Skip to content

Commit

Permalink
feat: Go error messaging improvements for lsp (#1272)
Browse files Browse the repository at this point in the history
<img width="1185" alt="Screenshot 2024-04-15 at 6 02 02 PM"
src="https://github.com/TBD54566975/ftl/assets/72891690/11878306-747c-4f7e-b74e-a78f56a5e1c4">
  • Loading branch information
worstell authored Apr 16, 2024
1 parent 5de29f5 commit 2500d32
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 35 deletions.
2 changes: 1 addition & 1 deletion backend/schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func SortErrorsByPosition(merr []error) {
if errors.As(merr[i], &ipe) && errors.As(merr[j], &jpe) {
ipp := ipe.Pos
jpp := jpe.Pos
return ipp.Line < jpp.Line || (ipp.Line == jpp.Line && ipp.Column < jpp.Column)
return ipp.Line < jpp.Line || (ipp.Line == jpp.Line && ipp.Column < jpp.Column) || (ipp.Line == jpp.Line && ipp.Column == jpp.Column && ipe.EndColumn < jpe.EndColumn)
}
return merr[i].Error() < merr[j].Error()
})
Expand Down
4 changes: 2 additions & 2 deletions buildengine/build_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ func TestExternalType(t *testing.T) {
testBuild(t, bctx, true, []assertion{
assertBuildProtoErrors(
"unsupported external type \"time.Month\"",
"invalid type \"ftl/external.ExternalResponse\"",
"invalid response type \"ftl/external.ExternalResponse\"",
"unsupported type \"time.Month\" for field \"Month\"",
"unsupported response type \"ftl/external.ExternalResponse\"",
),
})
}
2 changes: 1 addition & 1 deletion buildengine/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func assertBuildProtoErrors(msgs ...string) assertion {
errs = append(errs, *e)
}
schema.SortErrorsByPosition(errs)

for _, e := range errorList.Errors {
e.EndColumn = 0
}
Expand Down
24 changes: 15 additions & 9 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ func parseConfigDecl(pctx *parseContext, node *ast.CallExpr, fn *types.Func) {
tp := pctx.pkg.TypesInfo.Types[index.Index].Type
st, ok := visitType(pctx, index.Index.Pos(), tp).Get()
if !ok {
pctx.errors = append(pctx.errors, errorf(index.Index, "unsupported type %q", tp))
return
}

Expand Down Expand Up @@ -389,6 +390,9 @@ func visitTypeSpec(pctx *parseContext, node *ast.TypeSpec) {
enum.Name = strcase.ToUpperCamel(node.Name.Name)
enum.Type = typ
pctx.nativeNames[enum] = node.Name.Name
} else {
pctx.errors = append(pctx.errors, errorf(node, "unsupported type %q",
pctx.pkg.TypesInfo.TypeOf(node.Type).Underlying()))
}
} else {
visitType(pctx, node.Pos(), pctx.pkg.TypesInfo.Defs[node.Name].Type())
Expand Down Expand Up @@ -416,6 +420,8 @@ func visitValueSpec(pctx *parseContext, node *ast.ValueSpec) {
Value: value,
}
enum.Variants = append(enum.Variants, variant)
} else {
pctx.errors = append(pctx.errors, errorf(node, "unsupported type %q for enum variant %q", c.Type(), c.Name()))
}
}

Expand Down Expand Up @@ -496,11 +502,11 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb) {
resV, respOk := resp.Get()
if !reqOk {
pctx.errors = append(pctx.errors, tokenErrorf(params.At(1).Pos(), params.At(1).Name(),
"invalid request type %q", params.At(1).Type()))
"unsupported request type %q", params.At(1).Type()))
}
if !respOk {
pctx.errors = append(pctx.errors, tokenErrorf(results.At(0).Pos(), results.At(0).Name(),
"invalid response type %q", results.At(0).Type()))
"unsupported response type %q", results.At(0).Type()))
}
verb = &schema.Verb{
Pos: goPosToSchemaPos(node.Pos()),
Expand Down Expand Up @@ -652,6 +658,7 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type) optional.O
Metadata: metadata,
})
} else {
pctx.errors = append(pctx.errors, tokenErrorf(f.Pos(), f.Name(), "unsupported type %q for field %q", f.Type(), f.Name()))
fieldErrors = true
}
}
Expand All @@ -669,23 +676,22 @@ func visitConst(pctx *parseContext, cnode *types.Const) optional.Option[schema.V
case types.String:
value, err := strconv.Unquote(cnode.Val().String())
if err != nil {
pctx.errors = append(pctx.errors, tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "error parsing string constant"))
pctx.errors = append(pctx.errors, tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "unsupported string constant"))
return optional.None[schema.Value]()
}
return optional.Some[schema.Value](&schema.StringValue{Pos: goPosToSchemaPos(cnode.Pos()), Value: value})

case types.Int, types.Int64:
value, err := strconv.ParseInt(cnode.Val().String(), 10, 64)
if err != nil {
pctx.errors = append(pctx.errors, tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "error parsing int constant"))
pctx.errors = append(pctx.errors, tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "unsupported int constant"))
return optional.None[schema.Value]()
}
return optional.Some[schema.Value](&schema.IntValue{Pos: goPosToSchemaPos(cnode.Pos()), Value: int(value)})
default:
pctx.errors = append(pctx.errors, noEndColumnErrorf(cnode.Pos(), "unsupported basic type %q", b))
return optional.None[schema.Value]()
}
}
pctx.errors = append(pctx.errors, noEndColumnErrorf(cnode.Pos(), "unsupported const type %q", cnode.Type()))
return optional.None[schema.Value]()
}

