Skip to content

✨ Add TypeScript comment type support (issue #69)#82

Open
arnoox wants to merge 8 commits into
useblocks:mainfrom
arnoox:issue/69-typescript-support
Open

✨ Add TypeScript comment type support (issue #69)#82
arnoox wants to merge 8 commits into
useblocks:mainfrom
arnoox:issue/69-typescript-support

Conversation

@arnoox

@arnoox arnoox commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR implements #69 by adding TypeScript as a supported comment_type with .ts/.tsx defaults for source discovery.

Changes

  • Add ts to supported comment_type values in source discovery config.
  • Map TypeScript discovery extensions to .ts and .tsx.
  • Add tree-sitter TypeScript dependency and parser wiring in analyse utilities.
  • Add TypeScript scope/comment parsing tests.
  • Add discovery fixture coverage for TypeScript files.
  • Add integration analyse test coverage for TypeScript input.
  • Update docs for supported languages and discover examples.
  • Add changelog entry under Upcoming.

Verification

Ran:

  • uv run --group testing pytest tests/test_source_discover.py tests/test_src_trace.py tests/test_analyse_utils.py tests/test_analyse.py

Result:

  • 148 passed

Linked issue

@arnoox arnoox requested a review from patdhlk June 24, 2026 20:09
@ubmarco ubmarco self-requested a review July 1, 2026 18:38
# @C and C++ Scope Node Types, IMPL_C_2, impl, [FE_C_SUPPORT, FE_CPP]
CommentType.cpp: {"function_definition", "class_definition"},
CommentType.cs: {"method_declaration", "class_declaration", "property_declaration"},
CommentType.ts: {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Leading @-comments on exported declarations resolve to no scope (tagged_scope = None).

TypeScript wraps exported declarations in an export_statement node, but find_next_scope only matches a scope type directly or descends into a "block" sibling — never the export_statement wrapper. The comment's next named sibling is the export_statement, which matches nothing, so association falls through to None.

Reproduced with tree_sitter_typescript 0.23.2:

// @impl EXP_1
export function Foo() { return 1; }   // find_associated_scope(...) -> None
function Bar() { return 1; }          // (unexported) -> function_declaration ✓

Same for export class / export const / export default class. Since idiomatic TS exports most public declarations and the documented trace style is a leading comment, this silently drops the scope link for the common case. Fix: descend into export_statement in find_next_scope, mirroring the existing block special-case.

Comment thread src/sphinx_codelinks/analyse/utils.py Outdated
elif comment_type == CommentType.ts:
import tree_sitter_typescript # noqa: PLC0415

parsed_language = Language(tree_sitter_typescript.language_typescript())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.tsx files are parsed with the non-JSX grammar. config.py maps "ts": ["ts", "tsx"], so .tsx files are discovered and routed here, but language_typescript() cannot parse JSX. tree_sitter_typescript exposes language_tsx() for exactly this.

Reproduced with tree_sitter_typescript 0.23.2:

// @impl BTN_1
export function Button() { return <button/>; }
  • language_typescript()root_node.has_error = True (JSX misparsed), and find_associated_scope(...)None
  • language_tsx()has_error = False, parses cleanly

.tsx is advertised in the docs table, discovery defaults, and a discover fixture, yet no JSX is ever parsed in the tests. Fix: select language_tsx() for the .tsx extension (the TSX grammar is a superset that also parses plain .ts), or drop tsx from the advertised extensions until supported.

"function_declaration",
"class_declaration",
"method_definition",
"lexical_declaration",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lexical_declaration/variable_declaration make a leading comment bind to an intervening const/let/var instead of the function it documents. find_next_scope returns the first matching next sibling, and a const is a lexical_declaration that matches before the function does.

Reproduced with tree_sitter_typescript 0.23.2:

// @impl PROC_1
const helperFlag = true;
function processData() { return helperFlag; }

→ scope resolves to const helperFlag = true;, not processData — a wrong association.

Design tension: these node types are deliberately included so arrow-function consts (const testB = () => {}, as in demo.ts) are matched. So rather than dropping them, match a lexical_declaration only when its initializer is a function/arrow_function.

Comment thread tests/test_analyse.py

result = case["result"]
assert len(src_analyse.src_files) == result["num_src_files"]
assert len(src_analyse.oneline_warnings) == result["num_oneline_warnings"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

test_analyse_oneline_needs never asserts len(src_analyse.oneline_needs) — only num_src_files, num_oneline_warnings, and (below) num_comments. For the new ts case, num_oneline_warnings == 0 is satisfied equally by "extracted 1 one-line need" and "extracted nothing", so the test doesn't actually prove TypeScript one-line extraction works — a regression that silently produced zero needs would stay green.

demo.ts currently yields exactly one one-line need (id=TS_REQ_002). Suggest asserting it:

assert len(src_analyse.oneline_needs) == result["num_oneline_needs"]

(and add num_oneline_needs to each case).

Comment thread tests/test_analyse.py
{
"src_dir": TEST_DIR / "data" / "typescript",
"src_paths": [
TEST_DIR / "data" / "typescript" / "demo.ts",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No .tsx file with JSX is ever exercised through the analyse/tree-sitter pipeline. This case covers only demo.ts (plain TypeScript, no JSX), and the sole .tsx reference in the repo is the discover fixture (component.tsx = // component), which tests extension-based discovery only — its content is never parsed.

That's the gap that lets the .tsxlanguage_typescript() issue (see comment on utils.py L134) ship undetected. A .tsx fixture with real JSX, run through SourceAnalyse, would surface the has_error/lost-scope behavior immediately. Suggest adding a data/typescript/demo.tsx (JSX-returning component + a leading @-need) and a matching analyse case.

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.

Feature: Add TypeScript language support

2 participants