fix: ensure all refs except builtins are fully qualified (#1143)

This changes validation such that all refs except builtins, but
including self references must be fully qualified.

.golangci.yml

@@ -134,3 +134,4 @@ issues:
     - unused-parameter
     - "^loopclosure:"
     - 'shadow: declaration of "ctx" shadows declaration at'
+    - 'shadow: declaration of "ok" shadows declaration'

Justfile

@@ -72,4 +72,10 @@ npm-install:
 
 # Regenerate protos
 build-protos: npm-install
-  @mk {{SCHEMA_OUT}} : backend/schema -- "ftl-schema > {{SCHEMA_OUT}} && buf format -w && buf lint && cd backend/protos && buf generate"
+  @mk {{SCHEMA_OUT}} : backend/schema -- "ftl-schema > {{SCHEMA_OUT}} && buf format -w && buf lint && cd backend/protos && buf generate"
+
+integration-tests *test:
+  #!/bin/bash
+  set -euo pipefail
+  testName=${1:-}
+  go test -fullpath -count 1 -v -tags integration -run "$testName" ./integration

backend/controller/ingress/alias_test.go

@@ -17,10 +17,10 @@ func TestTransformFromAliasedFields(t *testing.T) {
 
 			data Test {
 				scalar String +alias json "bar"
-				inner Inner
-				array [Inner]
-				map {String: Inner}
-				optional Inner
+				inner test.Inner
+				array [test.Inner]
+				map {String: test.Inner}
+				optional test.Inner
 			}
 		}
 		`
@@ -77,10 +77,10 @@ func TestTransformToAliasedFields(t *testing.T) {
 
 			data Test {
 				scalar String +alias json "bar"
-				inner Inner
-				array [Inner]
-				map {String: Inner}
-				optional Inner
+				inner test.Inner
+				array [test.Inner]
+				map {String: test.Inner}
+				optional test.Inner
 			}
 		}
 		`

backend/controller/ingress/handler_test.go

@@ -42,16 +42,16 @@ func TestIngress(t *testing.T) {
 				foo String
 			}
 
-			verb getAlias(HttpRequest<AliasRequest>) HttpResponse<Empty, Empty>
+			verb getAlias(HttpRequest<test.AliasRequest>) HttpResponse<Empty, Empty>
 				+ingress http GET /getAlias
 
-			verb getPath(HttpRequest<PathParameterRequest>) HttpResponse<Empty, Empty>
+			verb getPath(HttpRequest<test.PathParameterRequest>) HttpResponse<Empty, Empty>
 				+ingress http GET /getPath/{username}
 
-			verb postMissingTypes(HttpRequest<MissingTypes>) HttpResponse<Empty, Empty>
+			verb postMissingTypes(HttpRequest<test.MissingTypes>) HttpResponse<Empty, Empty>
 				+ingress http POST /postMissingTypes
 
-			verb postJsonPayload(HttpRequest<JsonPayload>) HttpResponse<Empty, Empty>
+			verb postJsonPayload(HttpRequest<test.JsonPayload>) HttpResponse<Empty, Empty>
 				+ingress http POST /postJsonPayload
 		}
 	`)

backend/controller/ingress/ingress_test.go

@@ -79,7 +79,7 @@ func TestValidation(t *testing.T) {
 			schema:  `module test { data Test { mapValue {String: String} } }`,
 			request: obj{"mapValue": obj{"key1": "value1", "key2": "value2"}}},
 		{name: "DataRef",
-			schema:  `module test { data Nested { intValue Int } data Test { dataRef Nested } }`,
+			schema:  `module test { data Nested { intValue Int } data Test { dataRef test.Nested } }`,
 			request: obj{"dataRef": obj{"intValue": 10.0}}},
 		{name: "Optional",
 			schema:  `module test { data Test { intValue Int? } }`,
@@ -88,10 +88,10 @@ func TestValidation(t *testing.T) {
 			schema:  `module test { data Test { intValue Int? } }`,
 			request: obj{"intValue": 10.0}},
 		{name: "ArrayDataRef",
-			schema:  `module test { data Nested { intValue Int } data Test { arrayValue [Nested] } }`,
+			schema:  `module test { data Nested { intValue Int } data Test { arrayValue [test.Nested] } }`,
 			request: obj{"arrayValue": []any{obj{"intValue": 10.0}, obj{"intValue": 20.0}}}},
 		{name: "MapDataRef",
-			schema:  `module test { data Nested { intValue Int } data Test { mapValue {String: Nested} } }`,
+			schema:  `module test { data Nested { intValue Int } data Test { mapValue {String: test.Nested} } }`,
 			request: obj{"mapValue": obj{"key1": obj{"intValue": 10.0}, "key2": obj{"intValue": 20.0}}}},
 		{name: "OtherModuleRef",
 			schema:  `module other { data Other { intValue Int } } module test { data Test { otherRef other.Other } }`,

backend/controller/ingress/request_test.go

@@ -68,16 +68,16 @@ func TestBuildRequestBody(t *testing.T) {
 				foo String
 			}
 
-			verb getAlias(HttpRequest<AliasRequest>) HttpResponse<Empty, Empty>
+			verb getAlias(HttpRequest<test.AliasRequest>) HttpResponse<Empty, Empty>
 				+ingress http GET /getAlias
 
-			verb getPath(HttpRequest<PathParameterRequest>) HttpResponse<Empty, Empty>
+			verb getPath(HttpRequest<test.PathParameterRequest>) HttpResponse<Empty, Empty>
 				+ingress http GET /getPath/{username}
 
-			verb postMissingTypes(HttpRequest<MissingTypes>) HttpResponse<Empty, Empty>
+			verb postMissingTypes(HttpRequest<test.MissingTypes>) HttpResponse<Empty, Empty>
 				+ingress http POST /postMissingTypes
 
-			verb postJsonPayload(HttpRequest<JsonPayload>) HttpResponse<Empty, Empty>
+			verb postJsonPayload(HttpRequest<test.JsonPayload>) HttpResponse<Empty, Empty>
 				+ingress http POST /postJsonPayload
 		}
 	`)

