fix(lang-kotlin): support fun interface extraction via tree-sitter-kotlin re-vendor#2271
Open
glier wants to merge 3 commits into
Open
fix(lang-kotlin): support fun interface extraction via tree-sitter-kotlin re-vendor#2271glier wants to merge 3 commits into
fun interface extraction via tree-sitter-kotlin re-vendor#2271glier wants to merge 3 commits into
Conversation
…kotlin re-vendor Vendored tree-sitter-kotlin@0.3.8 (fwcd) parsed `fun interface Foo` as an ERROR node and dropped the declaration plus its abstract method, so functional (SAM) interfaces were never extracted. The fix landed upstream in fwcd/tree-sitter-kotlin#169 (closes abhigyanpatwari#87), merged to main 2025-04-25, but is not in any npm release (latest tag 0.3.8; main is the unreleased 0.4.0). Re-vendor the grammar from the unreleased fwcd main commit c8ac3d26: - refresh src/{parser.c,scanner.c,node-types.json,tree_sitter/*.h} and bindings/node/index.js; bump the vendor version 0.3.8 -> 0.4.0; record the pinned SHA + rationale in _vendoredBy and the vendor README. - switch the prebuild workflow's kotlin registry kind 'npm' -> 'vendored' (the fix is unreleased on npm, so prebuilds must build from the vendored C source, like swift/dart/proto). - add a hold to .github/vendored-grammars.json so the weekly auto-update monitor does not strict-inequality-revert the pin to the broken npm 0.3.8 (isNewer compares 0.3.8 != 0.4.0). - add 3 regression tests + a fixture asserting fun interfaces extract as Interface nodes with their abstract methods, and that plain-interface heritage still resolves. Existing KOTLIN_QUERIES need no change: the new grammar models `fun interface` as a class_declaration with an "interface" keyword child (plus an extra "fun" modifier child), which the existing interface rule already matches. Full Kotlin suite green against the new grammar (300 unit/cfg/resolver + 233 integration). NOTE: prebuilds/ are intentionally not in this commit. The version bump auto-triggers .github/workflows/build-tree-sitter-prebuilds.yml, which regenerates all 6 platform binaries from the vendored source in a separate PR. Until that lands, CI loads the committed 0.3.8 prebuild, so the new kotlin tests are red and the grammar change is inert at runtime. Merge the prebuild PR first or together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@glier is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
CI Report❌ Some checks failed Pipeline Status
Test Results
❌ 2 failed / 12881 passed 55 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
Collaborator
|
Please double check it with #2194. We need to make sure this fits into the matrix. For now, we only support tree-sitter on 0.21.x and you need to compile it locally on this tree-sitter version and double check if it can generate the grammar properly. |
magyargergo
requested changes
Jun 22, 2026
magyargergo
left a comment
magyargergo
left a comment
Collaborator
There was a problem hiding this comment.
Please check my comment above.
Contributor
Author
|
Thanks @magyargergo — I checked against #2194, and on the ABI/matrix side this already fits, so I want to confirm that and then ask you about the right way to get CI green, since the grammar-bump + prebuild flow is your pipeline. ABI / #2194 matrix — fits without a runtime bump:
Compiled + verified locally on
The red CI — and a process question for you: I'm not sure of the intended sequencing for a vendored-grammar bump in this repo — could you point me the right way? A couple of options I see:
Happy to do whichever fits your workflow — or follow a different process if there's an established one I've missed. |
Collaborator
|
I saw one ci job is red: https://github.com/abhigyanpatwari/GitNexus/actions/runs/27964323565/job/82753978209#step:4:1 I'll also check this locally if i can build it on tree-sitter 0.22.1 |
The kotlin `hold` added in the previous commit makes the tree-sitter upgrade-readiness report count it as a blocker — the report treats every held vendored grammar as frozen below a runtime upgrade (same as the intentionally-pinned tree-sitter-cpp and the ABI-held tree-sitter-c), "in-range ABI or not". So the report's blocker count goes 2 -> 3. Update the hardcoded count in test_issue_update_summary_regex_matches_current_report (and the _render_report docstring) accordingly — exactly as that test instructs: "if a grammar is added/removed or a pin/hold changes, update the expected counts". kotlin's ABI (14) is in range; the hold is what flags it, with the reason recorded in .github/vendored-grammars.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks for the link — didn't spot that failing job at first. Tracked it down: the Only thing left red is the 3 fun-interface tests — still the prebuild question from above. |
Two committed baselines pinned the pre-bump kotlin state and broke when the grammar was re-vendored (0.3.8 -> 0.4.0): - cli-commands.test.ts pinned the vendored kotlin package version at 0.3.8 -> update to 0.4.0. - bench/scope-capture/baselines.json: the new kotlin-fun-interface fixture joins the lang-resolution/kotlin-* corpus AND the new grammar parses `fun interface` as a class_declaration (not an ERROR node), so the capture fingerprint drifts. Rebaselined to the NEW grammar's fingerprint (verified by building the vendored parser.c against tree-sitter@0.21.1 and running measure.mjs --check); scaling ~0.83 (linear). Like the fun-interface integration tests, the scope-capture --check passes only once the regenerated prebuilds land; until then CI loads the committed 0.3.8 binary, so it stays red. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
We also need to regenerate the fingerprints for kotling benchmarks. https://github.com/abhigyanpatwari/GitNexus/actions/runs/27979931111/job/82807100858?pr=2271 |
Contributor
Author
|
Already did that in 6105cf7 — but I baselined the kotlin scope-capture fingerprint to the new grammar ( I can flip the baseline to the current old-grammar value so the bench goes green now, but the fun-interface tests need the new binary regardless — so it really all hinges on regenerating the kotlin prebuilds. Want me to handle that somehow, or will you kick |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The vendored
tree-sitter-kotlin@0.3.8(fwcd) parses a Kotlin functional (SAM) interface —fun interface Foo { ... }— as anERRORnode and drops the declaration and its abstract method. Sofun interfacetypes are never extracted (noInterfacenode, no method).Repro (old grammar):
→
(source_file (ERROR "fun" (user_type (type_identifier)) (simple_identifier)) (lambda_literal ...))The fix exists upstream: fwcd/tree-sitter-kotlin#169 (closes #87), merged to
main2025-04-25 — but it is not in any npm release (latest tag is still0.3.8, from 2024-08-03;mainis the unreleased0.4.0).Change
Re-vendor the grammar from the unreleased fwcd
maincommitc8ac3d26:src/{parser.c,scanner.c,node-types.json,tree_sitter/*.h}andbindings/node/index.js; bump the vendorversion0.3.8 → 0.4.0; record the pinned SHA + rationale in_vendoredByand the vendorREADME.md.kind: 'npm' → 'vendored'— the fix is unreleased on npm, so prebuilds must build from the vendored C source (like swift/dart/proto).holdto.github/vendored-grammars.jsonso the weekly auto-update monitor does not revert the pin:isNeweris strict-inequality, so0.3.8 !== 0.4.0would otherwise re-vendor the broken npm0.3.8over0.4.0.Interfacenodes with their methods, and that plain-interface heritage still resolves.No
KOTLIN_QUERIESchange needed: the new grammar modelsfun interface Fooas aclass_declarationwith an"interface"keyword child (plus an extra"fun"modifier child), which the existing interface rule(class_declaration "interface" (type_identifier) @name)already matches.Verification
Full Kotlin suite green against the new grammar (built locally for darwin-arm64):
test/unit/{kotlin-scope-captures,kotlin-static-marker}.test.ts,test/unit/cfg/kotlin-visitor.test.ts,test/integration/resolvers/{kotlin,kotlin-coverage}.test.ts→ 300 + 233 pass (incl. the 3 new fun-interface cases).test/unit/{vendored-grammars,grammar-update-monitor}.test.ts→ 28 pass.KOTLIN_QUERIESis present in the newnode-types.json(no silent node-type drift from the 0.3.8→main jump).prebuilds/are intentionally not in this PR — onlydarwin-arm64is buildable locally; the other 5 platforms require the cross-build CI.The version bump (
0.3.8 → 0.4.0) auto-triggersbuild-tree-sitter-prebuilds, which regenerates all 6 platform binaries from the vendored source. Until those binaries land,node-gyp-buildkeeps loading the committed0.3.8prebuild, so:Because this is a fork PR, the prebuild workflow's
aggregate(auto-PR) job is skipped — it will only upload artifacts. Recommended: a maintainer runsbuild-tree-sitter-prebuilds(workflow_dispatch,grammars=kotlin) to regenerate + commit the 6 binaries +SHA256SUMS, and that lands first or together with this PR. (SHA256SUMSis only written by that workflow, never verified at install/runtime.)🤖 Generated with Claude Code