Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add EndColumn field to errors #1177

Merged
merged 1 commit into from
Apr 4, 2024
Merged

fix: add EndColumn field to errors #1177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
550 changes: 280 additions & 270 deletions backend/protos/xyz/block/ftl/v1/schema/schema.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions backend/protos/xyz/block/ftl/v1/schema/schema.proto
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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