Skip to content

fix: restore Java parser fixes (#275/#278/#280), GDScript support (#316), and refresh stdio banner test#364

Closed
npkriami18 wants to merge 5 commits intotirth8205:mainfrom
npkriami18:fix/restore-java-gdscript-stdio
Closed

fix: restore Java parser fixes (#275/#278/#280), GDScript support (#316), and refresh stdio banner test#364
npkriami18 wants to merge 5 commits intotirth8205:mainfrom
npkriami18:fix/restore-java-gdscript-stdio

Conversation

@npkriami18
Copy link
Copy Markdown

Follow-up to #361 / #362. Same silent-revert pattern, three more victims.

What this PR does

Five cherry-picks onto main, preserving authorship:

Commit PR Author Tests restored
8104eb7 #278 @krmahadevan 3 Java inheritance/import-edge tests
9a88f20 #275 @krmahadevan 1 Java method-name test
088c281 #280 @krmahadevan 2 Java import-resolution tests
112a442 #316 @Mornor 9 GDScript tests
(new) 1 stdio-transport assertion refresh

Plus a 1-line test fix: TestServeMainTransport.test_stdio_calls_mcp_run_stdio was asserting calls == [{"transport": "stdio"}] but PR #290 (cc169af) had added show_banner=False to the production call without updating the test. Refreshed the assertion to match.

Why these were failing

Same root cause as #362: the 3cf31ec / cb9d25a chore: resolve merge conflicts with main commits accepted the stale feature-branch side of parser.py for the ReScript PR (#323) and Julia PR (#319). Both feature branches forked from db2d2df — before the Java/GDScript fixes landed — so git merge auto-resolved the Java and GDScript hunks in favour of the side that never had them. See #361 for the full pattern.

Test results

# Before (upstream/main @ b0f8527)
60 failed, 1015 passed, 1 skipped, 2 xpassed, 6 errors

# After (this branch, independent of #362)
45 failed, 1030 passed, 1 skipped, 2 xpassed, 6 errors

15 failures fixed. Remaining 45+6 are explicitly out of scope:

This PR and #362 are independent and can be merged in either order. Applying both together yields the full 60 → ≤8 improvement.

Quality gate

  • uv run ruff check code_review_graph/parser.py tests/test_main.py
  • uv run pytest tests/test_multilang.py::TestJavaParsing tests/test_multilang.py::TestJavaImportResolution tests/test_multilang.py::TestGDScriptParsing tests/test_main.py::TestServeMainTransport — 25 passed

Conflict resolution notes

  • parser.py EXTENSION_TO_LANGUAGE: kept both .res/.resi (ReScript, HEAD) and .gd (GDScript, cherry-pick side).
  • tests/test_multilang.py: kept HEAD side (preserves TestJuliaParsing class and ReScript cross-module resolver tests that the cherry-pick side did not have).

cc @tirth8205 — second restoration PR for the same merge-resolution bug tracked in #361. @krmahadevan @Mornor — pinging as original authors; please flag if I misread any intent during the cherry-pick.

krmahadevan and others added 5 commits April 21, 2026 19:21
…odes (tirth8205#278)

tree-sitter-java wraps extends/implements clauses in superclass and
super_interfaces nodes whose .text includes the keyword. The _get_bases()
function was storing the full text (e.g. "implements UserRepository")
as the INHERITS edge target instead of just the type name.

This caused inheritors_of queries to fail — the query looks up edges
by the qualified class name or bare name, neither of which matches
"implements UserRepository".

Adds a Java-specific branch in _get_bases() that drills into the AST
children to extract type_identifier nodes (including generic_type for
parameterized interfaces like IBar<String>). C#/Kotlin are unaffected
and retain the existing behavior.

(cherry picked from commit 8104eb7)
PR tirth8205#290 (cc169af) added show_banner=False to the production
mcp.run(transport="stdio") call to silence the FastMCP banner
under MCP stdio clients, but the assertion in
TestServeMainTransport.test_stdio_calls_mcp_run_stdio was never
refreshed to match. The test has been failing on main ever since.

Co-authored-by: Copilot <[email protected]>
…h8205#275)

tree-sitter-java places type_identifier (return type) before identifier
(method name) in method_declaration nodes.
The generic _get_name() loop matched type_identifier first,
causing methods to be indexed under their return type instead of their actual name.

For example:

* `public String getName()` was indexed as "String" instead of "getName", and
* `public ConfigBean getUtilityIngestionBean()`was indexed as "ConfigBean".

This broke callers_of,callees_of, and
children_of queries for any Java method with a non-void,
non-generic return type.

Adds a Java-specific branch in _get_name() that returns the first
identifier child for method_declaration nodes, following the same
pattern as the Go fix (field_identifier) from PR tirth8205#166.

Kotlin & Scala  is unaffected — its syntax places the name before the return type.

(cherry picked from commit 9a88f20)
Java imports like `import com.example.auth.User` were stored as raw
dot-notation strings because _do_resolve_module() had no Java branch.
This caused `importers_of` queries to return 0 — the query looks for
file path targets, but the stored edges had raw import strings.

Adds a Java branch that converts dot-notation to a relative path
(com/example/auth/User.java) and walks up from the caller's directory
to find the source root. This resolves same-source-root imports (the
common case in Maven modules).

Also handles:
- Static imports (import static pkg.Class.member) — strips the member
name and resolves to the class file
- Wildcard imports (import pkg.*) — skipped, can't resolve to one file
- JDK/library imports (java.util.*) — remain unresolved (no local file)

(cherry picked from commit 088c281)
@npkriami18
Copy link
Copy Markdown
Author

Closing in favour of a single unified restoration PR — combining all reverted parser fixes (#274, #275, #276, #278, #280, #285, #298, #316, #319) plus the _tool_manager FastMCP API drift and stdio banner test into one green PR.

@npkriami18 npkriami18 closed this Apr 21, 2026
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.

3 participants