Land upstream #1686 (maxmilian + bompus kernel/CG-28 follow-ups) onto current main. tree-sitter-typescript interface members (method_signature / property_signature) were never listed in the TS extractor, so platform .d.ts APIs had no declaration nodes for call edges. Mirrors on the Rust kernel path; keeps CG-28 damping for pure-interface declaration files; filters damped files from the explore RWR seed set. Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
This commit is contained in:
co-authored by
Colby McHenry
parent
8c047342cd
commit
ee83636acb
@@ -96,15 +96,36 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a
|
||||
if (testDir && fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/**
|
||||
* Type-level for the purposes of this gate: a type declaration, or a member
|
||||
* an interface declares.
|
||||
*
|
||||
* The second half is not a loosening. Since #1638 a `method_signature` /
|
||||
* `property_signature` is indexed as a `method` / `property` node, so a file
|
||||
* of nothing but interfaces no longer reads as nothing but `interface` kinds
|
||||
* — but a bodiless signature is on the same side of the line as the interface
|
||||
* that owns it, which is exactly how `getAmbientDeclarationPathsAmong` counts
|
||||
* it. What this still catches, and is here to catch, is a `function` or a
|
||||
* `class` creeping into the fixture: that would silently exempt the file and
|
||||
* make every assertion below vacuous.
|
||||
*/
|
||||
const isTypeLevel = (n: { id: string; kind: string }, filePath: string): boolean => {
|
||||
if (n.kind === 'interface' || n.kind === 'type_alias') return true;
|
||||
if (n.kind !== 'method' && n.kind !== 'property') return false;
|
||||
const interfaceIds = new Set(
|
||||
cg.getNodesInFile(filePath).filter((x) => x.kind === 'interface').map((x) => x.id),
|
||||
);
|
||||
return cg.getIncomingEdges(n.id)
|
||||
.some((e) => e.kind === 'contains' && interfaceIds.has(e.source));
|
||||
};
|
||||
|
||||
describe('fixture shape — if this rots, the gate below means nothing', () => {
|
||||
it('holds two declaration-only files that differ only in the banner', () => {
|
||||
for (const p of [HANDWRITTEN_DECL, GENERATED_DECL]) {
|
||||
const nodes = cg.getNodesInFile(p).filter((n) => n.kind !== 'file' && n.kind !== 'import');
|
||||
expect(nodes.length, `${p} declares nothing`).toBeGreaterThan(10);
|
||||
// Every symbol type-level, nothing with a body — the structural test the
|
||||
// penalty keys on. A `function`/`class` creeping in would silently exempt
|
||||
// the file and make every assertion below vacuous.
|
||||
expect(nodes.every((n) => n.kind === 'interface' || n.kind === 'type_alias'), `${p} has a non-type symbol`).toBe(true);
|
||||
// Nothing with a body — the structural test the penalty keys on.
|
||||
expect(nodes.every((n) => isTypeLevel(n, p)), `${p} has a non-type symbol`).toBe(true);
|
||||
}
|
||||
// Only one of them announces itself, so the CG-25 penalty is the ONLY
|
||||
// difference between the two — that is what makes them comparable.
|
||||
@@ -119,7 +140,7 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a
|
||||
// structure of any answer about that code.
|
||||
const nodes = cg.getNodesInFile(SHARED_TYPES).filter((n) => n.kind !== 'file' && n.kind !== 'import');
|
||||
expect(nodes.length).toBeGreaterThan(0);
|
||||
expect(nodes.every((n) => n.kind === 'interface' || n.kind === 'type_alias')).toBe(true);
|
||||
expect(nodes.every((n) => isTypeLevel(n, SHARED_TYPES))).toBe(true);
|
||||
expect(cg.getFile(SHARED_TYPES)?.generated).toBeFalsy();
|
||||
});
|
||||
|
||||
@@ -176,6 +197,23 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a
|
||||
expect(isAmbient(SHARED_TYPES)).toBe(false);
|
||||
expect(isAmbient(HANDWRITTEN_DECL)).toBe(true);
|
||||
});
|
||||
|
||||
it('still flags a shim whose interfaces now contribute method/property nodes', () => {
|
||||
// The silent-failure guard for #1638. Interface members are indexed, so a
|
||||
// pure-interface `.d.ts` no longer holds only `interface` kinds — and the
|
||||
// ambient rule is spelled as "EVERY declared symbol is type-level". Read
|
||||
// literally that stops flagging the moment the extractor improves, and
|
||||
// nothing else fails: the file just quietly ranks undamped again.
|
||||
//
|
||||
// Pinned from both ends on purpose. The `toBeGreaterThan(0)` half is what
|
||||
// keeps the other half honest — assert only the flag and this test would
|
||||
// still pass on an index where the members were never extracted at all,
|
||||
// which is precisely the state it exists to detect a regression FROM.
|
||||
const members = cg.getNodesInFile(HANDWRITTEN_DECL)
|
||||
.filter((n) => n.kind === 'method' || n.kind === 'property');
|
||||
expect(members.length, 'interface members are not indexed — see #1638').toBeGreaterThan(0);
|
||||
expect(cg.ambientDeclarationFilePredicate([HANDWRITTEN_DECL])(HANDWRITTEN_DECL)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the counter-case — a query that NAMES a declared type', () => {
|
||||
|
||||
@@ -566,6 +566,55 @@ interface Hprops {
|
||||
expect(refs.some((r) => r.referenceName === 'IOrderField')).toBe(true);
|
||||
});
|
||||
|
||||
it('indexes interface members, not just the interface itself', () => {
|
||||
// tree-sitter-typescript spells interface members `method_signature` /
|
||||
// `property_signature`, distinct from the class-member types the extractor
|
||||
// listed, so they were never captured (#1638). Java/C# are unaffected —
|
||||
// their grammars reuse `method_declaration`, already in their methodTypes.
|
||||
// The cost lands on `.d.ts` platform APIs: with no declaration node, call
|
||||
// sites through the interface have nothing to attach an edge to.
|
||||
const code = `
|
||||
export interface PlatformApi {
|
||||
fetchPage(id: string): Promise<string>;
|
||||
version: string;
|
||||
}
|
||||
`;
|
||||
const result = extractFromSource('api.d.ts', code);
|
||||
|
||||
const iface = result.nodes.find((n) => n.kind === 'interface' && n.name === 'PlatformApi');
|
||||
const method = result.nodes.find((n) => n.kind === 'method' && n.name === 'fetchPage');
|
||||
const prop = result.nodes.find((n) => n.kind === 'property' && n.name === 'version');
|
||||
expect(iface).toBeDefined();
|
||||
expect(method).toBeDefined();
|
||||
expect(prop).toBeDefined();
|
||||
|
||||
// Attached to the interface, not merely present. A member the graph holds
|
||||
// but hangs off the file is not a declaration a call edge can be resolved
|
||||
// through, which is the whole point of extracting it.
|
||||
const contained = result.edges
|
||||
.filter((e) => e.kind === 'contains' && e.source === iface!.id)
|
||||
.map((e) => e.target);
|
||||
expect(contained).toContain(method!.id);
|
||||
expect(contained).toContain(prop!.id);
|
||||
});
|
||||
|
||||
it('does not mint a top-level function from a type literal method signature', () => {
|
||||
// The failure mode the class-like guard on `method_signature` exists for
|
||||
// (#1638). `extractMethod` treats a method node with no class-like parent
|
||||
// as a free function — right for `method_definition`, wrong for a bodiless
|
||||
// signature, whose only home outside an interface is a type literal. Those
|
||||
// members are already extracted onto the alias (#359), so without the guard
|
||||
// the file gains a phantom `function stop` beside the real `Handle::stop`.
|
||||
const result = extractFromSource('t.ts', `
|
||||
export type Handle = { stop(): void; label: string };
|
||||
`);
|
||||
|
||||
const alias = result.nodes.find((n) => n.kind === 'type_alias' && n.name === 'Handle');
|
||||
expect(alias).toBeDefined();
|
||||
expect(result.nodes.find((n) => n.kind === 'method' && n.name === 'stop')).toBeDefined();
|
||||
expect(result.nodes.filter((n) => n.kind === 'function' && n.name === 'stop')).toEqual([]);
|
||||
});
|
||||
|
||||
it('should extract type references from interface method signatures', () => {
|
||||
const code = `
|
||||
import type { IPage } from '../PromoterList';
|
||||
@@ -899,10 +948,20 @@ export type Names = ['alpha', 'beta'];
|
||||
`;
|
||||
const result = extractFromSource('noise.ts', code);
|
||||
|
||||
// Since #1638 the fixture's own interfaces legitimately declare `id` / `name`
|
||||
// (`User::id`, `User::name`, `Service::name`), so membership in the name list
|
||||
// no longer implies a leak. What #634 guards is the *source*: a node minted
|
||||
// from a string literal in `Pick<User, 'id'>` or a tuple has no declaring
|
||||
// interface, so exclude anything a `contains` edge ties to one.
|
||||
const ifaceIds = new Set(result.nodes.filter((n) => n.kind === 'interface').map((n) => n.id));
|
||||
const declaredInInterface = new Set(
|
||||
result.edges.filter((e) => e.kind === 'contains' && ifaceIds.has(e.source)).map((e) => e.target)
|
||||
);
|
||||
const leaked = result.nodes.filter(
|
||||
(n) =>
|
||||
(n.kind === 'method' || n.kind === 'property') &&
|
||||
['id', 'name', 'foo', 'bar', 'alpha', 'beta'].includes(n.name)
|
||||
['id', 'name', 'foo', 'bar', 'alpha', 'beta'].includes(n.name) &&
|
||||
!declaredInInterface.has(n.id)
|
||||
);
|
||||
expect(leaked).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -53,7 +53,11 @@ describe('object-literal method extraction', () => {
|
||||
|
||||
// Each action's body was walked: fetchUser references its sibling `reset`,
|
||||
// so an in-store calls edge will resolve once the pipeline runs.
|
||||
const fetchUser = result.nodes.find((n) => n.name === 'fetchUser')!;
|
||||
// By KIND as well as name: the fixture's `Store` interface declares a
|
||||
// `fetchUser` too, and since #1638 that signature is a node of its own —
|
||||
// one that appears FIRST in the file, so a name-only lookup finds the
|
||||
// declaration and reads its return type where the action's body was meant.
|
||||
const fetchUser = result.nodes.find((n) => n.kind === 'function' && n.name === 'fetchUser')!;
|
||||
const fetchUserRefs = result.unresolvedReferences.filter((r) => r.fromNodeId === fetchUser.id);
|
||||
// `get().reset()` keeps its call receiver (#1683): the ref is the chain
|
||||
// `get().reset`, which the resolver binds to the store's own `reset`.
|
||||
|
||||
Reference in New Issue
Block a user