From 0e67d27b44b76e5c1539a3e38ce4f87d1da3deeb Mon Sep 17 00:00:00 2001 From: Akonwi Ngoh Date: Sat, 25 Jul 2026 00:30:37 -0400 Subject: [PATCH] fix(checker): reject private types in public APIs Fixes #337 --- compiler/checker/checker.go | 1 + compiler/checker/diagnostics.go | 1 + compiler/checker/public_api.go | 344 ++++++++++++++++++++++ compiler/checker/public_api_test.go | 227 ++++++++++++++ compiler/go/backend_test.go | 2 +- compiler/go/parity_test.go | 6 +- website/src/content/docs/guide/modules.md | 13 + 7 files changed, 590 insertions(+), 4 deletions(-) create mode 100644 compiler/checker/public_api.go create mode 100644 compiler/checker/public_api_test.go diff --git a/compiler/checker/checker.go b/compiler/checker/checker.go index 667cbffe..d2ea6569 100644 --- a/compiler/checker/checker.go +++ b/compiler/checker/checker.go @@ -623,6 +623,7 @@ func (c *Checker) Check() { c.checkStructFieldMapKeyTypes() c.checkRecursiveStructLayouts() c.checkGenericInstantiationCycles() + c.validatePublicAPI() // now that we're done with the aliases, use module paths for the import keys for alias, mod := range c.program.Imports { diff --git a/compiler/checker/diagnostics.go b/compiler/checker/diagnostics.go index e6358934..d85aafc7 100644 --- a/compiler/checker/diagnostics.go +++ b/compiler/checker/diagnostics.go @@ -51,6 +51,7 @@ const ( DiagnosticCodeGenericInstantiationCycle DiagnosticCode = "generic_instantiation_cycle" DiagnosticCodeMethodIntroducedGeneric DiagnosticCode = "method_introduced_generic_parameter" DiagnosticCodeInvalidMapKeyType DiagnosticCode = "invalid_map_key_type" + DiagnosticCodePrivateTypeExposure DiagnosticCode = "private_type_exposure" DiagnosticCodeMalformedTypeNode DiagnosticCode = "internal_malformed_type_node" DiagnosticCodeBranchTypeMismatch DiagnosticCode = "branch_type_mismatch" DiagnosticCodeNonExhaustiveValueIf DiagnosticCode = "non_exhaustive_value_if" diff --git a/compiler/checker/public_api.go b/compiler/checker/public_api.go new file mode 100644 index 00000000..6c7f7317 --- /dev/null +++ b/compiler/checker/public_api.go @@ -0,0 +1,344 @@ +package checker + +import ( + "fmt" + "reflect" + "sort" + + "github.com/akonwi/ard/parse" +) + +// validatePublicAPI ensures every type exposed by this module can be named by +// importing modules. Public nominal declarations are trusted at their boundary: +// their owning module validates their own fields and signatures separately. +func (c *Checker) validatePublicAPI() { + locations := publicDeclarationLocations(c.input) + fieldLocations := publicFieldLocations(c.input) + aliases := publicTypeAliases(c.input) + variables := topLevelVariableNames(c.input) + + for _, name := range sortedBoolMapKeys(aliases) { + public := aliases[name] + if public { + if symbol := c.scope.symbols[name]; symbol != nil { + c.validatePublicType("public type alias `"+name+"`", symbol.Type, locations[name]) + } + } + } + + for _, name := range sortedSymbolNames(c.scope.symbols) { + symbol := c.scope.symbols[name] + if _, alias := aliases[name]; alias { + continue + } + if variables[name] { + continue + } + loc := locations[name] + switch declaration := symbol.Type.(type) { + case *FunctionDef: + if !declaration.Private && !declaration.IsTest { + c.validatePublicFunction("function `"+name+"`", declaration, loc) + } + case *StructDef: + canonical := canonicalStructDefinition(declaration) + if canonical.Private || canonical.ModulePath != "" && canonical.ModulePath != c.modulePath { + continue + } + for _, field := range sortedTypeMapKeys(canonical.Fields) { + fieldLoc := loc + if byField := fieldLocations[canonical.Name]; byField != nil { + if specific, ok := byField[field]; ok { + fieldLoc = specific + } + } + c.validatePublicType(fmt.Sprintf("public field `%s`", field), canonical.Fields[field], fieldLoc) + } + owner := StructMethodOwner(canonical) + for _, methodName := range sortedFunctionMapKeys(c.program.StructMethods[owner]) { + method := c.program.StructMethods[owner][methodName] + if !method.Private { + c.validatePublicFunction(fmt.Sprintf("public method `%s.%s`", canonical.Name, methodName), method, loc) + } + } + case *Trait: + if declaration.private || declaration.ModulePath != "" && declaration.ModulePath != c.modulePath { + continue + } + for i := range declaration.methods { + method := &declaration.methods[i] + c.validatePublicFunction(fmt.Sprintf("public trait method `%s.%s`", declaration.Name, method.Name), method, loc) + } + case *Enum: + if declaration.Private || declaration.ModulePath != "" && declaration.ModulePath != c.modulePath { + continue + } + for _, methodName := range sortedFunctionMapKeys(declaration.Methods) { + method := declaration.Methods[methodName] + if !method.Private { + c.validatePublicFunction(fmt.Sprintf("public method `%s.%s`", declaration.Name, methodName), method, loc) + } + } + case *Union: + if declaration.Private || declaration.ModulePath != "" && declaration.ModulePath != c.modulePath { + continue + } + for _, member := range declaration.Types { + c.validatePublicType("public union `"+declaration.Name+"`", member, loc) + } + } + } + + for _, statement := range c.program.Statements { + variable, ok := statement.Stmt.(*VariableDef) + if !ok || variable.Mutable { + continue + } + c.validatePublicType("public variable `"+variable.Name+"`", variable.__type, locations[variable.Name]) + } +} + +func (c *Checker) validatePublicFunction(context string, function *FunctionDef, loc parse.Location) { + for _, parameter := range function.Parameters { + parameterLoc := loc + if parameter.Loc != (parse.Location{}) { + parameterLoc = parameter.Loc + } + c.validatePublicType(fmt.Sprintf("%s parameter `%s`", context, parameter.Name), parameter.Type, parameterLoc) + } + c.validatePublicType(context+" return type", function.ReturnType, loc) +} + +func (c *Checker) validatePublicType(context string, exposed Type, loc parse.Location) { + if privateName := privateNominalType(exposed, map[uintptr]bool{}); privateName != "" { + message := fmt.Sprintf("%s exposes private type `%s`", context, privateName) + c.addDiagnostic(Diagnostic{ + Kind: Error, + Code: DiagnosticCodePrivateTypeExposure, + Message: message, + Title: "Public API exposes private type", + Text: "Private types cannot appear in declarations visible outside their module.", + Primary: DiagnosticLabel{Span: c.sourceSpan(loc), Message: message}, + }) + } +} + +func privateNominalType(current Type, seen map[uintptr]bool) string { + if current == nil { + return "" + } + value := reflect.ValueOf(current) + if value.Kind() == reflect.Pointer && !value.IsNil() { + identity := value.Pointer() + if seen[identity] { + return "" + } + seen[identity] = true + } + + switch value := current.(type) { + case *MutableRef: + return privateNominalType(value.of, seen) + case *Maybe: + return privateNominalType(value.of, seen) + case *Result: + if name := privateNominalType(value.val, seen); name != "" { + return name + } + return privateNominalType(value.err, seen) + case *List: + return privateNominalType(value.of, seen) + case *FixedArray: + return privateNominalType(value.of, seen) + case *Chan: + return privateNominalType(value.of, seen) + case *Receiver: + return privateNominalType(value.of, seen) + case *Sender: + return privateNominalType(value.of, seen) + case *Map: + if name := privateNominalType(value.key, seen); name != "" { + return name + } + return privateNominalType(value.value, seen) + case *FunctionDef: + for _, parameter := range value.Parameters { + if name := privateNominalType(parameter.Type, seen); name != "" { + return name + } + } + return privateNominalType(value.ReturnType, seen) + case *TypeVar: + return privateNominalType(value.actual, seen) + case *StructDef: + canonical := canonicalStructDefinition(value) + if canonical.Private { + return canonical.Name + } + for _, argument := range value.TypeArgs { + if name := privateNominalType(argument, seen); name != "" { + return name + } + } + return "" + case *Enum: + if value.Private { + return value.Name + } + return "" + case *Union: + if value.Private { + return value.Name + } + return "" + case *Trait: + if value.private { + return value.Name + } + return "" + case *ForeignType: + for _, argument := range value.TypeArgs { + if name := privateNominalType(argument, seen); name != "" { + return name + } + } + } + return "" +} + +func sortedBoolMapKeys(values map[string]bool) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func sortedSymbolNames(values map[string]*Symbol) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func sortedTypeMapKeys(values map[string]Type) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func sortedFunctionMapKeys(values map[string]*FunctionDef) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func topLevelVariableNames(program *parse.Program) map[string]bool { + variables := map[string]bool{} + if program == nil { + return variables + } + for _, statement := range program.Statements { + switch declaration := statement.(type) { + case parse.VariableDeclaration: + variables[declaration.Name] = true + case *parse.VariableDeclaration: + variables[declaration.Name] = true + } + } + return variables +} + +func publicFieldLocations(program *parse.Program) map[string]map[string]parse.Location { + locations := map[string]map[string]parse.Location{} + if program == nil { + return locations + } + record := func(declaration parse.StructDefinition) { + fields := map[string]parse.Location{} + for _, field := range declaration.Fields { + if field.Type != nil { + fields[field.Name.Name] = field.Type.GetLocation() + } + } + locations[declaration.Name.Name] = fields + } + for _, statement := range program.Statements { + switch declaration := statement.(type) { + case parse.StructDefinition: + record(declaration) + case *parse.StructDefinition: + record(*declaration) + } + } + return locations +} + +func publicTypeAliases(program *parse.Program) map[string]bool { + aliases := map[string]bool{} + if program == nil { + return aliases + } + for _, statement := range program.Statements { + switch declaration := statement.(type) { + case parse.TypeDeclaration: + if len(declaration.Type) == 1 { + aliases[declaration.Name.Name] = !declaration.Private + } + case *parse.TypeDeclaration: + if len(declaration.Type) == 1 { + aliases[declaration.Name.Name] = !declaration.Private + } + } + } + return aliases +} + +func publicDeclarationLocations(program *parse.Program) map[string]parse.Location { + locations := map[string]parse.Location{} + if program == nil { + return locations + } + for _, statement := range program.Statements { + switch declaration := statement.(type) { + case parse.FunctionDeclaration: + locations[declaration.Name] = declaration.Location + case *parse.FunctionDeclaration: + locations[declaration.Name] = declaration.Location + case parse.StaticFunctionDeclaration: + locations[declaration.Path.String()] = declaration.Location + case *parse.StaticFunctionDeclaration: + locations[declaration.Path.String()] = declaration.Location + case parse.StructDefinition: + locations[declaration.Name.Name] = declaration.Location + case *parse.StructDefinition: + locations[declaration.Name.Name] = declaration.Location + case parse.TraitDefinition: + locations[declaration.Name.Name] = declaration.Location + case *parse.TraitDefinition: + locations[declaration.Name.Name] = declaration.Location + case parse.EnumDefinition: + locations[declaration.Name] = declaration.Location + case *parse.EnumDefinition: + locations[declaration.Name] = declaration.Location + case parse.TypeDeclaration: + locations[declaration.Name.Name] = declaration.Location + case *parse.TypeDeclaration: + locations[declaration.Name.Name] = declaration.Location + case parse.VariableDeclaration: + locations[declaration.Name] = declaration.Location + case *parse.VariableDeclaration: + locations[declaration.Name] = declaration.Location + } + } + return locations +} diff --git a/compiler/checker/public_api_test.go b/compiler/checker/public_api_test.go new file mode 100644 index 00000000..e258ddab --- /dev/null +++ b/compiler/checker/public_api_test.go @@ -0,0 +1,227 @@ +package checker_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/akonwi/ard/checker" + "github.com/akonwi/ard/parse" +) + +func TestPublicStructFieldCannotExposePrivateType(t *testing.T) { + assertPrivateTypeExposure(t, ` +private struct Hidden { + value: Int, +} + +struct Public { + hidden: Hidden?, +} +`, "public field `hidden` exposes private type `Hidden`") +} + +func TestPublicAPICannotExposeNestedPrivateTypes(t *testing.T) { + tests := []struct { + name string + source string + want string + }{ + { + name: "function return", + source: ` +private struct Hidden {} +fn expose() [Hidden] { [] } +`, + want: "function `expose` return type exposes private type `Hidden`", + }, + { + name: "function parameter callback", + source: ` +private struct Hidden {} +fn consume(callback: fn(Hidden) Int) { callback(Hidden{}) } +`, + want: "function `consume` parameter `callback` exposes private type `Hidden`", + }, + { + name: "generic argument", + source: ` +private struct Hidden {} +struct Box<$T> { value: $T } +fn expose() Box { Box{value: Hidden{}} } +`, + want: "function `expose` return type exposes private type `Hidden`", + }, + { + name: "inferred global", + source: ` +private struct Hidden {} +let state = Hidden{} +`, + want: "public variable `state` exposes private type `Hidden`", + }, + { + name: "union member", + source: ` +private struct Hidden {} +type Public = Int | Hidden +`, + want: "public union `Public` exposes private type `Hidden`", + }, + { + name: "trait method", + source: ` +private struct Hidden {} +trait Public { + fn reveal() Hidden +} +`, + want: "public trait method `Public.reveal` return type exposes private type `Hidden`", + }, + { + name: "struct method", + source: ` +private struct Hidden {} +struct Public {} +impl Public { + fn reveal() Hidden { Hidden{} } +} +`, + want: "public method `Public.reveal` return type exposes private type `Hidden`", + }, + { + name: "public alias", + source: ` +private struct Hidden {} +type Public = Hidden? +`, + want: "public type alias `Public` exposes private type `Hidden`", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assertPrivateTypeExposure(t, test.source, test.want) + }) + } +} + +func TestPublicAPIDoesNotTreatValuesAsNominalDeclarations(t *testing.T) { + result := parse.Parse([]byte(` +private struct Hidden {} +struct Public { hidden: Hidden } +let value = Public{hidden: Hidden{}} +`), "model.ard") + if len(result.Errors) > 0 { + t.Fatalf("parse errors: %v", result.Errors) + } + c := checker.New("model.ard", result.Program, nil) + c.Check() + count := 0 + for _, diagnostic := range c.Diagnostics() { + if diagnostic.Code == checker.DiagnosticCodePrivateTypeExposure { + count++ + } + } + if count != 1 { + t.Fatalf("private type exposure diagnostics = %d, want 1: %v", count, c.Diagnostics()) + } +} + +func TestPublicGenericDeclarationMayExposeItsTypeParameter(t *testing.T) { + result := parse.Parse([]byte(` +struct Box<$T> { value: $T } +fn wrap(value: $T) Box<$T> { Box<$T>{value: value} } +`), "model.ard") + if len(result.Errors) > 0 { + t.Fatalf("parse errors: %v", result.Errors) + } + c := checker.New("model.ard", result.Program, nil) + c.Check() + if c.HasErrors() { + t.Fatalf("unexpected diagnostics: %v", c.Diagnostics()) + } +} + +func TestImportedModuleRejectsPrivateTypeInPublicField(t *testing.T) { + projectDir := t.TempDir() + if err := os.WriteFile(filepath.Join(projectDir, "ard.toml"), []byte("name = \"privatefield\"\nard = \">= 0.1.0\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(projectDir, "model.ard"), []byte(` +private struct Hidden { value: Int } +struct Public { hidden: Hidden? } +fn make() Public { Public{hidden: Hidden{value: 1}} } +`), 0o644); err != nil { + t.Fatal(err) + } + mainPath := filepath.Join(projectDir, "main.ard") + if err := os.WriteFile(mainPath, []byte("use privatefield/model\nfn main() { let value = model::make() }\n"), 0o644); err != nil { + t.Fatal(err) + } + mainSource, err := os.ReadFile(mainPath) + if err != nil { + t.Fatal(err) + } + result := parse.Parse(mainSource, mainPath) + if len(result.Errors) > 0 { + t.Fatalf("parse errors: %v", result.Errors) + } + resolver, err := checker.NewModuleResolver(projectDir) + if err != nil { + t.Fatal(err) + } + c := checker.New(mainPath, result.Program, resolver) + c.Check() + if !c.HasErrors() { + t.Fatal("checker succeeded; expected imported module private type exposure diagnostic") + } + for _, diagnostic := range c.Diagnostics() { + if strings.Contains(diagnostic.Message, "public field `hidden` exposes private type `Hidden`") { + return + } + } + t.Fatalf("diagnostics = %v", c.Diagnostics()) +} + +func TestPrivateTypesMayRemainInsideModuleImplementation(t *testing.T) { + result := parse.Parse([]byte(` +private struct Hidden {} +private fn make_hidden() Hidden { Hidden{} } +struct Public { value: Int } +fn make_public() Public { Public{value: 1} } +`), "model.ard") + if len(result.Errors) > 0 { + t.Fatalf("parse errors: %v", result.Errors) + } + c := checker.New("model.ard", result.Program, nil) + c.Check() + if c.HasErrors() { + t.Fatalf("unexpected diagnostics: %v", c.Diagnostics()) + } +} + +func assertPrivateTypeExposure(t *testing.T, source string, want string) { + t.Helper() + result := parse.Parse([]byte(source), "model.ard") + if len(result.Errors) > 0 { + t.Fatalf("parse errors: %v", result.Errors) + } + + c := checker.New("model.ard", result.Program, nil) + c.Check() + if !c.HasErrors() { + t.Fatal("checker succeeded; expected private type exposure diagnostic") + } + for _, diagnostic := range c.Diagnostics() { + if strings.Contains(diagnostic.Message, want) { + return + } + } + messages := make([]string, len(c.Diagnostics())) + for i, diagnostic := range c.Diagnostics() { + messages[i] = diagnostic.Message + } + t.Fatalf("diagnostics = %q, want one containing %q", messages, want) +} diff --git a/compiler/go/backend_test.go b/compiler/go/backend_test.go index 01117cff..0acb0986 100644 --- a/compiler/go/backend_test.go +++ b/compiler/go/backend_test.go @@ -4325,7 +4325,7 @@ private struct internal_config { } fn make_user() User { User{first_name: "Ada", type: 1} } -fn main() internal_config { internal_config{secret_key: "s"} }`) +private fn make_internal_config() internal_config { internal_config{secret_key: "s"} }`) files := lowerProgramAST(t, program, Options{PackageName: "main"}) for _, field := range []string{"FirstName", "Type"} { if !astFilesHaveStructField(files, "User", field) { diff --git a/compiler/go/parity_test.go b/compiler/go/parity_test.go index 7a9417bb..41c44223 100644 --- a/compiler/go/parity_test.go +++ b/compiler/go/parity_test.go @@ -2391,14 +2391,14 @@ func TestGoTargetParityNestedGenericClosuresPreserveNamedTypeIdentity(t *testing number: Int, } - struct Context<$T> { + private struct Context<$T> { state: $T, next: fn(mut Value), } - type Handler = fn(mut Context<$T>, mut Value) + private type Handler = fn(mut Context<$T>, mut Value) - struct Box<$T> { + private struct Box<$T> { state: $T, handlers: [Handler<$T>], } diff --git a/website/src/content/docs/guide/modules.md b/website/src/content/docs/guide/modules.md index 770eb576..540e3db3 100644 --- a/website/src/content/docs/guide/modules.md +++ b/website/src/content/docs/guide/modules.md @@ -170,6 +170,19 @@ fn main() { Fields of a public Ard struct are public. Methods are public by default and can be marked `private`. +Because importing modules can use every public field and signature, public APIs cannot expose private types. This applies through containers and generic arguments as well as directly: + +```ard +private struct InternalState {} + +// Invalid: the public field exposes InternalState outside this module. +struct App { + state: InternalState?, +} +``` + +Make the exposed type public, or keep it entirely within private declarations. The same rule applies to public functions, methods, traits, unions, type aliases, and immutable top-level variables. + ```ard struct User { id: Int,