diff --git a/gitnexus-shared/src/scope-resolution/registries/lookup-core.ts b/gitnexus-shared/src/scope-resolution/registries/lookup-core.ts index 5fdf18057..b98423ca6 100644 --- a/gitnexus-shared/src/scope-resolution/registries/lookup-core.ts +++ b/gitnexus-shared/src/scope-resolution/registries/lookup-core.ts @@ -108,7 +108,31 @@ export function lookupCore( const perCandidate = new Map(); // ── Step 1: lexical scope-chain walk ────────────────────────────────── - const lexicalShadowed = walkLexicalChain(name, startScope, acceptedKinds, ctx, perCandidate); + // + // SKIPPED for a NAMED explicit receiver. `recv.name` names a MEMBER of + // whatever `recv` denotes; it is not a lexical reference to `name`, so a + // binding of the bare tail name in an enclosing scope is never the right + // answer. Steps 2 and 3 (receiver type / owner members) are the routes. + // + // Without this, `options.baseUrl` bound to an unrelated function-local + // `const baseUrl` in the same file. This is the residual half of the defect + // JS/TS block scopes narrowed in #2699 — blocks moved nested-block locals + // off the chain, but a local declared directly in the function body stayed + // on it, and no amount of extra scopes reaches that case. + // + // `this` / `self` are deliberately EXEMPT. For a self-receiver the members + // and the lexical chain legitimately overlap — a class body is itself a + // scope that binds its members — so Step 1 is a real resolution route + // there, not a coincidence. Measured on a 762-file corpus: skipping Step 1 + // for every explicit receiver dropped 711 edges, of which 43 were + // `this.member` reads reaching their own owner. Exempting the self names + // keeps those and still removes the 668 named-receiver false positives. + const skipLexical = + params.explicitReceiver !== undefined && + !IMPLICIT_RECEIVERS.includes(params.explicitReceiver.name); + const lexicalShadowed = skipLexical + ? false + : walkLexicalChain(name, startScope, acceptedKinds, ctx, perCandidate); // ── Step 2: type-binding / MRO walk (methods/fields) ────────────────── if (params.useReceiverTypeBinding && ctx.methodDispatch !== undefined) { @@ -297,7 +321,31 @@ function resolveReceiverOwner( return undefined; } -const IMPLICIT_RECEIVERS: readonly string[] = Object.freeze(['self', 'this']); +/** + * Names that denote the enclosing instance rather than an arbitrary object. + * + * Two consumers, and both want the same set: `resolveReceiverOwner` above + * tries them when no explicit receiver is present, and the Step-1 skip in + * `lookupCore` exempts them because for a SELF receiver the members and the + * lexical chain legitimately overlap — a class body is itself a scope that + * binds its members — whereas for a named receiver they never do. + * + * `$this` is matched because the receiver name arrives as the reference node's + * RAW SOURCE TEXT (`extractExplicitReceiver` returns `cap.text` verbatim), so + * PHP's `$this->x` presents as `"$this"`, sigil included. Listing the spelling + * keeps this a data table rather than a language switch — this module resolves + * language behaviour through `providers.*` and `params` only (see the header) + * — and it follows the ingestion-side twin, `THIS_RECEIVERS` in + * `gitnexus/src/core/ingestion/type-env.ts`, which has always listed the + * sigil'd spelling rather than stripping it. Stripping would carry the same + * false-positive surface anyway (a JS variable literally named `$this`). + * + * That twin also lists `Me`, deliberately NOT mirrored here: no entry in + * `SupportedLanguages` uses it, so it can only ever exempt a variable that + * happens to be called `Me`. The two lists are otherwise the same set, and + * nothing enforces that — see the drift guard noted in #2714. + */ +const IMPLICIT_RECEIVERS: readonly string[] = Object.freeze(['self', 'this', '$this']); function lookupReceiverType( startScope: ScopeId, diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index a2bb0a5a9..946362748 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -47,14 +47,15 @@ "_rebaselined_2563_instance_ownership": "#2563: csharp-using-static adds same-file ownership, local-function, overload, partial-class, and cross-namespace same-name coverage. Prior 75cf380209fa7d1a8a3ec873be1a9424b4e5173be0b08234c2291e8521a9b3c1 -> e05dc27456bde8175948586c9e7689033a378fa40e9ca4ce78cce41fbea0f2f8; scaling 1.058 < 1.5." }, "rust": { - "fingerprint": "655aed01cf1b6b84fa0c64d48dfb2526ecb67f47d90f0a91edabacd269a212db", + "fingerprint": "7f1240b38457468f06b7931e0c2c578f218f922774d0dc7e2ee6ef3b08d4d689", "scaling_budget": 1.5, "_rebaselined_dyn_trait_object_2604": "#2604: RUST_SCOPE_QUERY now captures function_signature_item (abstract trait methods, no body) as a scope + declaration, so a &dyn Trait receiver can dispatch a CALLS edge to the trait's own method. Additive capture shift across every bench fixture with a required trait method. Prior df369c5a5f8de7753fc8bab8b4108ef5081750974ea5085ba9a867675ac9eb29 -> f7742f65f14d7d6590df7f16303fc3cc9dc0c233cd80bf90c98b084933cd3846; scaling 1.033 < 1.5.", "_rebaselined_callable_flow_2522_review": "PR #2522 review hardening: callable operands retain expression/qualified identity and formals retain signature metadata. Prior 65e5bca66bb1ca117949409e8fb5c80ee69d6f1b5318908eaaecf08da0482e5c -> df369c5a5f8de7753fc8bab8b4108ef5081750974ea5085ba9a867675ac9eb29; scaling 1.065 < 1.5.", "_rebaselined_callable_flow_2522_followup": "PR #2522 follow-up: Rust fn-value callable flow facts with invocation/constructor-result suppression. Prior ac610bbe97666bf285923479dd7b43a2fe4c5354aae8df1bcbafdc04fb220f82 -> 65e5bca66bb1ca117949409e8fb5c80ee69d6f1b5318908eaaecf08da0482e5c; scaling 1.024 < 1.5.", "_rebaselined": "#1956 tri-review U1: rust-qualified-trait fixture (scoped + generic-of-scoped impl trait paths); bareTypeIdentifier now resolves scoped_type_identifier bases by their name: tail (additive, no existing-fixture drift); linear (~1.04). #1975: + rust-scoped-impl fixture (impl a::Inner / b::Inner inherent scoped impls) \u2014 legacy @definition.impl scoped arm + findEnclosingClassInfo inherent-impl scoped target; rust scope-extractor captures byte-identical. | #942: scope-resolution-only cleanup reworded fixture comments; capture byte-positions shift, capture LOGIC unchanged.", "_note": "PR #1934: F66/F68 let-binding pattern narrowing; F71 union (Struct-labeled, now materialized via legacy @definition.struct + resolvable); F72 macro FULLY WIRED \u2014 @declaration.macro/@reference.macro + MacroRegistry \u2192 USES edges to Macro nodes (never a same-named fn). + rust-macro / rust-union fixtures and merged with origin/main #1975 rust-scoped-impl; fingerprint re-baselined (scaling ~0.99, fixture_count 126). #1992: + rust-nested-tail-collision-generic and rust-generic-impl-same-method-name (F3) fixtures \u2014 pure fixture-corpus drift, no scope-extractor change; fixture_count 127->129, fingerprint 56ffc1c0->b00aea0f.", - "_rebaselined_import_disambiguation_2514": "#2514: added rust-import-* and rust-dup-* fixtures under lang-resolution for the range-binding ambiguity latch + import-disambiguated resolution (for-loops / struct destructuring across explicit/aliased/glob use imports). emitRustScopeCaptures is unchanged; the corpus fingerprint shifts purely because the fixture set grew (130 -> 174). Prior f7742f65f14d7d6590df7f16303fc3cc9dc0c233cd80bf90c98b084933cd3846 -> 655aed01cf1b6b84fa0c64d48dfb2526ecb67f47d90f0a91edabacd269a212db; scaling 1.06 < 1.5." + "_rebaselined_import_disambiguation_2514": "#2514: added rust-import-* and rust-dup-* fixtures under lang-resolution for the range-binding ambiguity latch + import-disambiguated resolution (for-loops / struct destructuring across explicit/aliased/glob use imports). emitRustScopeCaptures is unchanged; the corpus fingerprint shifts purely because the fixture set grew (130 -> 174). Prior f7742f65f14d7d6590df7f16303fc3cc9dc0c233cd80bf90c98b084933cd3846 -> 655aed01cf1b6b84fa0c64d48dfb2526ecb67f47d90f0a91edabacd269a212db; scaling 1.06 < 1.5.", + "_rebaselined_self_type_binding_2714": "#2714: a Rust `Self` type binding now records the enclosing impl's type instead of the literal 'Self'. `let fresh = Self { .. }` inside `impl User` binds `fresh: User`; recorded verbatim it bound `fresh: Self`, which resolves to nothing. The type-env channel already substituted this (type-extractors/rust.ts findEnclosingImplType); the scope-resolution channel did not, so the two disagreed. The gap was invisible while lookupCore Step 1 still walked the lexical chain for NAMED receivers \u2014 the impl scope binds the method by name, so fresh.validate() resolved by accident \u2014 and became a lost CALLS edge when #2714 stopped that walk. Only the rust fingerprint moves; the other 14 languages are byte-identical." }, "php": { "fingerprint": "4a688fa5a7016546f7f3c6d44de023608ae80c5b0e3670c16f6e61b3632608fd", diff --git a/gitnexus/src/core/ingestion/languages/rust/captures.ts b/gitnexus/src/core/ingestion/languages/rust/captures.ts index f8f80b9e4..cae5ee003 100644 --- a/gitnexus/src/core/ingestion/languages/rust/captures.ts +++ b/gitnexus/src/core/ingestion/languages/rust/captures.ts @@ -123,6 +123,31 @@ export function emitRustScopeCaptures( } } + // `Self` in a type binding names the enclosing impl's type, not a type + // called "Self". `let fresh = Self { … }` inside `impl User` binds + // `fresh: User`; recorded verbatim it binds `fresh: Self`, which resolves + // to nothing and leaves the receiver's type unknown. + // + // The type-env channel already substitutes this + // (`type-extractors/rust.ts` → `findEnclosingImplType`); the + // scope-resolution channel did not, so the two disagreed. That went + // unnoticed while `lookupCore` Step 1 still walked the lexical chain for + // named receivers — the impl scope binds the method by name, so + // `fresh.validate()` resolved by accident. #2714 stopped that walk for + // named receivers and the gap became a lost edge (#2699 follow-up). + const tbTypeNode = nodeMap['@type-binding.type']; + if (grouped['@type-binding.type']?.text === 'Self' && tbTypeNode !== undefined) { + const implNode = findEnclosingImpl(tbTypeNode); + const implTypeNode = implNode?.childForFieldName('type') ?? null; + if (implTypeNode !== null) { + grouped['@type-binding.type'] = syntheticCapture( + '@type-binding.type', + tbTypeNode, + implTypeNode.text, + ); + } + } + // Hoist return-type bindings from impl block functions to module level. // The auto-hoist in the scope-extractor places a type binding whose // anchor matches its innermost scope on the parent scope. By using the diff --git a/gitnexus/src/core/ingestion/tree-sitter-queries.ts b/gitnexus/src/core/ingestion/tree-sitter-queries.ts index 380cb8656..8eb1d8c3d 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -59,6 +59,18 @@ export const TYPESCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression))) @definition.function +; Generator EXPRESSIONS bound to a name (\`const g = function* () {}\`). Without +; these, the binding emitted a \`Const\` node rather than a \`Function\` one, so +; \`g()\` resolved to nothing: \`buildGraphTargetIndex\` only admits a callable +; node. Same construct and same binding semantics as the \`function_expression\` +; rules directly above, so same label. Covers the four variable-binding shapes +; (const/let and var, each plain and exported); a generator in an object-literal +; pair or a HOC wrapper is NOT covered and still falls through anonymous. +(lexical_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function))) @definition.function + (export_statement declaration: (lexical_declaration (variable_declarator @@ -71,6 +83,12 @@ export const TYPESCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +(export_statement + declaration: (lexical_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function)))) @definition.function + ; \`var\` closure bindings (#2693). The lexical rules above cover const/let; ; \`var\` is a different grammar node, so \`var f = (x) => x\` kept a Variable ; label while const/let got Function — and the CALLS edge that resolved through @@ -86,6 +104,11 @@ export const TYPESCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression))) @definition.function +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function))) @definition.function + (export_statement declaration: (variable_declaration (variable_declarator @@ -98,6 +121,12 @@ export const TYPESCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function)))) @definition.function + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; The pair's key field carries the meaningful name. Without these patterns, ; calls inside the arrow are attributed to the file (issue #1166), and the @@ -443,6 +472,18 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression))) @definition.function +; Generator EXPRESSIONS bound to a name (\`const g = function* () {}\`). Without +; these, the binding emitted a \`Const\` node rather than a \`Function\` one, so +; \`g()\` resolved to nothing: \`buildGraphTargetIndex\` only admits a callable +; node. Same construct and same binding semantics as the \`function_expression\` +; rules directly above, so same label. Covers the four variable-binding shapes +; (const/let and var, each plain and exported); a generator in an object-literal +; pair or a HOC wrapper is NOT covered and still falls through anonymous. +(lexical_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function))) @definition.function + (export_statement declaration: (lexical_declaration (variable_declarator @@ -455,6 +496,12 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +(export_statement + declaration: (lexical_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function)))) @definition.function + ; \`var\` closure bindings (#2693). The lexical rules above cover const/let; ; \`var\` is a different grammar node, so \`var f = (x) => x\` kept a Variable ; label while const/let got Function — and the CALLS edge that resolved through @@ -470,6 +517,11 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression))) @definition.function +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function))) @definition.function + (export_statement declaration: (variable_declaration (variable_declarator @@ -482,6 +534,12 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (generator_function)))) @definition.function + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; See TYPESCRIPT_QUERIES for rationale (issue #1166). (pair diff --git a/gitnexus/src/core/ingestion/workers/callable-id.ts b/gitnexus/src/core/ingestion/workers/callable-id.ts new file mode 100644 index 000000000..75d6bfcdd --- /dev/null +++ b/gitnexus/src/core/ingestion/workers/callable-id.ts @@ -0,0 +1,62 @@ +/** + * The id rules for a callable nested inside another callable (#2699). + * + * Extracted from `parse-worker.ts` for one reason: **three** phases there + * build these ids independently — the definition phase + * (`callableOwnQualifiedName`), the caller-attribution phase + * (`findEnclosingFunctionId`), and the worker-path node-id derivation in + * `processFileGroup`. An id they compute differently is not a test failure; + * the caller attaches to a node that does not exist, so the edge is dropped + * rather than reported. "Zero dangling edges" is what that looks like from + * outside, which is why the divergence #2714 fixed went unnoticed. + * + * These functions are pure and free of module-scope side effects, unlike + * `parse-worker.ts`, which posts a `ready` message to `parentPort` at import + * and therefore cannot be value-imported by a unit test at all. That is what + * makes the rule testable rather than merely commented. + * + * See `parse-worker.ts`'s `enclosingCallablePrefix` for how the prefix passed + * in here is derived, and why only genuinely nested callables get one. + */ + +import type { SyntaxNode } from '../utils/ast-helpers.js'; + +/** + * A function-local callable's own name segment: its name plus its declaration + * position. + * + * The name chain alone is not enough, and the gap is the language's, not the + * grammar's: ECMAScript creates an environment record per function AND per + * block, so sibling blocks in one function hold genuinely different bindings — + * + * function outer(a) { + * if (a) { const pick = …; return pick(1); } // one binding + * else { const pick = …; return pick(2); } // a DIFFERENT binding + * } + * + * — and both are `outer.pick` by name. Putting a block token in the qualifier + * would tag every local inside any `if`, the common case, and buy nothing over + * putting the position on the declaration itself: a declaration's own position + * is unique across every environment record it could belong to, without the + * qualifier having to enumerate them. One rule, no conditionals, O(1). + * + * Applied ONLY to locals. Top-level functions and class methods keep their + * bare/class-qualified ids, which is what keeps this off the symbols other + * files, saved queries and stored references actually address. + */ +export const localIdentity = (node: SyntaxNode, name: string): string => + `${name}@${node.startPosition.row}:${node.startPosition.column}`; + +/** + * The qualified name of a callable nested inside another callable — THE single + * definition of that rule, shared by all three id-building phases. + * + * A comment asking three call sites to stay in step is exactly the invariant + * that rots; routing them through one function makes divergence require + * deleting a call rather than editing a duplicated expression. + */ +export const nestedCallableQualifiedName = ( + prefix: string, + node: SyntaxNode, + name: string, +): string => `${prefix}.${localIdentity(node, name)}`; diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index c3a619a01..18bdd5bc0 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -1,4 +1,5 @@ import { parentPort, threadId, workerData } from 'node:worker_threads'; +import { localIdentity, nestedCallableQualifiedName } from './callable-id.js'; import Parser from 'tree-sitter'; import JavaScript from 'tree-sitter-javascript'; import TypeScript from 'tree-sitter-typescript'; @@ -767,28 +768,12 @@ function getMethodInfo( * and keep their existing ids byte-for-byte, which is what bounds the id churn * this change forces. * - * `localIdentity` below completes it. The name chain alone is not enough, and - * the gap is the language's, not the grammar's: ECMAScript creates an - * environment record per function AND per block, so sibling blocks in one - * function hold genuinely different bindings — - * - * function outer(a) { - * if (a) { const pick = …; return pick(1); } // one binding - * else { const pick = …; return pick(2); } // a DIFFERENT binding - * } - * - * — and both are `outer.pick` by name. Putting a block token in the qualifier - * would tag every local inside any `if`, the common case, and buy nothing over - * putting the position on the declaration itself: a declaration's own position - * is unique across every environment record it could belong to, without the - * qualifier having to enumerate them. One rule, no conditionals, O(1). - * - * Applied ONLY to locals. Top-level functions and class methods keep their - * bare/class-qualified ids, which is what keeps this off the symbols other - * files, saved queries and stored references actually address. + * `localIdentity` completes it, and both it and the shared + * `nestedCallableQualifiedName` rule now live in `./callable-id.ts`: this + * module posts a `ready` message to `parentPort` at import, so a unit test + * cannot value-import it, and a rule three phases must agree on has to be + * testable rather than merely commented (#2714). */ -const localIdentity = (node: SyntaxNode, name: string): string => - `${name}@${node.startPosition.row}:${node.startPosition.column}`; /** * Boundary for the enclosing-callable walk (#2699). @@ -873,9 +858,9 @@ const callableOwnQualifiedName = ( if (cached !== undefined) return cached; const efnResult = provider.methodExtractor?.extractFunctionName?.(fnNode, filePath); - // An anonymous callable has no name of its own, so it IS its position — - // `localIdentity` supplies the same suffix the local branch below appends, - // and the two must not stack. + // An anonymous callable has no name of its own, so it IS its position: the + // `ownName === null` branch below carries the position INSTEAD of a name, + // never in addition to one, so the two spellings cannot stack. const ownName = efnResult?.funcName ?? genericFuncName(fnNode) ?? null; const prefix = enclosingCallablePrefix(fnNode, filePath, provider); @@ -884,12 +869,11 @@ const callableOwnQualifiedName = ( ? cachedFindEnclosingClassInfo(fnNode, filePath, provider.resolveEnclosingOwner) : null; const owner = prefix ?? classInfo?.className; - const localName = localIdentity(fnNode, ownName ?? 'fn'); const result = prefix !== undefined - ? `${prefix}.${localName}` + ? nestedCallableQualifiedName(prefix, fnNode, ownName ?? 'fn') : ownName === null - ? localName + ? localIdentity(fnNode, 'fn') : owner ? `${owner}.${ownName}` : ownName; @@ -947,7 +931,16 @@ const findEnclosingFunctionId = ( const nestedPrefix = enclosingCallablePrefix(current, filePath, provider); const ownerName = nestedPrefix ?? classInfo?.className ?? standaloneMethodInfo?.receiverType ?? undefined; - const qualifiedName = ownerName ? `${ownerName}.${funcName}` : funcName; + // Lockstep with the other two id-building phases — see + // `nestedCallableQualifiedName`, which is the shared rule. When a + // nested prefix exists it IS `ownerName`, so this branch and the + // owner branch below cannot disagree about which prefix applies. + const qualifiedName = + nestedPrefix !== undefined + ? nestedCallableQualifiedName(nestedPrefix, current, funcName) + : ownerName + ? `${ownerName}.${funcName}` + : funcName; // Include # suffix to match definition-phase Method/Constructor IDs. // Use the same MethodExtractor (getMethodInfo) as the definition phase. // When same-arity collisions exist, also append ~type1,type2. @@ -2320,7 +2313,7 @@ const processFileGroup = ( qualifiedTypeName !== undefined ? qualifiedTypeName : nestedCallablePrefix !== undefined && definitionNode - ? `${nestedCallablePrefix}.${localIdentity(definitionNode, nodeName)}` + ? nestedCallableQualifiedName(nestedCallablePrefix, definitionNode, nodeName) : enclosingClassInfo ? `${enclosingClassInfo.className}.${nodeName}` : nodeName; diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 56b64cdbd..d2be2e1a0 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -91,13 +91,17 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // as the reason to re-check SCHEMA_BUMP against origin/main immediately before // merging, not just when the branch is cut; the same collision hit // INCREMENTAL_SCHEMA_VERSION in #2653/#2654. +// v27: generator EXPRESSIONS bound to a name emit a callable definition capture, +// and nested-callable caller attribution appends the localIdentity suffix the +// definition phase already used. Both are parse-time, so a warm cache would +// otherwise replay the old captures and ids verbatim. // v20: Java/Kotlin capture side-channels persist package and class-annotation // facts for shared Spring Bean resolution. // v19: Java enum constant bodies emit E$N Class nodes; anonymous naming uses // JLS 13.1 immediate-host chains (#2555). // v18: Worker$N anonymous bodies. v17: callable-value-flow operand identity. // v16: direct callee identity. -const SCHEMA_BUMP = 26; +const SCHEMA_BUMP = 27; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 731ea253a..eb46bfd82 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -518,8 +518,15 @@ export interface RepoMeta { * destroying the javac-compatible JLS identity of #2550/#2555/#2562. An index stamped * v18 therefore holds WRONG Java ids, and without this bump it passes the reuse gate * and keeps them on every unchanged file; force a full re-analyze instead. + * v20: a NAMED explicit receiver no longer resolves its member through the lexical + * scope chain (#2699 follow-up). `options.baseUrl` used to bind to an unrelated + * function-local `const baseUrl`; measured on a 762-file corpus this removes 709 + * such edges and adds none. `this`/`self` are exempt, so the 2 genuine self-alias + * reads it also covered are kept. A v19 index holds those false CALLS/ACCESSES on + * every unchanged file and would keep serving them through the reuse gate; force a + * full re-analyze instead. */ -export const INCREMENTAL_SCHEMA_VERSION = 19; +export const INCREMENTAL_SCHEMA_VERSION = 20; export interface IndexedRepo { repoPath: string; diff --git a/gitnexus/test/fixtures/rust-captures-golden/expected-captures.json b/gitnexus/test/fixtures/rust-captures-golden/expected-captures.json index 755630759..3eff888cb 100644 --- a/gitnexus/test/fixtures/rust-captures-golden/expected-captures.json +++ b/gitnexus/test/fixtures/rust-captures-golden/expected-captures.json @@ -117,11 +117,11 @@ }, "rust-constructor-type-inference/src/repo.rs": { "captureGroups": 15, - "digest": "5c0858427be24f2cc6ae346b9dc8294d85a530bcf2f529829d757b69f768dc65" + "digest": "27a1154fb22f567cc935badeb6751b402559009bc2879a2dbd5d3fc24ed502ec" }, "rust-constructor-type-inference/src/user.rs": { "captureGroups": 15, - "digest": "043cd8b9341ab299750f8d07f1d1ca34b714c4ecf69acba80ec827e93e3852c0" + "digest": "dd518ba2a0b041b78a714078fb7c897c820e6365c37a3d1fb67b8479d1d35e80" }, "rust-coverage/macros.rs": { "captureGroups": 7, @@ -165,11 +165,11 @@ }, "rust-default-constructor/src/repo.rs": { "captureGroups": 22, - "digest": "8abe9352adc39d7b197e7e042fbb17f5bdad89946bae7b5e8fa11c897102a7f5" + "digest": "d338afd4327e092f54d1a12e982b3346696f283db9d77b844a2476b519296bcb" }, "rust-default-constructor/src/user.rs": { "captureGroups": 22, - "digest": "c53db401a81fde2ffd5665393acb9cd605a62ec51c015c3aafb3f41c0897471f" + "digest": "7c5f68cb338041ea735aeeb8c0ae6d1da8f5f1f4be343d51a318549799ef5713" }, "rust-dup-fields-2/src/c_a.rs": { "captureGroups": 11, @@ -489,7 +489,7 @@ }, "rust-method-enrichment/src/lib.rs": { "captureGroups": 42, - "digest": "a4d9ca570fbb1ff1859a0b4f737aa3507b236f99700d37ded2c8c36518add567" + "digest": "7465fc5519b3f643f367ff8c0ae3769a0cb2dbd10e2ea7582bbcff8127a139eb" }, "rust-method-enrichment/src/main.rs": { "captureGroups": 18, @@ -605,11 +605,11 @@ }, "rust-scoped-multi-file/src/models/repo.rs": { "captureGroups": 20, - "digest": "9c41e8dea4bec804dc08ebb2af3ee06f6cbed2d4e72217331f4a42dd227ea6a9" + "digest": "ac717eb271403913640f21ced123449f2ff399e9a7ecf8c668a8d8a9996d8c59" }, "rust-scoped-multi-file/src/models/user.rs": { "captureGroups": 20, - "digest": "ab12913053296035ad20ebd2af9f2f9988f8b89d32af41f3915f8706f00fa600" + "digest": "bbc80d3aab883aa959627a8915ed18f1e90ea1061d65bc41eaf967a85ba91f05" }, "rust-self-struct-literal/main.rs": { "captureGroups": 11, @@ -617,7 +617,7 @@ }, "rust-self-struct-literal/models.rs": { "captureGroups": 32, - "digest": "e02d230c1215b3fd87fedbdaf98649a407fa1f2bfc61beb66f49d7ba070de7de" + "digest": "d4d831b113acf0a809b8aeec9a8ff02a2ce763fd649c16ceb5792c0d1a270047" }, "rust-self-this-resolution/src/repo.rs": { "captureGroups": 10, diff --git a/gitnexus/test/integration/block-scope-shadowing.test.ts b/gitnexus/test/integration/block-scope-shadowing.test.ts index b6979fdc2..fe964430d 100644 --- a/gitnexus/test/integration/block-scope-shadowing.test.ts +++ b/gitnexus/test/integration/block-scope-shadowing.test.ts @@ -14,19 +14,26 @@ * (`options.baseUrl`) mis-resolving to an unrelated function-local `const` of * the same name in the same file. * - * The cause is not block-specific: `lookupCore` Step 1 walks the lexical chain - * for every lookup, including explicit-receiver property reads, so - * `options.baseUrl` can bind to a local `baseUrl`. Block scopes do not fix that - * — they narrow it, by moving the local off the chain of any reference outside - * its block. The remaining case (a local declared directly in the function - * body) is unchanged and still mis-resolves; that is pre-existing and tracked - * separately. + * The cause was not block-specific: `lookupCore` Step 1 walked the lexical + * chain for EVERY lookup, including explicit-receiver property reads, so + * `options.baseUrl` could bind to a local `baseUrl`. Block scopes narrowed + * that — they moved a nested-block local off the chain of any reference + * outside its block — but a local declared directly in the FUNCTION BODY + * stayed on it, and no amount of extra scopes reaches that case. * - * So these tests pin the direction of the change in BOTH directions: the - * property read must not reach the block-local, and the genuine bare read of - * that same local must still emit its edge. Deleting the block-scope capture - * fails the first; over-suppressing (dropping block bindings instead of - * scoping them) fails the second. + * That residual half is fixed here too: Step 1 is now skipped when the site + * has a NAMED explicit receiver, since `recv.name` addresses a member of + * whatever `recv` denotes and never a lexical binding of the bare tail name. + * The second describe below pins it. What remains, deliberately, is that a + * `this`/`self` read can still bind lexically to a same-named local — that is + * the price of keeping the genuine self-alias reads Step 1 resolves correctly + * (`const self = this; self.member`), which is why those two names are exempt. + * + * So these tests pin the change in BOTH directions: a property read must not + * reach a same-named local, and a real member read must still resolve through + * the receiver's own type. Deleting the block-scope capture fails the first; + * over-suppressing — dropping block bindings rather than scoping them, or + * skipping Step 1 for `this` as well — fails the second. */ import { describe, expect, it, vi } from 'vitest'; import fs from 'node:fs'; @@ -114,3 +121,111 @@ describeIfWorkerBuilt('block scopes keep a property read off a same-named block expect(edges[0]).toContain('-> Property:box.ts:Box.baseUrl'); }); }); + +describeIfWorkerBuilt('a property read never resolves to a lexical binding of its own name', () => { + // The residual half. Block scopes moved a NESTED-block local off the chain of + // a reference outside that block, which removed 114 false edges on a 762-file + // corpus. A local declared directly in the FUNCTION BODY stayed on the chain, + // so `options.baseUrl` still bound to it — same defect, one scope level up, + // and not fixable by adding more scopes. + // + // Fixed in `lookupCore` instead: Step 1's lexical walk is skipped when the + // site has an explicit receiver. `recv.name` names a member of whatever + // `recv` denotes; a binding of the bare tail name in an enclosing scope is + // never the right answer. + + it('TypeScript: `options.baseUrl` does not ACCESS a function-body-level `const baseUrl`', async () => { + const edges = await accessEdgesFor( + 'body.ts', + [ + 'export function pick(options: { baseUrl?: string }, fallback: string): string {', + ' const baseUrl = fallback.trim();', + ' if (baseUrl.length > 0) return baseUrl;', + ' return options.baseUrl ?? fallback;', + '}', + '', + ].join('\n'), + ); + + expect(edges.filter((e) => e.endsWith('baseUrl'))).toEqual([]); + }); + + it('TypeScript: a real member read still resolves through the receiver type', async () => { + // The guard against over-suppression: skipping Step 1 must not take Steps + // 2 and 3 with it. `this.baseUrl` has an explicit receiver too, and it + // must still reach the class property. + const edges = await accessEdgesFor( + 'recv.ts', + [ + 'export class Box {', + " baseUrl = 'https://example.com';", + ' read(): string {', + ' return this.baseUrl;', + ' }', + '}', + '', + ].join('\n'), + ); + + expect(edges).toHaveLength(1); + expect(edges[0]).toContain('-> Property:recv.ts:Box.baseUrl'); + }); + + it('PHP: `$this` is exempt from the skip, like `this` and `self`', async () => { + // COMPANION INVARIANT, not a discriminating regression test — and that was + // measured, not assumed. The receiver name arrives as raw source text, so + // PHP's `$this->x` presents as `"$this"` and matched neither exempt name + // until #2714; but no PHP shape tried here depends on Step 1. This fixture + // (a closure reading `$this->…` inside a method that also declares a + // same-named local) produces byte-identical edge sets with `$this` present + // and absent from `IMPLICIT_RECEIVERS`, because Step 2 resolves the + // receiver's type first. + // + // It is kept for the same reason the `this.baseUrl` case above is: the + // exemption is protective. Every other language's self-receiver keeps its + // Step-1 route, and the 762-file corpus that measured "0 true edges lost" + // was TypeScript-only, so PHP's safety was never established by evidence. + // This pins that PHP member resolution through a self-receiver keeps + // working if Step 2's coverage ever changes. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-php-self-')); + try { + fs.writeFileSync( + path.join(dir, 'Box.php'), + [ + 'baseUrl . $this->helper();', + ' };', + ' return $fn() . $baseUrl;', + ' }', + '}', + '', + ].join('\n'), + 'utf-8', + ); + const result = await runPipelineFromRepo(dir, () => {}, { + workerPoolSize: 1, + workerUrlForTest: DIST_WORKER_URL, + keepLocalValueSymbols: true, + }); + const calls = result.graph.relationships + .filter((rel) => rel.type === 'CALLS') + .map((rel) => rel.targetId) + .sort(); + + // `$this->helper()` inside the closure reaches the class method, and the + // same-named local `$baseUrl` never becomes a call target. + expect(calls).toContain('Method:Box.php:Box.helper#0'); + expect(calls.filter((t) => t.includes('baseUrl'))).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/integration/cfg/__snapshots__/pipeline-pdg.test.ts.snap b/gitnexus/test/integration/cfg/__snapshots__/pipeline-pdg.test.ts.snap index d8337e88c..e36595271 100644 --- a/gitnexus/test/integration/cfg/__snapshots__/pipeline-pdg.test.ts.snap +++ b/gitnexus/test/integration/cfg/__snapshots__/pipeline-pdg.test.ts.snap @@ -3,7 +3,7 @@ exports[`U7 — C-family worker-mode --pdg pipeline > C#: --pdg off is byte-identical (zero PDG nodes/edges, stable golden digest) 1`] = ` { "byRelType": { - "CALLS": 7, + "CALLS": 6, "DEFINES": 3, "HAS_METHOD": 16, "MEMBER_OF": 9, @@ -18,8 +18,8 @@ exports[`U7 — C-family worker-mode --pdg pipeline > C#: --pdg off is byte-iden "Namespace": 1, "Process": 1, }, - "edgeDigest": "0497d26dbe060d36426bf10cde5db490a6d82c013e0835296723a4c00ef7442a", - "relationships": 38, + "edgeDigest": "60d3d5449952563f9364148494c5bce96c306dba921d140dc08176e99d7f2b3d", + "relationships": 37, "symbols": 24, } `; diff --git a/gitnexus/test/integration/closure-binding-labels.test.ts b/gitnexus/test/integration/closure-binding-labels.test.ts index f83d52d70..c0cafaa6e 100644 --- a/gitnexus/test/integration/closure-binding-labels.test.ts +++ b/gitnexus/test/integration/closure-binding-labels.test.ts @@ -479,6 +479,42 @@ describeIfWorkerBuilt('closure bindings resolve in the remaining languages (#269 expect(targets).toEqual(['Function:c.js:handler']); }); + + it('TypeScript: a generator EXPRESSION binding is a Function, like the other forms', async () => { + // `function*` as an expression is its own grammar node, matched by none of + // the closure-binding definition rules — so the binding emitted a `Const` + // and `g(1)` resolved to nothing, since `buildGraphTargetIndex` only + // admits a callable node. Same defect shape as the `var` case above. + const targets = await callTargetsFor( + 'gen.ts', + 'const g = function* (x: number) {\n yield x;\n};\n\nexport function caller() {\n return g(1);\n}\n', + ); + + expect(targets).toEqual(['Function:gen.ts:g']); + }); + + it('JavaScript: an exported `var` generator expression resolves too', async () => { + // Covers the two axes the rules multiply over — declaration keyword and + // export wrapper — in the language where `var` is idiomatic. + const targets = await callTargetsFor( + 'gen.js', + 'export var g = function* (x) {\n yield x;\n};\n\nexport function caller() {\n return g(1);\n}\n', + ); + + expect(targets).toEqual(['Function:gen.js:g']); + }); + + it('TypeScript: a generator DECLARATION is unaffected', async () => { + // The declaration form already resolved; it shares the emit path the new + // expression rules were inserted beside, so it is the guard against the + // insertion disturbing it. + const targets = await callTargetsFor( + 'decl.ts', + 'function* g(x: number) {\n yield x;\n}\n\nexport function caller() {\n return g(1);\n}\n', + ); + + expect(targets).toEqual(['Function:decl.ts:g']); + }); }); describeIfWorkerBuilt('a closure binding is a call TARGET, not yet a call SOURCE', () => { diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index bcd4a6542..6838c99eb 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -73,8 +73,8 @@ describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => { }); describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { - it('INCREMENTAL_SCHEMA_VERSION is bumped to 19 (class-body boundary fix, #2699)', () => { - expect(INCREMENTAL_SCHEMA_VERSION).toBe(19); + it('INCREMENTAL_SCHEMA_VERSION is bumped to 20 (named-receiver lexical fallback, #2699)', () => { + expect(INCREMENTAL_SCHEMA_VERSION).toBe(20); }); it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => { @@ -150,7 +150,12 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { // enclosing-callable walk on class DECLARATIONS only, so `Worker$1.run` was re-keyed // as `Worker.makeHandler.run@7:12`. Reusing it would keep those on unchanged files. expect(passesReuseGate(18)).toBe(false); + // A pre-v20 (v19) index holds the false CALLS/ACCESSES a NAMED explicit receiver + // used to mint through the lexical chain (`options.baseUrl` → a function-local + // `const baseUrl`) — 709 of them on a 762-file corpus. Reusing it would keep + // every one on unchanged files. + expect(passesReuseGate(19)).toBe(false); // A current-version stamp passes the gate (incremental top-up eligible). - expect(passesReuseGate(19)).toBe(true); + expect(passesReuseGate(20)).toBe(true); }); }); diff --git a/gitnexus/test/unit/callable-id-lockstep.test.ts b/gitnexus/test/unit/callable-id-lockstep.test.ts new file mode 100644 index 000000000..d22266418 --- /dev/null +++ b/gitnexus/test/unit/callable-id-lockstep.test.ts @@ -0,0 +1,77 @@ +/** + * #2699 / #2714 — the nested-callable id rule has ONE definition. + * + * Three phases in `parse-worker.ts` build the id of a callable nested inside + * another callable, independently: the definition phase + * (`callableOwnQualifiedName`), the caller-attribution phase + * (`findEnclosingFunctionId`), and the worker-path node-id derivation in + * `processFileGroup`. They must agree byte-for-byte, and when they do not the + * failure is SILENT — the caller id names a node that does not exist, so the + * edge is dropped rather than reported. "Zero dangling edges" is what that + * looks like from the outside, which is why it went unnoticed. + * + * Caller attribution really did omit the position suffix until #2714. The fix + * routed all three through `nestedCallableQualifiedName`; this file pins both + * halves of that — the rule's contract, and the fact that no call site has + * re-inlined it. + * + * The rule reads only `startPosition` off the node, so a positional stub is a + * complete input here; parsing real source would add a tree-sitter dependency + * without testing anything more of this function. + * + * The rules live in `callable-id.ts` rather than `parse-worker.ts` precisely so + * this file can exist: parse-worker posts a `ready` message to `parentPort` at + * import, so value-importing it from a unit test throws before any test runs. + */ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { nestedCallableQualifiedName } from '../../src/core/ingestion/workers/callable-id.js'; +import type { SyntaxNode } from '../../src/core/ingestion/utils/ast-helpers.js'; + +const nodeAt = (row: number, column: number): SyntaxNode => + ({ startPosition: { row, column } }) as unknown as SyntaxNode; + +describe('nestedCallableQualifiedName — the shared nested-callable id rule', () => { + it('qualifies by the enclosing callable AND the declaration position', () => { + expect(nestedCallableQualifiedName('run', nodeAt(3, 2), 'save')).toBe('run.save@3:2'); + }); + + it('takes the position from the node, never from the name', () => { + // Guards against a "fix" that formats the suffix from anything but the + // declaration site — the position is what makes the id unique. + expect(nestedCallableQualifiedName('outer', nodeAt(12, 9), 'fn')).toBe('outer.fn@12:9'); + }); + + it('separates same-named siblings in different blocks', () => { + // The case names alone cannot express (#2699): two `pick` bindings in the + // if/else arms of one function are genuinely different bindings, and both + // are `outer.pick` by name. + const first = nestedCallableQualifiedName('outer', nodeAt(2, 4), 'pick'); + const second = nestedCallableQualifiedName('outer', nodeAt(5, 4), 'pick'); + + expect(first).not.toBe(second); + }); + + it('carries a multi-level chain verbatim in the prefix', () => { + expect(nestedCallableQualifiedName('A.outer.mid', nodeAt(7, 0), 'inner')).toBe( + 'A.outer.mid.inner@7:0', + ); + }); +}); + +describe('no call site re-inlines the rule', () => { + it('parse-worker.ts contains no inlined `.${localIdentity(...)}` template', () => { + // The structural half. The unit assertions above would still pass if a + // fourth phase appeared and spelled the rule out by hand — which is + // exactly how the divergence #2714 fixed came to exist. This fails if any + // site reconstructs the id instead of calling the shared function. + const source = readFileSync( + fileURLToPath(new URL('../../src/core/ingestion/workers/parse-worker.ts', import.meta.url)), + 'utf8', + ); + const inlined = source.match(/\}\.\$\{localIdentity\(/g) ?? []; + + expect(inlined).toEqual([]); + }); +});