fix: monomorphising would fail under some circumstances (#880)

I originally intended to replace `DataRef` references to type parameters, with concrete `TypeParameter` instances, but this ended up conflating the parameter declaration and the reference. Instead, it's now left as a `DataRef`, and I've made `TypeParameter` no longer implement `Type`. Note that it being a `DataRef` doesn't make much sense either, because it's not a data structure, but that's part of a large issue where data references can also refer to verbs within the schema parsing. The correct solution here is to use an untyped `Ref` everwhere and resolve these to the correct reference types when validating. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
block · Feb 6, 2024 · bcbf8f7 · bcbf8f7
1 parent 2e61718
commit bcbf8f7
Show file tree

Hide file tree

Showing 12 changed files with 48 additions and 135 deletions.
diff --git a/backend/controller/ingress/ingress.go b/backend/controller/ingress/ingress.go
@@ -422,9 +422,6 @@ func validateValue(fieldType schema.Type, path path, value any, sch *schema.Sche
 		} else {
 			return validateValue(fieldType.Type, path, value, sch)
 		}
-
-	case *schema.TypeParameter:
-		panic("data structures with type parameters should be monomorphised")
 	}
 
 	if !typeMatches {
@@ -457,10 +454,10 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
 				field = f
 			}
 			for _, typeParam := range data.TypeParameters {
-				if typeParam.String() == key {
+				if typeParam.Name == key {
 					field = &schema.Field{
 						Name: key,
-						Type: typeParam,
+						Type: &schema.DataRef{Pos: typeParam.Pos, Name: typeParam.Name},
 					}
 				}
 			}
@@ -473,7 +470,7 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
 
 		switch field.Type.(type) {
 		case *schema.Bytes, *schema.Map, *schema.Optional, *schema.Time,
-			*schema.Unit, *schema.DataRef, *schema.Any, *schema.TypeParameter:
+			*schema.Unit, *schema.DataRef, *schema.Any:
 
 		case *schema.Int, *schema.Float, *schema.String, *schema.Bool:
 			if len(value) > 1 {

diff --git a/backend/schema/builtin.go b/backend/schema/builtin.go
@@ -1,111 +1,41 @@
 package schema
 
-import (
-	"golang.design/x/reflect"
-)
+import "golang.design/x/reflect"
+
+// BuiltinsSource is the schema source code for built-in types.
+const BuiltinsSource = `
+// Built-in types for FTL.
+builtin module builtin {
+  // HTTP request structure used for HTTP ingress verbs.
+  data HttpRequest<Body> {
+    method String
+    path String
+    pathParameters {String: String}
+    query {String: [String]}
+    headers {String: [String]}
+    body Body
+  }
+
+  // HTTP response structure used for HTTP ingress verbs.
+  data HttpResponse<Body> {
+    status Int
+    headers {String: [String]}
+    body Body
+  }
+
+  data Empty {}
+}
+`
+
+var builtinsModuleParsed = func() *Module {
+	module, err := moduleParser.ParseString("", BuiltinsSource)
+	if err != nil {
+		panic(err)
+	}
+	return module
+}()
 
 // Builtins returns a [Module] containing built-in types.
 func Builtins() *Module {
-	return reflect.DeepCopy(&Module{
-		Comments: []string{
-			"Built-in types for FTL.",
-		},
-		Builtin: true,
-		Name:    "builtin",
-		Decls: []Decl{
-			&Data{
-				Comments: []string{
-					"HTTP request structure used for HTTP ingress verbs.",
-				},
-				Name: "HttpRequest",
-				TypeParameters: []*TypeParameter{
-					{
-						Name: "Body",
-					},
-				},
-				Fields: []*Field{
-					{
-						Comments: []string{},
-						Name:     "method",
-						Type:     &String{},
-					},
-					{
-						Comments: []string{},
-						Name:     "path",
-						Type:     &String{},
-					},
-					{
-						Comments: []string{},
-						Name:     "pathParameters",
-						Type: &Map{
-							Key:   &String{},
-							Value: &String{},
-						},
-					},
-					{
-						Comments: []string{},
-						Name:     "query",
-						Type: &Map{
-							Key: &String{},
-							Value: &Array{
-								Element: &String{},
-							},
-						},
-					},
-					{
-						Comments: []string{},
-						Name:     "headers",
-						Type: &Map{
-							Key: &String{},
-							Value: &Array{
-								Element: &String{},
-							},
-						},
-					},
-					{
-						Comments: []string{},
-						Name:     "body",
-						Type: &TypeParameter{
-							Name: "Body",
-						},
-					},
-				},
-			},
-			&Data{
-				Comments: []string{
-					"HTTP response structure used for HTTP ingress verbs.",
-				},
-				Name: "HttpResponse",
-				TypeParameters: []*TypeParameter{
-					{
-						Name: "Body",
-					},
-				},
-				Fields: []*Field{
-					{
-						Comments: []string{},
-						Name:     "status",
-						Type:     &Int{},
-					},
-					{
-						Comments: []string{},
-						Name:     "headers",
-						Type: &Map{
-							Key: &String{},
-							Value: &Array{
-								Element: &String{},
-							},
-						},
-					},
-					{
-						Comments: []string{},
-						Name:     "body",
-						Type: &TypeParameter{
-							Name: "Body",
-						},
-					},
-				},
-			},
-		},
-	})
+	return reflect.DeepCopy(builtinsModuleParsed)
 }
diff --git a/backend/schema/data.go b/backend/schema/data.go
@@ -34,7 +34,7 @@ func (d *Data) Scope() Scope {
 
 // Monomorphise this data type with the given type arguments.
 //
-// Will return nil if it is not a parametric type.
+// If this data type has no type parameters, it will be returned as-is.
 //
 // This will return a new Data structure with all type parameters replaced with
 // the given types.
@@ -43,7 +43,7 @@ func (d *Data) Monomorphise(types ...Type) (*Data, error) {
 		return nil, fmt.Errorf("expected %d type arguments, got %d", len(d.TypeParameters), len(types))
 	}
 	if len(d.TypeParameters) == 0 {
-		return nil, nil
+		return d, nil
 	}
 	names := map[string]Type{}
 	for i, t := range d.TypeParameters {
@@ -166,8 +166,8 @@ func DataToSchema(s *schemapb.Data) *Data {
 
 // MonoType returns the monomorphised type of this data type if applicable, or returns the original type.
 func maybeMonomorphiseType(t Type, typeParameters map[string]Type) (Type, error) {
-	if t, ok := t.(*TypeParameter); ok {
-		if tp, ok := typeParameters[t.Name]; ok {
+	if t, ok := t.(*DataRef); ok {
+		if tp, ok := typeParameters[t.Name]; ok && t.Module == "" {
 			return tp, nil
 		}
 		return nil, fmt.Errorf("%s: unknown type parameter %q", t.Position(), t.Name)

diff --git a/backend/schema/data_test.go b/backend/schema/data_test.go
@@ -11,7 +11,7 @@ func TestMonomorphisation(t *testing.T) {
 		Name:           "Data",
 		TypeParameters: []*TypeParameter{{Name: "T"}},
 		Fields: []*Field{
-			{Name: "a", Type: &TypeParameter{Name: "T"}},
+			{Name: "a", Type: &DataRef{Name: "T"}},
 		},
 	}
 	actual, err := data.Monomorphise(&String{})

diff --git a/backend/schema/jsonschema_test.go b/backend/schema/jsonschema_test.go
@@ -39,8 +39,8 @@ var jsonSchemaSample = &Schema{
 				Name:           "Generic",
 				TypeParameters: []*TypeParameter{{Name: "K"}, {Name: "V"}},
 				Fields: []*Field{
-					{Name: "key", Type: &TypeParameter{Name: "K"}},
-					{Name: "value", Type: &TypeParameter{Name: "V"}},
+					{Name: "key", Type: &DataRef{Name: "K"}},
+					{Name: "value", Type: &DataRef{Name: "V"}},
 				},
 			},
 		}},

diff --git a/backend/schema/parser.go b/backend/schema/parser.go
@@ -15,7 +15,7 @@ var (
 	declUnion            = []Decl{&Data{}, &Verb{}, &Database{}}
 	nonOptionalTypeUnion = []Type{
 		&Int{}, &Float{}, &String{}, &Bytes{}, &Bool{}, &Time{}, &Array{},
-		&Map{}, &DataRef{}, &Unit{}, &Any{}, &TypeParameter{},
+		&Map{}, &DataRef{}, &Unit{}, &Any{},
 	}
 	typeUnion     = append(nonOptionalTypeUnion, &Optional{})
 	metadataUnion = []Metadata{&MetadataCalls{}, &MetadataIngress{}, &MetadataDatabases{}}

diff --git a/backend/schema/protobuf_dec.go b/backend/schema/protobuf_dec.go
@@ -60,8 +60,6 @@ func typeToSchema(s *schemapb.Type) Type {
 		return &Unit{Pos: posFromProto(s.Unit.Pos)}
 	case *schemapb.Type_Any:
 		return &Any{Pos: posFromProto(s.Any.Pos)}
-	case *schemapb.Type_Parameter:
-		return &TypeParameter{Pos: posFromProto(s.Parameter.Pos), Name: s.Parameter.Name}
 	}
 	panic(fmt.Sprintf("unhandled type: %T", s.Value))
 }

diff --git a/backend/schema/protobuf_enc.go b/backend/schema/protobuf_enc.go
@@ -121,9 +121,6 @@ func typeToProto(t Type) *schemapb.Type {
 
 	case *Optional:
 		return &schemapb.Type{Value: &schemapb.Type_Optional{Optional: t.ToProto().(*schemapb.Optional)}}
-
-	case *TypeParameter:
-		return &schemapb.Type{Value: &schemapb.Type_Parameter{Parameter: t.ToProto().(*schemapb.TypeParameter)}}
 	}
 	panic(fmt.Sprintf("unhandled type: %T", t))
 }
diff --git a/backend/schema/schema.go b/backend/schema/schema.go
@@ -63,11 +63,7 @@ func (s *Schema) ResolveDataRefMonomorphised(ref *DataRef) (*Data, error) {
 		return nil, fmt.Errorf("unknown data %v", ref)
 	}
 
-	if len(ref.TypeParameters) > 0 {
-		return data.Monomorphise(ref.TypeParameters...)
-	}
-
-	return data, nil
+	return data.Monomorphise(ref.TypeParameters...)
 }
 
 func (s *Schema) ResolveVerbRef(ref *VerbRef) *Verb {

diff --git a/backend/schema/typeparameter.go b/backend/schema/typeparameter.go
@@ -12,10 +12,8 @@ type TypeParameter struct {
 	Name string `parser:"@Ident" protobuf:"2"`
 }
 
-var _ Type = (*TypeParameter)(nil)
 var _ Decl = (*TypeParameter)(nil)
 
-func (*TypeParameter) schemaType()          {}
 func (t *TypeParameter) Position() Position { return t.Pos }
 func (t *TypeParameter) String() string     { return t.Name }
 func (t *TypeParameter) ToProto() protoreflect.ProtoMessage {

diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go
@@ -224,9 +224,6 @@ func genType(module *schema.Module, t schema.Type) string {
 
 	case *schema.Bytes:
 		return "[]byte"
-
-	case *schema.TypeParameter:
-		return t.Name
 	}
 	panic(fmt.Sprintf("unsupported type %T", t))
 }

diff --git a/go-runtime/compile/schema.go b/go-runtime/compile/schema.go
@@ -412,7 +412,7 @@ func visitStruct(pctx *parseContext, node ast.Node, tnode types.Type) (*schema.D
 
 func visitType(pctx *parseContext, node ast.Node, tnode types.Type) (schema.Type, error) {
 	if tparam, ok := tnode.(*types.TypeParam); ok {
-		return &schema.TypeParameter{Name: tparam.Obj().Id()}, nil
+		return &schema.DataRef{Pos: goPosToSchemaPos(node.Pos()), Name: tparam.Obj().Id()}, nil
 	}
 	switch underlying := tnode.Underlying().(type) {
 	case *types.Basic: