diff --git a/src/__tests__/ast-swift-module-scope.test.ts b/src/__tests__/ast-swift-module-scope.test.ts index efdc30003..c9d79d574 100644 --- a/src/__tests__/ast-swift-module-scope.test.ts +++ b/src/__tests__/ast-swift-module-scope.test.ts @@ -256,6 +256,68 @@ describe('Swift module-scope resolution (web-tree-sitter WASM)', () => { expect(calls.get('handled')?.resolvedTargetFile).toBe('Sources/App/Service.swift'); }); + it('does not resolve a call to a member the enclosing type inherits', async () => { + const { result } = await extractFiles([ + ['Sources/App/Base.swift', 'class Base {\n func work() -> Int { return 1 }\n}\n'], + ['Sources/App/Sub.swift', 'class Sub: Base {\n func run() -> Int { return work() }\n}\n'], + ['Sources/App/Global.swift', 'func work() -> Int { return 2 }\n'], + ]); + + // `Sub` inherits `work()` from `Base`, so that member is what the call runs. + // The top-level `func work()` in a third file is not, and an edge to it is an + // invented one. + const calls = new Map(result.callSites.map((c) => [c.calleeText, c])); + expect(calls.has('work')).toBe(true); + expect(calls.get('work')?.resolvedTargetFile).toBeUndefined(); + expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0); + }); + + it('does not resolve a call to a member an extension in another file adds', async () => { + const { result } = await extractFiles([ + ['Sources/App/Sub.swift', 'class Sub {\n func run() -> Int { return work() }\n}\n'], + ['Sources/App/Ext.swift', 'extension Sub {\n func work() -> Int { return 3 }\n}\n'], + ['Sources/App/Global.swift', 'func work() -> Int { return 2 }\n'], + ]); + + // The same shape with the member added by an extension rather than inherited. + const calls = new Map(result.callSites.map((c) => [c.calleeText, c])); + expect(calls.has('work')).toBe(true); + expect(calls.get('work')?.resolvedTargetFile).toBeUndefined(); + expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0); + }); + + it('does not resolve a call to a callable property a type declares', async () => { + const { result } = await extractFiles([ + ['Sources/App/Base.swift', 'class Base {\n let work: () -> Int = { 1 }\n}\n'], + ['Sources/App/Sub.swift', 'class Sub: Base {\n func run() -> Int { return work() }\n}\n'], + ['Sources/App/Global.swift', 'func work() -> Int { return 2 }\n'], + ]); + + // A property holding a closure is called under a bare name exactly like a + // method, so it shadows the module level the same way. A `property_declaration` + // is not one of the symbols the Swift query captures, which is why the member + // names are read off the tree rather than off the symbol list. + const calls = new Map(result.callSites.map((c) => [c.calleeText, c])); + expect(calls.has('work')).toBe(true); + expect(calls.get('work')?.resolvedTargetFile).toBeUndefined(); + expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0); + }); + + it('still resolves a call no type in the module declares as a member', async () => { + const { result } = await extractFiles([ + ['Sources/App/Base.swift', 'class Base {\n func other() -> Int { return 1 }\n}\n'], + ['Sources/App/Sub.swift', 'class Sub: Base {\n func run() -> Int { return work() }\n}\n'], + ['Sources/App/Global.swift', 'func work() -> Int { return 2 }\n'], + ]); + + // The control for the two cases above: no member is named `work`, so the + // module-level function is the only candidate and the edge still stands. + const references = result.edges.filter((e) => e.relation === 'REFERENCES'); + expect(references).toHaveLength(1); + expect(references[0]?.from).toBe('Sources/App/Sub.swift'); + expect(references[0]?.to).toBe('Sources/App/Global.swift'); + }); + it('does not resolve a type nested inside another file', async () => { const { result } = await extractFiles([ [ diff --git a/src/wiki-engine/code-knowledge/ast/call-resolver.ts b/src/wiki-engine/code-knowledge/ast/call-resolver.ts index d6509d4ef..c4a3e59fa 100644 --- a/src/wiki-engine/code-knowledge/ast/call-resolver.ts +++ b/src/wiki-engine/code-knowledge/ast/call-resolver.ts @@ -1,7 +1,7 @@ import type { AstCallSite, AstImport, AstSymbol } from "./types.js"; import type { ResolvedImport } from "./import-resolver.js"; import type { SwiftModuleSymbolIndex } from "./module-scope.js"; -import { findSwiftModuleSymbol } from "./module-scope.js"; +import { findSwiftModuleSymbol, swiftModuleDeclaresMember } from "./module-scope.js"; export interface ImportBindingMap { /** Local name → exported symbol id in target file */ @@ -110,7 +110,17 @@ function resolveOneCall( // the walker reports those bindings per site: `run(work:) { work() }` calls // its parameter, so claiming a sibling file's `func work()` here would // invent an edge. Missing a resolution is the better failure. - if (swiftModules && !site.localBindings?.includes(callee)) { + // + // A member of the enclosing type wins over a module-level declaration in the + // same way, and it does not have to be declared in this file to do so: a + // superclass, an `extension` or a protocol default implementation anywhere in + // the module puts it in scope. `swiftModuleDeclaresMember` is what stands in + // for the inheritance graph this layer does not build. + if ( + swiftModules && + !site.localBindings?.includes(callee) && + !swiftModuleDeclaresMember(swiftModules, site.fromFile, callee) + ) { const moduleSymbol = findSwiftModuleSymbol(swiftModules, site.fromFile, callee, ["function", "class"]); if (moduleSymbol) { return { diff --git a/src/wiki-engine/code-knowledge/ast/index.ts b/src/wiki-engine/code-knowledge/ast/index.ts index 7f97bfe98..2b9ea4c5f 100644 --- a/src/wiki-engine/code-knowledge/ast/index.ts +++ b/src/wiki-engine/code-knowledge/ast/index.ts @@ -5,6 +5,7 @@ import { type CodeFact } from "../code-extractors.js"; import { structuralEdgesToCodeFacts, unresolvedImportsToGaps } from "./adapt-code-facts.js"; import { buildImportBindingsForFile, callResolutionWeight, resolveCallSites } from "./call-resolver.js"; import { buildFileExistenceChecker, resolveImportSpecifier } from "./import-resolver.js"; +import type { SwiftMemberName } from "./module-scope.js"; import { buildSwiftModuleSymbolIndex, findSwiftModuleSymbol } from "./module-scope.js"; import { ensureAstReady } from "./parser-registry.js"; import type { AstExtractionGap, AstImplementsSite, StructuralEdge, StructuralGraphResult } from "./types.js"; @@ -46,6 +47,7 @@ export async function extractStructuralGraph( const { repoRoot, files } = options; const symbols: StructuralGraphResult["symbols"] = []; const swiftModuleSymbols: StructuralGraphResult["symbols"] = []; + const swiftMemberNames: SwiftMemberName[] = []; const imports: StructuralGraphResult["imports"] = []; const callSites: StructuralGraphResult["callSites"] = []; const implementsSites: AstImplementsSite[] = []; @@ -76,6 +78,7 @@ export async function extractStructuralGraph( filesParsed++; symbols.push(...walked.symbols); swiftModuleSymbols.push(...walked.swiftModuleSymbols); + swiftMemberNames.push(...walked.swiftMemberNames); imports.push(...walked.imports); callSites.push(...walked.callSites); implementsSites.push(...walked.implementsSites); @@ -92,8 +95,10 @@ export async function extractStructuralGraph( // SwiftPM target see each other with no import statement. Index the module // scopes once so conformance and call resolution can fall back to them — // over the module-visible declarations only, since a method or a `private` - // declaration is not reachable by name from a sibling file. - const swiftModules = buildSwiftModuleSymbolIndex(swiftModuleSymbols); + // declaration is not reachable by name from a sibling file. The member names + // go in too, not as candidates but as the names a bare call inside a type may + // be referring to instead of the module level. + const swiftModules = buildSwiftModuleSymbolIndex(swiftModuleSymbols, swiftMemberNames); const resolvedImports = new Map>>(); const resolvedKeys = new Set(); diff --git a/src/wiki-engine/code-knowledge/ast/module-scope.ts b/src/wiki-engine/code-knowledge/ast/module-scope.ts index d284ca87e..f8ab2e495 100644 --- a/src/wiki-engine/code-knowledge/ast/module-scope.ts +++ b/src/wiki-engine/code-knowledge/ast/module-scope.ts @@ -58,11 +58,19 @@ export function swiftModuleScope(relativePath: string): string | undefined { return undefined; } +/** A name a type declares as a member, and the file that declares it. */ +export interface SwiftMemberName { + file: string; + name: string; +} + export interface SwiftModuleSymbolIndex { /** Module scope key → every declaration found in that module. */ byModule: Map; /** File → its module scope key, for files that sit inside a known module. */ scopeOfFile: Map; + /** Module scope key → every name a type in that module declares as a member. */ + memberNames: Map>; } /** @@ -75,10 +83,35 @@ export interface SwiftModuleSymbolIndex { * resolve from another file, which is exactly the fabricated edge this layer * exists to avoid. `walk.ts` decides it, at the point where the declaration node * is still in hand. + * + * `members` is the complement the lookup below cannot do without: the names + * that belong to a type's body. They are not candidates — a bare name never + * reaches a member of another file's type — but they say when a bare name is + * not a candidate for the module level either, which is what + * `swiftModuleDeclaresMember` is for. Names rather than symbols, because a + * property is callable under a bare name and is not a symbol this layer + * extracts. */ -export function buildSwiftModuleSymbolIndex(symbols: AstSymbol[]): SwiftModuleSymbolIndex { +export function buildSwiftModuleSymbolIndex( + symbols: AstSymbol[], + members: SwiftMemberName[] +): SwiftModuleSymbolIndex { const byModule = new Map(); const scopeOfFile = new Map(); + const memberNames = new Map>(); + + for (const member of members) { + const scope = swiftModuleScope(member.file); + if (!scope) { + continue; + } + const names = memberNames.get(scope); + if (names) { + names.add(member.name); + } else { + memberNames.set(scope, new Set([member.name])); + } + } for (const symbol of symbols) { const scope = swiftModuleScope(symbol.file); @@ -94,7 +127,37 @@ export function buildSwiftModuleSymbolIndex(symbols: AstSymbol[]): SwiftModuleSy } } - return { byModule, scopeOfFile }; + return { byModule, scopeOfFile, memberNames }; +} + +/** + * Whether a type in `fromFile`'s module declares `name` as a member. + * + * A bare call inside a type runs that type's member when one is named after the + * callee, and the member can come from anywhere: the type itself, a superclass, + * an `extension` in a third file, a protocol's default implementation. None of + * those are visible to a lookup that only knows the module's top-level + * declarations, so a module-level function of the same name looks like the only + * candidate and is claimed as the target. + * + * Answering the question per type would need the inheritance and conformance + * graph of the whole module, which this layer does not build. Asking it of the + * module — does *any* type declare this member? — needs only the names already + * extracted, and errs the way the rest of the layer errs: a call that does turn + * out to be the module-level one, made from a module where some unrelated type + * declares a member of the same name, loses its edge. A missing edge still shows + * up as a gap; an invented one is read as a fact. + */ +export function swiftModuleDeclaresMember( + index: SwiftModuleSymbolIndex, + fromFile: string, + name: string +): boolean { + const scope = index.scopeOfFile.get(fromFile) ?? swiftModuleScope(fromFile); + if (!scope) { + return false; + } + return index.memberNames.get(scope)?.has(name) === true; } /** diff --git a/src/wiki-engine/code-knowledge/ast/walk.ts b/src/wiki-engine/code-knowledge/ast/walk.ts index f73493d43..b597b2d3b 100644 --- a/src/wiki-engine/code-knowledge/ast/walk.ts +++ b/src/wiki-engine/code-knowledge/ast/walk.ts @@ -10,6 +10,7 @@ import { normalizeImportSpecifier, parseImportBindings } from "./import-bindings.js"; +import type { SwiftMemberName } from "./module-scope.js"; import { grammarForExtension, getLanguage, getParser, getQuery } from "./parser-registry.js"; import type { AstCallSite, AstImplementsSite, AstImport, AstSymbol, AstSymbolKind } from "./types.js"; @@ -20,6 +21,19 @@ export interface FileWalkResult { * name. Empty for every other language, and a subset of `symbols` for Swift. */ swiftModuleSymbols: AstSymbol[]; + /** + * Swift only: every name a type in this file declares as a member, paired + * with the file so the module index can scope it. Disjoint from + * `swiftModuleSymbols` — a member is not reachable by a bare name from a + * sibling file. What they are needed for is the opposite question: a bare + * name *inside* a type may be one of these, in which case it is not the + * module-level declaration of the same name. + * + * Names, not symbols: a `let work: () -> Int` is callable under a bare name + * just like a `func work()`, and a property is not a symbol this walker + * extracts, so the question cannot be answered off the symbol list. + */ + swiftMemberNames: SwiftMemberName[]; imports: AstImport[]; callSites: AstCallSite[]; implementsSites: AstImplementsSite[]; @@ -35,18 +49,19 @@ export function isAstParseableFile(relativePath: string): boolean { export function walkFile(file: CodeCollectedFile): FileWalkResult { const symbols: AstSymbol[] = []; const swiftModuleSymbols: AstSymbol[] = []; + const swiftMemberNames: SwiftMemberName[] = []; const imports: AstImport[] = []; const callSites: AstCallSite[] = []; const implementsSites: AstImplementsSite[] = []; const parseErrors: string[] = []; if (!isAstParseableFile(file.relativePath)) { - return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors }; + return { symbols, swiftModuleSymbols, swiftMemberNames, imports, callSites, implementsSites, parseErrors }; } if (Buffer.byteLength(file.content, "utf8") > MAX_FILE_BYTES) { parseErrors.push(`skipped large file: ${file.relativePath}`); - return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors }; + return { symbols, swiftModuleSymbols, swiftMemberNames, imports, callSites, implementsSites, parseErrors }; } const variant = grammarForExtension(path.extname(file.relativePath))!; @@ -59,12 +74,12 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult { tree = parser.parse(file.content); } catch (error) { parseErrors.push(`parse failed: ${file.relativePath}: ${error instanceof Error ? error.message : String(error)}`); - return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors }; + return { symbols, swiftModuleSymbols, swiftMemberNames, imports, callSites, implementsSites, parseErrors }; } if (!tree) { parseErrors.push(`parse returned null: ${file.relativePath}`); - return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors }; + return { symbols, swiftModuleSymbols, swiftMemberNames, imports, callSites, implementsSites, parseErrors }; } try { @@ -74,6 +89,13 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult { // call would re-walk the enclosing declaration once for each of its calls. const swiftShadowedNames = variant === "swift" ? buildSwiftShadowedNames(tree.rootNode) : undefined; + if (variant === "swift") { + const names = new Set(); + collectSwiftMemberNames(tree.rootNode, names); + for (const name of names) { + swiftMemberNames.push({ file: file.relativePath, name }); + } + } for (const match of query.matches(tree.rootNode)) { const byName = new Map(match.captures.map((c) => [c.name, c.node])); @@ -166,7 +188,7 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult { tree.delete(); } - return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors }; + return { symbols, swiftModuleSymbols, swiftMemberNames, imports, callSites, implementsSites, parseErrors }; } /** @@ -210,6 +232,59 @@ function isSwiftModuleVisible(decl: Node): boolean { ); } +/** + * tree-sitter-swift puts the members of a class, a struct, an actor or an + * `extension` in a `class_body`, an enum's in an `enum_class_body` and a + * protocol's requirements in a `protocol_body`. + */ +const SWIFT_TYPE_BODIES = new Set(["class_body", "enum_class_body", "protocol_body"]); + +/** + * Every name the types in a file declare as a member. + * + * Read off the tree rather than off the symbols the query captures, because the + * query captures functions and types only: a `let work: () -> Int` is called as + * `work()` exactly like a `func work()`, and so is a `var` holding a closure, so + * a set built from function captures alone still lets the module-level fallback + * claim a bare call that one of them answers. + * + * A declaration names itself one of two ways, and both are taken here: + * + * - a `name` field holding an identifier — a method, a nested type, a + * `typealias`, an `init`, an enum case; + * - a `pattern` child — how `property_declaration` and + * `protocol_property_declaration` carry their name, including the several + * names of `let (a, b) = ...`. Descending the pattern is what + * `collectSwiftShadowedNames` already does for a local binding. + * + * A `subscript_declaration` has neither (its `name` field is the return type) and + * is not reachable by a bare name anyway, so it contributes nothing. + * + * A declaration inside a function body is deliberately not a member: it sits + * under `statements`, not under a type body, and nothing outside that body can + * be referring to it. The enclosing-scope bindings on the call site already + * cover that case. + */ +function collectSwiftMemberNames(node: Node, names: Set): void { + if (SWIFT_TYPE_BODIES.has(node.type)) { + for (const member of namedChildrenOf(node)) { + const name = member.childForFieldName("name"); + if (name && (name.type === "simple_identifier" || name.type === "type_identifier")) { + addSwiftName(name.text, names); + continue; + } + for (const child of namedChildrenOf(member)) { + if (child.type === "pattern") { + collectSwiftShadowedNames(child, names); + } + } + } + } + for (const child of namedChildrenOf(node)) { + collectSwiftMemberNames(child, names); + } +} + function symbolId(file: string, kind: AstSymbolKind, name: string): string { const kindLabel = kind.charAt(0).toUpperCase() + kind.slice(1); return `${file}#${kindLabel}:${name}`;