Skip to content

Commit

Permalink
fix: add EndColumn field to errors
Browse files Browse the repository at this point in the history
  • Loading branch information
wesbillman committed Apr 4, 2024
1 parent 253ad31 commit 6bc5418
Show file tree
Hide file tree
Showing 7 changed files with 356 additions and 315 deletions.
550 changes: 280 additions & 270 deletions backend/protos/xyz/block/ftl/v1/schema/schema.pb.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions backend/protos/xyz/block/ftl/v1/schema/schema.proto
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ message EnumVariant {
message Error {
string msg = 1;
Position pos = 2;
int64 endColumn = 3;
}

message ErrorList {
Expand Down
20 changes: 12 additions & 8 deletions backend/schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,40 @@ import (
)

type Error struct {
Msg string `json:"msg" protobuf:"1"`
Pos Position `json:"pos" protobuf:"2"`
Err error `protobuf:"-"` // Wrapped error, if any
Msg string `json:"msg" protobuf:"1"`
Pos Position `json:"pos" protobuf:"2"`
EndColumn int `json:"endCol" protobuf:"3"`
Err error `protobuf:"-"` // Wrapped error, if any
}

type ErrorList struct {
Errors []Error `json:"errors" protobuf:"1"`
}

func (e Error) Error() string { return fmt.Sprintf("%s: %s", e.Pos, e.Msg) }
func (e Error) Error() string { return fmt.Sprintf("%s-%d: %s", e.Pos, e.EndColumn, e.Msg) }
func (e Error) Unwrap() error { return e.Err }

func Errorf(pos Position, format string, args ...any) Error {
return Error{Msg: fmt.Sprintf(format, args...), Pos: pos}
func Errorf(pos Position, endColumn int, format string, args ...any) Error {
return Error{Msg: fmt.Sprintf(format, args...), Pos: pos, EndColumn: endColumn}
}

func Wrapf(pos Position, err error, format string, args ...any) Error {
func Wrapf(pos Position, endColumn int, err error, format string, args ...any) Error {
if format == "" {
format = "%s"
} else {
format += ": %s"
}
// Propagate existing error position if available
var newPos Position
var newEndColumn int
if perr := (Error{}); errors.As(err, &perr) {
newPos = perr.Pos
newEndColumn = perr.EndColumn
args = append(args, perr.Msg)
} else {
newPos = pos
newEndColumn = endColumn
args = append(args, err)
}
return Error{Msg: fmt.Sprintf(format, args...), Pos: newPos, Err: err}
return Error{Msg: fmt.Sprintf(format, args...), Pos: newPos, EndColumn: newEndColumn, Err: err}
}
6 changes: 6 additions & 0 deletions frontend/src/protos/xyz/block/ftl/v1/schema/schema_pb.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

77 changes: 47 additions & 30 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"sync"
"unicode"
"unicode/utf8"

"github.com/alecthomas/types/optional"
"golang.org/x/exp/maps"
Expand Down Expand Up @@ -44,12 +45,23 @@ type NativeNames map[schema.Decl]string

type enums map[string]*schema.Enum

func errorf(pos token.Pos, format string, args ...interface{}) schema.Error {
return schema.Errorf(goPosToSchemaPos(pos), format, args...)
func tokenErrorf(pos token.Pos, tokenText string, format string, args ...interface{}) schema.Error {
goPos := goPosToSchemaPos(pos)
endColumn := goPos.Column
if len(tokenText) > 0 {
endColumn += utf8.RuneCountInString(tokenText)
}
return schema.Errorf(goPosToSchemaPos(pos), endColumn, format, args...)
}

func errorf(node ast.Node, format string, args ...interface{}) schema.Error {
pos, endCol := goNodePosToSchemaPos(node)
return schema.Errorf(pos, endCol, format, args...)
}

func wrapf(pos token.Pos, err error, format string, args ...interface{}) schema.Error {
return schema.Wrapf(goPosToSchemaPos(pos), err, format, args...)
func wrapf(node ast.Node, err error, format string, args ...interface{}) schema.Error {
pos, endCol := goNodePosToSchemaPos(node)
return schema.Wrapf(pos, endCol, err, format, args...)
}

// ExtractModuleSchema statically parses Go FTL module source into a schema.Module.
Expand Down Expand Up @@ -78,7 +90,7 @@ func ExtractModuleSchema(dir string) (NativeNames, *schema.Module, error) {
if len(pkg.Errors) > 0 {
for _, perr := range pkg.Errors {
if len(pkg.Syntax) > 0 {
merr = append(merr, wrapf(pkg.Syntax[0].Pos(), perr, "%s", pkg.PkgPath))
merr = append(merr, wrapf(pkg.Syntax[0], perr, "%s", pkg.PkgPath))
} else {
merr = append(merr, fmt.Errorf("%s: %w", pkg.PkgPath, perr))
}
Expand All @@ -89,7 +101,7 @@ func ExtractModuleSchema(dir string) (NativeNames, *schema.Module, error) {
err := goast.Visit(file, func(node ast.Node, next func() error) (err error) {
defer func() {
if err != nil {
err = wrapf(node.Pos(), err, "")
err = wrapf(node, err, "")
}
}()
switch node := node.(type) {
Expand Down Expand Up @@ -164,21 +176,21 @@ func visitCallExpr(pctx *parseContext, node *ast.CallExpr) error {

func parseCall(pctx *parseContext, node *ast.CallExpr) error {
if len(node.Args) != 3 {
return errorf(node.Pos(), "call must have exactly three arguments")
return errorf(node, "call must have exactly three arguments")
}
_, verbFn := deref[*types.Func](pctx.pkg, node.Args[1])
if verbFn == nil {
if sel, ok := node.Args[1].(*ast.SelectorExpr); ok {
return errorf(node.Args[1].Pos(), "call first argument must be a function but is an unresolved reference to %s.%s", sel.X, sel.Sel)
return errorf(node.Args[1], "call first argument must be a function but is an unresolved reference to %s.%s", sel.X, sel.Sel)
}
return errorf(node.Args[1].Pos(), "call first argument must be a function but is %T", node.Args[1])
return errorf(node.Args[1], "call first argument must be a function but is %T", node.Args[1])
}
if pctx.activeVerb == nil {
return nil
}
moduleName, ok := ftlModuleFromGoModule(verbFn.Pkg().Path()).Get()
if !ok {
return errorf(node.Args[1].Pos(), "call first argument must be a function in an ftl module")
return errorf(node.Args[1], "call first argument must be a function in an ftl module")
}
ref := &schema.Ref{
Pos: goPosToSchemaPos(node.Pos()),
Expand All @@ -196,12 +208,12 @@ func parseConfigDecl(pctx *parseContext, node *ast.CallExpr, fn *types.Func) err
var err error
name, err = strconv.Unquote(literal.Value)
if err != nil {
return wrapf(node.Pos(), err, "")
return wrapf(node, err, "")
}
}
}
if name == "" {
return errorf(node.Pos(), "config and secret declarations must have a single string literal argument")
return errorf(node, "config and secret declarations must have a single string literal argument")
}
index := node.Fun.(*ast.IndexExpr) //nolint:forcetypeassert

Expand Down Expand Up @@ -296,6 +308,11 @@ func goPosToSchemaPos(pos token.Pos) schema.Position {
return schema.Position{Filename: p.Filename, Line: p.Line, Column: p.Column, Offset: p.Offset}
}

func goNodePosToSchemaPos(node ast.Node) (schema.Position, int) {
p := fset.Position(node.Pos())
return schema.Position{Filename: p.Filename, Line: p.Line, Column: p.Column, Offset: p.Offset}, fset.Position(node.End()).Column
}

func visitGenDecl(pctx *parseContext, node *ast.GenDecl) error {
switch node.Tok {
case token.TYPE:
Expand All @@ -310,13 +327,13 @@ func visitGenDecl(pctx *parseContext, node *ast.GenDecl) error {
switch dir.(type) {
case *directiveExport:
if len(node.Specs) != 1 {
return errorf(node.Pos(), "error parsing ftl export directive: expected exactly one type "+
return errorf(node, "error parsing ftl export directive: expected exactly one type "+
"declaration")
}
if pctx.module.Name == "" {
pctx.module.Name = pctx.pkg.Name
} else if pctx.module.Name != pctx.pkg.Name {
return errorf(node.Pos(), "type export directive must be in the module package")
return errorf(node, "type export directive must be in the module package")
}
if t, ok := node.Specs[0].(*ast.TypeSpec); ok {
if _, ok := pctx.pkg.TypesInfo.TypeOf(t.Type).Underlying().(*types.Basic); ok {
Expand Down Expand Up @@ -392,7 +409,7 @@ func visitValueSpec(pctx *parseContext, node *ast.ValueSpec) error {
}
c, ok := pctx.pkg.TypesInfo.Defs[node.Names[0]].(*types.Const)
if !ok {
return errorf(node.Pos(), "could not extract enum %s: expected exactly one variant name", enum.Name)
return errorf(node, "could not extract enum %s: expected exactly one variant name", enum.Name)
}
value, err := visitConst(c)
if err != nil {
Expand Down Expand Up @@ -425,7 +442,7 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb, e
if pctx.module.Name == "" {
pctx.module.Name = pctx.pkg.Name
} else if pctx.module.Name != pctx.pkg.Name {
return nil, errorf(node.Pos(), "function export directive must be in the module package")
return nil, errorf(node, "function export directive must be in the module package")
}

case *directiveIngress:
Expand All @@ -447,13 +464,13 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb, e
fnt := pctx.pkg.TypesInfo.Defs[node.Name].(*types.Func) //nolint:forcetypeassert
sig := fnt.Type().(*types.Signature) //nolint:forcetypeassert
if sig.Recv() != nil {
return nil, errorf(node.Pos(), "ftl:export cannot be a method")
return nil, errorf(node, "ftl:export cannot be a method")
}
params := sig.Params()
results := sig.Results()
reqt, respt, err := checkSignature(sig)
if err != nil {
return nil, wrapf(node.Pos(), err, "")
return nil, wrapf(node, err, "")
}
var req schema.Type
if reqt != nil {
Expand Down Expand Up @@ -520,13 +537,13 @@ func ftlModuleFromGoModule(pkgPath string) optional.Option[string] {
func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type) (*schema.Ref, error) {
named, ok := tnode.(*types.Named)
if !ok {
return nil, errorf(pos, "expected named type but got %s", tnode)
return nil, tokenErrorf(pos, tnode.String(), "expected named type but got %s", tnode)
}
nodePath := named.Obj().Pkg().Path()
if !strings.HasPrefix(nodePath, pctx.pkg.PkgPath) {
destModule, ok := ftlModuleFromGoModule(nodePath).Get()
if !ok {
return nil, errorf(pos, "struct declared in non-FTL module %s", nodePath)
return nil, tokenErrorf(pos, nodePath, "struct declared in non-FTL module %s", nodePath)
}
dataRef := &schema.Ref{
Pos: goPosToSchemaPos(pos),
Expand All @@ -537,7 +554,7 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type) (*schema.R
arg := named.TypeArgs().At(i)
typeArg, err := visitType(pctx, pos, arg)
if err != nil {
return nil, errorf(pos, "type parameter %s: %v", arg.String(), err)
return nil, tokenErrorf(pos, arg.String(), "type parameter %s: %v", arg.String(), err)
}

// Fully qualify the Ref if needed
Expand Down Expand Up @@ -570,7 +587,7 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type) (*schema.R
})
typeArg, err := visitType(pctx, pos, named.TypeArgs().At(i))
if err != nil {
return nil, errorf(pos, "type parameter %s: %v", param.Obj().Name(), err)
return nil, tokenErrorf(pos, param.Obj().Name(), "type parameter %s: %v", param.Obj().Name(), err)
}
dataRef.TypeParameters = append(dataRef.TypeParameters, typeArg)
}
Expand Down Expand Up @@ -604,18 +621,18 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type) (*schema.R

s, ok := named.Underlying().(*types.Struct)
if !ok {
return nil, errorf(pos, "expected struct but got %s", named)
return nil, tokenErrorf(pos, named.String(), "expected struct but got %s", named)
}
for i := range s.NumFields() {
f := s.Field(i)
ft, err := visitType(pctx, f.Pos(), f.Type())
if err != nil {
return nil, errorf(f.Pos(), "field %s: %v", f.Name(), err)
return nil, tokenErrorf(f.Pos(), f.Name(), "field %s: %v", f.Name(), err)
}

// Check if field is exported
if len(f.Name()) > 0 && unicode.IsLower(rune(f.Name()[0])) {
return nil, errorf(f.Pos(), "params field %s must be exported by starting with an uppercase letter", f.Name())
return nil, tokenErrorf(f.Pos(), f.Name(), "params field %s must be exported by starting with an uppercase letter", f.Name())
}

// Extract the JSON tag and split it to get just the field name
Expand Down Expand Up @@ -662,10 +679,10 @@ func visitConst(cnode *types.Const) (schema.Value, error) {
}
return &schema.IntValue{Pos: goPosToSchemaPos(cnode.Pos()), Value: int(value)}, nil
default:
return nil, errorf(cnode.Pos(), "unsupported basic type %s", b)
return nil, tokenErrorf(cnode.Pos(), b.Name(), "unsupported basic type %s", b)
}
}
return nil, errorf(cnode.Pos(), "unsupported const type %s", cnode.Type())
return nil, tokenErrorf(cnode.Pos(), cnode.Type().String(), "unsupported const type %s", cnode.Type())
}

func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) (schema.Type, error) {
Expand Down Expand Up @@ -713,7 +730,7 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) (schema.Type
return &schema.Float{Pos: goPosToSchemaPos(pos)}, nil

default:
return nil, errorf(pos, "unsupported basic type %s", underlying)
return nil, tokenErrorf(pos, underlying.Name(), "unsupported basic type %s", underlying)
}

case *types.Struct:
Expand Down Expand Up @@ -754,10 +771,10 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type) (schema.Type
if underlying.String() == "any" {
return &schema.Any{Pos: goPosToSchemaPos(pos)}, nil
}
return nil, errorf(pos, "unsupported type %q", tnode)
return nil, tokenErrorf(pos, tnode.String(), "unsupported type %q", tnode)

default:
return nil, errorf(pos, "unsupported type %T", tnode)
return nil, tokenErrorf(pos, tnode.String(), "unsupported type %T", tnode)
}
}

Expand Down
2 changes: 1 addition & 1 deletion go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,5 +239,5 @@ func TestErrorReporting(t *testing.T) {
}
pwd, _ := os.Getwd()
_, _, err := ExtractModuleSchema("testdata/failing")
assert.EqualError(t, err, filepath.Join(pwd, `testdata/failing/failing.go`)+`:14:2: call must have exactly three arguments`)
assert.EqualError(t, err, filepath.Join(pwd, `testdata/failing/failing.go`)+`:14:2-46: call must have exactly three arguments`)
}
15 changes: 9 additions & 6 deletions lsp/lsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,20 @@ func publishErrors(errByFilename map[string]errSet, s *Server) {
sourceName := "ftl"
severity := protocol.DiagnosticSeverityError

length, err := getLineOrWordLength(filename, pp.Line, pp.Column, false)
if err != nil {
s.logger.Errorf(err, "Failed to get line or word length")
continue
// If the end column is not set, set it to the length of the word.
if e.EndColumn-1 == pp.Column {
length, err := getLineOrWordLength(filename, pp.Line, pp.Column, false)
if err != nil {
s.logger.Errorf(err, "Failed to get line or word length")
continue
}
e.EndColumn = pp.Column + length
}
endColumn := pp.Column + length

diagnostics = append(diagnostics, protocol.Diagnostic{
Range: protocol.Range{
Start: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(pp.Column - 1)},
End: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(endColumn - 1)},
End: protocol.Position{Line: uint32(pp.Line - 1), Character: uint32(e.EndColumn - 1)},
},
Severity: &severity,
Source: &sourceName,
Expand Down

0 comments on commit 6bc5418

Please sign in to comment.