fix: Invalid cron schedule does not include positional information (#…

…1659)

Metadata positioning was not being populated so it was offsetting from the top of the file.

backend/schema/validate.go

@@ -540,7 +540,7 @@ func validateVerbMetadata(scopes Scopes, module *Module, n *Verb) (merr []error)
 		case *MetadataCronJob:
 			_, err := cron.Parse(md.Cron)
 			if err != nil {
-				merr = append(merr, err)
+				merr = append(merr, errorf(md, "verb %s: invalid cron expression %q: %v", n.Name, md.Cron, err))
 			}
 			if _, ok := n.Request.(*Unit); !ok {
 				merr = append(merr, errorf(md, "verb %s: cron job can not have a request type", n.Name))

go-runtime/compile/parser.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 
 	"github.com/alecthomas/participle/v2"
-	"github.com/alecthomas/participle/v2/lexer"
 
 	"github.com/TBD54566975/ftl/backend/schema"
 )
@@ -23,20 +22,28 @@ type directiveWrapper struct {
 }
 
 //sumtype:decl
-type directive interface{ directive() }
+type directive interface {
+	directive()
+	SetPosition(pos schema.Position)
+}
 
 type exportable interface {
 	IsExported() bool
 }
 
 type directiveVerb struct {
-	Pos lexer.Position
+	Pos schema.Position
 
 	Verb   bool `parser:"@'verb'"`
 	Export bool `parser:"@'export'?"`
 }
 
 func (*directiveVerb) directive() {}
