diff --git a/common/schema/typeresolver.go b/common/schema/typeresolver.go index f25991c05..50bda410b 100644 --- a/common/schema/typeresolver.go +++ b/common/schema/typeresolver.go @@ -1,6 +1,7 @@ package schema import ( + "errors" "fmt" "strings" @@ -77,10 +78,8 @@ func NewScopes() Scopes { if err := scopes.Add(optional.None[*Module](), builtins.Name, builtins); err != nil { panic(err) } - for _, decl := range builtins.Decls { - if err := scopes.Add(optional.Some(builtins), decl.GetName(), decl); err != nil { - panic(err) - } + if err := scopes.AddModuleDecls(builtins); err != nil { + panic(err) } // Push an empty scope for other modules to be added to. scopes = scopes.Push() @@ -128,6 +127,20 @@ func (s *Scopes) Add(owner optional.Option[*Module], name string, symbol Symbol) return nil } +// AddModuleDecls adds all decls in a module to the current scope. +func (s *Scopes) AddModuleDecls(module *Module) error { + errs := []error{} + for _, decl := range module.Decls { + if err := s.Add(optional.Some(module), decl.GetName(), decl); err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errors.Join(errs...) + } + return nil +} + func (s Scopes) ResolveType(t Type) *ModuleDecl { switch t := t.(type) { case Named: diff --git a/common/schema/typeresolver_test.go b/common/schema/typeresolver_test.go index 9c9820cb6..fea1c9ce2 100644 --- a/common/schema/typeresolver_test.go +++ b/common/schema/typeresolver_test.go @@ -14,12 +14,41 @@ func TestTypeResolver(t *testing.T) { t T } verb test(test.Request) Empty + + // This module has it's own definition of HttpRequest + data HttpRequest { + } + } + `) + assert.NoError(t, err) + otherModule, err := ParseModuleString("", ` + module other { + data External { + } } `) assert.NoError(t, err) scopes := NewScopes() err = scopes.Add(optional.None[*Module](), module.Name, module) assert.NoError(t, err) + err = scopes.Add(optional.None[*Module](), otherModule.Name, otherModule) + assert.NoError(t, err) + + // Resolving "HTTPRequest" should return builtin.HttpRequest + httpRequest := scopes.Resolve(Ref{Name: "HttpRequest"}) + assert.Equal(t, httpRequest.Module.MustGet().Name, "builtin") + + // Push a new scope for "test" module's decls + scopes = scopes.Push() + assert.NoError(t, scopes.AddModuleDecls(module)) + + // Resolving "HTTPRequest" should return test.HttpRequest now that we've pushed the new scope + httpRequest = scopes.Resolve(Ref{Name: "HttpRequest"}) + assert.Equal(t, httpRequest.Module.MustGet().Name, "test") + + // Resolving "external" should fail + external := scopes.Resolve(Ref{Name: "External"}) + assert.Equal(t, external, nil) // Resolve a builtin. actualInt, _ := ResolveAs[*Int](scopes, Ref{Name: "Int"}) diff --git a/common/schema/validate.go b/common/schema/validate.go index 5a28a53d5..663011492 100644 --- a/common/schema/validate.go +++ b/common/schema/validate.go @@ -70,7 +70,7 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema schema.Modules = slices.DeleteFunc(schema.Modules, func(m *Module) bool { return m.Name == builtins.Name }) schema.Modules = append([]*Module{builtins}, schema.Modules...) - scopes := NewScopes() + generalScopes := NewScopes() // Validate dependencies if err := validateDependencies(schema); err != nil { @@ -82,7 +82,7 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema if module == builtins { continue } - if err := scopes.Add(optional.None[*Module](), module.Name, module); err != nil { + if err := generalScopes.Add(optional.None[*Module](), module.Name, module); err != nil { merr = append(merr, err) } } @@ -106,6 +106,12 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema merr = append(merr, err) } + // Push current decls + scopes := generalScopes.Push() + if err := scopes.AddModuleDecls(module); err != nil { + merr = append(merr, err) + } + indent := 0 err := Visit(module, func(n Node, next func() error) error { save := scopes @@ -115,6 +121,11 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema } indent++ defer func() { indent-- }() + + // Validate all children before validating the current node. + if err := next(); err != nil { + return err + } switch n := n.(type) { case *Ref: mdecl := scopes.Resolve(*n) @@ -222,7 +233,7 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema *EnumVariant, *Config, *Secret, *Topic, *DatabaseRuntime, *DatabaseRuntimeConnections, *Data, *Field: } - return next() + return nil }) if err != nil { merr = append(merr, err) @@ -258,8 +269,9 @@ func ValidateModule(module *Module) error { merr = append(merr, err) } scopes = scopes.Push() - // Key is : - duplicateDecls := map[string]Decl{} + if err := scopes.AddModuleDecls(module); err != nil { + merr = append(merr, err) + } _ = Visit(module, func(n Node, next func() error) error { //nolint:errcheck if scoped, ok := n.(Scoped); ok { @@ -267,14 +279,14 @@ func ValidateModule(module *Module) error { scopes = scopes.PushScope(scoped.Scope()) defer func() { scopes = pop }() } + + // Validate all children before validating the current node. + if err := next(); err != nil { + return err + } + if n, ok := n.(Decl); ok { tname := typeName(n) - duplKey := tname + ":" + n.GetName() - if dupl, ok := duplicateDecls[duplKey]; ok { - merr = append(merr, errorf(n, "duplicate %s %q, first defined at %s", tname, n.GetName(), dupl.Position())) - } else { - duplicateDecls[duplKey] = n - } if !ValidateName(n.GetName()) { merr = append(merr, errorf(n, "%s name %q is invalid", tname, n.GetName())) } else if _, ok := primitivesScope[n.GetName()]; ok { @@ -351,17 +363,12 @@ func ValidateModule(module *Module) error { } } - case *MetadataRetry: - if n.Catch != nil && n.Catch.Module == "" { - n.Catch.Module = module.Name - } - case Type, Metadata, Value, IngressPathComponent, DatabaseConnector, *Module, *Schema, *Optional, *TypeParameter, *EnumVariant, *Config, *Secret, *DatabaseRuntime, *DatabaseRuntimeConnections, *Database, *Enum: } - return next() + return nil }) merr = cleanErrors(merr) @@ -908,9 +915,6 @@ func validateQueryParamsPayloadType(n Node, r Type, v *Verb, reqOrResp string) e func validateVerbSubscriptions(module *Module, v *Verb, md *MetadataSubscriber, scopes Scopes) (merr []error) { merr = []error{} - if md.Topic.Module == "" { - md.Topic.Module = module.Name - } topicDecl := scopes.Resolve(*md.Topic) if topicDecl == nil { // errors for invalid refs are handled elsewhere @@ -959,9 +963,6 @@ func validateRetries(module *Module, retry *MetadataRetry, requestType optional. merr = append(merr, errorf(retry, "catch can only be defined on verbs")) return } - if retry.Catch.Module == "" { - retry.Catch.Module = module.Name - } catchDecl := scopes.Resolve(*retry.Catch) if catchDecl == nil { if retry.Catch.Module != "" && retry.Catch.Module != module.Name && !schema.Ok() { diff --git a/common/schema/validate_test.go b/common/schema/validate_test.go index 000d71774..b89a233da 100644 --- a/common/schema/validate_test.go +++ b/common/schema/validate_test.go @@ -104,8 +104,8 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - "3:20: ingress verb a: request type Empty must be builtin.HttpRequest", - "3:27: ingress verb a: response type Empty must be builtin.HttpResponse", + "3:20: ingress verb a: request type builtin.Empty must be builtin.HttpRequest", + "3:27: ingress verb a: response type builtin.Empty must be builtin.HttpResponse", }}, {name: "IngressBodyTypes", schema: ` @@ -136,8 +136,8 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - "11:22: ingress verb any: GET request type HttpRequest must have a body of unit not Any", - "11:52: ingress verb any: response type HttpResponse must have a body of bytes, string, data structure, unit, float, int, bool, map, or array not Any", + "11:22: ingress verb any: GET request type builtin.HttpRequest must have a body of unit not Any", + "11:52: ingress verb any: response type builtin.HttpResponse must have a body of bytes, string, data structure, unit, float, int, bool, map, or array not Any", "16:31: ingress verb pathInvalid: cannot use path parameter \"invalid\" with request type String as it has multiple path parameters, expected Data or Map type", "16:41: ingress verb pathInvalid: cannot use path parameter \"extra\" with request type String as it has multiple path parameters, expected Data or Map type", "18:31: ingress verb pathMissing: request pathParameter type one.Path does not contain a field corresponding to the parameter \"missing\"", @@ -152,7 +152,7 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - "3:24: ingress verb bytes: GET request type HttpRequest must have a body of unit not Bytes", + "3:24: ingress verb bytes: GET request type builtin.HttpRequest must have a body of unit not Bytes", }}, {name: "Array", schema: ` @@ -222,8 +222,8 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - `4:21: duplicate config "FTL_ENDPOINT", first defined at 3:20`, - `5:21: duplicate config "FTL_ENDPOINT", first defined at 3:20`, + `4:21: duplicate declaration of "FTL_ENDPOINT" at 3:20`, + `5:21: duplicate declaration of "FTL_ENDPOINT" at 3:20`, }, }, {name: "DuplicateSecrets", @@ -235,18 +235,10 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - `4:6: duplicate secret "MY_SECRET", first defined at 3:6`, - `5:6: duplicate secret "MY_SECRET", first defined at 3:6`, + `4:6: duplicate declaration of "MY_SECRET" at 3:6`, + `5:6: duplicate declaration of "MY_SECRET" at 3:6`, }, }, - {name: "ConfigAndSecretsWithSameName", - schema: ` - module one { - config FTL_ENDPOINT String - secret FTL_ENDPOINT String - } - `, - }, {name: "DuplicateDatabases", schema: ` module one { @@ -255,7 +247,7 @@ func TestValidate(t *testing.T) { } `, errs: []string{ - `4:6: duplicate database "MY_DB", first defined at 3:6`, + `4:6: duplicate declaration of "MY_DB" at 3:6`, }, }, {name: "ValueEnumMismatchedVariantTypes",