API, Core: Fix errorprone warnings #9419
Conversation
| @@ -166,6 +166,7 @@ public void clear() { | |||
| wrapperSet.clear(); | |||
| } | |||
|
|
|||
| @SuppressWarnings("CollectionUndefinedEquality") | |||
There was a problem hiding this comment.
containsAll uses CharSequenceWrapper for equals to ensure consistent
hashing and equals behaviour similar to CharSequenceMap (javadoc of CharSequenceMap explains it). But the tool is unable to figure it out I guess. Hence, suppressing here and below two places.
| @@ -54,7 +54,7 @@ public boolean equals(Object other) { | |||
| CharSequenceWrapper that = (CharSequenceWrapper) other; | |||
|
|
|||
| if (wrapped instanceof String && that.wrapped instanceof String) { | |||
| return wrapped.equals(that.wrapped); | |||
| return wrapped.toString().contentEquals(that.wrapped); | |||
There was a problem hiding this comment.
see
https://errorprone.info/bugpattern/UndefinedEquals check For CharSequence
There was a problem hiding this comment.
Thanks for looking into this @ajantha-bhat, but there is a fair chance that this was intentional for performance reasons.
There was a problem hiding this comment.
I see. We can suppress the warning in that case then.
@aokolnychyi: Thoughts?
There was a problem hiding this comment.
Reverted the fix and suppressed instead.
|
cc: @aokolnychyi |
|
Retriggering build due to flaky test |
api/src/main/java/org/apache/iceberg/util/CharSequenceWrapper.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Looks good to me, I am not sure about one comment.
I was meaning to do this, thanks @ajantha-bhat!
There was a problem hiding this comment.
Thanks for fixing this @ajantha-bhat and @aokolnychyi for the review 👍
* API, Core: Fix errorprone warnings * Address comments * Remove comment
* API, Core: Fix errorprone warnings * Address comments * Remove comment
* API, Core: Fix errorprone warnings * Address comments * Remove comment
./gradlew clean build -x testis reporting some warnings as seen below.After this PR,
./gradlew clean build -x testis green.