mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-28 01:31:23 +00:00
test(typescript): pin documented HOC trade-offs and close var-form parity gap
Addresses the four findings on PR #1261 (Claude bot review for #1261). All findings flagged missing assertion tests for behaviour already documented in code comments — none reported a real bug. The verdict was "production-ready with minor follow-ups"; these tests strengthen the documentation-to-test contract. [medium #1] Array-method false-positive Pin `const found = items.find((item) => predicate(item))` → `predicate.attributedTo === 'found'` as an accepted FP. The const is a value, never invoked, so no incoming CALLS edge ever points at it; the outgoing edge is a minor mis-attribution we accept rather than maintain a HOC allowlist. [medium #2] Nested HOCs (`memo(forwardRef(...))`) — no phantom Function:Wrapped Two integration tests in `typescript-hoc-wrapped.test.ts`: 1. `Wrapped` is NOT a Function node (the outer call's first arg is a call_expression, not an arrow — no @declaration.function pattern matches the outer shape). 2. The deepest arrow's `helper()` call is NOT attributed to Function:Wrapped (the deepest arrow is anonymous because call_expression.parent is `arguments`, not `variable_declarator`), and no Function-sourced CALLS originate from `nested.tsx`. [medium #3] Multi-arrow argument dedup Pin `const x = call(() => first(), () => second())` — both arrows share the same `arguments → call_expression → variable_declarator` ancestor chain on the legacy DAG, so both attribute to "x". Documents the registry-primary dedup story alongside. [low #4] `var X = HOC(...)` parity gap Registry-primary `query.ts` had `(variable_declaration ...)` HOC patterns but legacy `tree-sitter-queries.ts` (TS + JS) did not. Closes the gap by mirroring two `(variable_declaration ...)` HOC patterns into both legacy sections so the parity gate stays tight even if a codebase mixes `var X = HOC(...)` with `const X = HOC(...)`. Validation - Targeted: 41/41 (28 unit + 13 integration) on registry-primary. - Broader TS suite: 60/60 across 4 resolver test files. - CI parity gate (`typescript.test.ts`): 236/236 on legacy DAG and 236/236 on registry-primary. - Prettier clean. ESLint clean (5 pre-existing non-null-assertion warnings in the test file, unrelated). tsc --noEmit clean. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
ReidenXerx
parent
8d2eae8d3c
commit
861cbe66a5
3 changed files with 161 additions and 9 deletions
|
|
@ -85,14 +85,16 @@ export const TYPESCRIPT_QUERIES = `
|
|||
value: (function_expression)) @definition.function
|
||||
|
||||
; HOC-wrapped variable declarations: \`const X = HOC((args) => { ... })\`.
|
||||
; Mirrors the four registry-primary patterns in
|
||||
; \`languages/typescript/query.ts\` so the legacy Call-Resolution DAG and
|
||||
; the registry-primary pipeline produce the same set of \`Function\` nodes
|
||||
; — required for the CI parity gate. Covers React.forwardRef / memo /
|
||||
; useCallback / useMemo / observer / debounce / user-defined HOC
|
||||
; factories. See \`tsExtractFunctionName\` for the resolution logic and
|
||||
; the \`query.ts\` comment for the full anchor-discipline rationale and
|
||||
; the chained-array-method trade-off.
|
||||
; Mirrors the registry-primary patterns in \`languages/typescript/query.ts\`
|
||||
; so the legacy Call-Resolution DAG and the registry-primary pipeline
|
||||
; produce the same set of \`Function\` nodes — required for the CI parity
|
||||
; gate. Covers React.forwardRef / memo / useCallback / useMemo / observer
|
||||
; / debounce / user-defined HOC factories. The \`var X = HOC(...)\` form is
|
||||
; mirrored too (registry-primary has it) so that codebases mixing \`var\` and
|
||||
; \`const\` see identical attribution on both pipelines. See
|
||||
; \`tsExtractFunctionName\` for the resolution logic and the \`query.ts\`
|
||||
; comment for the full anchor-discipline rationale and the chained-
|
||||
; array-method trade-off.
|
||||
(lexical_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
|
|
@ -123,6 +125,22 @@ export const TYPESCRIPT_QUERIES = `
|
|||
arguments: (arguments
|
||||
(function_expression)))))) @definition.function
|
||||
|
||||
; \`var X = HOC(...)\` parity with registry-primary. Legacy code (and any
|
||||
; transpiler output that downlevels \`const\` to \`var\`) hits this shape.
|
||||
(variable_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
value: (call_expression
|
||||
arguments: (arguments
|
||||
(arrow_function))))) @definition.function
|
||||
|
||||
(variable_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
value: (call_expression
|
||||
arguments: (arguments
|
||||
(function_expression))))) @definition.function
|
||||
|
||||
; Variable/constant declarations (non-function values).
|
||||
; Overlap with @definition.function patterns is handled by parse-worker dedup.
|
||||
(lexical_declaration
|
||||
|
|
@ -302,7 +320,9 @@ export const JAVASCRIPT_QUERIES = `
|
|||
; HOC-wrapped variable declarations: \`const X = HOC((args) => { ... })\`.
|
||||
; See TYPESCRIPT_QUERIES section above for the full rationale (issue #1166
|
||||
; follow-up — covers forwardRef / memo / useCallback / useMemo / observer
|
||||
; / debounce / user-defined HOC factories).
|
||||
; / debounce / user-defined HOC factories). Both \`const\` and \`var\` forms
|
||||
; are mirrored so JS code that uses \`var\` (or transpiler output) gets the
|
||||
; same attribution as the registry-primary path.
|
||||
(lexical_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
|
|
@ -333,6 +353,21 @@ export const JAVASCRIPT_QUERIES = `
|
|||
arguments: (arguments
|
||||
(function_expression)))))) @definition.function
|
||||
|
||||
; \`var X = HOC(...)\` parity with registry-primary.
|
||||
(variable_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
value: (call_expression
|
||||
arguments: (arguments
|
||||
(arrow_function))))) @definition.function
|
||||
|
||||
(variable_declaration
|
||||
(variable_declarator
|
||||
name: (identifier) @name
|
||||
value: (call_expression
|
||||
arguments: (arguments
|
||||
(function_expression))))) @definition.function
|
||||
|
||||
; Variable/constant declarations (non-function values).
|
||||
; Overlap with @definition.function patterns is handled by parse-worker dedup.
|
||||
(lexical_declaration
|
||||
|
|
|
|||
|
|
@ -239,4 +239,68 @@ describe('TypeScript HOC-wrapped variable declarations', () => {
|
|||
const functionStrays = stray.filter((c) => c.sourceLabel === 'Function');
|
||||
expect(functionStrays, 'no other Function sources for doStuff calls').toEqual([]);
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────
|
||||
// Documented limitation: deeply-nested HOCs (`memo(forwardRef(...))`).
|
||||
//
|
||||
// The fixture `nested.tsx` documents that the OUTER pattern requires
|
||||
// the arrow to be a direct grandchild of the const's `call_expression`
|
||||
// value — when the arrow is wrapped in another `call_expression`
|
||||
// (`memo(forwardRef(arrow))`), the pattern misses and the deepest
|
||||
// arrow stays anonymous. The const itself (`Wrapped`) is also NOT a
|
||||
// Function: the immediate arg of the outer `memo(...)` call is a
|
||||
// `call_expression` (`forwardRef(...)`), not an arrow / fn-expression,
|
||||
// so no `@declaration.function` pattern matches the outer shape either.
|
||||
//
|
||||
// We assert ABSENCE here (rather than positive resolution) so that any
|
||||
// future change to the patterns or to `tsExtractFunctionName` that
|
||||
// accidentally starts matching nested HOCs surfaces immediately. A
|
||||
// proper fix for nested HOCs would require deciding which level wins
|
||||
// the name (outer wrapper? deepest behaviour-arrow?) and is out of
|
||||
// scope for this PR.
|
||||
// ─────────────────────────────────────────────────────────────────
|
||||
|
||||
it('nested HOCs (memo(forwardRef(...))): Wrapped is NOT a Function (known limitation)', () => {
|
||||
// The outer const `Wrapped` matches NO `@declaration.function` pattern
|
||||
// because the outer call's first argument is itself a call_expression,
|
||||
// not an arrow_function / function_expression. It should be picked up
|
||||
// as a Variable by `@definition.const` (or skipped entirely) — but it
|
||||
// must NOT appear as a Function node.
|
||||
const functions = new Set(getNodesByLabel(result, 'Function'));
|
||||
expect(functions, 'Wrapped (nested HOC) must NOT be a Function node').not.toContain('Wrapped');
|
||||
});
|
||||
|
||||
it('nested HOCs: helper() call inside the deepest arrow does NOT source from Function:Wrapped', () => {
|
||||
// Calls inside the doubly-wrapped arrow have no named ancestor (deepest
|
||||
// arrow is anonymous because `call_expression.parent` is `arguments`,
|
||||
// not `variable_declarator`; the outer `memo` and `forwardRef` calls
|
||||
// are themselves anonymous expressions). So calls in `nested.tsx` must
|
||||
// either source from File or not be attributed to `Wrapped` at all.
|
||||
//
|
||||
// The negative assertion is what matters: a future change that wrongly
|
||||
// attributes the deepest arrow to its outer const would silently corrupt
|
||||
// impact analysis for any real code that nests HOCs (e.g.,
|
||||
// `memo(forwardRef(...))` UI primitives).
|
||||
const helperCalls = getRelationships(result, 'CALLS').filter(
|
||||
(c) => c.sourceFilePath === 'src/nested.tsx' && c.target === 'helper',
|
||||
);
|
||||
expect(helperCalls.length, 'helper call must still be captured').toBeGreaterThan(0);
|
||||
|
||||
const fromWrapped = helperCalls.filter((c) => c.source === 'Wrapped');
|
||||
expect(
|
||||
fromWrapped,
|
||||
'helper call must NOT be attributed to Function:Wrapped (deepest arrow stays anonymous)',
|
||||
).toEqual([]);
|
||||
|
||||
// Defensive: there should be no Function-sourced edges from anywhere in
|
||||
// `nested.tsx` (everything is anonymous or module-level).
|
||||
const allNestedCalls = getRelationships(result, 'CALLS').filter(
|
||||
(c) => c.sourceFilePath === 'src/nested.tsx',
|
||||
);
|
||||
const functionSourced = allNestedCalls.filter((c) => c.sourceLabel === 'Function');
|
||||
expect(
|
||||
functionSourced,
|
||||
'no Function-sourced CALLS from nested.tsx (all anchors should be File)',
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -531,4 +531,57 @@ describe('issue #1166 follow-up — HOC-wrapped variable declarations', () => {
|
|||
`);
|
||||
expect(names).toContain('Legacy');
|
||||
});
|
||||
|
||||
// ─── Documented trade-offs: pin behaviour so future readers aren't surprised ───
|
||||
|
||||
it('accepted false-positive: `const x = arr.find((y) => p(y))` attributes p to "x"', () => {
|
||||
// The HOC-wrapped pattern (arrow's parent is `arguments`, grandparent is
|
||||
// `call_expression`, great-grandparent is `variable_declarator`) is broad
|
||||
// enough to also match chained array-method callbacks like Array#find /
|
||||
// Array#some / Array#every — where `x` is a *value* (the result of the
|
||||
// method), not a function. The arrow inside borrows the const's name and
|
||||
// its inner calls attribute to it.
|
||||
//
|
||||
// This is documented in `languages/typescript/query.ts` as an accepted
|
||||
// trade-off because:
|
||||
// 1. `x` is never invoked as a function, so no incoming CALLS edge is
|
||||
// ever created — the spurious `Function:x` is graph-isolated on the
|
||||
// incoming side.
|
||||
// 2. The outgoing edge `Function:x → predicate` is a minor mis-attribution
|
||||
// (the call IS happening, just not from a "function called x"), and
|
||||
// the alternative — narrowing the pattern to known HOC names — would
|
||||
// require maintaining a wrapper allowlist that breaks for every new
|
||||
// HOC factory.
|
||||
//
|
||||
// We pin this here so a future change that tightens the pattern is forced
|
||||
// to update this test and re-evaluate the trade-off explicitly.
|
||||
const sites = collectCallAttributions(`
|
||||
const found = items.find((item) => predicate(item));
|
||||
`);
|
||||
expect(findCall(sites, 'predicate')?.attributedTo).toBe('found');
|
||||
});
|
||||
|
||||
it('multi-arrow argument: both arrows resolve to the same const name (legacy DAG path)', () => {
|
||||
// `const x = call(() => first(), () => second())` — both arrows share the
|
||||
// same `arguments → call_expression → variable_declarator` ancestor chain,
|
||||
// so `tsExtractFunctionName`'s third branch returns "x" for each. Calls
|
||||
// inside both arrows therefore attribute to "x" on the legacy DAG path.
|
||||
//
|
||||
// In the registry-primary pipeline the two arrows produce two candidate
|
||||
// `Function:x` defs that the qualified-name dedup collapses into one
|
||||
// (last-write-wins by symbol range). The end result is the same set of
|
||||
// CALLS edges sourced from "x"; only the def's range changes. We pin the
|
||||
// legacy attribution here because that's what the unit harness exercises.
|
||||
//
|
||||
// The pattern is rare in real code (few APIs take two callbacks both
|
||||
// worth tracking), but it does exist in some `register(setup, teardown)`
|
||||
// / `addEventListener('event', handler, { once })` shaped helpers. If a
|
||||
// future change drops one of the calls or attributes it elsewhere, we
|
||||
// want to know.
|
||||
const sites = collectCallAttributions(`
|
||||
const x = call(() => first(), () => second());
|
||||
`);
|
||||
expect(findCall(sites, 'first')?.attributedTo).toBe('x');
|
||||
expect(findCall(sites, 'second')?.attributedTo).toBe('x');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue