/** * #2699 — JS/TS `statement_block` scopes, and the false ACCESSES edges they * remove. * * Enabling `(statement_block) @scope.block` for TS/JS dropped 114 ACCESSES * edges across a 762-file corpus with `added: 0`. That looked like a * regression, so it was measured rather than assumed: all 274 emitting * reference sites behind those 114 edges were classified by re-reading the * source at the site. Every one of the 114 had at least one site of the form * `receiver.name`, and none was bare-identifier-only. (269 sites classified as * member reads outright; the 5 remaining were classifier artifacts — the name * also occurred earlier on the line, as in `a.b.declLine` for `b` — and are * member reads too.) So every dropped edge was a PROPERTY read * (`options.baseUrl`) mis-resolving to an unrelated function-local `const` of * the same name in the same file. * * 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. * * 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'; import os from 'node:os'; import path from 'node:path'; import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; import { DIST_WORKER_URL, distWorkerExists } from '../helpers/worker-parse.js'; vi.setConfig({ testTimeout: 90_000 }); /** `ACCESSES` edges in a one-file repo, as `source -> target` id pairs. */ const accessEdgesFor = async (filename: string, source: string): Promise => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-block-scope-')); try { fs.writeFileSync(path.join(dir, filename), source, 'utf-8'); const result = await runPipelineFromRepo(dir, () => {}, { workerPoolSize: 1, workerUrlForTest: DIST_WORKER_URL, // `pruneLocalSymbols` drops inert function-local value symbols — ~94% of // them on a real corpus — so in a two-line fixture the `const` under test // is deleted before any edge can name it, and both arms return []. That // is why earlier synthetic attempts at this edge class all read as "no // difference". Keeping them is what makes the fixture discriminate. keepLocalValueSymbols: true, }); return result.graph.relationships .filter((rel) => rel.type === 'ACCESSES') .map((rel) => `${rel.sourceId} -> ${rel.targetId}`) .sort(); } finally { fs.rmSync(dir, { recursive: true, force: true }); } }; // The member read sits OUTSIDE the block on purpose. Inside it, the block is on // the reference's own lexical chain and the property would bind to the local in // either arm — so an inside-the-block fixture cannot discriminate. const SHADOWED = [ 'export function pickBaseUrl(options: { baseUrl?: string }, fallback: string): string {', ' if (fallback.length > 0) {', ' const baseUrl = fallback.trim();', ' return baseUrl;', ' }', ' return options.baseUrl ?? fallback;', '}', '', ].join('\n'); const describeIfWorkerBuilt = distWorkerExists() ? describe : describe.skip; describeIfWorkerBuilt('block scopes keep a property read off a same-named block local', () => { it('TypeScript: `options.baseUrl` does not ACCESS the block-local `const baseUrl`', async () => { const edges = await accessEdgesFor('pick.ts', SHADOWED); expect(edges.filter((e) => e.endsWith('baseUrl') && e.includes('pickBaseUrl'))).toEqual([]); }); it('TypeScript: a real property read still resolves past a same-named block local', async () => { // Companion invariant, not a discriminating regression test: this edge is // identical in both arms. It exists because the test above only proves an // edge went away, which a change that dropped Block-kind bindings entirely // would also satisfy. Asserting the surviving edge SET — exactly one, and // pointing at the class property rather than the block local — is what // separates "correctly scoped" from "deleted". const edges = await accessEdgesFor( 'box.ts', [ 'export class Box {', " baseUrl = 'https://example.com';", ' pick(fallback: string): string {', ' if (fallback.length > 0) {', ' const baseUrl = fallback.trim();', ' return baseUrl;', ' }', ' return this.baseUrl;', ' }', '}', '', ].join('\n'), ); // Matched on the target rather than the whole id: the method node carries // an overload index (`Box.pick#1`) that is orthogonal to what this pins. expect(edges).toHaveLength(1); 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 }); } }); });