Skip to content

fix(typescript-extractor): capture parenless arrow params and abstract classes#442

Open
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/typescript-extractor-edge-cases
Open

fix(typescript-extractor): capture parenless arrow params and abstract classes#442
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/typescript-extractor-edge-cases

Conversation

@tirth8205

Copy link
Copy Markdown
Contributor

Problem

Two structural-analysis gaps in the TypeScript/JavaScript extractor:

  1. Parenless single-param arrow functionsconst g = x => doThing(x) puts the lone parameter under the arrow_function's parameter (singular) field, not inside a formal_parameters node. extractVariableDeclarations only looked for parameters/formal_parameters, so the parameter was silently dropped (params: []). This form is ubiquitous in map/filter callbacks.

  2. Abstract classesabstract class Foo {} parses as abstract_class_declaration, not class_declaration. Neither processTopLevelNode nor processExportStatement matched that node type, so abstract classes disappeared entirely: no class entry, no methods/properties, and export abstract class X {} produced no export entry either. Whole base-class/service hierarchies vanished from the graph.

Fix

  • In extractVariableDeclarations, read valueNode.childForFieldName("parameter") first and use it as the single param when present; otherwise fall back to the existing parameters/formal_parameters lookup.
  • Add case "abstract_class_declaration" alongside case "class_declaration" in both processTopLevelNode and processExportStatement. extractClass already locates the name via type_identifier/identifier and reads class_body, so it works unchanged for the abstract variant.

Testing

  • Added packages/core/src/plugins/extractors/__tests__/typescript-extractor.test.ts (modeled on the sibling extractor tests, loading the tree-sitter-typescript WASM grammar) with cases for the parenless arrow param and for exported/non-exported abstract classes.
  • The new assertions fail before the fix (params []; classes/exports empty) and pass after.
  • vitest run for the core package is green (35 files, 697 tests, no regressions); tsc --noEmit exits 0; ESLint on the changed files is clean.

🤖 Generated with Claude Code

…t classes

A single-parameter arrow function written without parentheses
(`x => f(x)`) exposes its lone parameter under the `parameter`
(singular) field, not inside a `formal_parameters` node, so the
extractor silently dropped it. Now read that field first.

`abstract class Foo {}` parses as `abstract_class_declaration`, a node
type neither processTopLevelNode nor processExportStatement matched, so
abstract classes (and their methods/exports) vanished from the
structural graph. Both switches now handle `abstract_class_declaration`
alongside `class_declaration`; extractClass works unchanged.

Adds typescript-extractor.test.ts covering both cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@thejesh23 thejesh23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. Abstract method signatures still dropped from methods
extractClass only matches method_definition, but abstract members parse as abstract_method_signature (e.g. abstract run(): void;). For the common case of an abstract base class declaring its contract, the resulting classes[].methods is empty or partial — exactly the hierarchy this PR is trying to recover. The new abstract-class test uses a concrete run() {} body, so this gap is not exercised.

2. Decorated abstract classes
TS decorators (@Injectable() abstract class X {}) wrap the class in a decorator + class_declaration/abstract_class_declaration pair, and when combined with export the node layout shifts again. Neither processTopLevelNode nor processExportStatement walks past a leading decorator sibling, so decorated abstract services (very common in Nest/Angular code) still disappear. Worth at least one test covering @injectable() abstract class X {}.

3. Sibling Dart PR has the same shape
PR #435 adds the Dart extractor and Dart also has abstract class plus parenless single-param closures via =>; the same two omissions (abstract-method-signature extraction, decorator/annotation wrapping) are likely to recur there. Worth aligning the fixes/tests across the two extractors before both land.

Nit: singleParam.text will inline any incidental whitespace/comments between the bare identifier and =>; harmless today but singleParam.childForFieldName("name")?.text ?? singleParam.text would be more defensive.

extractClass only matched method_definition, so abstract-class members
declared as abstract_method_signature (`abstract run(): void;`) or
method_signature (`run(): void;`) were silently dropped, leaving the
recovered abstract-class hierarchy with empty or partial methods. Accept
both signature node types alongside method_definition.

Adds tests covering a mixed abstract/concrete class, a purely abstract
contract, and decorated (and decorated+exported) abstract classes to
lock the already-correct decorator dispatch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tirth8205

Copy link
Copy Markdown
Contributor Author

1. Abstract method signatures dropped from methods — Fixed. extractClass now accepts abstract_method_signature and method_signature in addition to method_definition. Worth noting the node type depends on the abstract keyword: abstract run(): void; parses as abstract_method_signature, but a bare run(): void; body member (also valid in an abstract class) parses as method_signature — handling only the former would still drop the latter, so both are accepted. Added tests for a mixed abstract/concrete class (methods === ['run','helper']) and a purely-abstract contract (['find','save']).

2. Decorated abstract classes — Verified against tree-sitter-typescript 0.23.2: for @Injectable() abstract class X {} the top-level node is abstract_class_declaration with the decorator as an inner child, not a leading sibling; for @Injectable() export abstract class X {} and export @Injectable() abstract class X {} the export_statement carries abstract_class_declaration as a child. All three are already dispatched correctly (the name lookup .find(type_identifier|identifier) skips the decorator), so no code change was needed. Added regression tests for the decorated and decorated+exported forms to lock this.

3. Dart parity (#435) — Left as a separate follow-up. #435 is already merged, so the two can't land together; Dart is a distinct extractor with a different grammar and would need its own node-shape verification rather than assuming parity. Out of scope for this TS-only PR.

Nit (singleParam.text) — Keeping as-is. A node's .text is its own source span, so for const g = x /* c */ => x the parameter field points at the identifier x and .text is exactly 'x' — no incidental whitespace/comments are inlined. The suggested singleParam.childForFieldName('name')?.text ?? singleParam.text is a no-op here: singleParam is the bare identifier and has no name field, so the first branch is always undefined and it falls through to .text regardless.

Full core suite green (701/701), tsc clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants