fix(code-knowledge): resolve Swift symbols across files in the same module (#847)

* fix(code-knowledge): resolve Swift symbols across files in the same module

Swift declarations are module-scoped, but the AST track resolved calls and
conformances with a file-scoped model: declared in this file, or reachable
through a resolved import. That model is correct for TS/Go/Python, where a
cross-file symbol has to be imported. Swift needs no import inside a module,
so a conformance or a call to a symbol in a sibling file emitted nothing.

Add a third, Swift-only level of lookup after the existing two: a declaration
elsewhere in the same module, with the module boundary read from the layout
SwiftPM mandates (Sources/<Target>/, Tests/<Target>/). Outside that layout no
scope is claimed, and a name declared more than once in the module emits
nothing rather than picking one arbitrarily.

Fixes #843

* fix(code-knowledge): index only module-visible Swift declarations

Addresses the three P1 findings the automated review raised on the first
revision of #847. All three were real.

The fallback widened the candidate set without narrowing eligibility. The
candidate set is "every declaration in the module"; what a bare name can
actually reach is strictly smaller, and that has to be decided where the
declaration and its modifiers are still in hand.

- module-scope.ts: the module key now keeps the package root, so
  Packages/A/Sources/App and Packages/B/Sources/App stay two modules
  instead of merging into Sources/App.
- walk.ts: a declaration enters the module index only when it is top-level
  and not private/fileprivate. Neither fact survives into AstSymbol, so it
  is computed in the walker and surfaced as FileWalkResult.swiftModuleSymbols.
- index.ts: feeds that subset to the module index.

queries.ts and the shared AstSymbol type are untouched, and the tie rule
agreed in #843 is unchanged: only module-visible declarations can tie.

Verification on the merge result (main a8ab8e00 + this change):
- real-CLI e2e, 15/15 assertions, three new negative probes each in a file
  that declares nothing else; both cross-package directions stay clean while
  each package resolves to its own Proto.swift
- four mutations, each removing one guard, each caught by exactly the
  matching test and nothing else
- ast-swift-module-scope.test.ts 15 cases; related suite 68 -> 83 passed
- oxlint --deny-warnings and tsc --noEmit: rc=0 on this change and rc=0 on
  unmodified main, same binary and flags, so the comparison is real

* fix(code-knowledge): take the innermost SwiftPM marker as the module boundary

Addresses the P1 the automated review raised on the previous head.

Keeping the package root only fixes packages that sit side by side. A package
vendored under the outer package's own Tests/ has two markers on its path, and
the scan took the first: Tests/Fixtures/A/Sources/App and
Tests/Fixtures/B/Sources/App were both scoped to Tests/Fixtures, which is the
same merge as the earlier package-root finding, reached one level out.

The scan now runs from the end, so the innermost marker wins. The direction
also decides how a layout this function misreads can fail: an inner marker can
only yield a scope nested inside the true module, which loses a resolution,
while an outer one can span two real modules and fabricate an edge. The layer
already prefers a missing edge to a wrong one, so the bias is deliberate and
is stated in the docstring and in the tests.

Verification on the merge result (main a8ab8e00 + this change):
- real-CLI e2e, 20/20 assertions; both vendored fixture packages now resolve to
  their own Proto.swift while neither cross-package direction produces an edge
- five mutations, each removing one guard, each caught by exactly the matching
  tests and nothing else
- ast-swift-module-scope.test.ts 18 cases; related suite 68 -> 86 passed
- oxlint --deny-warnings and tsc --noEmit: rc=0 here and rc=0 on unmodified
  main, same binary and flags

* fix(code-knowledge): a name an enclosing scope binds is not a module reference

Addresses the P1 the automated review raised on the previous head.

The fallback answered "which module does this bare call belong to" without
first asking whether the call is a module-level reference at all. With
A.swift declaring func work() and B.swift declaring
func run(work: () -> Void) { work() }, Swift calls the parameter, and the
lookup resolved it to A.swift and emitted a fabricated cross-file REFERENCES
edge. The receiver fallback had the same hole for a local value shadowing a
type name.

The walker now records, per call site, the names an enclosing scope binds:
function and closure parameters, local let/var, and what a
for / if let / guard let / catch let / case let introduces. Both module-wide
lookups decline when the callee or receiver is one of those names. The walk
stops at the call own ancestors, so a binding in an unrelated function of the
same file shadows nothing; it over-collects only within the scopes it does
visit, which costs a resolution rather than inventing an edge.

This is the same mistake as the earlier four findings on a different axis:
eligibility was narrowed at one end of the edge (can a sibling reach this
declaration) and not at the other (can this call reach anything module-level).

Verification on the merge result (main a8ab8e00 + this change):
- real-CLI e2e, 24/24 assertions; ShadowParam.swift (its only call bound by
  its own parameter) ends with 0 outgoing edges, ShadowMixed.swift (one bound
  call, one unbound) with exactly 1, and the heuristic track is byte-identical
- seven mutations, each removing one guard, each caught by exactly the matching
  tests and nothing else; originals restored byte for byte
- ast-swift-module-scope.test.ts 24 cases; affected set derived from the
  changed modules: 93/106 passed on unmodified main and 93/106 here
- Node 24.14.0, the runtime the matrix gained while this sat in review, on the
  merge result (main 5fb316c7, which carries #861): the two Swift AST test
  files pass, 2/2. On this branch tree alone Node 24 aborts during the first
  Swift parse (Fatal process out of memory: Zone) and a command-line
  --wasm-tier-up-filter does not help, which is why #861 has to set the flag
  inside the worker.
- oxlint --deny-warnings and tsc --noEmit: rc=0 here and rc=0 on unmodified
  main, same binary and flags

* fix(code-knowledge): keep a call's own name from counting as a shadow

Two follow-ups from review.

The closed rule collected every identifier in the enclosing scopes, and that
included the callee of a sibling call: in `func run() { work(); work() }` each
call read the other's `work` as evidence, so both cross-file resolutions were
suppressed. The name a call goes through is a use, not a binding. Arguments are
still walked, so a closure that does bind a name keeps counting.

`SWIFT_IDENTIFIER` also admitted ASCII only, so a parameter named `π` was
dropped from the set and a sibling `func π()` won the fallback. Swift
identifiers are not ASCII.

Both shapes now have cases; all four mutations the rule has to survive are
killed by the suite.

* fix(code-knowledge): collect Swift shadowing once per declaration, for any identifier

Two follow-ups from review, both in the same set of names.

The character class guarding `addSwiftName` admitted Unicode letters after the
previous pass but still rejected escaped identifiers such as `repeat` -- which
Swift requires when a name collides with a keyword -- and symbol or emoji
names. A parameter with such a name never reached `localBindings`, so a sibling
`func` of the same name won the fallback and a fabricated cross-file edge was
emitted. Every caller passes the text of a `simple_identifier` or a
`type_identifier`, which is the grammar's own verdict that the token is a name,
so the class was the whole defect and nothing else is tested there now.

The set was also rebuilt for every call by re-walking that call's ancestors.
The union over those ancestors is exactly the top-level declaration that holds
the call -- every ancestor sits inside it, and the declaration is one of them --
so the declaration is now read once per file and each call site looks its answer
up. Same names, minus the quadratic term: a function holding N calls went from
roughly N^2 AST visits to a single traversal per declaration.

Reading the declaration whole is a shade coarser than the per-call walk: a name
occurring only inside the call now counts as well. That costs nothing where the
field is consumed, because the resolver asks about the callee and the receiver,
and both are callee positions, skipped below as uses. A name can only enter the
set through a position that is not the very call it would resolve.

Verification against the pushed head 891da0826b, same fixtures, same runtime:

- 16-shape before/after matrix (5 shadowed shapes, 2 of the dropped identifiers,
  2 scope cases, 7 controls). The only rows that move are the two identifier
  shapes, each 1 fabricated edge -> 0. Every other row is identical, including
  the controls for those two shapes, which still resolve.
- The two new identifier cases fail on the unmodified head (2 failed | 31 passed
  of 33) and pass here (33/33), so they pin the defect rather than the change.
- Five mutations, each removing one guard: reinstating the ASCII shape test,
  keeping the callee as evidence, scoping to the nearest function instead of the
  declaration, looking the declaration up by endIndex, collecting type
  positions. All five are killed; walk.ts restored byte for byte afterwards.
- ast-swift-module-scope.test.ts 29 -> 33 cases. The four AST suites: 65 passed.
- The quadratic term, measured on one function holding the same call N times:
  200 -> 800 calls is 477 ms -> 8428 ms on the head (x17.7, and it overruns the
  15 s test timeout) against 12.6 ms -> 29.5 ms here.
- oxlint --deny-warnings --report-unused-disable-directives and tsc --noEmit:
  rc=0. No reference to the two helpers removed here is left in src/.

* fix(code-knowledge): treat an argument as a use, not a binding, in Swift shadowing

Two follow-ups from review, both about positions that mention a name without
binding it.

The review bot reported that `consume(work)` before a bare `work()` suppresses
the call: the argument's mention of `work` entered the shadowing set, the
resolver saw its callee there and dropped the edge. The bot labels it P2; it is
also older than this branch. The per-call walk it replaced had the same hole --
it excluded the call's own subtree, but a mention in a *sibling* call was still
counted, and `consume(work)` is a sibling of `work()`.

Rebuilding the set from the whole declaration lost that exclusion entirely, and
that loss was a real regression the bot did not report: `work(work)` counts its
own argument as evidence and stops resolving through its callee. The previous
commit message claimed reading the declaration whole "costs nothing where the
field is consumed, because ... callee positions are skipped". That is wrong.
Skipping the callee position says nothing about a mention of the same name
reaching the set from anywhere else in the declaration -- its own argument list
included.

Both are the same defect in one direction: an argument is an expression
position, so it can mention a name but never bind one. A marker now follows the
walk into `value_arguments` and is cleared on the way into a `lambda_literal`,
since a closure handed over as an argument still opens its own scope and its
parameters and captures do bind. Nothing enumerates binding constructs, so the
rule stays closed.

Verification spans three states of walk.ts, same fixtures, same runtime:
891da0826b (the branch's starting point, before the previous pass), da6f894e
(the pushed head, after it) and this commit.

- 18-shape matrix, covering every shape the review rounds produced: four
  shadowed shapes (parameter, stored property, generic parameter, closure
  capture list), three identifier shapes (Unicode, escaped, symbol), two
  repeated-call shapes (two calls, twelve calls), the inherited-member shape,
  the bot's two argument shapes, `work(work)`, a closure passed to itself, a
  bare mention beside a call, and three controls. Against the pushed head no row
  is worse and three rows move up, all of them positions this commit is about:

    bot shape, argument mention after the call   no edge -> 1 edge
    bot shape, argument mention before the call  no edge -> 1 edge
    work(work)                                   no edge -> 1 edge

  The third is the regression above: it resolves on 891da0826b, does not resolve
  on the pushed head, and resolves again here. Against 891da0826b no row is
  worse either; the escaped and symbol shapes go from 2 edges to 1, because
  before the previous pass a name written with backticks or as a symbol was
  dropped from the set, so the call inside the shadowing declaration resolved
  along with the one that should.
- Every shadowed shape -- parameter, stored property, generic parameter,
  closure capture list, Unicode parameter, inherited member -- still resolves to
  no edge, and the three controls still resolve.
- ast-swift-module-scope.test.ts 29 -> 33 -> 36 cases. The four AST suites: 68
  passed. With the test file at 36 cases and walk.ts at 891da0826b the suite is
  3 failed | 33 passed -- escaped, symbol and the argument mention, which is
  what pinning those defects looks like. With walk.ts at the pushed head it is
  2 failed | 34 passed, the argument mention and `work(work)`: the two shapes
  this commit repairs. The closure-parameter case passes against both, so it
  guards a rule rather than recording new behaviour.
- Four mutations, each removing one guard: ignoring the argument marker, not
  clearing it inside a closure, never marking arguments, and no longer skipping
  callee identifiers. All four are killed; walk.ts restored byte for byte
  afterwards (sha256 62332202bcc5c72a53417bd0fc9be8d0b88f674237a606104d20a5f7816c2673).
- oxlint --deny-warnings --report-unused-disable-directives and tsc --noEmit:
  rc=0. The repo's lint script also adds --type-aware; that half is exercised by
  CI.

Still open, unchanged and out of scope here: inherited and cross-file extension
members, tracked in #909. It needs a member table the AST index does not build
today, so no amount of argument-position handling reaches it.
This commit is contained in:
Smilewithoutfalling
2026-09-30 14:01:02 +08:00
committed by GitHub
parent a8ab138b0a
commit 2aaf3db543
6 changed files with 1041 additions and 14 deletions
@@ -0,0 +1,621 @@
import { describe, it, expect, beforeEach } from 'vitest';
import type { CodeCollectedFile } from '../wiki-engine/code-knowledge/code-collector.js';
import { extractStructuralGraphAsFacts } from '../wiki-engine/code-knowledge/ast/index.js';
import { swiftModuleScope } from '../wiki-engine/code-knowledge/ast/module-scope.js';
import { resetParserRegistryForTests } from '../wiki-engine/code-knowledge/ast/parser-registry.js';
function makeFile(relativePath: string, content: string): CodeCollectedFile {
return {
path: `/virtual/${relativePath}`,
relativePath,
language: 'swift',
sha256: 'test',
content,
};
}
const REPO_ROOT = '/virtual';
async function extractFiles(files: Array<[string, string]>) {
return extractStructuralGraphAsFacts({
repoRoot: REPO_ROOT,
files: files.map(([relativePath, content]) => makeFile(relativePath, content)),
});
}
describe('Swift module scope', () => {
it('reads the module boundary from a SwiftPM layout', () => {
expect(swiftModuleScope('Sources/App/Models.swift')).toBe('Sources/App');
expect(swiftModuleScope('Sources/App/Nested/Deep.swift')).toBe('Sources/App');
expect(swiftModuleScope('Tests/AppTests/ModelsTests.swift')).toBe('Tests/AppTests');
expect(swiftModuleScope('Sources\\App\\Models.swift')).toBe('Sources/App');
});
it('keeps the package root in the module boundary', () => {
// Two packages in one repository can name their targets the same. The scope
// has to span the path up to the target, or the two would be one module and
// a name in one could resolve into the other.
const a = swiftModuleScope('Packages/A/Sources/App/Models.swift');
const b = swiftModuleScope('Packages/B/Sources/App/Models.swift');
expect(a).toBe('Packages/A/Sources/App');
expect(b).toBe('Packages/B/Sources/App');
expect(a).not.toBe(b);
});
it('takes the innermost marker, so a package vendored under Tests/ keeps its own root', () => {
// A repository may vendor whole packages under a directory it already named
// `Tests`/`Sources`. The inner marker is those packages' boundary; taking the
// outer one would scope `Tests/Fixtures/A/...` and `Tests/Fixtures/B/...` to
// the same `Tests/Fixtures` and merge two packages.
expect(swiftModuleScope('Tests/Fixtures/A/Sources/App/Models.swift')).toBe('Tests/Fixtures/A/Sources/App');
expect(swiftModuleScope('Tests/Fixtures/A/Sources/App/Deep/Models.swift')).toBe('Tests/Fixtures/A/Sources/App');
// A directory merely *named* `Tests` inside a target is not a boundary when
// it cannot hold a target directory of its own — the file directly under it
// leaves the real marker the only candidate.
expect(swiftModuleScope('Sources/App/Tests/Helper.swift')).toBe('Sources/App');
// The bias this direction buys: a marker this function mistakes for a
// package root only ever yields a scope nested INSIDE the true module, so it
// under-scopes (loses a resolution) instead of spanning two real modules.
expect(swiftModuleScope('Sources/App/Tests/Sub/Helper.swift')).toBe('Sources/App/Tests/Sub');
});
it('refuses to invent a module where the layout states none', () => {
// No `Sources/` or `Tests/` segment: an arbitrary directory tree says
// nothing about Swift's module boundary, so no scope is claimed.
expect(swiftModuleScope('App/Models.swift')).toBeUndefined();
expect(swiftModuleScope('MySources/App/Models.swift')).toBeUndefined();
// A file sitting directly under Sources/ has no target directory.
expect(swiftModuleScope('Sources/App.swift')).toBeUndefined();
expect(swiftModuleScope('Sources/App/Models.ts')).toBeUndefined();
});
});
describe('Swift module-scope resolution (web-tree-sitter WASM)', () => {
beforeEach(() => {
resetParserRegistryForTests();
});
it('resolves a conformance to a protocol declared in another file of the same module', async () => {
const { result } = await extractFiles([
['Sources/App/Protocols.swift', 'protocol LocalProto {\n func describe() -> String\n}\n'],
[
'Sources/App/Models.swift',
'struct Point: LocalProto {\n func describe() -> String { return "point" }\n}\n',
],
]);
const implementsEdges = result.edges.filter((e) => e.relation === 'IMPLEMENTS');
expect(implementsEdges).toHaveLength(1);
expect(implementsEdges[0]?.from).toBe('Sources/App/Models.swift');
expect(implementsEdges[0]?.to).toBe('Sources/App/Protocols.swift');
expect(implementsEdges[0]?.evidence[0]?.note).toBe('Point implements LocalProto');
});
it('resolves a call to a function declared in another file of the same module', async () => {
const { result } = await extractFiles([
['Sources/App/Math.swift', 'func helper() -> Int { return 1 }\n'],
['Sources/App/Runner.swift', 'func run() -> Int {\n return helper()\n}\n'],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Runner.swift');
expect(references[0]?.to).toBe('Sources/App/Math.swift');
expect(references[0]?.confidence).toBe('INFERRED');
});
it('resolves a receiver call whose type lives in another file of the same module', async () => {
const { result } = await extractFiles([
['Sources/App/Service.swift', 'class Service {\n func ping() -> Int { return 1 }\n}\n'],
['Sources/App/App.swift', 'func run() -> Int {\n return Service.ping()\n}\n'],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.to).toBe('Sources/App/Service.swift');
});
it('keeps a symbol from a different target unresolved', async () => {
const { result } = await extractFiles([
['Sources/Other/Remote.swift', 'protocol RemoteProto { }\n'],
['Sources/App/Models.swift', 'struct Point: RemoteProto { }\n'],
]);
// A separate target is a separate module: without an import the name is not
// visible, and emitting an edge here would be fabrication.
expect(result.edges.filter((e) => e.relation === 'IMPLEMENTS')).toHaveLength(0);
});
it('emits nothing when the name is declared more than once in the module', async () => {
const { result } = await extractFiles([
['Sources/App/A.swift', 'protocol Dup { }\n'],
['Sources/App/B.swift', 'protocol Dup { }\n'],
['Sources/App/C.swift', 'struct S: Dup { }\n'],
]);
// Two candidates mean the layout cannot say which file defines it, so the
// resolution declines rather than picking one at random.
expect(result.edges.filter((e) => e.relation === 'IMPLEMENTS')).toHaveLength(0);
});
it('does not guess a module outside a SwiftPM layout', async () => {
const { result } = await extractFiles([
['App/Protocols.swift', 'protocol LocalProto { }\n'],
['App/Models.swift', 'struct Point: LocalProto { }\n'],
]);
expect(result.edges.filter((e) => e.relation === 'IMPLEMENTS')).toHaveLength(0);
});
it('leaves same-file resolution unchanged', async () => {
const { result } = await extractFiles([
[
'Sources/App/All.swift',
[
'protocol LocalProto { }',
'',
'struct Point: LocalProto { }',
'',
'func helper() -> Int { return 1 }',
'',
'func run() -> Int { return helper() }',
'',
].join('\n'),
],
]);
const implementsEdges = result.edges.filter((e) => e.relation === 'IMPLEMENTS');
expect(implementsEdges).toHaveLength(1);
expect(implementsEdges[0]?.from).toBe('Sources/App/All.swift');
expect(implementsEdges[0]?.to).toBe('Sources/App/All.swift');
// A same-file call still resolves to EXTRACTED; it must not be downgraded
// to the cross-file path now that the fallback exists.
const helperCall = result.callSites.find((c) => c.calleeText === 'helper');
expect(helperCall?.confidence).toBe('EXTRACTED');
expect(helperCall?.resolvedTargetFile).toBe('Sources/App/All.swift');
});
it('does not resolve a file-scoped declaration from another file', async () => {
const { result } = await extractFiles([
[
'Sources/App/Internal.swift',
[
'private func hidden() -> Int { return 1 }',
'fileprivate func alsoHidden() -> Int { return 2 }',
'func visible() -> Int { return 3 }',
'',
].join('\n'),
],
[
'Sources/App/Runner.swift',
[
'func run() -> Int {',
' let a = hidden()',
' let b = alsoHidden()',
' let c = visible()',
' return a + b + c',
'}',
'',
].join('\n'),
],
]);
const calls = new Map(result.callSites.map((c) => [c.calleeText, c]));
for (const name of ['hidden', 'alsoHidden', 'visible']) {
expect(calls.has(name)).toBe(true);
}
// Same file, same call shape, same `-> Int` signature: the only variable is
// the modifier on the declaration. `private` and `fileprivate` stop at the
// file that declares them; the unmodified function is module-wide.
expect(calls.get('hidden')?.resolvedTargetFile).toBeUndefined();
expect(calls.get('alsoHidden')?.resolvedTargetFile).toBeUndefined();
expect(calls.get('visible')?.resolvedTargetFile).toBe('Sources/App/Internal.swift');
});
it('does not resolve a method or a protocol requirement from another file', async () => {
const { result } = await extractFiles([
[
'Sources/App/Service.swift',
[
'struct Service {',
' func handle() -> Int { return 1 }',
'}',
'',
'protocol Handler {',
' func respond() -> Int',
'}',
'',
'func handled() -> Int { return 2 }',
'',
].join('\n'),
],
[
'Sources/App/Runner.swift',
[
'func run() -> Int {',
' let a = handle()',
' let b = respond()',
' let c = handled()',
' return a + b + c',
'}',
'',
].join('\n'),
],
]);
const calls = new Map(result.callSites.map((c) => [c.calleeText, c]));
for (const name of ['handle', 'respond', 'handled']) {
expect(calls.has(name)).toBe(true);
}
// A member is reached through its container, not by a bare name, so only the
// top-level function is something a sibling file can call on its own.
expect(calls.get('handle')?.resolvedTargetFile).toBeUndefined();
expect(calls.get('respond')?.resolvedTargetFile).toBeUndefined();
expect(calls.get('handled')?.resolvedTargetFile).toBe('Sources/App/Service.swift');
});
it('does not resolve a type nested inside another file', async () => {
const { result } = await extractFiles([
[
'Sources/App/Outer.swift',
[
'struct Outer {',
' struct Config {',
' static func make() -> Int { return 1 }',
' }',
'}',
'',
'struct TopLevelConfig {',
' static func make() -> Int { return 2 }',
'}',
'',
].join('\n'),
],
[
'Sources/App/Builder.swift',
[
'func build() -> Int {',
' let a = Config.make()',
' let b = TopLevelConfig.make()',
' return a + b',
'}',
'',
].join('\n'),
],
]);
const calls = new Map(result.callSites.map((c) => [c.calleeText, c]));
for (const name of ['Config.make', 'TopLevelConfig.make']) {
expect(calls.has(name)).toBe(true);
}
expect(calls.get('Config.make')?.resolvedTargetFile).toBeUndefined();
expect(calls.get('TopLevelConfig.make')?.resolvedTargetFile).toBe('Sources/App/Outer.swift');
});
it('does not merge same-named targets of different packages', async () => {
const { result } = await extractFiles([
['Packages/A/Sources/App/Proto.swift', 'protocol Shared { }\n'],
['Packages/B/Sources/App/Model.swift', 'struct S: Shared { }\n'],
]);
// `Packages/A/Sources/App` and `Packages/B/Sources/App` share their last two
// segments but are separate modules, so the name stays unresolved.
expect(result.edges.filter((e) => e.relation === 'IMPLEMENTS')).toHaveLength(0);
});
it('resolves inside a nested package target', async () => {
const { result } = await extractFiles([
['Packages/A/Sources/App/Proto.swift', 'protocol Shared { }\n'],
['Packages/A/Sources/App/Model.swift', 'struct S: Shared { }\n'],
]);
// The control for the case above: the same layout, one package, so the
// conformance must still resolve.
const implementsEdges = result.edges.filter((e) => e.relation === 'IMPLEMENTS');
expect(implementsEdges).toHaveLength(1);
expect(implementsEdges[0]?.to).toBe('Packages/A/Sources/App/Proto.swift');
});
it('does not merge two packages vendored under the same Tests directory', async () => {
const { result } = await extractFiles([
['Tests/Fixtures/A/Sources/App/Proto.swift', 'protocol Shared { }\n'],
['Tests/Fixtures/B/Sources/App/Model.swift', 'struct S: Shared { }\n'],
]);
// Both paths carry an outer `Tests` segment. A scan that took the first
// marker would scope both to `Tests/Fixtures` — the same merge the package
// root fix rules out one level down, just reached through the outer package.
expect(result.edges.filter((e) => e.relation === 'IMPLEMENTS')).toHaveLength(0);
});
it('resolves inside a package vendored under a Tests directory', async () => {
const { result } = await extractFiles([
['Tests/Fixtures/A/Sources/App/Proto.swift', 'protocol Shared { }\n'],
['Tests/Fixtures/A/Sources/App/Model.swift', 'struct S: Shared { }\n'],
]);
// The control for the case above: the same layout, one fixture package.
const implementsEdges = result.edges.filter((e) => e.relation === 'IMPLEMENTS');
expect(implementsEdges).toHaveLength(1);
expect(implementsEdges[0]?.to).toBe('Tests/Fixtures/A/Sources/App/Proto.swift');
});
});
describe('Swift module-scope resolution yields to enclosing bindings', () => {
beforeEach(() => {
resetParserRegistryForTests();
});
// Every case below pairs a *shadowed* call with an unshadowed one of the same
// name, and the shadowed one gets its own file. Edges are file-to-file, so
// putting both in one file would make the assertion true either way: the
// resolved call would supply the very edge the unresolved call must not.
it('does not resolve a call to a parameter of the enclosing function', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Shadowed.swift', 'func shadowed(work: () -> Int) -> Int {\n return work()\n}\n'],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return work()\n}\n'],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('does not resolve a call to a local binding', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
[
'Sources/App/Shadowed.swift',
'func shadowed() -> Int {\n let work = { 1 }\n return work()\n}\n',
],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return work()\n}\n'],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
});
it('does not resolve a call to a guard binding, which is a sibling statement', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
[
'Sources/App/Shadowed.swift',
'func shadowed(opt: (() -> Int)?) -> Int {\n guard let work = opt else { return 0 }\n return work()\n}\n',
],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return work()\n}\n'],
]);
// `guard let` binds into the *rest of the block*, not into a nested scope,
// so the call sits beside the guard instead of inside it. A walk that only
// looked at ancestors would miss this one.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
});
it('does not resolve a call to a closure parameter', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
[
'Sources/App/Shadowed.swift',
'func shadowed(handler: (() -> Int) -> Int) -> Int {\n return handler { work in work() }\n}\n',
],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return work()\n}\n'],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
});
it('still resolves when the binding belongs to a different function of the same file', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
[
'Sources/App/Mixed.swift',
// Only the second function binds `work`, so exactly one of the two calls
// may resolve. The assertion is two-sided: it fails if the binding is
// ignored (both resolve, and the graph keeps one edge per resolved call)
// and it fails under a position-blind per-file rule (neither resolves).
// The binding set therefore has to be per call site, not per file.
'func caller() -> Int {\n return work()\n}\n\nfunc param(work: () -> Int) -> Int {\n return work()\n}\n',
],
]);
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Mixed.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('does not resolve a receiver that an enclosing scope binds', async () => {
const { result } = await extractFiles([
['Sources/App/Widget.swift', 'class Widget {\n static func make() -> Int { return 1 }\n}\n'],
['Sources/App/Shadowed.swift', 'func shadowed(Widget: Int) -> Int {\n return Widget.make()\n}\n'],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return Widget.make()\n}\n'],
]);
// The receiver fallback leans on a naming convention, so it has to yield to
// a scope that actually binds the name.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
expect(references[0]?.to).toBe('Sources/App/Widget.swift');
});
it('does not resolve a call an enclosing stored property shadows', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
[
'Sources/App/Stored.swift',
['struct S {', ' let work: () -> Void', ' func run() {', ' work()', ' }', '}', ''].join('\n'),
],
]);
// Swift reads an unqualified `work` inside a method as `self.work`, so the
// stored property binds the name — even though it is neither a parameter nor
// a local, the two shapes the collector once looked for.
expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0);
});
it('does not resolve a receiver an enclosing generic parameter shadows', async () => {
const { result } = await extractFiles([
['Sources/App/Factory.swift', 'class Factory {\n static func make() -> Int { return 1 }\n}\n'],
['Sources/App/Generic.swift', 'func run<Factory: Maker>() -> Int {\n return Factory.make()\n}\n'],
]);
// `Factory` in this position is the generic parameter, not the sibling
// class the receiver fallback would otherwise claim.
expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0);
});
it('does not resolve a call a closure capture list shadows', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Captured.swift', 'func outer() -> Int {\n let g = { [work = makeWork()] in work() }\n return g()\n}\n'],
]);
// The capture list introduces `work` inside the closure. The lambda's own
// parameter list is empty, which is all the old collector inspected.
expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0);
});
it('does not let one call stand in for another in the same body', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Twice.swift', 'func run() -> Int {\n work()\n work()\n return 0\n}\n'],
]);
// Neither call binds `work`; each one only *uses* it. Counting the other
// call's identifier as evidence would suppress both, so the over-collection
// this rule accepts has a floor: a call's own name sits below it.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(2);
for (const reference of references) expect(reference.to).toBe('Sources/App/Worker.swift');
});
it('recognises a binding whose name is not ASCII', async () => {
const { result } = await extractFiles([
['Sources/App/Pi.swift', 'func π() -> Int { return 1 }\n'],
['Sources/App/Runner.swift', 'func run(π: () -> Int) -> Int {\n return π()\n}\n'],
]);
// Swift identifiers are not ASCII. A class that only admits [A-Za-z] drops
// the parameter and lets the sibling `func π()` win the fallback.
expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0);
});
it('recognises a binding whose name has to be escaped', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func `repeat`() -> Int { return 1 }\n'],
['Sources/App/Shadowed.swift', 'func shadowed(`repeat`: () -> Int) -> Int {\n return `repeat`()\n}\n'],
['Sources/App/Plain.swift', 'func plain() -> Int {\n return `repeat`()\n}\n'],
]);
// A name that collides with a keyword is written between backticks, and the
// grammar reports it that way on both sides — the declaration and the call
// carry the same token, so leaving the escaped form out of the set is the
// only thing that breaks the pair.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('recognises a binding whose name is a symbol', async () => {
// U+1F680, written as an escape so the case stays readable in an editor.
const rocket = '\u{1F680}';
const { result } = await extractFiles([
['Sources/App/Worker.swift', `func ${rocket}() -> Int { return 1 }\n`],
['Sources/App/Shadowed.swift', `func shadowed(${rocket}: () -> Int) -> Int {\n return ${rocket}()\n}\n`],
['Sources/App/Plain.swift', `func plain() -> Int {\n return ${rocket}()\n}\n`],
]);
// Swift admits symbol names, emoji included. A class built out of Unicode
// *letters* still excludes every one of them.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Plain.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('resolves every call of a declaration that binds the name nowhere', async () => {
const body = Array.from({ length: 12 }, () => ' work()').join('\n');
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Many.swift', `func run() -> Int {\n${body}\n return 0\n}\n`],
]);
// The shadowing set is read off the declaration once and handed to every
// call inside it. Nothing binds `work` here, so all twelve calls resolve --
// a set looked up for the wrong declaration, or emptied after the first
// call, would show up as a shortfall rather than as a wrong edge.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(12);
for (const reference of references) expect(reference.to).toBe('Sources/App/Worker.swift');
});
it('does not let a type position stand in for a binding', async () => {
const { result } = await extractFiles([
['Sources/App/Factory.swift', 'func Logger() -> Int { return 1 }\n'],
['Sources/App/Use.swift', 'func run(logger: Logger) -> Int {\n return Logger()\n}\n'],
]);
// `Logger` in the parameter list is a *type*, and only a value binding can
// shadow a call. Counting the type position would suppress the resolution
// and drop the edge to the sibling function that actually answers for it.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Use.swift');
expect(references[0]?.to).toBe('Sources/App/Factory.swift');
});
it('does not let an argument mention stand in for a binding', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Run.swift', 'func run() {\n consume(work)\n work()\n}\n'],
]);
// Passing a function along is a *use* of its name. Counting that mention as
// a binding suppresses the call on the next line, and the edge it should
// have carried disappears. The mention may sit on either side of the call.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Run.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('resolves a call that passes its own name', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Run.swift', 'func run() {\n work(work)\n}\n'],
]);
// The declaration binds `work` nowhere, so both the callee and the argument
// refer to the sibling function. Reading the declaration whole made the
// argument the evidence that suppressed the callee -- the same mention, and
// the same call.
const references = result.edges.filter((e) => e.relation === 'REFERENCES');
expect(references).toHaveLength(1);
expect(references[0]?.from).toBe('Sources/App/Run.swift');
expect(references[0]?.to).toBe('Sources/App/Worker.swift');
});
it('still shadows a call whose name a closure parameter binds', async () => {
const { result } = await extractFiles([
['Sources/App/Worker.swift', 'func work() -> Int { return 1 }\n'],
['Sources/App/Run.swift', 'func run() {\n consume({ work in work() })\n}\n'],
]);
// A closure handed over as an argument opens its own scope: the argument
// marker must be cleared on the way in, or the parameter stops counting and
// the inner call resolves to the sibling function it does not mean.
expect(result.edges.filter((e) => e.relation === 'REFERENCES')).toHaveLength(0);
});
});
@@ -1,5 +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";
export interface ImportBindingMap {
/** Local name → exported symbol id in target file */
@@ -51,7 +53,8 @@ export function resolveCallSites(
callSites: AstCallSite[],
imports: AstImport[],
resolved: Map<string, ResolvedImport | undefined>,
symbolsByFile: Map<string, AstSymbol[]>
symbolsByFile: Map<string, AstSymbol[]>,
swiftModules?: SwiftModuleSymbolIndex
): AstCallSite[] {
const bindingsByFile = new Map<string, ImportBindingMap>();
return callSites.map((site) => {
@@ -60,14 +63,15 @@ export function resolveCallSites(
bindings = buildImportBindingsForFile(site.fromFile, imports, resolved, symbolsByFile);
bindingsByFile.set(site.fromFile, bindings);
}
return resolveOneCall(site, symbolsByFile, bindings);
return resolveOneCall(site, symbolsByFile, bindings, swiftModules);
});
}
function resolveOneCall(
site: AstCallSite,
symbolsByFile: Map<string, AstSymbol[]>,
bindings: ImportBindingMap
bindings: ImportBindingMap,
swiftModules?: SwiftModuleSymbolIndex
): AstCallSite {
const callee = site.calleeText;
@@ -97,6 +101,27 @@ function resolveOneCall(
return { ...site, resolvedTargetFile: importedFile, confidence: "INFERRED" };
}
// Swift module scope: a symbol declared elsewhere in the same module is
// visible without any import, so the same-file and import lookups above
// cannot be the only ones. INFERRED rather than EXTRACTED because the
// module boundary itself is read off the directory layout, not the syntax.
//
// A name bound by an enclosing scope wins over every module-level one, and
// 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)) {
const moduleSymbol = findSwiftModuleSymbol(swiftModules, site.fromFile, callee, ["function", "class"]);
if (moduleSymbol) {
return {
...site,
resolvedTargetId: moduleSymbol.id,
resolvedTargetFile: moduleSymbol.file,
confidence: "INFERRED"
};
}
}
return site;
}
@@ -125,6 +150,21 @@ function resolveOneCall(
return { ...site, resolvedTargetFile: site.fromFile, confidence: "INFERRED" };
}
// `Service.make()` where `Service` is declared in another file of the same
// Swift module. Swift convention capitalises type names, so a receiver that
// matches a module-local class declaration is a type reference and not a
// local value — the same heuristic the same-file branch above already uses.
//
// The heuristic is a convention, not a rule, so it still has to yield to the
// scopes that actually bind the name: a parameter or local called `Service`
// shadows the type, and the receiver then has nothing to do with this module.
if (swiftModules && !site.localBindings?.includes(recv)) {
const moduleClass = findSwiftModuleSymbol(swiftModules, site.fromFile, recv, ["class"]);
if (moduleClass) {
return { ...site, resolvedTargetFile: moduleClass.file, receiver: recv, confidence: "INFERRED" };
}
}
return site;
}
+18 -4
View File
@@ -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 { buildSwiftModuleSymbolIndex, findSwiftModuleSymbol } from "./module-scope.js";
import { ensureAstReady } from "./parser-registry.js";
import type { AstExtractionGap, AstImplementsSite, StructuralEdge, StructuralGraphResult } from "./types.js";
import { isAstParseableFile, walkFile } from "./walk.js";
@@ -44,6 +45,7 @@ export async function extractStructuralGraph(
const { repoRoot, files } = options;
const symbols: StructuralGraphResult["symbols"] = [];
const swiftModuleSymbols: StructuralGraphResult["symbols"] = [];
const imports: StructuralGraphResult["imports"] = [];
const callSites: StructuralGraphResult["callSites"] = [];
const implementsSites: AstImplementsSite[] = [];
@@ -73,6 +75,7 @@ export async function extractStructuralGraph(
}
filesParsed++;
symbols.push(...walked.symbols);
swiftModuleSymbols.push(...walked.swiftModuleSymbols);
imports.push(...walked.imports);
callSites.push(...walked.callSites);
implementsSites.push(...walked.implementsSites);
@@ -85,6 +88,13 @@ export async function extractStructuralGraph(
symbolsByFile.set(sym.file, list);
}
// Swift resolves symbols module-wide, not file-wide: two files under the same
// 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);
const resolvedImports = new Map<string, Awaited<ReturnType<typeof resolveImportSpecifier>>>();
const resolvedKeys = new Set<string>();
@@ -117,7 +127,7 @@ export async function extractStructuralGraph(
gaps.push(...unresolvedImportsToGaps(imports.filter((i) => !i.isTypeOnly), resolvedKeys));
const resolvedCalls = resolveCallSites(callSites, imports, resolvedImports, symbolsByFile);
const resolvedCalls = resolveCallSites(callSites, imports, resolvedImports, symbolsByFile, swiftModules);
for (const call of resolvedCalls) {
if (!call.resolvedTargetFile || call.resolvedTargetFile === call.fromFile) {
@@ -143,8 +153,10 @@ export async function extractStructuralGraph(
}
// IMPLEMENTS edges: resolve each implemented interface name to its defining
// file via (a) same-file interface symbols or (b) imported bindings. Names
// that resolve to neither (e.g. ambient/global types) are skipped.
// file via (a) same-file interface symbols, (b) imported bindings, or (c) for
// Swift, a declaration elsewhere in the same module — a conformance to a
// protocol of the same module needs no import. Names that resolve to none of
// these (e.g. ambient/global types) are skipped.
for (const site of implementsSites) {
const bindings = buildImportBindingsForFile(site.fromFile, imports, resolvedImports, symbolsByFile);
const localInterfaces = symbolsByFile.get(site.fromFile) ?? [];
@@ -154,7 +166,9 @@ export async function extractStructuralGraph(
if (sameFile) {
targetFile = site.fromFile;
} else {
targetFile = bindings.localToFile.get(ifaceName);
targetFile =
bindings.localToFile.get(ifaceName) ??
findSwiftModuleSymbol(swiftModules, site.fromFile, ifaceName, ["interface"])?.file;
}
if (!targetFile) {
continue;
@@ -0,0 +1,126 @@
import type { AstSymbol, AstSymbolKind } from "./types.js";
/**
* Swift has no source-level package or module declaration — unlike Go's
* `package` clause or Python's file-as-module rule, the module boundary is a
* build-system fact that the AST cannot read. The only layout with a mandated
* shape is SwiftPM: everything under `<package>/Sources/<Target>/` is one
* module, and everything under `<package>/Tests/<Target>/` is another (test
* targets see the library through `@testable import`, not by being the same
* module).
*
* The key spans the path *up to and including* the target directory, not just
* the `Sources/<Target>` tail. One repository can hold several Swift packages
* side by side, and `Packages/A/Sources/App` and `Packages/B/Sources/App` are
* two different modules that happen to share their last two segments; keying on
* the tail alone would merge them and let a name in one package resolve into
* the other.
*
* The marker that counts is the *innermost* one, because a package can also be
* nested under a directory the outer package already named `Sources` or `Tests`:
* `Tests/Fixtures/A/Sources/App` is package A's target `App`, not a target of the
* outer package. Derived from the path shape alone, this is still an inference,
* so the scan is deliberately biased towards under-scoping — see the loop below.
*
* Outside that layout this function returns `undefined` on purpose. Guessing a
* module boundary from an arbitrary directory tree would fabricate edges
* between files that Swift actually keeps apart, and a wrong edge is worse than
* a missing one: the missing one still surfaces as a gap.
*/
export function swiftModuleScope(relativePath: string): string | undefined {
const normalized = relativePath.replace(/\\/gu, "/");
if (!normalized.toLowerCase().endsWith(".swift")) {
return undefined;
}
const segments = normalized.split("/");
// The innermost marker wins, so the scan runs from the end. A repository may
// vendor whole packages beneath a directory of its own named `Tests` or
// `Sources` — `Tests/Fixtures/A/Sources/App` and `Tests/Fixtures/B/Sources/App`
// are two packages that share an outer `Tests` segment, and taking the first
// marker would scope both to `Tests/Fixtures` and merge them.
//
// The direction also decides how a layout this function misreads can fail.
// An inner marker can only ever yield a scope nested *inside* the true module,
// which loses a resolution; an outer one can span two real modules, which
// fabricates an edge. Missing beats wrong here for the same reason it does
// everywhere else in this layer.
//
// The loop stops before the last two segments: they have to hold the marker
// and the target directory, so `Sources/App.swift` states no target and gets
// no scope.
for (let index = segments.length - 3; index >= 0; index--) {
const segment = segments[index];
if (segment !== "Sources" && segment !== "Tests") {
continue;
}
return segments.slice(0, index + 2).join("/");
}
return undefined;
}
export interface SwiftModuleSymbolIndex {
/** Module scope key → every declaration found in that module. */
byModule: Map<string, AstSymbol[]>;
/** File → its module scope key, for files that sit inside a known module. */
scopeOfFile: Map<string, string>;
}
/**
* Index the declarations that a sibling file can reach by name.
*
* The caller supplies module-visible declarations only — top-level, and not
* `private` / `fileprivate`. Neither fact survives into `AstSymbol`, so it
* cannot be re-checked here; handing this function the full symbol list instead
* would let a method, a protocol requirement or a file-scoped declaration
* 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.
*/
export function buildSwiftModuleSymbolIndex(symbols: AstSymbol[]): SwiftModuleSymbolIndex {
const byModule = new Map<string, AstSymbol[]>();
const scopeOfFile = new Map<string, string>();
for (const symbol of symbols) {
const scope = swiftModuleScope(symbol.file);
if (!scope) {
continue;
}
scopeOfFile.set(symbol.file, scope);
const bucket = byModule.get(scope);
if (bucket) {
bucket.push(symbol);
} else {
byModule.set(scope, [symbol]);
}
}
return { byModule, scopeOfFile };
}
/**
* Find the single declaration of `name` in the same Swift module as `fromFile`.
*
* Returns `undefined` when the name is declared in another module, not at all,
* or more than once inside this one. An ambiguous name means the layout cannot
* say which file it lives in, so the caller records nothing rather than picking
* one arbitrarily — the same reasoning the module-import gap already follows.
*
* Declarations in `fromFile` are excluded: the same-file lookup in the caller
* already covers those, and admitting them here would let a same-file match
* arrive through the cross-file path.
*/
export function findSwiftModuleSymbol(
index: SwiftModuleSymbolIndex,
fromFile: string,
name: string,
kinds: readonly AstSymbolKind[]
): AstSymbol | undefined {
const scope = index.scopeOfFile.get(fromFile) ?? swiftModuleScope(fromFile);
if (!scope) {
return undefined;
}
const matches = (index.byModule.get(scope) ?? []).filter(
(symbol) => symbol.name === name && symbol.file !== fromFile && kinds.includes(symbol.kind)
);
return matches.length === 1 ? matches[0] : undefined;
}
@@ -28,6 +28,27 @@ export interface AstCallSite {
line: number;
calleeText: string;
receiver?: string;
/**
* Swift only. The names that could bind this call's callee (or receiver),
* gathered from the top-level declaration that encloses the call site:
* function and closure parameters, local `let`/`var`, and whatever a
* `for` / `if let` / `guard let` / `catch let` / `case let` introduces, plus
* stored properties, generic parameters and closure capture lists.
*
* A call whose callee (or receiver) is one of these names refers to that
* binding, so a module-wide lookup must not claim it: `run(work:) { work() }`
* calls the parameter, not a sibling file's `func work()`. Only the syntax
* tree knows this, which is why it is recorded here and not recomputed in the
* resolver.
*
* Collected per declaration rather than per call site, so it is deliberately
* coarse: the declaration is read whole, and a name that merely occurs in it —
* passed around as an argument rather than bound — counts. Both directions of
* error are not equal here: an extra name costs a resolution, a missing one
* invents a cross-file edge. Absent for non-Swift files and for sites with no
* such binding.
*/
localBindings?: string[];
resolvedTargetId?: string;
resolvedTargetFile?: string;
confidence: ManifestConfidence;
+212 -7
View File
@@ -1,5 +1,7 @@
import path from "node:path";
import type { Node } from "web-tree-sitter";
import type { CodeCollectedFile } from "../code-collector.js";
import {
collectExportLineStarts,
@@ -13,6 +15,11 @@ import type { AstCallSite, AstImplementsSite, AstImport, AstSymbol, AstSymbolKin
export interface FileWalkResult {
symbols: AstSymbol[];
/**
* Swift only: the declarations a sibling file of the same module can reach by
* name. Empty for every other language, and a subset of `symbols` for Swift.
*/
swiftModuleSymbols: AstSymbol[];
imports: AstImport[];
callSites: AstCallSite[];
implementsSites: AstImplementsSite[];
@@ -27,18 +34,19 @@ export function isAstParseableFile(relativePath: string): boolean {
export function walkFile(file: CodeCollectedFile): FileWalkResult {
const symbols: AstSymbol[] = [];
const swiftModuleSymbols: AstSymbol[] = [];
const imports: AstImport[] = [];
const callSites: AstCallSite[] = [];
const implementsSites: AstImplementsSite[] = [];
const parseErrors: string[] = [];
if (!isAstParseableFile(file.relativePath)) {
return { symbols, imports, callSites, implementsSites, parseErrors };
return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors };
}
if (Buffer.byteLength(file.content, "utf8") > MAX_FILE_BYTES) {
parseErrors.push(`skipped large file: ${file.relativePath}`);
return { symbols, imports, callSites, implementsSites, parseErrors };
return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors };
}
const variant = grammarForExtension(path.extname(file.relativePath))!;
@@ -51,17 +59,21 @@ 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, imports, callSites, implementsSites, parseErrors };
return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors };
}
if (!tree) {
parseErrors.push(`parse returned null: ${file.relativePath}`);
return { symbols, imports, callSites, implementsSites, parseErrors };
return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors };
}
try {
const query = getQuery(variant);
const exportLineStarts = collectExportLineStarts(variant, tree.rootNode);
// One traversal per file, shared by every call site below: doing this per
// call would re-walk the enclosing declaration once for each of its calls.
const swiftShadowedNames =
variant === "swift" ? buildSwiftShadowedNames(tree.rootNode) : undefined;
for (const match of query.matches(tree.rootNode)) {
const byName = new Map(match.captures.map((c) => [c.name, c.node]));
@@ -96,7 +108,7 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult {
const lineStart = decl.startPosition.row + 1;
const lineEnd = decl.endPosition.row + 1;
const exported = isExportedSymbol(variant, decl.startIndex, file.content, lineStart, exportLineStarts);
symbols.push({
const symbol: AstSymbol = {
id: symbolId(file.relativePath, kind, symbolName),
kind,
name: symbolName,
@@ -104,7 +116,13 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult {
lineStart,
lineEnd,
exported
});
};
symbols.push(symbol);
// Swift files in one module see each other without any import, so the
// module index needs exactly the declarations a sibling can reach.
if (variant === "swift" && isSwiftModuleVisible(decl)) {
swiftModuleSymbols.push(symbol);
}
continue;
}
@@ -115,11 +133,14 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult {
const receiver = byName.get("call.receiver")?.text;
const member = byName.get("call.member")?.text;
const calleeText = callee ?? (receiver && member ? `${receiver}.${member}` : callNode.text);
const localBindings =
swiftShadowedNames === undefined ? [] : swiftShadowedNamesAt(callNode, swiftShadowedNames);
callSites.push({
fromFile: file.relativePath,
line,
calleeText,
receiver,
...(localBindings.length > 0 ? { localBindings } : {}),
confidence: "INFERRED"
});
continue;
@@ -145,10 +166,194 @@ export function walkFile(file: CodeCollectedFile): FileWalkResult {
tree.delete();
}
return { symbols, imports, callSites, implementsSites, parseErrors };
return { symbols, swiftModuleSymbols, imports, callSites, implementsSites, parseErrors };
}
/**
* Whether the other files of a Swift module can reach this declaration by name.
*
* There is no `import` between the files of one module, so a sibling file sees
* every top-level declaration that is not narrowed to its own file. Two
* exclusions follow, and both are load-bearing when a name is looked up
* module-wide:
*
* - **Not top-level.** A method, a protocol requirement or a type nested in
* another type is reached through its container, not by a bare name. Admitting
* one would let an unqualified call in one file bind to an unrelated method in
* another, and two same-named members would also look like an ambiguous module
* name and suppress a resolution that was correct.
* - **Not file-scoped.** `private` and `fileprivate` narrow a declaration to the
* file that declares it (or to its enclosing declaration), so a sibling cannot
* see it. `private(set)` narrows only the setter and is *not* file-scoped;
* the grammar reports it as `private(set)`, which the comparison below leaves
* alone.
*
* Absence of a `modifiers` child means the default, `internal`, which the whole
* module sees.
*
* `namedChildren` is typed `(Node | null)[]` in web-tree-sitter, so both child
* lookups below are null-guarded with `?.`. The `?.` is load-bearing: without it
* the callbacks would have to reason about a null hole, and `tsc --noEmit`
* rejects them.
*/
function isSwiftModuleVisible(decl: Node): boolean {
if (decl.parent?.type !== "source_file") {
return false;
}
const modifiers = decl.namedChildren.find((child) => child?.type === "modifiers");
if (!modifiers) {
return true;
}
return !modifiers.namedChildren.some(
(child) =>
child?.type === "visibility_modifier" && (child.text === "private" || child.text === "fileprivate")
);
}
function symbolId(file: string, kind: AstSymbolKind, name: string): string {
const kindLabel = kind.charAt(0).toUpperCase() + kind.slice(1);
return `${file}#${kindLabel}:${name}`;
}
/** web-tree-sitter types `namedChildren` as `(Node | null)[]`; drop the holes. */
function namedChildrenOf(node: Node): Node[] {
return node.namedChildren.filter((child): child is Node => child !== null);
}
function addSwiftName(name: string, names: Set<string>): void {
// `_` is the "no internal name" placeholder, not a binding. Nothing else is
// tested here: every caller passes the text of a `simple_identifier` or a
// `type_identifier`, which is the grammar's own verdict that the token is a
// name, so there is no shape left to check. The character class that used to
// guard this was the defect — widened to Unicode letters it still dropped
// escaped identifiers such as `` `repeat` `` (which Swift requires when a name
// collides with a keyword) and symbol or emoji names, so a parameter with such
// a name never reached `localBindings` and a same-named sibling function won
// the fallback.
if (name !== "_") {
names.add(name);
}
}
/**
* The names in one top-level declaration that stop a call inside it from
* resolving through Swift module scope.
*
* `call-resolver` decides *which module* a bare call belongs to, but only the
* syntax tree knows whether the callee is genuinely a module-level declaration:
* `run(work:) { work() }` calls its parameter, and a `let work = ...` above the
* call wins over a sibling file's `func work()`. Resolving those against the
* module fabricates a cross-file edge, which is worse than missing one, so the
* names are gathered here — while the tree is still in hand — and carried on the
* call site.
*
* The question asked is deliberately *closed*: does the name occur anywhere else
* in this declaration? Every construct that can introduce a name — a parameter, a
* local `let`, `if let`, `guard let`, `for … in`, `catch let`, `case let`, a
* stored property, a generic parameter, a closure capture list — puts that name
* somewhere else in the declaration, whether or not this walk understands the
* construct. Enumerating the constructs instead is precisely what makes a
* whitelist the defect: there is always one more binding form, and that was the
* shape of every review round this file has had.
*
* The cost is paid on the other side, deliberately. A name that merely *appears*
* in the declaration — a value passed around rather than bound, a sibling
* statement's local — suppresses the resolution too. Over-suppressing costs a
* resolution; the opposite error invents an edge, and that asymmetry is the
* point.
*/
function collectSwiftShadowedNames(node: Node, names: Set<string>, insideArgument = false): void {
if (node.type === "simple_identifier" || node.type === "type_identifier") {
// An argument is an expression position: `consume(work)` mentions `work`, it
// does not bind it. Counting the mention suppresses the resolution of every
// `work()` in the declaration — including the call that passes its own name,
// `work(work)`, whose callee is the very thing the argument names.
if (!insideArgument) {
addSwiftName(node.text, names);
}
return;
}
// A type position such as `Int` in `(work: Int)` names a type, not a value;
// collecting it would shadow every call to a same-named function. A generic
// parameter is a `type_identifier` too, but it never sits under `user_type`,
// so it still lands in the set.
if (node.type === "user_type") {
return;
}
// The name a *call* goes through is a use, not a binding. Without this,
// `work(); work()` has each call count the other one's identifier as evidence,
// and both cross-file resolutions are suppressed. Arguments are still walked,
// so a closure that does bind a name — `handler { work in work() }` — keeps
// counting.
if (node.type === "call_expression") {
for (const child of namedChildrenOf(node)) {
if (child.type === "navigation_expression" || child.type === "simple_identifier") {
continue;
}
collectSwiftShadowedNames(child, names, insideArgument);
}
return;
}
// A closure opens a scope, so its parameters and captures bind for real however
// the closure itself was reached — including as an argument — and the argument
// marker is cleared on the way in. Everything else keeps the marker it was
// given.
const childInsideArgument = node.type === "lambda_literal" ? false : insideArgument || node.type === "value_arguments";
for (const child of namedChildrenOf(node)) {
collectSwiftShadowedNames(child, names, childInsideArgument);
}
}
/**
* The shadowing names of every top-level declaration in a file.
*
* Built once per file, not once per call. The ancestors a call could be shadowed
* by are the scopes between it and the file, and their union is exactly the
* top-level declaration that holds the call — so reading that declaration whole
* yields the same names, at one traversal per declaration instead of a subtree
* walk per call. The per-call version made a function holding N calls cost
* O(N²) AST visits.
*
* Read whole, the declaration no longer excludes the call's own subtree, and
* that side effect mattered: a name appearing only in the call's own arguments
* used to be invisible, so `work(work)` still resolved through its callee.
* Marking argument positions restores that — and goes one step further, since a
* mention in a *sibling* call (`consume(work)` before a bare `work()`) was
* counted as a binding by the older version as well. An argument is an
* expression position: it can mention a name, never bind one. A closure reached
* through an argument still opens its own scope, so its parameters and captures
* are collected.
*
* Keyed by `startIndex`: top-level declarations do not overlap, and tree-sitter
* hands out a fresh wrapper on every navigation, so node identity is not
* something a `Map` can be built on.
*/
function buildSwiftShadowedNames(root: Node): Map<number, string[]> {
const byDeclaration = new Map<number, string[]>();
for (const declaration of namedChildrenOf(root)) {
const names = new Set<string>();
collectSwiftShadowedNames(declaration, names);
byDeclaration.set(declaration.startIndex, [...names]);
}
return byDeclaration;
}
/**
* The shadowing names for one call site, read off the map built above.
*
* The scope that can shadow the name is the top-level declaration holding the
* call: it is the outermost ancestor below the file, and the file's own module
* level is what the fallback resolves *against*, so it cannot also be what
* shadows the name. A call that *is* a top-level statement has no such
* declaration above it, and the map holds only its own bare callee — which is
* skipped as a callee position, leaving the empty set the per-call walk
* produced.
*/
function swiftShadowedNamesAt(node: Node, byDeclaration: Map<number, string[]>): string[] {
let scope: Node = node;
while (scope.parent && scope.parent.type !== "source_file") {
scope = scope.parent;
}
return byDeclaration.get(scope.startIndex) ?? [];
}