Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,4 @@ issues:
- unused-parameter
- "^loopclosure:"
- 'shadow: declaration of "ctx" shadows declaration at'
- 'shadow: declaration of "ok" shadows declaration'
8 changes: 7 additions & 1 deletion Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 8 additions & 8 deletions backend/controller/ingress/alias_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
`
Expand Down Expand Up @@ -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
}
}
`
Expand Down
8 changes: 4 additions & 4 deletions backend/controller/ingress/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
`)
Expand Down
6 changes: 3 additions & 3 deletions backend/controller/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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? } }`,
Expand All @@ -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 } }`,
Expand Down
8 changes: 4 additions & 4 deletions backend/controller/ingress/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
`)
Expand Down
2 changes: 1 addition & 1 deletion backend/schema/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
30 changes: 2 additions & 28 deletions backend/schema/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sort"
"strings"

"github.com/alecthomas/types/optional"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/proto"

Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions backend/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
28 changes: 16 additions & 12 deletions backend/schema/typeresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
Expand All @@ -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
Expand Down Expand Up @@ -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}
}
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions backend/schema/typeresolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/alecthomas/assert/v2"
"github.com/alecthomas/types/optional"
)

func TestTypeResolver(t *testing.T) {
Expand All @@ -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"})
Expand Down
Loading