UseLambdaForFunctionalInterface: keep the receiver of implicit getClass() - #980
Draft
martinfrancois wants to merge 3 commits into
Draft
Conversation
…ss() An anonymous class has its own `this`; a lambda uses the enclosing lexical `this`. The recipe's `usesThis` guard only recognises an explicit `this` identifier, so an unqualified `getClass()` in the functional method slipped past it and the conversion silently changed the returned runtime class from the anonymous implementation to the enclosing class. Add a `usesImplicitGetClass` guard that keeps the anonymous class when the functional method resolves `java.lang.Object getClass()` with no receiver or with a bare `super` receiver, either as an invocation or as a `super::getClass` member reference. It does not descend into nested class or nested anonymous class bodies, which declare their own `this`, but it does visit the enclosing expression and the arguments of a qualified `new`, which are evaluated in the outer scope. A skipped site is reported in the data table as "calls `getClass()` on the anonymous instance". Worth weighing on review: a `getClass` call with no type attribution now blocks conversion, because its receiver cannot be proven; an outer-qualified `Test.super.getClass()` keeps its receiver either way and is deliberately left convertible; and `Test.this.getClass()` was already blocked by `usesThis`. No existing test expectation changes.
…instance, not just getClass()
4 tasks
martinfrancois
marked this pull request as draft
August 17, 2026 08:08
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.
Suggested review order: 24 of 52 (Score: 5)
Review first: #979
What's changed?
UseLambdaForFunctionalInterfaceno longer converts an anonymous class to a lambda when the body of that anonymous class callsgetClass()on the anonymous instance itself. Three written forms land on that instance, and all three now block the conversion:getClass(), meaning a call written with no receiver at all;super.getClass()withsuperwritten on its own and no class name qualifying it, called a baresuperhere, in contrast with the outer-qualifiedTest.super.getClass()under the limitations below;super::getClass, again with a baresuper.A call in one of those three forms named
getClasswhose LST node carries no method type, because the source lacks complete type attribution, blocks the conversion too: the recipe cannot prove what it resolves to, and blocking is the fail-safe direction on missing type information (dontUseLambdaWhenGetClassCannotBeAttributed).Any refusal by this new check records the text "calls
getClass()on the anonymous instance" in theAnonymousFunctionalInterfaceImplementationsdata table, which already exists onmainalong with the recording of a reason for every refusal, so this change adds one reason text and no new plumbing.dataTableRecordsImplicitGetClassasserts that such a refusal produces exactly one row, withconvertiblefalse and that reason text.Before
Inside
class Test.Actual after the recipe
Using the recipe on current
main.Expected after the recipe
(unchanged)The recipe leaves the input above exactly as written.
The new check is a private method
usesImplicitGetClass, called fromconversionBlocker, the existing private method that runs the individual reasons for refusing a conversion and returns the first that applies, directly after the existingusesThischeck.What's your motivation?
Recipe:
org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface.An anonymous class has its own
this. A lambda does not, and uses the enclosingthisinstead. An instance method call written with no receiver takesthisas its target reference (JLS 15.12.4.1), and asuper::getClassmethod reference is evaluated against that samethis(JLS 15.13.3). The recipe already refuses to convert when the body mentionsthis, but an unqualifiedgetClass()has nothistoken for that check to find.The converted code then returns a different value at run time. In the input above, the anonymous class is declared inside
Test, sojavaccompiles it to the binary classTest$1and thegetClass()call returns theClassobject forTest$1. After the conversionmainperforms, the same call returns theClassobject forTest. The compiler reports nothing, so the changed class reaches logging output, cache keys andgetClass().getResource(...)lookups unnoticed.When the anonymous class is created inside a static method and its body calls
super.getClass(), the output produced bymaindoes not compile at all: the body becomes() -> super.getClass(), whichjavacrejects withnon-static variable super cannot be referenced from a static context.dontUseLambdaWhenSuperGetClasscovers exactly that shape. Reproduced on 2.40.0 and on 2.41.0-SNAPSHOT built frommain.Confirmed real-world execution
OverlayQuickTile.javaat868ef81e.org.openrewrite.recipe:rewrite-static-analysis:2.41.0.The released recipe changes an anonymous
Runnableinto a lambda. Its unqualifiedgetClass()call then returns the enclosingOverlayQuickTileclass instead of the anonymous implementation class, silently changing the application log tag.Anything in particular you'd like reviewers to focus on?
No existing test changed its expectation: the test file has 251 added lines and no deleted lines.
Where the check stops, then two limitations:
usesImplicitGetClasswalks the anonymous class body but does not descend into a class declaration written inside it, local or member, nor into a nested anonymous class body: code there has athisof its own, so agetClass()there keeps its receiver and must not block the outer conversion (useLambdaWhenOnlyANestedAnonymousClassCallsGetClass). It does visit the arguments of a nested anonymous class creation and its enclosing instance expression, the qualifier written before.newinouter.new Inner() { ... }, since both are evaluated in the outer scope (dontUseLambdaWhenEnclosingExpressionOfQualifiedNewCallsGetClass), and it descends into a lambda written in the body, which has nothisof its own, so an unqualifiedgetClass()there still lands on the anonymous instance (dontUseLambdaWhenImplicitGetClassIsNested).Test.super.getClass()is still converted, because that call names the enclosing instance both before and after the conversion. A related effect on binary names that this change does not address either: converting an anonymous class that has further anonymous classes nested inside it makesjavacnumber the remaining ones differently on the next compile, so what used to beTest$1$1becomesTest$1. The recipe renames nothing itself, and this happens onmainfor every such conversion.toString(),hashCode()orequals(...)is still converted, and the converted code can still return a different result at run time, for the same reasongetClass()does. Separately, an anonymous class created in a static method whose body makes a baresuper.call to a method other thangetClass()is still converted into code that does not compile. Neither case is changed here; both behave as onmain.Have you considered any alternatives or workarounds?
One alternative is to block every unqualified call to a
java.lang.Objectmethod, not onlygetClass(). I kept it togetClass()becausejava.lang.Object.getClass()isfinaland cannot be overridden, so the change in the returned value is certain, whiletoString(),hashCode()andequals(...)are often overridden on purpose and a wider check would refuse conversions that are correct today. Widening it is two lines insideusesImplicitGetClass, plus tests for the wider behaviour that I have not written and so cannot size: the check today compares the call against aMethodMatcherbuilt for the signaturejava.lang.Object getClass(), and the wider version would instead test thatmethod.getMethodType().getDeclaringType()isjava.lang.Object, whatever the method name is. Say so in review and I will change it here.Any additional context
Pre-existing tests changed: None.
This change adds 9 tests to
UseLambdaForFunctionalInterfaceTest. Without the code change in this pull request, 8 fail. They cover these cases:getClass()whose converted code returns a different class, such asdontUseLambdaWhenImplicitGetClasssuperwhose converted code does not compile, including the method-reference caseuseLambdaWhenOnlyANestedAnonymousClassCallsGetClassgetClasscall without type attribution, where leaving the source unchanged is the safe resultThe ninth test passes either way and shows that a correct conversion is not blocked.
This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.
I ran the formatter with the repository's
.editorconfig. It also wanted to re-indent lines that this change does not touch, so I left those alone and kept the diff limited to this change.Checklist
./gradlew buildlocally, and committed any resulting changes torecipes.csv