+
+func (d *directiveVerb) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveVerb) String() string {
 	if d.Export {
 		return "ftl:verb export"
@@ -48,13 +55,18 @@ func (d *directiveVerb) IsExported() bool {
 }
 
 type directiveData struct {
-	Pos lexer.Position
+	Pos schema.Position
 
 	Data   bool `parser:"@'data'"`
 	Export bool `parser:"@'export'?"`
 }
 
 func (*directiveData) directive() {}
+
+func (d *directiveData) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveData) String() string {
 	if d.Export {
 		return "ftl:data export"
@@ -66,13 +78,18 @@ func (d *directiveData) IsExported() bool {
 }
 
 type directiveEnum struct {
-	Pos lexer.Position
+	Pos schema.Position
 
 	Enum   bool `parser:"@'enum'"`
 	Export bool `parser:"@'export'?"`
 }
 
 func (*directiveEnum) directive() {}
+
+func (d *directiveEnum) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveEnum) String() string {
 	if d.Export {
 		return "ftl:enum export"
@@ -84,13 +101,18 @@ func (d *directiveEnum) IsExported() bool {
 }
 
 type directiveTypeAlias struct {
-	Pos lexer.Position
+	Pos schema.Position
 
 	TypeAlias bool `parser:"@'typealias'"`
 	Export    bool `parser:"@'export'?"`
 }
 
 func (*directiveTypeAlias) directive() {}
+
+func (d *directiveTypeAlias) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveTypeAlias) String() string {
 	if d.Export {
 		return "ftl:typealias export"
@@ -110,6 +132,11 @@ type directiveIngress struct {
 }
 
 func (*directiveIngress) directive() {}
+
+func (d *directiveIngress) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveIngress) String() string {
 	w := &strings.Builder{}
 	fmt.Fprintf(w, "ftl:ingress %s", d.Method)
@@ -127,6 +154,10 @@ type directiveCronJob struct {
 
 func (*directiveCronJob) directive() {}
 
+func (d *directiveCronJob) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveCronJob) String() string {
 	return fmt.Sprintf("cron %s", d.Cron)
 }
@@ -141,6 +172,10 @@ type directiveRetry struct {
 
 func (*directiveRetry) directive() {}
 
+func (d *directiveRetry) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveRetry) String() string {
 	components := []string{"retry"}
 	if d.Count != nil {
@@ -162,6 +197,10 @@ type directiveSubscriber struct {
 
 func (*directiveSubscriber) directive() {}
 
+func (d *directiveSubscriber) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveSubscriber) String() string {
 	return fmt.Sprintf("subscribe %s", d.Name)
 }
@@ -175,6 +214,10 @@ type directiveExport struct {
 
 func (*directiveExport) directive() {}
 
+func (d *directiveExport) SetPosition(pos schema.Position) {
+	d.Pos = pos
+}
+
 func (d *directiveExport) String() string {
 	return "export"
 }
@@ -199,23 +242,25 @@ func parseDirectives(node ast.Node, fset *token.FileSet, docs *ast.CommentGroup)
 			continue
 		}
 		pos := fset.Position(line.Pos())
-		// TODO: We need to adjust position information embedded in the schema.
+		ppos := schema.Position{
+			Filename: pos.Filename,
+			Line:     pos.Line,
+			Column:   pos.Column + 2, // Skip "//"
+		}
+
 		directive, err := directiveParser.ParseString(pos.Filename, line.Text[2:])
 		if err != nil {
-			// Adjust the Participle-reported position relative to the AST node.
 			var scerr *schema.Error
 			var perr participle.Error
 			if errors.As(err, &perr) {
-				ppos := schema.Position{}
-				ppos.Filename = pos.Filename
-				ppos.Column += pos.Column + 2
-				ppos.Line = pos.Line
 				scerr = schema.Errorf(ppos, ppos.Column, "%s", perr.Message())
 			} else {
 				scerr = wrapf(node, err, "")
 			}
 			return nil, scerr
 		}
+
+		directive.Directive.SetPosition(ppos)
 		directives = append(directives, directive.Directive)
 	}
 	return directives, nil

go-runtime/compile/schema_test.go

@@ -14,6 +14,7 @@ import (
 	"golang.org/x/tools/go/packages"
 
 	"github.com/TBD54566975/ftl/backend/schema"
+	"github.com/TBD54566975/ftl/internal/errors"
 	"github.com/TBD54566975/ftl/internal/exec"
 	"github.com/TBD54566975/ftl/internal/log"
 	"github.com/TBD54566975/ftl/internal/slices"
@@ -535,3 +536,27 @@ func TestErrorReporting(t *testing.T) {
 	}
 	assert.Equal(t, expected, actual)
 }
+
+// Where parsing is correct but validation of the schema fails
+func TestValidationFailures(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+	ctx := log.ContextWithNewDefaultLogger(context.Background())
+	pwd, _ := os.Getwd()
+	err := exec.Command(ctx, log.Debug, "testdata/validation", "go", "mod", "tidy").RunBuffered(ctx)
+	assert.NoError(t, err)
+	_, err = ExtractModuleSchema("testdata/validation", &schema.Schema{})
+	assert.Error(t, err)
+	errs := errors.UnwrapAll(err)
+
+	filename := filepath.Join(pwd, `testdata/validation/validation.go`)
+	actual := slices.Map(errs, func(e error) string {
+		return strings.TrimPrefix(e.Error(), filename+":")
+	})
+	expected := []string{
+		`11:3-3: verb badYear: invalid cron expression "* * * * * 9999": value 9999 out of allowed year range of 0-3000`,
+		`16:3-3: verb allZeroes: invalid cron expression "0 0 0 0 0": value 0 out of allowed day of month range of 1-31`,
+	}
+	assert.Equal(t, expected, actual)
+}

go-runtime/compile/testdata/failing/failing.go

@@ -150,3 +150,13 @@ type TypeEnum3 interface{ ExportedMethod() }
 
 //ftl:enum
 type NoMethodsTypeEnum interface{}
+
+//ftl:cron * * * * *
+func GoodCron(ctx context.Context) error {
+	return nil
+}
+
+//ftl:cron 0 0 0 0 0
+func BadCron(ctx context.Context) error {
+	return nil
+}

go-runtime/compile/testdata/validation/ftl.toml

@@ -0,0 +1,2 @@
+module = "validation"
+language = "go"

go-runtime/compile/testdata/validation/go.mod

@@ -0,0 +1,43 @@
+module ftl/validation
+
+go 1.22.2
+
+replace github.com/TBD54566975/ftl => ../../../..
+
+require github.com/TBD54566975/ftl v0.0.0-00010101000000-000000000000
+
+require (
+	connectrpc.com/connect v1.16.1 // indirect
+	connectrpc.com/grpcreflect v1.2.0 // indirect
+	connectrpc.com/otelconnect v0.7.0 // indirect
+	github.com/alecthomas/concurrency v0.0.2 // indirect
+	github.com/alecthomas/participle/v2 v2.1.1 // indirect
+	github.com/alecthomas/types v0.16.0 // indirect
+	github.com/alessio/shellescape v1.4.2 // indirect
+	github.com/danieljoos/wincred v1.2.0 // indirect
+	github.com/go-logr/logr v1.4.2 // indirect
+	github.com/go-logr/stdr v1.2.2 // indirect
+	github.com/godbus/dbus/v5 v5.1.0 // indirect
+	github.com/jackc/pgpassfile v1.0.0 // indirect
+	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
+	github.com/jackc/pgx/v5 v5.6.0 // indirect
+	github.com/jackc/puddle/v2 v2.2.1 // indirect
+	github.com/jpillora/backoff v1.0.0 // indirect
+	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
+	github.com/mattn/go-isatty v0.0.20 // indirect
+	github.com/multiformats/go-base36 v0.2.0 // indirect
+	github.com/swaggest/jsonschema-go v0.3.70 // indirect
+	github.com/swaggest/refl v1.3.0 // indirect
+	github.com/zalando/go-keyring v0.2.4 // indirect
+	go.opentelemetry.io/otel v1.27.0 // indirect
+	go.opentelemetry.io/otel/metric v1.27.0 // indirect
+	go.opentelemetry.io/otel/trace v1.27.0 // indirect
+	golang.org/x/crypto v0.23.0 // indirect
+	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
+	golang.org/x/mod v0.17.0 // indirect
+	golang.org/x/net v0.25.0 // indirect
+	golang.org/x/sync v0.7.0 // indirect
+	golang.org/x/sys v0.20.0 // indirect
+	golang.org/x/text v0.15.0 // indirect
+	google.golang.org/protobuf v1.34.1 // indirect
+)