Support compatible Kotlin AST diff trees#1101
Open
thromel wants to merge 1 commit into
Open
Conversation
5fe7b6a to
79d9299
Compare
79d9299 to
fbd2f3c
Compare
fbd2f3c to
ed5205d
Compare
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
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.
Fixes #1100.
Why
Issue #1100 shows a real mismatch in AST-diff for Java-to-Kotlin changes. Java files use the JDT-style GumTree labels, while Kotlin files were still using tree-sitter labels such as
class_declaration,function_declaration, andproperty_declaration. That made the two trees harder to match even when the source-level change was a straightforward Java-to-Kotlin conversion.This PR changes AST-diff mode only. Refactoring detection still uses the existing Kotlin PSI/UML pipeline.
What changed
CompilationUnit,PackageDeclaration,ImportDeclaration,TypeDeclaration,FieldDeclaration,MethodDeclaration,SingleVariableDeclaration,Block,ReturnStatement,MethodInvocation,InfixExpression, and comments.astDiff=true.TypeDeclaration, so comparing declaration labels would hide Java-to-Kotlin diffs.=asASSIGNMENT_OPERATOR.QualifiedNamelabeledkotlin.math.*covers the full source text, including.*.Why the fixture diff is large
No fixture files were removed. The large deletion count is inside generated JSON snapshots.
The old Kotlin AST-diff tree came from tree-sitter and included many low-level syntax nodes. The new tree is smaller and Java-compatible. For example, Kotlin now emits
CompilationUnit,TypeDeclaration,MethodDeclaration,FieldDeclaration,VariableDeclarationFragment,INFIX_EXPRESSION_OPERATOR, andASSIGNMENT_OPERATORinstead of tree-sitter-specific nodes such assource_file,class_declaration,function_declaration, andproperty_declaration.That means the expected mapping snapshots had to be regenerated. The changed fixture folders are limited to the two commits listed in
java2kotlin.json.Behavior notes
The new visitor is compatibility-oriented. It is not trying to model every Kotlin PSI node as a perfect JDT equivalent. It maps the Kotlin constructs that matter for the current AST-diff path to Java-style labels, then falls back to generic expression or statement wrappers when a construct does not have a dedicated mapping yet.
That keeps the issue fix scoped: Java-to-Kotlin AST diffs now get comparable tree roots and common declaration/body structure, without changing the refactoring model or the non-AST-diff Kotlin path.
Review feedback addressed
Claude Code with the Opus model alias reviewed the change before the PR was opened. I addressed the concrete findings around in-body comments, non-ASCII source offsets, and avoiding silent visitor failures.
puku-cli with the Opus model reviewed the PR after it was opened. One finding held up against the code: wildcard imports had a label/source-span mismatch. This PR now ranges the full wildcard import text and bounds contained-node lookup by the owning PSI element.
Nikos's follow-up commit on
masteradded Java-to-Kotlin operator mappings for infix operators, assignment operators, and variable declaration=. This branch is rebased on that work and adapts it to the compatible Kotlin AST: those operators are now represented with compatible node names instead of tree-sitter operator node names.The puku review also flagged cross-language detection as a possible regression. I checked that against the previous behavior. Before this PR, Java and Kotlin were already treated as cross-language because Java used
TypeDeclarationand Kotlin used tree-sitter declaration labels. The new language-identity check preserves that behavior after Kotlin starts emitting Java-compatible declaration labels.Tests
git diff --check./gradlew test --tests org.refactoringminer.astDiff.tests.KotlinCompatibleTreeVisitorTest --tests org.refactoringminer.astDiff.tests.JavaToKotlinDiffTest --no-daemon./gradlew test --tests org.refactoringminer.astDiff.tests.TreeFromParserTest --tests org.refactoringminer.astDiff.tests.TreeMatcherTest --tests org.refactoringminer.astDiff.tests.KotlinCompatibleTreeVisitorTest --tests org.refactoringminer.astDiff.tests.JavaToKotlinDiffTest --tests org.refactoringminer.astDiff.tests.PythonDiffTest --tests org.refactoringminer.astDiff.tests.TypeScriptDiffTest --no-daemon./gradlew test --tests org.refactoringminer.test.TestKotlinDatasetRefactorings --no-daemon