backend/schema/data.go

@@ -199,7 +199,7 @@ func maybeMonomorphiseType(t Type, typeParameters map[string]Type) (Type, error)
 		if tp, ok := typeParameters[t.Name]; ok {
 			return tp, nil
 		}
-		return nil, fmt.Errorf("%s: unknown type parameter %q", t.Position(), t.Name)
+		return nil, fmt.Errorf("%s: unknown type parameter %q", t.Position(), t)
 	}
 	return t, nil
 }

backend/schema/module.go

@@ -8,6 +8,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/alecthomas/types/optional"
 	"golang.org/x/exp/maps"
 	"google.golang.org/protobuf/proto"
 
@@ -43,41 +44,14 @@ func (m *Module) Scan(src any) error {
 	}
 }
 
-// Scope returns a scope containing all the declarations in this module.
-func (m *Module) Scope() Scope {
-	scope := Scope{}
-	for _, d := range m.Decls {
-		switch d := d.(type) {
-		case *Data:
-			scope[d.Name] = ModuleDecl{m, d}
-
-		case *Verb:
-			scope[d.Name] = ModuleDecl{m, d}
-
-		case *Enum:
-			scope[d.Name] = ModuleDecl{m, d}
-
-		case *Config:
-			scope[d.Name] = ModuleDecl{m, d}
-
-		case *Secret:
-			scope[d.Name] = ModuleDecl{m, d}
-
-		case *Database:
-			scope[d.Name] = ModuleDecl{m, d}
-		}
-	}
-	return scope
-}
-
 // Resolve returns the declaration in this module with the given name, or nil
 func (m *Module) Resolve(ref Ref) *ModuleDecl {
 	if ref.Module != "" && ref.Module != m.Name {
 		return nil
 	}
 	for _, d := range m.Decls {
 		if d.GetName() == ref.Name {
-			return &ModuleDecl{m, d}
+			return &ModuleDecl{optional.Some(m), d}
 		}
 	}
 	return nil

backend/schema/schema_test.go

@@ -44,7 +44,7 @@ module todo {
   }
 
   verb create(todo.CreateRequest) todo.CreateResponse
-      +calls todo.destroy  +database calls todo.testdb 
+      +calls todo.destroy  +database calls todo.testdb
 
   verb destroy(builtin.HttpRequest<todo.DestroyRequest>) builtin.HttpResponse<todo.DestroyResponse, String>
       +ingress http GET /todo/destroy/{name}
@@ -81,7 +81,7 @@ func TestImports(t *testing.T) {
 		data Data {
 			ref other.Data
 			ref another.Data
-			ref Generic<new.Data>
+			ref test.Generic<new.Data>
 		}
 		verb myVerb(test.Data) test.Data
 			+calls verbose.verb
@@ -174,8 +174,8 @@ func TestParsing(t *testing.T) {
 					data CreateListResponse {}
 
 					// Create a new list
-					verb createList(todo.CreateListRequest) CreateListResponse
-						+calls createList
+					verb createList(todo.CreateListRequest) todo.CreateListResponse
+						+calls todo.createList
 				}
 			`,
 			expected: &Schema{
@@ -305,7 +305,7 @@ func TestParsing(t *testing.T) {
 						value T
 					}
 
-					verb test(Data<String>) Data<String>
+					verb test(test.Data<String>) test.Data<String>
 				}
 				`,
 			expected: &Schema{

backend/schema/typeresolver.go

@@ -2,8 +2,9 @@ package schema
 
 import (
 	"fmt"
-	"runtime/debug"
 	"strings"
+
+	"github.com/alecthomas/types/optional"
 )
 
 // Resolver may be implemented be a node in the AST to resolve references within itself.
@@ -22,7 +23,7 @@ type Scope map[string]ModuleDecl
 
 // ModuleDecl is a declaration associated with a module.
 type ModuleDecl struct {
-	Module *Module // May be nil.
+	Module optional.Option[*Module]
 	Symbol Symbol
 }
 
@@ -70,10 +71,14 @@ func NewScopes() Scopes {
 	builtins := Builtins()
 	// Empty scope tail for builtins.
 	scopes := Scopes{primitivesScope, Scope{}}
-	if err := scopes.Add(nil, builtins.Name, builtins); err != nil {
+	if err := scopes.Add(optional.None[*Module](), builtins.Name, builtins); err != nil {
 		panic(err)
 	}
-	scopes = scopes.PushScope(builtins.Scope())
+	for _, decl := range builtins.Decls {
+		if err := scopes.Add(optional.Some(builtins), decl.GetName(), decl); err != nil {
+			panic(err)
+		}
+	}
 	// Push an empty scope for other modules to be added to.
 	scopes = scopes.Push()
 	return scopes
@@ -111,11 +116,8 @@ func (s Scopes) Push() Scopes {
 }
 
 // Add a declaration to the current scope.
-func (s *Scopes) Add(owner *Module, name string, symbol Symbol) error {
+func (s *Scopes) Add(owner optional.Option[*Module], name string, symbol Symbol) error {
 	end := len(*s) - 1
-	if name == "destroy" {
-		debug.PrintStack()
-	}
 	if prev, ok := (*s)[end][name]; ok {
 		return fmt.Errorf("%s: duplicate declaration of %q at %s", symbol.Position(), name, prev.Symbol.Position())
 	}
@@ -132,7 +134,7 @@ func (s Scopes) ResolveType(t Type) *ModuleDecl {
 		return s.Resolve(*t)
 
 	case Symbol:
-		return &ModuleDecl{nil, t}
+		return &ModuleDecl{optional.None[*Module](), t}
 
 	default:
 		return nil
@@ -160,9 +162,11 @@ func (s Scopes) Resolve(ref Ref) *ModuleDecl {
 					return decl
 				}
 			} else {
-				for _, d := range mdecl.Module.Decls {
-					if d.GetName() == ref.Name {
-						return &ModuleDecl{mdecl.Module, d}
+				if module, ok := mdecl.Module.Get(); ok {
+					for _, d := range module.Decls {
+						if d.GetName() == ref.Name {
+							return &ModuleDecl{mdecl.Module, d}
+						}
 					}
 				}
 			}

backend/schema/typeresolver_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/alecthomas/assert/v2"
+	"github.com/alecthomas/types/optional"
 )
 
 func TestTypeResolver(t *testing.T) {
@@ -12,12 +13,13 @@ func TestTypeResolver(t *testing.T) {
 			data Request<T> {
 				t T
 			}
-			verb test(Request<String>) Empty
+			verb test(test.Request<String>) Empty
 		}
 	`)
 	assert.NoError(t, err)
 	scopes := NewScopes()
-	scopes = scopes.PushScope(module.Scope())
+	err = scopes.Add(optional.None[*Module](), module.Name, module)
+	assert.NoError(t, err)
 
 	// Resolve a builtin.
 	actualInt, _ := ResolveAs[*Int](scopes, Ref{Name: "Int"})