diff --git a/buildengine/build_go_test.go b/buildengine/build_go_test.go index def7eea677..19903ca0ae 100644 --- a/buildengine/build_go_test.go +++ b/buildengine/build_go_test.go @@ -40,7 +40,7 @@ func TestExternalType(t *testing.T) { } testBuild(t, bctx, "", "unsupported external type", []assertion{ assertBuildProtoErrors( - `unsupported external type "time.Month"`, + `unsupported external type "time.Month"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`, `unsupported type "time.Month" for field "Month"`, `unsupported response type "ftl/external.ExternalResponse"`, ), diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index 37ba8ad9d9..23b3b79f60 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -793,10 +793,19 @@ func getGoExternalType(fqName string) (_import string, _type string, err error) if err != nil { return "", "", err } - pkg := im[strings.LastIndex(im, "/")+1:] + + var pkg string if i := strings.LastIndex(im, " "); i != -1 { // import has an alias and this will be the package pkg = im[:i] + im = im[i+1:] + } + unquoted, err := strconv.Unquote(im) + if err != nil { + return "", "", fmt.Errorf("failed to unquote import %q: %w", im, err) + } + if pkg == "" { + pkg = unquoted[strings.LastIndex(unquoted, "/")+1:] } typeName := fqName[strings.LastIndex(fqName, ".")+1:] return im, fmt.Sprintf("%s.%s", pkg, typeName), nil diff --git a/go-runtime/compile/schema_test.go b/go-runtime/compile/schema_test.go index abf3c665de..4416f56c70 100644 --- a/go-runtime/compile/schema_test.go +++ b/go-runtime/compile/schema_test.go @@ -577,11 +577,12 @@ func TestErrorReporting(t *testing.T) { // failing/child/child.go expectedChild := []string{ `9:2-6: unsupported type "uint64" for field "Body"`, - `14:2-2: unsupported external type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"`, + `14:2-2: unsupported external type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`, `14:2-7: unsupported type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.NonFTLType" for field "Field"`, `19:6-41: declared type github.com/blah.lib.NonFTLType in typemap does not match native type github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType`, `24:6-6: multiple Go type mappings found for "ftl/failing/child.MultipleMappings"`, `34:2-13: enum variant "SameVariant" conflicts with existing enum variant of "EnumVariantConflictParent" at "196:2"`, + `37:18-18: schema declaration with name "FTL_ENDPOINT" already exists for module "failing"; previously declared at "38:18"`, } assert.Equal(t, expectedParent, actualParent) assert.Equal(t, expectedChild, actualChild) diff --git a/go-runtime/schema/call/analyzer.go b/go-runtime/schema/call/analyzer.go index 153b7b3127..469118a917 100644 --- a/go-runtime/schema/call/analyzer.go +++ b/go-runtime/schema/call/analyzer.go @@ -63,8 +63,9 @@ func validateCallExpr(pass *analysis.Pass, node *ast.CallExpr) { } else { lhsPkgPath = lhsObject.Pkg().Path() } + var lhsIsExternal bool - if !common.IsPathInPkg(pass.Pkg, lhsPkgPath) { + if !common.IsPathInModule(pass.Pkg, lhsPkgPath) { lhsIsExternal = true } diff --git a/go-runtime/schema/common/common.go b/go-runtime/schema/common/common.go index a25a42eb8c..93fe1ca546 100644 --- a/go-runtime/schema/common/common.go +++ b/go-runtime/schema/common/common.go @@ -96,7 +96,7 @@ func runExtractDeclsFunc[T schema.Decl, N ast.Node](extractFunc ExtractDeclFunc[ if !ok { return } - if obj != nil && !IsPathInPkg(pass.Pkg, obj.Pkg().Path()) { + if obj != nil && !IsPathInModule(pass.Pkg, obj.Pkg().Path()) { return } md, ok := GetFactForObject[*ExtractedMetadata](pass, obj).Get() @@ -239,12 +239,16 @@ func IsType[T types.Type](t types.Type) bool { return ok } -// IsPathInPkg returns true if the given path is in the package. -func IsPathInPkg(pkg *types.Package, path string) bool { +// IsPathInModule returns true if the given path is in the module. +func IsPathInModule(pkg *types.Package, path string) bool { if path == pkg.Path() { return true } - return strings.HasPrefix(path, pkg.Path()+"/") + moduleName, err := FtlModuleFromGoPackage(pkg.Path()) + if err != nil { + return false + } + return strings.HasPrefix(path, "ftl/"+moduleName) } // GetObjectForNode returns the types.Object for the given node. @@ -324,9 +328,9 @@ func extractRef(pass *analysis.Pass, pos token.Pos, named *types.Named) optional } nodePath := named.Obj().Pkg().Path() - if !IsPathInPkg(pass.Pkg, nodePath) && IsExternalType(named.Obj()) { - //TODO: link to external type docs - NoEndColumnErrorf(pass, pos, "unsupported external type %q", GetNativeName(named.Obj())) + if !IsPathInModule(pass.Pkg, nodePath) && IsExternalType(named.Obj()) { + NoEndColumnErrorf(pass, pos, "unsupported external type %q; see FTL docs on using external types: %s", + GetNativeName(named.Obj()), "tbd54566975.github.io/ftl/docs/reference/externaltypes/") return optional.None[schema.Type]() } @@ -437,7 +441,7 @@ func ExtractTypeForNode(pass *analysis.Pass, obj types.Object, node ast.Node, in return ExtractType(pass, node.Pos(), pass.TypesInfo.TypeOf(typ.Sel)) } - if !IsPathInPkg(pass.Pkg, im.Path()) && !strings.HasPrefix(im.Path(), "ftl/") { + if !IsPathInModule(pass.Pkg, im.Path()) && !strings.HasPrefix(im.Path(), "ftl/") { // Non-FTL return optional.Some[schema.Type](&schema.Any{}) } diff --git a/go-runtime/schema/data/analyzer.go b/go-runtime/schema/data/analyzer.go index b0a1a0ab73..d9197e8760 100644 --- a/go-runtime/schema/data/analyzer.go +++ b/go-runtime/schema/data/analyzer.go @@ -41,7 +41,7 @@ func Extract(pass *analysis.Pass, node *ast.TypeSpec, obj types.Object) optional func extractData(pass *analysis.Pass, pos token.Pos, named *types.Named) optional.Option[*schema.Data] { fset := pass.Fset nodePath := named.Obj().Pkg().Path() - if !common.IsPathInPkg(pass.Pkg, nodePath) { + if !common.IsPathInModule(pass.Pkg, nodePath) { return optional.None[*schema.Data]() } diff --git a/go-runtime/schema/extract.go b/go-runtime/schema/extract.go index 6332b68846..6610a48123 100644 --- a/go-runtime/schema/extract.go +++ b/go-runtime/schema/extract.go @@ -145,7 +145,8 @@ func combineAllPackageResults(results map[*analysis.Analyzer][]any, diagnostics refResults := make(map[schema.RefKey]refResult) extractedDecls := make(map[schema.Decl]types.Object) // for identifying duplicates - declKeys := make(map[string]tuple.Pair[types.Object, schema.Position]) + typeUniqueness := make(map[string]tuple.Pair[types.Object, schema.Position]) + globalUniqueness := make(map[string]tuple.Pair[types.Object, schema.Position]) for _, r := range fResults { fr, ok := r.(finalize.Result) if !ok { @@ -163,15 +164,23 @@ func combineAllPackageResults(results map[*analysis.Analyzer][]any, diagnostics } copyFailedRefs(refResults, fr.Failed) for decl, obj := range fr.Extracted { - key := getDeclKey(decl) - if value, ok := declKeys[key]; ok && value.A != obj { + // check for duplicates and add the Decl to the module schema + typename := common.GetDeclTypeName(decl) + typeKey := fmt.Sprintf("%s-%s", typename, decl.GetName()) + if value, ok := typeUniqueness[typeKey]; ok && value.A != obj { // decls redeclared in subpackage combined.Errors = append(combined.Errors, schema.Errorf(decl.Position(), decl.Position().Column, - "duplicate %s declaration for %q; already declared at %q", common.GetDeclTypeName(decl), + "duplicate %s declaration for %q; already declared at %q", typename, combined.Module.Name+"."+decl.GetName(), value.B)) continue } - declKeys[key] = tuple.Pair[types.Object, schema.Position]{A: obj, B: decl.Position()} + if value, ok := globalUniqueness[decl.GetName()]; ok && value.A != obj { + combined.Errors = append(combined.Errors, schema.Errorf(decl.Position(), decl.Position().Column, + "schema declaration with name %q already exists for module %q; previously declared at %q", + decl.GetName(), combined.Module.Name, value.B)) + } + typeUniqueness[typeKey] = tuple.Pair[types.Object, schema.Position]{A: obj, B: decl.Position()} + globalUniqueness[decl.GetName()] = tuple.Pair[types.Object, schema.Position]{A: obj, B: decl.Position()} extractedDecls[decl] = obj } maps.Copy(combined.NativeNames, fr.NativeNames) @@ -350,19 +359,3 @@ func goQualifiedNameForWidenedType(obj types.Object, metadata []schema.Metadata) } return nativeName, nil } - -// criteria for uniqueness within a given decl type -// used for detecting duplicate declarations -func getDeclKey(decl schema.Decl) string { - typename := common.GetDeclTypeName(decl) - switch d := decl.(type) { - case *schema.Config: - return fmt.Sprintf("%s-%s-%s", typename, d.Name, d.Type) - case *schema.Secret: - return fmt.Sprintf("%s-%s-%s", typename, d.Name, d.Type) - case *schema.Topic: - return fmt.Sprintf("%s-%s-%s", typename, d.Name, d.Event) - default: - return fmt.Sprintf("%s-%s", typename, d.GetName()) - } -} diff --git a/go-runtime/schema/verb/analyzer.go b/go-runtime/schema/verb/analyzer.go index 7bcad54d71..ff91515548 100644 --- a/go-runtime/schema/verb/analyzer.go +++ b/go-runtime/schema/verb/analyzer.go @@ -3,6 +3,7 @@ package verb import ( "go/ast" "go/types" + "unicode" "github.com/alecthomas/types/optional" @@ -63,9 +64,12 @@ func Extract(pass *analysis.Pass, root *ast.FuncDecl, obj types.Object) optional } func checkSignature(pass *analysis.Pass, node *ast.FuncDecl, sig *types.Signature) (req, resp optional.Option[*types.Var]) { - if expVerbName := strcase.ToUpperCamel(node.Name.Name); node.Name.Name != expVerbName { - common.Errorf(pass, node, "unexpected verb name %q, did you mean to use %q instead?", node.Name.Name, - expVerbName) + if node.Name.Name == "" { + common.Errorf(pass, node, "verb function must be named") + return optional.None[*types.Var](), optional.None[*types.Var]() + } + if !unicode.IsUpper(rune(node.Name.Name[0])) { + common.Errorf(pass, node, "verb name must be exported") return optional.None[*types.Var](), optional.None[*types.Var]() }