fix: some fixes! (#2066)

- fixes a code gen issue for types.ftl.go
- validate global uniquness on Decl names (required because ref->Decl
resolution breaks if two Decls have the same name, even of different
types)
- fixes logic that determines if path is local because pass.Pkg may not
be root module package

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"`,
 		),

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

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)

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
 	}
 

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{})
 				}

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]()
 	}
 

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())
-	}
-}

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]()
 	}