Expand All @@ -696,6 +702,9 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) optional.Opt
switch underlying := tnode.Underlying().(type) {
case *types.Basic:
if named, ok := tnode.(*types.Named); ok {
if _, ok := visitType(pctx, pos, named.Underlying()).Get(); !ok {
return optional.None[schema.Type]()
}
nodePath := named.Obj().Pkg().Path()
if pctx.enums[named.Obj().Name()] != nil {
return optional.Some[schema.Type](&schema.Ref{
Expand Down Expand Up @@ -736,7 +745,6 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) optional.Opt
return optional.Some[schema.Type](&schema.Float{Pos: goPosToSchemaPos(pos)})

default:
pctx.errors = append(pctx.errors, noEndColumnErrorf(pos, "unsupported basic type %q", underlying))
return optional.None[schema.Type]()
}

Expand All @@ -746,7 +754,6 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) optional.Opt
if ref, ok := visitStruct(pctx, pos, tnode).Get(); ok {
return optional.Some[schema.Type](ref)
}
pctx.errors = append(pctx.errors, noEndColumnErrorf(pos, "invalid type %q", tnode))
return optional.None[schema.Type]()
}

Expand All @@ -773,7 +780,6 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) optional.Opt
if ref, ok := visitStruct(pctx, pos, tnode).Get(); ok {
return optional.Some[schema.Type](ref)
}
pctx.errors = append(pctx.errors, noEndColumnErrorf(pos, "invalid type %q", tnode))
return optional.None[schema.Type]()
}

Expand Down
34 changes: 12 additions & 22 deletions go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,45 +246,35 @@ func TestErrorReporting(t *testing.T) {
filename := filepath.Join(pwd, `testdata/failing/failing.go`)
assert.EqualError(t, errors.Join(merr...),
filename+":10:13-35: config and secret declarations must have a single string literal argument\n"+
filename+":13:2-10: unsupported type \"error\" for field \"BadParam\"\n"+
filename+":13:2-2: unsupported type \"error\"\n"+
filename+":16:2-2: unsupported basic type \"uint64\"\n"+
filename+":16:2-17: unsupported type \"uint64\" for field \"AnotherBadParam\"\n"+
filename+":19:3-3: unexpected token \"verb\" (expected Directive)\n"+
filename+":25:1-1: invalid type \"ftl/failing.Request\"\n"+
filename+":25:1-1: invalid type \"ftl/failing.Response\"\n"+
filename+":25:36-39: invalid request type \"ftl/failing.Request\"\n"+
filename+":25:50-50: invalid response type \"ftl/failing.Response\"\n"+
filename+":25:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":25:50-50: unsupported response type \"ftl/failing.Response\"\n"+
filename+":26:16-29: call first argument must be a function in an ftl module\n"+
filename+":27:2-46: call must have exactly three arguments\n"+
filename+":28:16-25: call first argument must be a function\n"+
filename+":33:1-1: invalid type \"ftl/failing.Response\"\n"+
filename+":33:1-2: must have at most two parameters (context.Context, struct)\n"+
filename+":33:69-69: invalid response type \"ftl/failing.Response\"\n"+
filename+":38:1-1: invalid type \"ftl/failing.Response\"\n"+
filename+":33:69-69: unsupported response type \"ftl/failing.Response\"\n"+
filename+":38:22-27: first parameter must be of type context.Context but is ftl/failing.Request\n"+
filename+":38:37-43: second parameter must be a struct but is string\n"+
filename+":38:53-53: invalid response type \"ftl/failing.Response\"\n"+
filename+":43:1-1: invalid type \"ftl/failing.Response\"\n"+
filename+":38:53-53: unsupported response type \"ftl/failing.Response\"\n"+
filename+":43:43-47: second parameter must not be ftl.Unit\n"+
filename+":43:59-59: invalid response type \"ftl/failing.Response\"\n"+
filename+":48:1-1: invalid type \"ftl/failing.Response\"\n"+
filename+":43:59-59: unsupported response type \"ftl/failing.Response\"\n"+
filename+":48:1-2: first parameter must be context.Context\n"+
filename+":48:18-18: invalid response type \"ftl/failing.Response\"\n"+
filename+":53:1-1: invalid type \"ftl/failing.Request\"\n"+
filename+":48:18-18: unsupported response type \"ftl/failing.Response\"\n"+
filename+":53:1-2: must have at most two results (struct, error)\n"+
filename+":53:41-44: invalid request type \"ftl/failing.Request\"\n"+
filename+":58:1-1: invalid type \"ftl/failing.Request\"\n"+
filename+":53:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":58:1-2: must at least return an error\n"+
filename+":58:36-39: invalid request type \"ftl/failing.Request\"\n"+
filename+":62:1-1: invalid type \"ftl/failing.Request\"\n"+
filename+":62:35-38: invalid request type \"ftl/failing.Request\"\n"+
filename+":58:36-39: unsupported request type \"ftl/failing.Request\"\n"+
filename+":62:35-38: unsupported request type \"ftl/failing.Request\"\n"+
filename+":62:48-48: must return an error but is ftl/failing.Response\n"+
filename+":67:1-1: invalid type \"ftl/failing.Request\"\n"+
filename+":67:41-44: invalid request type \"ftl/failing.Request\"\n"+
filename+":67:41-44: unsupported request type \"ftl/failing.Request\"\n"+
filename+":67:55-55: first result must be a struct but is string\n"+
filename+":67:63-63: must return an error but is string\n"+
filename+":67:63-63: second result must not be ftl.Unit\n"+
filename+":74:1-1: verb \"WrongResponse\" already exported\n"+
filename+":79:6-6: invalid type \"ftl/failing.BadStruct\"\n"+
filename+":80:2-12: struct field unexported must be exported by starting with an uppercase letter",
)
}
Expand Down

0 comments on commit 2500d32

Please sign in to comment.