fix: handle refs with module = “”

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:

common/schema/typeresolver_test.go

@@ -14,12 +14,41 @@ func TestTypeResolver(t *testing.T) {
 				t T
 			}
 			verb test(test.Request<String>) 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"})

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,23 +269,24 @@ func ValidateModule(module *Module) error {
 		merr = append(merr, err)
 	}
 	scopes = scopes.Push()
-	// Key is <type>:<name>
-	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 {
 			pop := scopes
 			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() {

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<Any, Unit, Unit> must have a body of unit not Any",
-				"11:52: ingress verb any: response type HttpResponse<Any, Any> 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<Any, Unit, Unit> must have a body of unit not Any",
+				"11:52: ingress verb any: response type builtin.HttpResponse<Any, Any> 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<Bytes, Unit, Unit> must have a body of unit not Bytes",
+				"3:24: ingress verb bytes: GET request type builtin.HttpRequest<Bytes, Unit, Unit> 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",