[cleanup] Replace Collections singleton factory usages

[xiangfu0]

Summary

This PR addresses the follow-up from #18835 and the review comment on #18837 by modernizing immutable collection factory usage while keeping targeted Checkstyle guards for empty collection factories.

Changes:

• Replaced Collections.singletonList(...) with List.of(...) where the element is non-null.
• Replaced Collections.singletonMap(...) with Map.of(...) where keys/values are non-null.
• Replaced Collections.singleton(...) with Set.of(...).
• Replaced Collections.emptyList(), Collections.emptySet(), and Collections.emptyMap() with List.of(), Set.of(), and Map.of() where mutability semantics are unchanged.
• Added Checkstyle guards for Collections.emptyList, Collections.emptySet, and Collections.emptyMap, including method references.
• Preserved mutable semantics at mutating call sites, including the materialized-view dependent-list filtering in PinotHelixResourceManager.deleteTable().
• Kept Collections.singletonList(), Collections.singleton(), and Collections.singletonMap() out of the Checkstyle ban, but documented that Collections.singleton* should only be used when an element, key, or value argument is intentionally null because List.of(null), Set.of(null), and Map.of(...) with nulls throw NullPointerException.
• Preserved intentional null test semantics with explicit null-preserving constructions where modern factories would throw NullPointerException.
• Updated agent-facing docs (AGENTS.md, CLAUDE.md, .github/copilot-instructions.md, and kb/) with the review rule: block Collections.emptyList/Collections.emptySet/Collections.emptyMap, prefer modern factories for non-null immutable literals, scan method references too, check mutating callers before replacing empty factories, and do not blanket-ban Collections.singleton*, but only allow it for intentional null element/key/value cases.

Why

The modern immutable factories are the preferred style for non-null collection literals. Collections.emptyList(), Collections.emptySet(), and Collections.emptyMap() should not be reintroduced, so Checkstyle now blocks them. Singleton factories are handled by review judgment instead of a blanket ban because they can still be valid when null elements, keys, or values are intentional.

User Manual / Contributor Guidance

For new Pinot code, use List.of(), Set.of(), and Map.of() instead of Collections.emptyList(), Collections.emptySet(), and Collections.emptyMap(); Checkstyle enforces this. Prefer List.of(...), Set.of(...), and Map.of(...) for non-null immutable collection literals. Use Collections.singleton* only when an element, key, or value argument is intentionally null; otherwise prefer List.of(...), Set.of(...), and Map.of(...). If a test intentionally needs null contents, keep that intent explicit and local to the test. Before replacing empty collection factories, check whether callers mutate the returned collection and copy locally if needed.

Sample Table Config / Queries

No table config or query syntax changes are introduced. Query behavior is unchanged; this is a source cleanup only.

Validation

• rg -n "Collections(\\.|::)(emptyList|emptySet|emptyMap)\\b" --glob '*.java' --glob '*.jj' --glob '!target/**' .
• git diff --check
• ./mvnw spotless:apply
• ./mvnw checkstyle:check
• ./mvnw license:format
• ./mvnw license:check
• ./mvnw -DskipTests test-compile
  • Initial full pass caught two missed method references; after fixing them, resumed compilation passed through the remaining modules.
  • A transient local Immutables annotation processor failure in pinot-minion-builtin-tasks passed on immediate rerun with -rf :pinot-minion-builtin-tasks -e.
• ./mvnw -pl pinot-controller -Dtest=PinotHelixResourceManagerStatelessTest#testUpdateTableConfigRefreshesBrokerAndServerCaches test
• ./mvnw spotless:apply -pl pinot-controller
• ./mvnw checkstyle:check -pl pinot-controller
• ./mvnw license:format -pl pinot-controller
• ./mvnw license:check -pl pinot-controller
• ./mvnw spotless:apply -pl pinot-core,pinot-query-runtime
• ./mvnw checkstyle:check -pl pinot-core,pinot-query-runtime
• ./mvnw license:format -pl pinot-core,pinot-query-runtime
• ./mvnw license:check -pl pinot-core,pinot-query-runtime
• ./mvnw -pl pinot-server -Dtest=TableTierResourceTest test
• ./mvnw spotless:apply -pl pinot-server
• ./mvnw checkstyle:check -pl pinot-server
• ./mvnw license:format -pl pinot-server
• ./mvnw license:check -pl pinot-server

[codecov-commenter]

Codecov Report

❌ Patch coverage is 59.23567% with 128 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.76%. Comparing base (63bf8dc) to head (7322e1c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
.../apache/pinot/client/admin/SegmentAdminClient.java	0.00%	5 Missing and 1 partial ⚠️
...e/pinot/common/utils/FileUploadDownloadClient.java	0.00%	4 Missing ⚠️
...roller/workload/scheme/TablePropagationScheme.java	20.00%	2 Missing and 2 partials ⚠️
...r/broker/helix/MultiClusterHelixBrokerStarter.java	0.00%	3 Missing ⚠️
...troller/api/resources/PinotDdlRestletResource.java	57.14%	3 Missing ⚠️
...ources/PinotInstanceAssignmentRestletResource.java	25.00%	3 Missing ⚠️
...roller/api/resources/PinotTaskRestletResource.java	0.00%	3 Missing ⚠️
...ntroller/helix/core/PinotHelixResourceManager.java	80.00%	3 Missing ⚠️
...lix/core/minion/PinotHelixTaskResourceManager.java	57.14%	3 Missing ⚠️
...pache/pinot/core/auth/BasicAuthPrincipalUtils.java	25.00%	3 Missing ⚠️
... and 74 more

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #18837   +/-   ##
=========================================
  Coverage     64.76%   64.76%           
  Complexity     1322     1322           
=========================================
  Files          3393     3393           
  Lines        211022   211022           
  Branches      33135    33135           
=========================================
  Hits         136676   136676           
+ Misses        63330    63326    -4     
- Partials      11016    11020    +4     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-21	64.76% <59.23%> (ø)
temurin	64.76% <59.23%> (ø)
unittests	64.76% <59.23%> (ø)
unittests1	56.97% <55.08%> (-0.01%)	⬇️
unittests2	37.18% <33.75%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Jackie-Jiang]

Suggest not banning this completely. It make sense to use it when element is null. As long as we remove most usage of it, AI should be able to use List.of() when applicable.

We can also replace usages of Collections.singleton() and Collections.singletonMap().
We can remove usage of Collections.emptyList()/Set()/Map()

[xiangfu0]

Addressed. I removed the hard Checkstyle ban, replaced the remaining Collections.singleton(), Collections.emptyList(), Collections.emptySet(), and Collections.emptyMap() usages with Set.of(), List.of(), and Map.of() where applicable, and kept null-preserving cases explicit instead of banning the old APIs outright.

[xiangfu0]

Follow-up update: kept the Checkstyle guard targeted to Collections.emptyList and Collections.emptyMap (including method references), while leaving Collections.singletonList/Collections.singletonMap unblocked so null-preserving test/compat cases remain possible. Agent docs and C7.12 now call out that distinction explicitly.

[xiangfu0]

Also updated the agent docs/C7.12 to make the singleton exception stricter: Collections.singleton* should only be used when an element/key/value argument is intentionally null, since List.of(null), Set.of(null), and Map.of(...) with null keys or values throw NullPointerException.

[xiangfu0]

Follow-up: added Collections.emptySet to the same Checkstyle guard and agent/C7.12 docs, including method references. Empty list/set/map factories are now all blocked in favor of List.of()/Set.of()/Map.of(). Collections.singleton* remains allowed only for intentional null element/key/value cases because the modern factories reject nulls.