Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions __tests__/explore-declaration-only.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
});

Expand Down Expand Up @@ -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', () => {
Expand Down
61 changes: 60 additions & 1 deletion __tests__/extraction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -842,10 +891,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([]);
});
Expand Down
6 changes: 5 additions & 1 deletion __tests__/object-literal-methods.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
expect(fetchUserRefs.map((r) => r.referenceName)).toContain('reset');

Expand Down
29 changes: 27 additions & 2 deletions codegraph-kernel/src/tsjs/extractors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,33 @@ impl<'t> Walker<'t> {
let name = self.text(name_node).to_string();

// TS/JS field definitions carry an explicit `type` field; the generic
// scan is for other languages (#808).
let type_text = node.child_by_field_name("type").map(|t| {
// scan is for other languages (#808). A `property_signature` (an
// interface member, #1638) carries a `type` field and no value, so it
// reads the type field too: the generic scan's exclusion list covers
// `identifier` but not the `property_identifier` an interface member is
// named with, so it would stop on the name and make the signature repeat
// it (`counts counts`) instead of naming the type. Mirrors
// extractProperty's isTsJsField.
let is_ts_js_field = matches!(
node.kind(),
"public_field_definition" | "field_definition" | "property_signature"
);
let type_node = if is_ts_js_field {
node.child_by_field_name("type")
} else {
(0..node.named_child_count()).filter_map(|i| node.named_child(i)).find(|c| {
!matches!(
c.kind(),
"modifier"
| "modifiers"
| "identifier"
| "accessor_list"
| "accessors"
| "equals_value_clause"
)
})
};
let type_text = type_node.map(|t| {
let raw = self.text(t);
raw.strip_prefix(':').unwrap_or(raw).trim_start().to_string()
});
Expand Down
50 changes: 42 additions & 8 deletions codegraph-kernel/src/tsjs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,32 @@ impl Variant {
/// typescriptExtractor.methodTypes / javascriptExtractor.methodTypes.
fn is_method_type(v: Variant, kind: &str) -> bool {
kind == "method_definition"
|| (v.is_ts() && kind == "public_field_definition")
|| (v.is_ts() && matches!(kind, "public_field_definition" | "method_signature"))
|| (!v.is_ts() && kind == "field_definition")
}

/// typescriptExtractor.propertyTypes. The interface counterpart of
/// `public_field_definition`: it carries no value, so it is always a property
/// and never goes through classify_ts_class_member (#1638).
fn is_property_type(v: Variant, kind: &str) -> bool {
v.is_ts() && kind == "property_signature"
}

/// Method node types that spell a SIGNATURE — a declaration with no body (#1638).
///
/// They are a method of whatever type declares them and nothing on their own, so
/// they must not take `extract_method`'s "no class-like parent, so treat it as a
/// free function" fallback. The other method types can: a `method_definition`
/// outside a class really is a function. This one appears outside a class only
/// inside a type literal (`type Handle = { stop(): void }`), whose members
/// `extract_ts_type_alias_members` already extracts and attaches to the alias
/// (#359) — take the fallback and the file gains a phantom top-level
/// `function stop` beside the real `Handle::stop`. Mirrors the TS extractor's
/// SIGNATURE_METHOD_NODE_TYPES (extraction/tree-sitter.ts).
fn is_signature_method_type(kind: &str) -> bool {
kind == "method_signature"
}

fn is_function_type(kind: &str) -> bool {
matches!(kind, "function_declaration" | "arrow_function" | "function_expression")
}
Expand Down Expand Up @@ -622,7 +644,9 @@ impl<'t> Walker<'t> {
} else if is_class_type(self.variant, kind) {
self.extract_class(node);
skip_children = true;
} else if is_method_type(self.variant, kind) {
} else if is_method_type(self.variant, kind)
&& (!is_signature_method_type(kind) || self.inside_class_like())
{
if classify_ts_class_member(node) == Member::Property {
let prop = self.extract_property(node);
if let (Some((row, name)), Some(value)) = (prop, node.child_by_field_name("value")) {
Expand Down Expand Up @@ -664,12 +688,22 @@ impl<'t> Walker<'t> {
self.extract_call(node);
} else if kind == "new_expression" {
self.extract_instantiation(node);
} else if self.variant.is_ts()
&& matches!(kind, "property_signature" | "method_signature")
&& self.inside_class_like()
{
let parent = self.top_row();
self.extract_type_annotations(node, parent);
} else if is_property_type(self.variant, kind) && self.inside_class_like() {
// NOTE: `property_signature` / `method_signature` used to be handled
// here together, hanging their type annotations off the ENCLOSING
// INTERFACE — the only anchor available while the members themselves
// went unextracted. Since #1638 `method_signature` is a method type
// and `property_signature` a property type, so the method branch
// above claims the first (under the same inside_class_like guard
// this branch had) and this one extracts the second as a real node.
// The `references` edges survive — extract_method and
// extract_property each call extract_type_annotations — but now hang
// off the member, the more precise anchor: `Api::fetch → PageId`
// says which member wants the type, where `Api → PageId` only said
// the file did.
self.extract_property(node);
self.scan_fn_ref_subtree(node, 0);
skip_children = true;
}

if !skip_children {
Expand Down
Loading