Skip to content

Commit

Permalink
fix: directive error positions and messages (#1756)
Browse files Browse the repository at this point in the history
Fixes #1635

I also noticed that the directive errors were being shown on the `func`
instead of the location of the directive so added `GetPosition` to
directives as well.

![Screenshot 2024-06-12 at 3 52
24 PM](https://github.com/TBD54566975/ftl/assets/51647/d5aaa6d2-2bba-4727-85b2-fd3965bb9ada)

and

![Screenshot 2024-06-12 at 3 53
31 PM](https://github.com/TBD54566975/ftl/assets/51647/a5d60dcb-c67b-4799-8445-529f36cfe724)
  • Loading branch information
wesbillman authored Jun 12, 2024
1 parent b601080 commit 44163b4
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
41 changes: 39 additions & 2 deletions go-runtime/compile/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type directiveWrapper struct {
type directive interface {
directive()
SetPosition(pos schema.Position)
GetPosition() schema.Position
}

type exportable interface {
Expand All @@ -48,6 +49,10 @@ func (d *directiveVerb) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveVerb) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveVerb) String() string {
if d.Export {
return "ftl:verb export"
Expand All @@ -71,6 +76,10 @@ func (d *directiveData) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveData) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveData) String() string {
if d.Export {
return "ftl:data export"
Expand All @@ -94,6 +103,10 @@ func (d *directiveEnum) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveEnum) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveEnum) String() string {
if d.Export {
return "ftl:enum export"
Expand All @@ -117,6 +130,10 @@ func (d *directiveTypeAlias) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveTypeAlias) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveTypeAlias) String() string {
if d.Export {
return "ftl:typealias export"
Expand All @@ -141,6 +158,10 @@ func (d *directiveIngress) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveIngress) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveIngress) String() string {
w := &strings.Builder{}
fmt.Fprintf(w, "ftl:ingress %s", d.Method)
Expand All @@ -162,8 +183,12 @@ func (d *directiveCronJob) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveCronJob) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveCronJob) String() string {
return fmt.Sprintf("cron %s", d.Cron)
return fmt.Sprintf("ftl:cron %s", d.Cron)
}

type directiveRetry struct {
Expand All @@ -180,6 +205,10 @@ func (d *directiveRetry) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveRetry) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveRetry) String() string {
components := []string{"retry"}
if d.Count != nil {
Expand All @@ -205,6 +234,10 @@ func (d *directiveSubscriber) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveSubscriber) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveSubscriber) String() string {
return fmt.Sprintf("subscribe %s", d.Name)
}
Expand All @@ -222,8 +255,12 @@ func (d *directiveExport) SetPosition(pos schema.Position) {
d.Pos = pos
}

func (d *directiveExport) GetPosition() schema.Position {
return d.Pos
}

func (d *directiveExport) String() string {
return "export"
return "ftl:export"
}

var directiveParser = participle.MustBuild[directiveWrapper](
Expand Down
20 changes: 10 additions & 10 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ func noEndColumnErrorf(pos token.Pos, format string, args ...interface{}) *schem
return tokenErrorf(pos, "", format, args...)
}

func unexpectedDirectiveErrorf(dir directive, format string, args ...interface{}) *schema.Error {
return schema.Errorf(dir.GetPosition(), 0, format, args...)
}

func tokenErrorf(pos token.Pos, tokenText string, format string, args ...interface{}) *schema.Error {
goPos := goPosToSchemaPos(pos)
endColumn := goPos.Column
Expand Down Expand Up @@ -95,12 +99,6 @@ func (e errorSet) add(err *schema.Error) {
e[err.Error()] = err
}

func (e errorSet) addAll(errs ...*schema.Error) {
for _, err := range errs {
e.add(err)
}
}

func legacyExtractModuleSchema(dir string, sch *schema.Schema, out *analyzers.ExtractResult) error {
pkgs, err := packages.Load(&packages.Config{
Dir: dir,
Expand Down Expand Up @@ -313,7 +311,7 @@ func extractTopicDecl(pctx *parseContext, node *ast.CallExpr, stack []ast.Node)
if _, ok := dir.(*directiveExport); ok {
export = true
} else {
pctx.errors.add(errorf(node, "unexpected directive attached for topic: %T", dir))
pctx.errors.add(unexpectedDirectiveErrorf(dir, "unexpected directive %q attached for topic", dir))
}
}

Expand Down Expand Up @@ -539,7 +537,7 @@ func parseFSMDecl(pctx *parseContext, node *ast.CallExpr, stack []ast.Node) {
MaxBackoff: retryDir.MaxBackoff,
})
} else {
pctx.errors.add(errorf(node, "unexpected directive attached for FSM: %T", dir))
pctx.errors.add(unexpectedDirectiveErrorf(dir, "unexpected directive %q attached for FSM", dir))
}
}
}
Expand Down Expand Up @@ -1216,8 +1214,10 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb) {
Pos: dir.Pos,
Name: dir.Name,
})
case *directiveData, *directiveEnum, *directiveTypeAlias, *directiveExport:
pctx.errors.add(errorf(node, "unexpected directive %T", dir))
case *directiveExport:
pctx.errors.add(unexpectedDirectiveErrorf(dir, "unexpected directive %q attached for verb, did you mean to use '//ftl:verb export' instead", dir))
case *directiveData, *directiveEnum, *directiveTypeAlias:
pctx.errors.add(unexpectedDirectiveErrorf(dir, "unexpected directive %q attached for verb", dir))
}
}
if !isVerb {
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 @@ -510,7 +510,7 @@ func TestErrorReporting(t *testing.T) {
`21:14-44: duplicate database declaration at 20:14-44`,
`24:2-10: unsupported type "error" for field "BadParam"`,
`27:2-17: unsupported type "uint64" for field "AnotherBadParam"`,
`31:1-2: unexpected directive *compile.directiveExport`,
`30:3-0: unexpected directive "ftl:export" attached for verb, did you mean to use '//ftl:verb export' instead`,
`36:36-39: unsupported request type "ftl/failing.Request"`,
`36:50-50: unsupported response type "ftl/failing.Response"`,
`37:16-29: call first argument must be a function but is an unresolved reference to lib.OtherFunc`,
Expand Down

0 comments on commit 44163b4

Please sign in to comment.