Cleanup: Replace ImmutableMap.of() with Map.of() #17091
Conversation
xiangfu0
force-pushed
the
cleanup/replace-immutable-map-with-java-standard
branch
7 times, most recently
from
46e83f4 to
5f73ed1
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR replaces Guava's ImmutableMap.of() with Java's native Map.of() throughout the codebase. The change modernizes the code by using Java's built-in collections factory methods introduced in Java 9, reducing dependency on the Guava library. Additionally, a checkstyle rule is added to prevent future use of ImmutableMap.
Key changes:
- Replaces all instances of
ImmutableMap.of()andImmutableMap.builder()patterns withMap.of()orMap.ofEntries() - Updates
Map.copyOf()calls to usejava.util.Mapinstead of Guava'sImmutableMap.copyOf() - Adds a checkstyle rule to disallow importing
com.google.common.collect.ImmutableMap
Reviewed Changes
Copilot reviewed 82 out of 82 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| config/checkstyle.xml | Adds checkstyle rule to prevent ImmutableMap imports |
| Multiple production files | Replaces ImmutableMap.of() with Map.of() and ImmutableMap.copyOf() with Map.copyOf() |
| Multiple test files | Replaces ImmutableMap.of() with Map.of() in test code |
| pinot-common/src/main/codegen/templates/Parser.jj | Removes unused ImmutableMap import |
...gration-tests/src/test/java/org/apache/pinot/integration/tests/tpch/TblToAvroMultiValue.java
Show resolved
Hide resolved
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/tpch/TblToAvro.java
Show resolved
Hide resolved
xiangfu0
force-pushed
the
cleanup/replace-immutable-map-with-java-standard
branch
2 times, most recently
from
02aef8e to
43d321a
Compare
…d ImmutableMap.of; refactor imports and builders to Java Map APIs; remove unused ImmutableMap imports
xiangfu0
force-pushed
the
cleanup/replace-immutable-map-with-java-standard
branch
from
43d321a to
aebfe93
Compare
yashmayya
left a comment
yashmayya
left a comment
There was a problem hiding this comment.
Looks like a bunch of tests are failing due to this change?
| <!-- Disallow importing com.google.common.collect.ImmutableMap (use java.util.Map instead) --> | ||
| <module name="IllegalImport"> | ||
| <property name="illegalClasses" value="com.google.common.collect.ImmutableMap"/> | ||
| </module> |
There was a problem hiding this comment.
Why is this rule needed - did we find any issues in Guava's immutable map?
There was a problem hiding this comment.
No, just it's just for code conventions
seems to be flaky |
yashmayya
left a comment
yashmayya
left a comment
There was a problem hiding this comment.
Thanks for the cleanup!
xiangfu0
restored the
cleanup/replace-immutable-map-with-java-standard
branch
xiangfu0
deleted the
cleanup/replace-immutable-map-with-java-standard
branch
2 participants
Cleanup: Replace ImmutableMap.of() with Map.of()