Add tests to reproduce exceptions in UseVarForGenericsConstructors #259
Conversation
| """, """ | ||
| package com.example.app; | ||
|
|
||
| import java.util.List; |
There was a problem hiding this comment.
Hmm; we might want to clear out unused imports.
| import java.util.List; |
There was a problem hiding this comment.
Thought about that too but added a note here java-lang-var.yml L20
There was a problem hiding this comment.
Hmm; but wouldn't we at least be able to maybeRemoveImport the type on left side of the assignment when replaced with var (or maybe only if the left side is an interface)? If there's any other uses still then the maybe part should pick up on that.
There was a problem hiding this comment.
Oh wasn't aware of this. This should be rolled out to every Recipe in org.openrewrite.java.migrate.lang.var
joanvr
added
bug
There was a problem hiding this comment.
Thank you very much for your contribution. I've been following up your PR also from the slack support channel. And indeed it's a nice and interesting improvement!
I've seen that you've added applicability code to avoid the unsupported case of generic variables with bounds, so it looks good for me :)
Would you like me to merge it already or you want to do some final changes?
|
Hi @joanvr Thanks for the review, I'd like to apply Tims suggestion first, I think I'll be done with this PR after lunch. |
…rGenericMethodInvocations and UseVarForGenericsConstructors
|
@joanvr ready for merge from my point |
What's changed?
UseVarForGenericsConstructors failed for some edge cases. They are documented by #257 and are fixed in this PR.
What's your motivation?
Close #257
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
No, would lead to broken recipes.
Any additional context
Checklist
./gradlew licenseFormatI've updated the documentation (if applicable)