Forward visit methods declared on a delegate superclass under adapt()#8172
Merged
Conversation
TreeVisitorAdapter collected proxy-forwarded methods from delegate.getClass().getDeclaredMethods() (the leaf only), so preVisit/postVisit/visitX declared on a superclass never fired under cross-language adapt(). Walk the delegate hierarchy up to (not into) the iso-visitor instead, leaf-first so the most-derived override wins. Also de-qualifies inline class names and trims comments in the file.
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.
What's changed
TreeVisitorAdaptercollected the visit-like methods to proxy-forward fromdelegate.getClass().getDeclaredMethods()— the leaf class only. SopreVisit/postVisit/visitXdeclared on a superclass of the delegate were never forwarded, and silently did not fire when the visitor was adapted to another language (C#/Python/Kotlin/Groovy). Java is unaffected because it runs natively, withoutadapt()— which is why this stayed hidden.The fix walks the delegate's class hierarchy from the leaf up to — but not into — the language iso-visitor (
delegateType), collecting the user's visit-like methods at every level (leaf-first, so the most-derived override wins). Stopping at the iso-visitor matters: crossing into it would forward the framework's ownvisitXto the foreign-language delegate and break native traversal.Why it matters
Any visitor that factors shared behavior into a base class (e.g. a common scanning base that overrides
preVisit/postVisit) misbehaves under cross-languageadapt()today, with no error — the overrides just don't run.Tests
Two regression tests in
TreeVisitorAdapterTest:preVisitdeclared on aTreeVisitorsuperclass is forwarded.visitIdentifierdeclared on aJavaIsoVisitorsuperclass is forwarded (exercises the iso-visitor stop-boundary).Both fail on
mainand pass with this change; existing adapter tests,rewrite-groovy, andrewrite-corestay green.Also
Incidental cleanup of the touched file: inline fully-qualified class names replaced with imports, and a few verbose comments trimmed.