mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-02 02:11:29 +00:00
* fix(impact): fail closed on id-less targets and follow ??/||/?: callable values (#3354) #3354 reports `impact` returning a byte-identical 1037/CRITICAL/`exact` result for three unrelated targets, with only `target.name` differing. The reporter's `target` had no `id` and no `filePath`, which the traversal path always emits, and a synthetic reproduction of their monorepo (pnpm, Cloudflare worker-configuration.d.ts in five packages, Hono, a Durable Object) resolves every target correctly on main. So the identical result was not reproduced. The repro did surface two real gaps and one hardening point: - `_runImpactBFS` now throws when the target has no node id. Every caller already catches, so impact reports `impactedCount: null, risk: UNKNOWN` instead of a normal-looking blast radius that cannot be about this symbol. - Callable-value flow followed only a single designator on the RHS, so `const sweep = env.__sweep ?? runSweep; await sweep(env)` produced no flow and `scheduled` was missing as a caller of `runSweep`, while the result still claimed `epistemic: exact`. Each branch of `??`, `||`, `or`, and `?:` now flows into the binding (language-neutral: operator and field names, no language checks). Parse cache bumped 104 -> 112 (105-111 are claimed by open PR #3326). - A whitespace-only `target_uid` (strict adapters materialize omitted optional strings) is treated as omitted and falls back to the name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(mcp): treat a non-string uid as omitted instead of throwing (#3354) Review follow-up on #3373. `query.uid?.trim()` called `.trim()` on a client-supplied value, and the MCP envelope is not type-validated, so `context({uid: 42})` threw a TypeError that `context()` does not catch. Before #3373 the same input ended as a structured not_found. A non-string uid now counts as omitted, which matches how normalizeToolParams already treats a non-string `target_uid`. The impact and trace not-found messages print a trimmed uid only when it is a non-blank string, so a strict adapter sending " " sees the name it searched for instead of `' '`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(ingestion): expand ??/?:/ternary branches through a provider hook (#3354) Review follow-up on #3373. The shared value-alternatives rule keys on tree-sitter field names, and several grammars spell the same construct differently, so the expansion never fired for them: - Kotlin `elvis_expression` has no fields. - Swift uses `value`/`if_nil` and `if_true`/`if_false`. - Dart uses `first`/`second`, and its conditional has no `condition` field. - Python `a if c else b` has no fields. The `'?:'` operator entry was dead, since no bundled grammar emits it. Add an optional `valueAlternatives` hook to CallableFlowCaptureOptions, consulted before the shared rule, and implement it in the Kotlin, Swift, Dart, Python and Ruby providers. Shared code still names no language. Ruby's statement-bodied `if`/`unless`/`elsif` also carries `condition`/`consequence`/`alternative`, so the shared ternary rule dug an identifier out of an arbitrary statement (`g = h; 0` flowed `h`) and produced a wrong CALLS edge. The Ruby hook now keeps a multi-statement branch as one opaque source, as before #3373. Tests: new provider fixtures for Python, Kotlin, Swift, Dart and Ruby (including a Ruby negative), and TS chain, parenthesized, callable-left and `&&` negative cases. Captures goldens gain one entry each for the new fixtures. SCHEMA_BUMP stays 112 (same unreleased PR). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(ingestion): keep long ||/??/?: chains linear in callable-flow capture (#3354) Expanding value-selecting sources into per-branch flows made long chains super-linear and, past a few thousand operands, a stack overflow. A generated `w === "k0" || w === "k1" || ...` keyword table took 6.7 s at 1000 operands and 48 s at 2000, and threw RangeError at 8000. Two causes, both fixed without changing any emitted capture: - valueAlternatives recursed once per operator level and spread the partial results at every level. It is now an explicit stack that writes to one output array. The left-to-right order, the paren unwrapping, and the provider-hook contract (`[node]` means opaque) are unchanged. - Every alternative ran its visibility walks from its own leaf, which can be as deep as the chain is long, all the way to the root. Also, tree-sitter's `parent` re-descends from the root, so each step costs the node's depth. The walks now jump between "anchor" nodes, the only nodes any check can match: region ids and formal owners. The nearest anchor is memoized per node across the file, and parents come from a map recorded by the one DFS the synthesizer already does. After the fix: 0.34 s / 0.23 s / 1.6 s at 1000 / 2000 / 8000 operands. That is within about 1.2x of main without the expansion; what remains is tree-sitter query time. Capture fingerprints are byte-identical before and after the fix for all 16 scope-capture bench languages and for the Python harness. A `typescript-deep-chain` case added to the scope-capture bench guards the scaling: 1.11-1.22 now, 7.5-8.0 before. A unit test pins a 10000-operand chain: it must still yield the seed for a callable operand, the copy for a formal at the deepest leaf, and the invoke. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(kotlin): see through braced if-branches in callable alternatives (#3354) tree-sitter-kotlin wraps a braced branch as `control_structure_body > statements > <expr>`, so for any non-empty block kotlinValueAlternatives saw exactly one named child (`statements`) and pushed the wrapper itself as the branch value. operandSyntax emits nothing for a `statements` node, so `val run = if (c) { ::f } else { ::g }; run()` produced no flow edges, and the "multi-statement block stays opaque" guard could never fire. Descend one level through `statements` and require exactly one non-comment expression there. Empty blocks (`{}` has no named children), multi-statement blocks, and `if` without `else` still return the whole `if` as one opaque source. The doc comment now describes that. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(ruby): skip only the multi-statement branch in callable alternatives (#3354) rubyValueAlternatives returned `[node]` (opaque) as soon as one branch held more than one statement. Two things went wrong because of that: - At the top level, `run = if c then g = h; 0 else method(:f) end` dropped the single-statement `else`, so `f` got no flow edge. - In an elsif chain, the outer `if` pushed the `elsif` node as a branch. The shared loop called the hook on it again, got `[elsif]` back, and used the whole elsif subtree as one source. So in `if a then method(:run_a) elsif b then g = h; 0 else method(:run_b) end` the `run_b` edge was lost. The hook now walks the elsif chain itself and skips each multi-statement branch, while every single-statement branch still becomes an alternative. This cannot add a wrong edge: each emitted alternative is a value the conditional really evaluates to, and nothing is taken from the skipped branch. Before, that branch did not contribute a resolvable value either. A whole conditional used as a source becomes a qualified-name seed, which resolves to nothing when it has more than one identifier leaf. With a single identifier leaf, it could even seed the condition variable. When no branch is a single statement, the conditional still stays one opaque source, as before. Ruby captures golden: ruby-callable-alternatives/app.rb goes from 33 to 58 capture groups. 23 of them come from the new fixture functions (checked against the old source). The other 2 are the new branch alternatives, `statement_if -> run_sweep` and `elsif_chain -> run_b`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(ingestion): flow the right operand of && / and into callable bindings (#3354) `a && b` / `a and b` yields `a` when it is falsy and `b` otherwise. A falsy value is never a callable, so the right operand is the only one that can be invoked later. The capture left `&&` unexpanded, and the compound source became a qualified seed that resolves to nothing: `const run = x && f; run()` gained no edge to `f` while impact claimed `exact`. Python `x and f or g` reached only `g`. The shared expansion now maps `&&` / `and` to the right branch only. It recurses, so `x and f or g` reaches both `f` and `g`. Where `&&` yields a boolean (Java, C#, Go, Rust, C, C++, PHP, Zig), the destination cannot be invoked, so the flow never meets a call. A before/after CALLS diff over all 83 lang-resolution fixtures that contain `&&` or `and` shows exactly one new edge, logicalAnd -> runAndRight. PHP and Ruby bind low-precedence `and` looser than `=`, so `$g = $x and $y` never reaches this rule. The Python golden digest changes only for the extended python-callable-alternatives fixture. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(ingestion): keep operator branches of a value-selecting source opaque (#3354) The fan-out sent every branch of `??` / `||` / `or` / `?:` to emitAssignmentFact on its own, including branches that compute a value. `Handlers.fallback === run || fb` then emitted the comparison as a seed whose qualified text sliced to receiver `Handlers`, member `run`, and resolveBoundMemberCandidates minted a CALLS edge to `Handlers.run`. The same happened for `this.state !== run ?? this.fallback`, `this.state + run || this.fallback`, and Python `self.state != run or self.fallback`. Before the fan-out, the whole compound source was one opaque seed. A branch that is a binary operator expression now contributes nothing. The check uses the field vocabulary valueBranches already reads (a `left`/`right` pair, or an `operator`/`operators`/`op` token after the expression start), not grammar type names. Member accesses that field their `.`/`->` as `operator` (Ruby `call`, C/C++ `field_expression`) also field a member name through the list memberParts uses, now shared as memberNameNode, so they stay designators. Unary `&f`/`*fp` lead with their operator and stay designators. Call results were already dropped by emitAssignmentFact, and lambdas and callable references are unchanged. CALLS-edge diff over 87 lang-resolution fixtures (505 -> 501 edges): only the four false edges above were removed, and none were added. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(ingestion): pin the LEFT operand of ?? / or / elvis per provider (#3354) The Python, Kotlin, Swift and Dart "override ?? fn" cases put an unresolvable parameter on the left, so a fan-out that kept only the last operand still passed them. Each fixture now adds a callableLeft case with a real function on the left and a parameter on the right (`run_left or fallback`, `::runLeft ?: fallback`, `runLeft ?? fallback`), and each language asserts `callableLeft → runLeft`. Mutation check: dropping the left branch from the shared `??`/`||`/`or` rule and making the four provider hooks return only their last operand fails all four new tests, while the existing right-operand tests still pass. Capture goldens were regenerated with UPDATE_GOLDEN=1: - python app.py: 35 -> 85 groups. 35 -> 75 is stale drift from8ad0d8db7, which added the comparison cases without regenerating. 75 -> 85 is the new case: 2 declarations, 2 scopes, the variable, the `run()` reference, and 4 callable-flow captures (seed -> run_left, copy <- fallback, formal, invoke). - swift App.swift: 25 -> 36 groups for the same new case, plus Swift's type-binding for the fallback parameter. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(bench): re-baseline scope-capture fingerprints for #3354 callable alternatives This PR makes a callable chosen by a value-selecting source flow every branch it can yield (??, ||, or, && right operand, ternary, statement if, elvis). It also keeps an operator branch opaque. Both change @callable-flow captures, and the PR adds *-callable-alternatives regression fixtures that these benches glob. To verify, the BASE (merge-base233ca2849) and HEAD emitters were run over the same HEAD fixture corpus plus the synthetic source. Every added or removed match is an @callable-flow.* match on one of those sources. The pre-existing corpus is byte-identical for all 16 scope-capture languages and for Python. Emitter delta, then corpus growth: - ruby: +5/-0 app.rb (+1 file, +58 groups) - swift: +6/-3 App.swift (+1 file, +36 groups) - dart: +6/-3 app.dart (+1 file, +33 groups) - kotlin: +8/-2 App.kt (+1 file, +71 groups) - typescript: +18/-14 4 files (+288 groups) - python: +11/-7 app.py (+1 file, +85 groups) The removed matches are whole-expression seeds and copies that carried a qualified name of the compound source. They are replaced by one seed or copy per operand. Synthetic scaling counts are unchanged and every scaling ratio stays under budget. Per-language notes are in baselines.json under _rebaselined_3354_callable_alternatives. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * refactor(mcp): share one blank-uid check across impact, trace and symbol lookup (#3354) The "trimmed uid, or omitted when blank/non-string" rule was inlined four times, three of them trimming twice. nonBlankUid() owns it now; behaviour is unchanged. The whitespace and empty target_uid tests collapse into one it.each. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(cfg): correct the Kotlin and Python grammar-field notes (#3354) The Kotlin CFG visitor claimed no control-flow node has fields; a parse of the vendored grammar shows if_expression fields condition/consequence/ alternative (when/for/while/do/try/elvis are fieldless). The Python harvest note listed conditional_expression's children like field names; the node is fieldless and they are positional. Both notes were misleading reviewers of the #3354 value-alternatives hooks. Comment-only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(ingestion): skip optional-grammar suites in callable-alternatives providers test (#3354) Kotlin, Swift and Dart grammars are optional installs. Guard their describe blocks with isLanguageAvailable, as swift.test.ts and dart.test.ts do, so an install without one of them skips instead of failing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(ingestion): probe the Dart parser before enabling its providers suite (#3354) isLanguageAvailable only proves the module loaded; tree-sitter-dart can still fail on setLanguage. Probe loadParser/loadLanguage and skip on failure, the same guard dart.test.ts uses. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
138 lines
6.2 KiB
TypeScript
138 lines
6.2 KiB
TypeScript
/**
|
|
* Direct unit tests for `synthesizeCallableFlowCaptures` — the shared
|
|
* AST-walking capture synthesizer behind every language's
|
|
* `*_CALLABLE_CAPTURE_OPTIONS` block (#2522 review: the 1,100-line producer
|
|
* had no test naming it; only downstream consumers were covered).
|
|
*
|
|
* Runs over the JavaScript grammar with a minimal options object so the
|
|
* assertions pin the SYNTHESIZER's semantics (assignment forms, member
|
|
* paths, subscripts, formals, produced-value guards), not any language's
|
|
* option tuning.
|
|
*/
|
|
|
|
import { describe, it, expect } from 'vitest';
|
|
import { synthesizeCallableFlowCaptures } from '../../../src/core/ingestion/utils/callable-flow-captures.js';
|
|
import { getJsParser } from '../../../src/core/ingestion/languages/javascript/query.js';
|
|
import { getTreeSitterBufferSize } from '../../../src/core/ingestion/constants.js';
|
|
|
|
const OPTIONS = {
|
|
functionNodeTypes: new Set(['function_declaration', 'arrow_function', 'function_expression']),
|
|
callNodeTypes: new Set(['call_expression']),
|
|
parameterListNodeTypes: new Set(['formal_parameters', 'arguments']),
|
|
parameterNodeTypes: new Set(['identifier', 'rest_pattern', 'assignment_pattern']),
|
|
bindingNodeTypes: new Set(['variable_declarator']),
|
|
assignmentNodeTypes: new Set(['assignment_expression']),
|
|
identifierNodeTypes: new Set(['identifier', 'property_identifier']),
|
|
} as const;
|
|
|
|
function factsFor(src: string): Array<Record<string, string>> {
|
|
const tree = getJsParser().parse(src, undefined, { bufferSize: getTreeSitterBufferSize(src) });
|
|
if (tree === null) throw new Error('parse failed');
|
|
return synthesizeCallableFlowCaptures(tree.rootNode, OPTIONS).map((match) => {
|
|
const out: Record<string, string> = {};
|
|
for (const [tag, cap] of Object.entries(match)) {
|
|
if (cap !== undefined) out[tag] = cap.text;
|
|
}
|
|
return out;
|
|
});
|
|
}
|
|
|
|
function byTag(facts: Array<Record<string, string>>, tag: string): Array<Record<string, string>> {
|
|
return facts.filter((fact) => fact[tag] !== undefined);
|
|
}
|
|
|
|
describe('synthesizeCallableFlowCaptures (shared synthesizer, #2522)', () => {
|
|
it('emits a seed for a declared-function initializer and an invoke at the call through it', () => {
|
|
const facts = factsFor('function target() {}\nconst h = target;\nh(1);\n');
|
|
expect(byTag(facts, '@callable-flow.seed')).toMatchObject([
|
|
{
|
|
'@callable-flow.destination': 'h',
|
|
'@callable-flow.target-name': 'target',
|
|
},
|
|
]);
|
|
expect(byTag(facts, '@callable-flow.invoke')).toMatchObject([
|
|
{
|
|
'@callable-flow.callee': 'h',
|
|
'@callable-flow.invocation-kind': 'indirect',
|
|
'@callable-flow.arity': '1',
|
|
},
|
|
]);
|
|
});
|
|
|
|
it('emits one formal fact per parameter with its index', () => {
|
|
const facts = factsFor('function take(a, b) {}\n');
|
|
expect(byTag(facts, '@callable-flow.formal')).toMatchObject([
|
|
{ '@callable-flow.binding': 'a', '@callable-flow.parameter-index': '0' },
|
|
{ '@callable-flow.binding': 'b', '@callable-flow.parameter-index': '1' },
|
|
]);
|
|
});
|
|
|
|
it('emits an argument fact for a function passed by name', () => {
|
|
const facts = factsFor('function target() {}\nfunction wire(cb) { cb(); }\nwire(target);\n');
|
|
expect(byTag(facts, '@callable-flow.argument')).toMatchObject([
|
|
{ '@callable-flow.source': 'target', '@callable-flow.parameter-index': '0' },
|
|
]);
|
|
});
|
|
|
|
it('binds subscripted destinations and callees to the container, not the index (#2522 fix)', () => {
|
|
const facts = factsFor(
|
|
'function target() {}\nfunction entry(i) {\n const tbl = [];\n tbl[i] = target;\n tbl[i]();\n}\n',
|
|
);
|
|
expect(byTag(facts, '@callable-flow.seed')).toMatchObject([
|
|
{ '@callable-flow.destination': 'tbl', '@callable-flow.target-name': 'target' },
|
|
]);
|
|
expect(byTag(facts, '@callable-flow.invoke')).toMatchObject([
|
|
{ '@callable-flow.callee': 'tbl' },
|
|
]);
|
|
});
|
|
|
|
it('emits a member-call invoke only when the member cell has a visible store (#2522 fix)', () => {
|
|
const stored = factsFor('function target() {}\nfunction go(o) { o.run = target; o.run(); }\n');
|
|
expect(byTag(stored, '@callable-flow.invoke')).toMatchObject([
|
|
{ '@callable-flow.callee': 'run' },
|
|
]);
|
|
|
|
const unstored = factsFor('function go(map) { map.get("x"); }\n');
|
|
expect(byTag(unstored, '@callable-flow.invoke')).toEqual([]);
|
|
});
|
|
|
|
it('treats call results as produced values, not callable designators', () => {
|
|
const facts = factsFor('function make() {}\nconst h = make();\n');
|
|
expect(byTag(facts, '@callable-flow.seed')).toEqual([]);
|
|
expect(byTag(facts, '@callable-flow.copy')).toEqual([]);
|
|
});
|
|
|
|
it('suppresses bare-name sources entirely under bareNamesAreCalls (#2522 Ruby fix)', () => {
|
|
const src = 'function target() {}\nconst h = target;\n';
|
|
const withDefault = factsFor(src);
|
|
expect(byTag(withDefault, '@callable-flow.seed')).toHaveLength(1);
|
|
|
|
const tree = getJsParser().parse(src);
|
|
if (tree === null) throw new Error('parse failed');
|
|
const suppressed = synthesizeCallableFlowCaptures(tree.rootNode, {
|
|
...OPTIONS,
|
|
bareNamesAreCalls: true,
|
|
});
|
|
expect(suppressed.filter((match) => match['@callable-flow.seed'] !== undefined)).toEqual([]);
|
|
});
|
|
|
|
it('expands a 10000-operand `||` chain and keeps its callable and formal operands (#3354 review)', () => {
|
|
// Each `||` nests one level deeper, so the expansion used to recurse once
|
|
// per operand (stack overflow near 8000) and walk each leaf's full depth.
|
|
// The formal `w` sits at the deepest leaf: its visibility still has to be
|
|
// found through the enclosing function, 10000 levels up.
|
|
const operands = Array.from({ length: 10000 }, (_, i) => `w === "k${i}"`);
|
|
operands.splice(5000, 0, 'target');
|
|
operands.unshift('w');
|
|
const facts = factsFor(
|
|
`function target() {}\nfunction isKw(w) {\n const f = ${operands.join(' || ')};\n f();\n}\n`,
|
|
);
|
|
expect(byTag(facts, '@callable-flow.seed')).toMatchObject([
|
|
{ '@callable-flow.destination': 'f', '@callable-flow.target-name': 'target' },
|
|
]);
|
|
expect(byTag(facts, '@callable-flow.copy')).toMatchObject([
|
|
{ '@callable-flow.destination': 'f', '@callable-flow.source': 'w' },
|
|
]);
|
|
expect(byTag(facts, '@callable-flow.invoke')).toMatchObject([{ '@callable-flow.callee': 'f' }]);
|
|
});
|
|
});
|