Skip to content

Index TypeScript abstract classes, not just concrete ones - #1033

Open
aala-conga wants to merge 2 commits into
tirth8205:stagingfrom
aala-conga:fix/abstract-class-typescript
Open

aala-conga wants to merge 2 commits into
tirth8205:stagingfrom
aala-conga:fix/abstract-class-typescript

Conversation

@aala-conga

Copy link
Copy Markdown

Fixes #1031.

Tree-sitter gives an abstract class its own node type, abstract_class_declaration, and
_CLASS_TYPES listed only class_declaration for typescript/tsx. An abstract base therefore
contributed no Class node — and the concrete methods hosted on it lost their parent and were not
emitted either, so shared behaviour on a base class was invisible to the graph while every subclass
was indexed.

The fix adds one string to each of the two lists. abstract_class_declaration is TypeScript-specific;
Java reuses class_declaration and is not affected.

Measured

On an Angular codebase (1696 Class nodes before):

before after
abstract classes indexed 0 / 21 (0 %) 21 / 21 (100 %)
concrete classes indexed 634 / 641 (99 %) unchanged
Class nodes total 1696 1717
the base method's node <file>::handle <file>::Base.handle

Java on the same run: 4 abstract classes, 4 indexed, before and after.

Tests

TestTypeScriptAbstractClasses in tests/test_parser.py, a sibling of
TestTypeScriptTypeDeclarations (the #737 class, whose shape it copies). Four cases: the
declaration itself, the methods that disappear with it, the tsx list (the fix touches two lists,
and a .ts-only test would have covered only one), and reachability by qualified name in a built
graph — the form the symptom actually took.

All four fail on staging and pass with the fix:

$ pytest tests/test_parser.py::TestTypeScriptAbstractClasses -q   # staging
4 failed in 0.74s
$ pytest tests/test_parser.py::TestTypeScriptAbstractClasses -q   # with the fix
4 passed in 0.65s

Full suite, fresh clone of staging at f0e4eb7, Python 3.13, pip install -e ".[dev]":

run result
staging, untouched — baseline 3963 passed, 773 skipped, 2 xfailed, 2 xpassed (349 s)
+ the fix 3963 passed, 773 skipped, 2 xfailed, 2 xpassed (321 s)
+ the fix + these tests 3967 passed, 773 skipped, 2 xfailed, 2 xpassed (333 s)

Nothing in the existing suite depended on abstract classes being absent.

Scope

This does not close #1031's downstream effects on its own. callers_of on a method inherited from an
abstract base is still 0, because this.method() remains bound to the enclosing class rather than
walked up the now-complete INHERITS chain — that is the subject of the follow-up discussion on
#984. What this change does fix is the inheritance chain breaking at an abstract intermediate, and
the node naming of base methods.

I did not check Kotlin, PHP or C#, which also have abstract classes and which I have no codebase to
measure on.

🤖 Generated with Claude Code

Tree-sitter gives an abstract class its own node type,
abstract_class_declaration, and _CLASS_TYPES listed only
class_declaration for typescript/tsx. An abstract base therefore
contributed no Class node, and the concrete methods hosted on it lost
their parent and were not emitted either — so shared behaviour on a base
class was invisible to the graph while every subclass was indexed.

Measured on an Angular codebase: abstract classes indexed 0/21 -> 21/21,
concrete classes unchanged at 634/641. Java is not affected
(abstract_class_declaration is TS-specific; Java reuses class_declaration).

Adds TestTypeScriptAbstractClasses, a sibling of the tirth8205#737 class, covering
the declaration, the methods that disappear with it, the tsx list, and
reachability by qualified name in a built graph. All four fail on staging.

See: tirth8205#1031

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.25 (LOW) — 7 changed function(s)/class(es), 17 affected flow(s), 1 test gap(s)

Risk-scored changes

Risk Level Symbol Location Tested
0.25 low tests/test_parser.py::TestTypeScriptAbstractClasses._project tests/test_parser.py:2691 yes
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses tests/test_parser.py:2659 no
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses.setup_method tests/test_parser.py:2670 yes
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses.test_abstract_class_becomes_a_node tests/test_parser.py:2698 (test)
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses.test_concrete_method_of_an_abstract_class_is_contained_... tests/test_parser.py:2707 (test)
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses.test_abstract_class_in_tsx_becomes_a_node tests/test_parser.py:2726 (test)
0.05 low tests/test_parser.py::TestTypeScriptAbstractClasses.test_abstract_base_is_reachable_in_a_built_graph tests/test_parser.py:2740 (test)

Affected execution flows

  • visit_FunctionDef — criticality 0.41, 55 node(s) across 1 file(s)
  • visit_AsyncFunctionDef — criticality 0.41, 55 node(s) across 1 file(s)
  • visit_ClassDef — criticality 0.41, 54 node(s) across 1 file(s)
  • visit_If — criticality 0.36, 3 node(s) across 1 file(s)
  • repo_relative_path — criticality 0.36, 9 node(s) across 1 file(s)
  • ...and 12 more affected flow(s)

Test gaps

  • tests/test_parser.py::TestTypeScriptAbstractClasses (tests/test_parser.py:2659)

Token savings: this graph-backed report used ~202,054 fewer tokens (~91%) than reading every changed file in full (estimated, chars/4 approximation).


Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

This branch has not been deployed

No deployments
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.

[Bug]: TypeScript. abstract_class_declaration is missing from _CLASS_TYPES, so no abstract class is indexed`

2 participants