MINOR: Replace Collections and Arrays factory methods with Java 9+ equivalents in tools - #23344
Conversation
ConfigCommand already uses Set.of() elsewhere in this file, for example in the listConfigResources calls. This replaces the two remaining Collections.singleton() usages for consistency, and removes the java.util.Collections import that is no longer needed. Set.of() rejects null elements, but both call sites pass a freshly constructed object, so the replacement is safe here. verified with `./gradlew tools:test --tests ConfigCommandTest`
m1a2st
left a comment
There was a problem hiding this comment.
Thanks for the patch! Could you also clean up the tools test module? There are a few files there with the same pattern.
Replaces Collections.singleton(), singletonList() and emptyMap() with Set.of(), List.of() and Map.of() in ConnectInternalTopicsTest and DumpLogSegmentsTest, and removes the now-unused java.util.Collections imports. Two singletonMap() call sites are intentionally left as they are, since they map a key to a null value and Map.of() does not accept null: ConfigCommandTest#511 and ShareGroupCommandTest#265. Collections.nCopies(), disjoint() and emptyIterator() are also untouched, as they have no List.of()-style equivalent. verified with `./gradlew tools:test --tests DumpLogSegmentsTest --tests ConnectInternalTopicsTest`
|
Thanks for the review! I've pushed a second commit that cleans up the tools test module
I deliberately left a few call sites alone:
Let me know if you'd prefer a different scope |
Replaces Arrays.asList() with List.of() in DumpLogSegmentsTest and StreamsGroupCommandTest, as suggested in review. The three call sites in DumpLogSegmentsTest only read through the returned ListIterator (hasNext/next/previous), so an immutable list is fine. The one in StreamsGroupCommandTest is immediately copied into an ArrayList, so mutability of the source does not matter. The java.util.Arrays imports are kept, since both files still use Arrays.stream(). verified with `./gradlew tools:test --tests DumpLogSegmentsTest --tests StreamsGroupCommandTest`
|
Done — One note: I kept the |
m1a2st
left a comment
There was a problem hiding this comment.
Thanks for the patch, LGTM
Replaces legacy collection factory methods in the tools module with
their Set.of() / List.of() / Map.of() equivalents, and removes imports
that become unused
Changed
Collections.emptyMap() → Map.of(), Arrays.asList() → List.of()
Deliberately left unchanged
Collections.singletonMap() with a null value; Map.of() rejects nulls
List.of()-style equivalent
Arrays.stream()
Verification
DumpLogSegmentsTest --tests ConnectInternalTopicsTest --tests
StreamsGroupCommandTest
tools:spotlessCheck
Reviewers: Ken Huang s7133700@gmail.com