Run OpenRewrite Java 17 recipe #3759
Conversation
a0aa352 to
2de7b56
Compare
| // Special handle SortedSets because they are fast to compare | ||
| // because their elements must be in the same order to be equivalent Sets. | ||
| if (key1 instanceof Collection<?> set) { | ||
| if (key1 instanceof SortedSet<?> set) { |
There was a problem hiding this comment.
Hi @timtebeek! May I ask your opinion on this one?
I executed the Java 17 recipe on the AssertJ codebase, and as far as I can tell, there was an incorrect migration.
Initially, it was:
and, after the recipe run, became:
- if (key1 instanceof SortedSet) {
- if (!compareOrderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare, visited)) {
+ if (key1 instanceof Collection<?> set) {
+ if (!compareOrderedCollection(set, (Collection<?>) key2, currentPath, toCompare, visited)) {Do you think it's something to report as an OpenRewrite issue?
There was a problem hiding this comment.
Apologies for the late reply! I was out 🏂🏻. Indeed looks like a bug; I think it's best not to make any change if there's a mismatch between the righthand side of the instanceof and the cast. Would you mind logging an issue? I'm catching up here still.
Great to see you're running these recipes; I hope they've helped you so far already.
There was a problem hiding this comment.
No worries!
Yes, they definitely helped! Still going through the review of the 400+ files 🙃
I'll log an issue in the upcoming days 👍
|
|
||
| // Check List, as element order matters this comparison is faster than using unordered comparison. | ||
| if (key1 instanceof Collection<?> list) { | ||
| if (key1 instanceof List<?> list) { |
There was a problem hiding this comment.
@timtebeek this is the second one, which originally was:
and, after the recipe run, became:
- if (key1 instanceof List) {
- if (!compareOrderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare, visited)) {
+ if (key1 instanceof Collection<?> list) {
+ if (!compareOrderedCollection(list, (Collection<?>) key2, currentPath, toCompare, visited)) {78d4196 to
3daa3c1
Compare
3daa3c1 to
c352ade
Compare
See https://docs.openrewrite.org/recipes/java/migrate/upgradetojava17.