Fix ErrorProne warnings in :file-collections and :input-tracking #28887
Conversation
|
@bot-gradle test QFL without PTS |
|
I've triggered the following builds with parameters: |
mlopatkin
requested review from
bamboo and
abstratt
and removed request for
a team
| @@ -35,6 +35,7 @@ public abstract class LazilyInitializedFileCollection extends CompositeFileColle | |||
| * @deprecated Use the overload accepting the TaskDependencyFactory | |||
| */ | |||
| @Deprecated | |||
| @SuppressWarnings("InlineMeSuggester") | |||
There was a problem hiding this comment.
💭 The offending plugin has been fixed, so we can remove the usage in 9.0
| @@ -185,7 +185,7 @@ public void replaceAll(BiFunction<? super Object, ? super Object, ?> function) { | |||
| } | |||
|
|
|||
| @Override | |||
| public Object putIfAbsent(Object key, Object value) { | |||
| public synchronized Object putIfAbsent(Object key, Object value) { | |||
| Object oldValue; | |||
| synchronized (delegate) { | |||
There was a problem hiding this comment.
🤔 Since delegate is also a Properties instance (thus synchronized) and we're now synchronizing this Properties instance, what's the point of synchronized (delegate) here?
There was a problem hiding this comment.
🤔 Since
delegateis also aPropertiesinstance (thussynchronized) and we're now synchronizing thisPropertiesinstance, what's the point ofsynchronized (delegate)here?
Well, making these methods synchronized doesn't change much in this regard. I guess, I was either over-defensive when writing these methods and didn't look if the method is synchronized or not, or this is a leftover of some earlier code state when there were some extra steps in the synchronized block.
TBH, the synchronization contract is still broken for system properties, because user code gets a new Properties instance every time. I'd rather replace this fix with a class-level suppression and address synchronizations concerns separately.
We don't use errorprone annotations yet.
mlopatkin
force-pushed
the
ml/errorprone-file-collections-input-tracking
branch
from
001ce19
to
f3fd5ab
Compare
|
@bot-gradle test this |
|
I've triggered the following builds for you. Click here to see all build failures. |
Reviewing cheatsheet
Before merging the PR, comments starting with