mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
fix(scope-resolution): resolve calls through a closure-valued binding (#2693)
`val f = { }; f()` emitted no CALLS edge in Kotlin or Swift, so `impact` on
such a symbol under-reported to zero — the same false all-clear as #2687.
The cause was not, as first suspected, that these languages fail to feed
`callable-value-flow`. They do: `synthesizeCallableFlowCaptures` is called
from 15 language capture modules, and Kotlin already resolves reassignment
through the pass (`var f = ::a; if (c) f = ::b; f(1)` reaches both targets).
Their captures are already exactly right — the seed names the binding as its
own callable, per the anonymous-callable convention in
callable-flow-captures.ts.
They died one layer later, at the `buildGraphTargetIndex` gate:
if (!isCallable(def) && providerTarget?.(def) !== true) continue;
`isCallable` is Function/Method/Constructor, but the scope-resolution layer
declares a closure binding with its VALUE label (Kotlin/Swift `Property`),
and `isCallableValueTarget` is implemented by exactly one provider — COBOL.
So the binding never entered `graphTargets`; `lexicalCallableLookup` then
returned `shadowed: true` with no targets, which also suppressed the
workspace-wide fallback, and the seed resolved to nothing.
Only the graph knows a value binding holds a callable — since #2687 it emits
a single `Function` node for one. So value bindings now resolve their graph
id first and are admitted on the label of the node they actually reach.
This is self-limiting: a genuine constant keeps its own Const/Property node,
so `resolveDefGraphId`'s qualified key hits before the label-agnostic
`simpleKey` fallback can reach a same-named callable. Only a binding whose
own value node was replaced by a callable one gets through.
No scope kind changes — Kotlin's `lambda_literal` stays `@scope.block`, so
#1757 smart-cast semantics are untouched by construction. The fix is
language-neutral: it discriminates on the graph node label, never on a
language name.
Dart is fixed separately; its root cause is independent.
This commit is contained in:
parent
89bbdcf566
commit
cacc99bc8e
2 changed files with 98 additions and 12 deletions
|
|
@ -10,6 +10,7 @@ import type {
|
|||
CallableFlowInvokeSite,
|
||||
CallableFlowOperand,
|
||||
CallableFlowSite,
|
||||
NodeLabel,
|
||||
ParsedFile,
|
||||
ScopeId,
|
||||
SymbolDefinition,
|
||||
|
|
@ -747,19 +748,49 @@ function buildGraphTargetIndex(
|
|||
const out = new Map<string, Target>();
|
||||
const byAnchor = buildGraphCallableAnchorIndex(graph);
|
||||
for (const def of scopes.defs.byId.values()) {
|
||||
if (!isCallable(def) && providerTarget?.(def) !== true) continue;
|
||||
const callableDef = isCallable(def) || providerTarget?.(def) === true;
|
||||
// #2693: a closure bound to a name (`val f = { }`) is a callable the def
|
||||
// type cannot see. The scope-resolution layer declares such a binding with
|
||||
// its VALUE label (Kotlin/Swift `Property`, Dart `Variable`) while #2687
|
||||
// makes the graph emit a single `Function` node for it. Only the graph
|
||||
// knows, so value bindings are resolved first and admitted below on the
|
||||
// label of the node they actually reach.
|
||||
if (!callableDef && !VALUE_BINDING_DEF_TYPES.has(def.type)) continue;
|
||||
const anchorKey = definitionAnchorKey(def);
|
||||
const anchored = anchorKey === undefined ? undefined : byAnchor.get(anchorKey);
|
||||
const id =
|
||||
anchored?.length === 1 ? anchored[0] : resolveDefGraphId(def.filePath, def, nodeLookup);
|
||||
if (id === undefined) continue;
|
||||
// A genuine constant keeps its own `Const`/`Property`/`Variable` node, so
|
||||
// `resolveDefGraphId`'s qualified key hits before its label-agnostic
|
||||
// `simpleKey` fallback can reach a same-named callable — only a binding
|
||||
// whose own value node was replaced by a callable one gets through here.
|
||||
if (!callableDef && !isCallableGraphNode(graph, id)) continue;
|
||||
// Overloads can intentionally share one graph node ID. Index by the
|
||||
// definition identity so contextual signature narrowing still sees the
|
||||
// complete overload set before a selected target collapses to graph ID.
|
||||
if (id !== undefined) out.set(def.nodeId, { id, def });
|
||||
out.set(def.nodeId, { id, def });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Value-binding labels whose initializer can be a callable. Kept separate from
|
||||
* `isCallable` because these are admitted on graph-node evidence, never on the
|
||||
* def type alone.
|
||||
*/
|
||||
const VALUE_BINDING_DEF_TYPES: ReadonlySet<NodeLabel> = new Set<NodeLabel>([
|
||||
'Const',
|
||||
'Property',
|
||||
'Static',
|
||||
'Variable',
|
||||
]);
|
||||
|
||||
function isCallableGraphNode(graph: KnowledgeGraph, id: string): boolean {
|
||||
const label = graph.getNode(id)?.label;
|
||||
return label === 'Function' || label === 'Method' || label === 'Constructor';
|
||||
}
|
||||
|
||||
interface CanonicalCallableTargets {
|
||||
/** Definition identity remains the key; declaration keys may point at the definition target. */
|
||||
readonly targets: ReadonlyMap<string, Target>;
|
||||
|
|
|
|||
|
|
@ -13,14 +13,16 @@
|
|||
* these rely on the #2687 pre-scan collapsing the pair; a regression there
|
||||
* would surface here as a twin rather than a wrong label.
|
||||
*
|
||||
* The label alone does not make `f()` resolve — free-call resolution runs off
|
||||
* the per-language scope-resolution queries. Go, Python and C++ now also carry a
|
||||
* The label alone does not make `f()` resolve. Go, Python and C++ carry a
|
||||
* `@declaration.function` capture anchored on the inner closure literal, so
|
||||
* calls resolve there too (asserted in the second describe). Kotlin, Swift and
|
||||
* Dart still lack a `@scope.function` whose range matches the closure literal —
|
||||
* Kotlin deliberately scopes `lambda_literal` as a BLOCK (#1757) — and an
|
||||
* unaligned declaration anchor mis-attributes callers, so those three keep the
|
||||
* label fix only.
|
||||
* free-call resolution finds the def directly. Kotlin, Swift and Dart cannot
|
||||
* take that route — they lack a `@scope.function` whose range matches the
|
||||
* closure literal (Kotlin deliberately scopes `lambda_literal` as a BLOCK,
|
||||
* #1757) and an unaligned declaration anchor mis-attributes callers.
|
||||
*
|
||||
* #2693 resolves those three through `callable-value-flow` instead: the graph
|
||||
* node this file asserts IS the evidence that admits the binding as a callable
|
||||
* target, so a regression in the labels above now also breaks call resolution.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
|
|
@ -156,9 +158,18 @@ const callTargetsFor = async (filename: string, source: string): Promise<string[
|
|||
};
|
||||
|
||||
describeIfWorkerBuilt('calls to a closure binding resolve to its Function node', () => {
|
||||
// The label change alone is not enough: each language also needs a
|
||||
// `@declaration.function` anchored on the inner closure literal, so the def is
|
||||
// owned by the closure's own scope and free-call resolution can find it.
|
||||
// Two independent routes reach the same outcome.
|
||||
//
|
||||
// Go, Python and C++ take the DECLARATION route: a `@declaration.function`
|
||||
// anchored on the inner closure literal, so the def is owned by the closure's
|
||||
// own scope and free-call resolution finds it directly.
|
||||
//
|
||||
// Kotlin, Swift and Dart cannot — an unaligned declaration anchor
|
||||
// mis-attributes callers, and Kotlin scopes `lambda_literal` as a BLOCK on
|
||||
// purpose (#1757, smart casts). They take the CALLABLE-VALUE-FLOW route
|
||||
// instead (#2693): their capture layer already emits a `seed` naming the
|
||||
// binding as its own callable, and `buildGraphTargetIndex` admits the
|
||||
// binding because the graph node #2687 created for it is a `Function`.
|
||||
|
||||
it('Go: Handler(1) resolves', async () => {
|
||||
const targets = await callTargetsFor(
|
||||
|
|
@ -186,4 +197,48 @@ describeIfWorkerBuilt('calls to a closure binding resolve to its Function node',
|
|||
|
||||
expect(targets).toContain('Function:main.cpp:handler');
|
||||
});
|
||||
|
||||
it('Kotlin: handler(1) resolves', async () => {
|
||||
const targets = await callTargetsFor(
|
||||
'App.kt',
|
||||
'val handler = { x: Int -> x }\n\nfun caller(): Int {\n return handler(1)\n}\n',
|
||||
);
|
||||
|
||||
expect(targets).toContain('Function:App.kt:handler');
|
||||
});
|
||||
|
||||
it('Swift: handler(1) resolves', async () => {
|
||||
const targets = await callTargetsFor(
|
||||
'App.swift',
|
||||
'let handler = { (x: Int) -> Int in return x }\n\nfunc caller() -> Int {\n return handler(1)\n}\n',
|
||||
);
|
||||
|
||||
expect(targets).toContain('Function:App.swift:handler');
|
||||
});
|
||||
});
|
||||
|
||||
describeIfWorkerBuilt('a non-callable value binding stays edge-free', () => {
|
||||
// The suppression must key on an actual callable value. Widening
|
||||
// `buildGraphTargetIndex` to admit value bindings whose graph node is a
|
||||
// `Function` is safe only because a genuine constant keeps its own
|
||||
// `Const`/`Property`/`Variable` node, so `resolveDefGraphId`'s qualified key
|
||||
// hits before the label-agnostic `simpleKey` fallback can reach a callable.
|
||||
|
||||
it('Kotlin: a constant sharing its name with a function mints no CALLS to the constant', async () => {
|
||||
const targets = await callTargetsFor(
|
||||
'Shadow.kt',
|
||||
'val maxSize = 10\n\nfun size(): Int {\n return maxSize\n}\n',
|
||||
);
|
||||
|
||||
expect(targets).toEqual([]);
|
||||
});
|
||||
|
||||
it('Kotlin: a property initialised from a call is not itself callable', async () => {
|
||||
const targets = await callTargetsFor(
|
||||
'Made.kt',
|
||||
'fun make(): Int = 1\n\nval made = make()\n\nfun caller(): Int {\n return made\n}\n',
|
||||
);
|
||||
|
||||
expect(targets).toEqual(['Function:Made.kt:make']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue