diff --git a/AGENTS.md b/AGENTS.md index b638134..2538ee9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,16 @@ This applies broadly, including: - test-scope inference from test-library APIs - effect or state boundary evidence derived from known dependency APIs +Call-graph expansion is an adapter contract, not a per-rule convenience. Apply it uniformly at the fact site so every consumer of that fact sees the expanded evidence. For property-test facts, the expansion root is the whole property construction that reaches the test runner, not only the final assertion callback. That includes: + +- generator expressions and helper bindings used to build generated inputs +- named helper functions and local function literals +- non-function top-level bindings such as generator values +- higher-order property builders when the language structure can trace a callback or returned property function +- operation-sequence operations and assertions derived through helpers + +Do not add a narrow expansion path only for the newest failing fixture. Prefer a single closure helper that all fact emitters for that evidence category call through. If full expansion is not possible in a language adapter without semantic analysis, implement the strongest structural expansion available, document the remaining gap in the adapter test or issue, and avoid pretending a bare identifier match proves reachability. + The intent is to reject forgery by incidental names while accepting normal local factoring. A test file should not fail just because it wraps `QCheck`, `Crowbar`, `testing/quick`, or a Swift property-check helper in a local helper. Conversely, a file should not pass merely because it contains a local variable or unrelated declaration with a trusted-looking name. For generated property tests, adapters should emit structurally rich generated-input evidence rather than policy booleans. Report the generated inputs and syntactic uses where each generated value participates in the property body. These uses are anti-vacuity evidence only; they do not need to prove value flow to an assertion or decision API. Constant properties such as unit generators with `fun ()` or ignored closure arguments may be useful regression assertions, but they should not be emitted as generated-input-backed coverage. diff --git a/go/main.go b/go/main.go index 951bc4d..a765f79 100644 --- a/go/main.go +++ b/go/main.go @@ -289,10 +289,14 @@ func goArchitectureFileFact(path string, loadedFile goLoadedFileContext) (archit imports := goImports(syntaxFile) identifiers := goIdentifiers(syntaxFile) apiReferences := goAPIReferences(syntaxFile) + callableArgumentReferences := goSemanticCallableArgumentReferences(syntaxFile, loadedFile.typesInfo) + for reference := range callableArgumentReferences { + apiReferences[reference] = struct{}{} + } decisionSurface := declaredDecisionSurface(syntaxFile) decisionProducts := declaredDecisionProducts(syntaxFile) decisionReferences := declaredDecisionReferences(syntaxFile) - propertyEvidence := goPropertyEvidence(syntaxFile) + propertyEvidence := goPropertyEvidence(syntaxFile, callableArgumentReferences) if metadata.moduleType != "stateTest" { for index := range propertyEvidence.checks { propertyEvidence.checks[index].operationSequences = []goOperationSequenceEvidence{} @@ -398,6 +402,36 @@ func goSemanticQualifiedReferences(syntaxFile *ast.File, info *types.Info, selfP return references } +// goSemanticCallableArgumentReferences records function symbols that are passed +// as values to another call, e.g. quick.Check(makeProp(Decide), nil). They are +// real API references even though they are not syntactic calls themselves. +func goSemanticCallableArgumentReferences(syntaxFile *ast.File, info *types.Info) map[string]struct{} { + references := map[string]struct{}{} + if info == nil { + return references + } + ast.Inspect(syntaxFile, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + for _, argument := range call.Args { + ast.Inspect(argument, func(argumentNode ast.Node) bool { + identifier, ok := argumentNode.(*ast.Ident) + if !ok { + return true + } + if _, ok := info.Uses[identifier].(*types.Func); ok { + references[identifier.Name] = struct{}{} + } + return true + }) + } + return true + }) + return references +} + func importPath(importSpec *ast.ImportSpec) string { path, err := strconv.Unquote(importSpec.Path.Value) if err != nil { @@ -816,7 +850,10 @@ type goOperationSequenceEvidence struct { assertions map[string]struct{} } -func goPropertyEvidence(syntaxFile *ast.File) goPropertyEvidenceResult { +func goPropertyEvidence( + syntaxFile *ast.File, + callableArgumentReferences map[string]struct{}, +) goPropertyEvidenceResult { globalFunctions := goFunctionDeclarationsByName(syntaxFile) quickNames := goImportNamesByPath(syntaxFile)["testing/quick"] result := goPropertyEvidenceResult{} @@ -841,6 +878,43 @@ func goPropertyEvidence(syntaxFile *ast.File) goPropertyEvidenceResult { references[reference] = struct{}{} } } + // Also harvest every identifier in the call-site argument + // expressions and expand it through the call graph. A property built + // by a higher-order helper — e.g. quick.Check(makeProp(decide), nil) — + // names `decide` only as a value passed at the call site (not a call), + // so it is invisible to call/type reference collection; capture it + // here and follow `makeProp`'s body through the property functions. + for _, argument := range call.Args { + ast.Inspect(argument, func(node ast.Node) bool { + name := "" + switch typed := node.(type) { + case *ast.Ident: + name = typed.Name + case *ast.SelectorExpr: + name = typed.Sel.Name + } + if name == "" { + return true + } + if goPredeclaredIdentifier(name) { + return true + } + _, isPropertyFunction := propertyFunctions[name] + _, isCallableArgument := callableArgumentReferences[name] + if !isPropertyFunction && !isCallableArgument { + return true + } + if isCallableArgument { + references[name] = struct{}{} + } + if referencedFunction, ok := propertyFunctions[name]; ok { + for nested := range goReachablePropertyReferences(&referencedFunction, propertyFunctions, map[string]bool{}) { + references[nested] = struct{}{} + } + } + return true + }) + } result.checks = append(result.checks, goPropertyCheckEvidence{ references: references, generatedInputs: goGeneratedInputEvidenceForProperty(propertyFunction, propertyFunctions), @@ -852,6 +926,19 @@ func goPropertyEvidence(syntaxFile *ast.File) goPropertyEvidenceResult { return result } +func goPredeclaredIdentifier(name string) bool { + switch name { + case "nil", "true", "false", "iota": + return true + case "append", "cap", "clear", "close", "complex", "copy", "delete", "imag", "len", "make", "max", "min", "new", "panic", "print", "println", "real", "recover": + return true + case "any", "bool", "byte", "comparable", "complex64", "complex128", "error", "float32", "float64", "int", "int8", "int16", "int32", "int64", "rune", "string", "uint", "uint8", "uint16", "uint32", "uint64", "uintptr": + return true + default: + return false + } +} + func goPropertyCheckFacts(evidence goPropertyEvidenceResult) []propertyCheckFact { facts := make([]propertyCheckFact, 0, len(evidence.checks)) for _, check := range evidence.checks { @@ -966,11 +1053,50 @@ func goQuickCheckPropertyFunction( return &propertyFunction } return nil + case *ast.CallExpr: + identifier, ok := firstArgument.Fun.(*ast.Ident) + if !ok { + return nil + } + propertyBuilder, ok := propertyFunctions[identifier.Name] + if !ok { + return nil + } + return goReturnedPropertyFunction(&propertyBuilder) default: return nil } } +func goReturnedPropertyFunction(propertyBuilder *goPropertyFunction) *goPropertyFunction { + if propertyBuilder == nil { + return nil + } + var returned *goPropertyFunction + ast.Inspect(propertyBuilder.body, func(node ast.Node) bool { + if returned != nil { + return false + } + returnStatement, ok := node.(*ast.ReturnStmt) + if !ok { + return true + } + for _, result := range returnStatement.Results { + functionLiteral, ok := result.(*ast.FuncLit) + if !ok { + continue + } + returned = &goPropertyFunction{ + functionType: functionLiteral.Type, + body: functionLiteral.Body, + } + return false + } + return true + }) + return returned +} + func goReachablePropertyReferences( propertyFunction *goPropertyFunction, propertyFunctions map[string]goPropertyFunction, diff --git a/go/main_test.go b/go/main_test.go index d489c4d..6eb5ea5 100644 --- a/go/main_test.go +++ b/go/main_test.go @@ -263,6 +263,40 @@ func DecideSend() string { } } +func TestLintBackendArchitectureCoversDecisionReachedThroughPropertyHelper(t *testing.T) { + backendRoot := newBackendFixture(t, map[string]string{ + "internal/routing/decision.go": `// @archlint.module core +// @archlint.domain routing +package routing + +func Decide(x int) bool { return x > 0 } +`, + "internal/routing/decision_test.go": `// @archlint.module test +// @archlint.domain routing +package routing + +import ( + "testing" + "testing/quick" +) + +// makeProp builds the property behind a higher-order helper, so Decide is named +// only at the quick.Check call site, not inside the resolved property body. +func makeProp(decide func(int) bool) func(int) bool { + return func(x int) bool { return decide(x) || x <= 0 } +} + +func TestDecideProperty(t *testing.T) { + if err := quick.Check(makeProp(Decide), nil); err != nil { + t.Fatal(err) + } +} +`, + }) + + assertNoViolations(t, lintBackendArchitecture(backendRoot)) +} + func TestLintBackendArchitectureRejectsHandlerWithOnlyArbitraryCoreTypeReference(t *testing.T) { backendRoot := newBackendFixture(t, map[string]string{ "internal/httpapi/http_decision.go": `// @archlint.module core diff --git a/ocaml/test.sh b/ocaml/test.sh index 1726dfc..4e17811 100644 --- a/ocaml/test.sh +++ b/ocaml/test.sh @@ -150,22 +150,23 @@ cat > "$TMPDIR/test/test_closure.ml" <<'ML' @archlint.domain demo.closure *) let gen_b = - QCheck2.Gen.map (fun x -> ignore (Closure_core.decide_b x); x) QCheck2.Gen.int + ignore (Closure_core.decide_b 0); + QCheck2.Gen.small_int let totality name f = - QCheck2.Test.make ~name QCheck2.Gen.int (fun x -> ignore (f x); true) + QCheck2.Test.make ~name QCheck2.Gen.small_int (fun x -> ignore (f x); true) let prop_a = - QCheck2.Test.make ~name:"a" QCheck2.Gen.int (fun x -> + QCheck2.Test.make ~name:"a" QCheck2.Gen.small_int (fun x -> Closure_core.decide_a x || x >= 0) -let prop_b = QCheck2.Test.make ~name:"b" gen_b (fun x -> x = x) +let prop_b = QCheck2.Test.make ~name:"b" gen_b (fun (x : int) -> x = x) let prop_c = totality "c" (fun x -> Closure_core.decide_c x) let () = - QCheck2.Test.check_exn prop_a; - QCheck2.Test.check_exn prop_b; - QCheck2.Test.check_exn prop_c + ignore prop_a; + ignore prop_b; + ignore prop_c ML eval "$(opam env --switch "${ARCHLINT_OPAM_SWITCH:-$ROOT/ocaml}" --set-switch --shell=sh)" @@ -338,6 +339,6 @@ let prop = ignore Linkonly_core.decide_link; true) -let () = QCheck2.Test.check_exn prop +let () = ignore prop ML expect_violation "core module property tests must reference every decision API: decide_link" constant_lint diff --git a/swift/Sources/SwiftArchLint/main.swift b/swift/Sources/SwiftArchLint/main.swift index cda0c15..2a904c4 100644 --- a/swift/Sources/SwiftArchLint/main.swift +++ b/swift/Sources/SwiftArchLint/main.swift @@ -513,7 +513,7 @@ final class ArchitectureVisitor: SyntaxVisitor { guard isPropertyCheckCall(node) else { return .visitChildren } - let referenceVisitor: APIReferenceVisitor = propertyClosureReferenceVisitor(for: node) + let referenceVisitor: APIReferenceVisitor = propertyConstructionReferenceVisitor(for: node) let expandedReferences: [String] = expandedAPIReferences( from: referenceVisitor.apiReferences, visited: [] @@ -674,18 +674,11 @@ final class ArchitectureVisitor: SyntaxVisitor { } } - private func propertyClosureReferenceVisitor( + private func propertyConstructionReferenceVisitor( for node: FunctionCallExprSyntax ) -> APIReferenceVisitor { let referenceVisitor: APIReferenceVisitor = APIReferenceVisitor(viewMode: .sourceAccurate) - if let trailingClosure: ClosureExprSyntax = node.trailingClosure { - referenceVisitor.walk(trailingClosure) - } - for argument: LabeledExprSyntax in node.arguments { - if let closure: ClosureExprSyntax = argument.expression.as(ClosureExprSyntax.self) { - referenceVisitor.walk(closure) - } - } + referenceVisitor.walk(node) return referenceVisitor } diff --git a/swift/test.sh b/swift/test.sh index eda04e2..e7cfc00 100644 --- a/swift/test.sh +++ b/swift/test.sh @@ -265,6 +265,61 @@ struct SQLiteMailSyncStateSchema { EOF assert_passes "$static_property_decision_surface_fixture" +closure_fixture="$(new_fixture closure)" +cat > "$closure_fixture/apps/ios/MailApp/Backend/ClosureDecider.swift" <<'EOF' +// @archlint.module core +// @archlint.domain backend.closure +enum ClosureDecider { + static func decideA(_ shard: Int) -> Bool { + shard >= 0 + } + + static func decideB(_ shard: Int) -> Bool { + shard < 0 + } + + static func decideC(_ shard: Int) -> Bool { + shard == 0 + } +} +EOF +cat > "$closure_fixture/apps/ios/MailAppTests/ClosureDeciderTests.swift" <<'EOF' +// @archlint.module test +// @archlint.domain backend.closure +import PropertyBased +import Testing + +func helper(_ shard: Int) -> Bool { + ClosureDecider.decideC(shard) +} + +@Test +func closureProperty() async { + await propertyCheck( + input: Gen.int(in: 0...10).map { shard in + _ = ClosureDecider.decideB(shard) + return shard + } + ) { shard in + #expect(ClosureDecider.decideA(shard) || helper(shard) || shard >= 0) + } +} +EOF +cat > "$closure_fixture/apps/ios/MailApp/Backend/ClosureClient.swift" <<'EOF' +// @archlint.module shell +// @archlint.domain backend.closure +import Foundation + +struct ClosureClient { + func handle() -> URLRequest { + let path = ClosureDecider.decideA(1) ? "/v1/accounts" : "/v1/accounts" + let url = URL(string: "http://localhost" + path)! + return URLRequest(url: url) + } +} +EOF +assert_passes "$closure_fixture" + arbitrary_core_type_reference_fixture="$(new_fixture arbitrary-core-type-reference)" cat > "$arbitrary_core_type_reference_fixture/apps/ios/MailApp/Backend/HTTPMailBackendDecider.swift" <<'EOF' // @archlint.module core diff --git a/typescript/src/main.ts b/typescript/src/main.ts index da287c7..bd9b998 100644 --- a/typescript/src/main.ts +++ b/typescript/src/main.ts @@ -71,6 +71,7 @@ type Facts = { sharedState: Map>; propertyChecks: PropertyCheckFact[]; functionReferences: Map>; + functionBodies: Map; interfaceLogicEvidence: InterfaceLogicEvidence; }; @@ -462,6 +463,7 @@ function emptyFacts(): Facts { sharedState: new Map(), propertyChecks: [], functionReferences: new Map(), + functionBodies: new Map(), interfaceLogicEvidence: { functionBodies: [], constructorBodies: [], @@ -533,6 +535,7 @@ function recordFunctionDeclaration(facts: Facts, declaration: ts.FunctionDeclara if (declaration.body !== undefined) { addUnique(facts.interfaceLogicEvidence.functionBodies, name); facts.functionReferences.set(name, apiReferencesInNode(declaration.body)); + facts.functionBodies.set(name, declaration); } recordReturnTypeProducts(facts, declaration.type); } @@ -559,8 +562,10 @@ function recordVariableStatement(facts: Facts, statement: ts.VariableStatement, } addUnique(facts.interfaceLogicEvidence.functionBodies, name); facts.functionReferences.set(name, apiReferencesInNode(declaration.initializer)); + facts.functionBodies.set(name, declaration.initializer); } else if (declaration.initializer !== undefined && !isLiteralLike(declaration.initializer)) { addUnique(facts.interfaceLogicEvidence.derivedValueBodies, name); + facts.functionReferences.set(name, apiReferencesInNode(declaration.initializer)); } } } @@ -622,11 +627,19 @@ function propertyCheckForCall(facts: Facts, call: ts.CallExpression): PropertyCh if (!isFastCheckPropertyCall(call.expression) || call.arguments.length === 0) { return undefined; } - const callback = call.arguments[call.arguments.length - 1]; - if (callback === undefined || !isFunctionLikeExpression(callback)) { + const callbackExpression = call.arguments[call.arguments.length - 1]; + if (callbackExpression === undefined) { + return undefined; + } + const callback = propertyCallbackForExpression(facts, callbackExpression); + if (callback === undefined) { return undefined; } - const references = sorted(expandedApiReferences(facts, apiReferencesInNode(callback), new Set())); + const directReferences = call.arguments.reduce( + (acc, argument) => union(acc, propertyConstructionReferences(facts, argument)), + new Set(), + ); + const references = sorted(expandedApiReferences(facts, directReferences, new Set())); const generatedInputs = callback.parameters.flatMap((parameter) => bindingNames(parameter.name).map((name) => ({ name, @@ -637,6 +650,64 @@ function propertyCheckForCall(facts: Facts, call: ts.CallExpression): PropertyCh return { references, generatedInputs, operationSequences }; } +function propertyConstructionReferences(facts: Facts, node: ts.Node): Set { + const references = apiReferencesInNode(node); + for (const identifier of identifiersInNode(node)) { + if (facts.functionReferences.has(identifier)) { + for (const expanded of expandedApiReferences(facts, new Set([identifier]), new Set())) { + references.add(expanded); + } + } else { + references.add(identifier); + } + } + return references; +} + +function identifiersInNode(node: ts.Node): Set { + const identifiers = new Set(); + const visit = (current: ts.Node): void => { + if (ts.isIdentifier(current) && !isDeclarationName(current)) { + identifiers.add(current.text); + } + ts.forEachChild(current, visit); + }; + visit(node); + return identifiers; +} + +function propertyCallbackForExpression(facts: Facts, expression: ts.Expression): ts.FunctionLikeDeclaration | undefined { + if (isFunctionLikeExpression(expression)) { + return expression; + } + if (ts.isIdentifier(expression)) { + return facts.functionBodies.get(expression.text); + } + if (ts.isCallExpression(expression) && ts.isIdentifier(expression.expression)) { + const builder = facts.functionBodies.get(expression.expression.text); + if (builder?.body !== undefined) { + return returnedFunctionLike(builder.body); + } + } + return undefined; +} + +function returnedFunctionLike(node: ts.Node): ts.FunctionLikeDeclaration | undefined { + let returned: ts.FunctionLikeDeclaration | undefined; + const visit = (current: ts.Node): void => { + if (returned !== undefined) { + return; + } + if (ts.isReturnStatement(current) && current.expression !== undefined && isFunctionLikeExpression(current.expression)) { + returned = current.expression; + return; + } + ts.forEachChild(current, visit); + }; + visit(node); + return returned; +} + function isFastCheckPropertyCall(expression: ts.Expression): boolean { if (!ts.isPropertyAccessExpression(expression)) { return false; @@ -942,6 +1013,14 @@ function intersection(left: Set, right: Set): Set { return new Set([...left].filter((value) => right.has(value))); } +function union(left: Set, right: Set): Set { + const values = new Set(left); + for (const value of right) { + values.add(value); + } + return values; +} + function hasIntersection(left: Set, right: Set): boolean { for (const value of left) { if (right.has(value)) { diff --git a/typescript/test.sh b/typescript/test.sh index 19f2bd9..fa63eaf 100755 --- a/typescript/test.sh +++ b/typescript/test.sh @@ -78,6 +78,54 @@ test("decision property", () => { }); TS +cat > "$TMPDIR/src/closure.ts" <<'TS' +// @archlint.module core +// @archlint.domain demo.closure + +export function decideA(value: number): boolean { + return value >= 0; +} + +export function decideB(value: number): boolean { + return value < 0; +} + +export function decideC(value: number): boolean { + return value === 0; +} +TS + +cat > "$TMPDIR/tests/closure.test.mts" <<'TS' +// @archlint.module test +// @archlint.domain demo.closure + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import * as fc from "fast-check"; +import { decideA, decideB, decideC } from "../src/closure.js"; + +const genB = fc.integer().map((value) => { + decideB(value); + return value; +}); + +function makeProp(decide: (value: number) => boolean) { + return (value: number) => { + decide(value); + return true; + }; +} + +test("closure properties", () => { + fc.assert( + fc.property(fc.integer(), (value) => { + assert.equal(typeof decideA(value), "boolean"); + }), + ); + fc.assert(fc.property(genB, makeProp(decideC))); +}); +TS + npm --prefix "$ROOT/typescript" install >/dev/null npm --prefix "$ROOT/typescript" run --silent typecheck uv run --project "$ROOT" python "$ROOT/evaluate.py" \