Skip to content

SONARJAVA-6777: Implement S9401: IdentityHashMap keys should not be boxed primitive types - #6152

Open
nathsou wants to merge 3 commits into
masterfrom
new-rule/S9401
Open

nathsou wants to merge 3 commits into
masterfrom
new-rule/S9401

Conversation

@nathsou

@nathsou nathsou commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implement S9401 for IdentityHashMap constructors and Guava Maps.newIdentityHashMap().
  • Detect all eight boxed primitive wrapper key types.
  • Add focused semantic and no-bytecode tests.
  • Generate rule metadata and activate the rule in Sonar way.

Links

AI disclosure

  • LLM model used for implementation: GPT-5.6 Sol Low

@nathsou nathsou self-assigned this Sep 15, 2026
@hashicorp-vault-sonar-prod

Copy link
Copy Markdown
Contributor

SONARJAVA-6777

Comment thread java-checks/src/main/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheck.java Outdated
class IdentityHashMapBoxedKeyCheckGuavaSample {

void boxedKeys() {
Map<Boolean, String> booleans = Maps.newIdentityHashMap(); // Noncompliant

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should show the precise location where the issue is raised, and the issue message on at least one instance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 3de3f09. The first Guava case now asserts the complete issue message and underlines the precise Maps.newIdentityHashMap() invocation. The focused IdentityHashMapBoxedKeyCheckTest suite passes.

<code>null</code> even when the map contains a key with the same primitive value.</p>
<p>Use a map implementation that compares keys by value when the key represents a primitive value. If reference equality is required, use key objects
whose identity is deliberate and stable.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be an "How to fix" and compliant non-compliant subsection. Did you use RIS 😜

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My skill explicitly asks to use RIS but after looking at the substeps the agent took, I had a recently expired token so it decided to write the RSPEC manually. This should be fixed in newer PRs of mine.

@gitar-bot

gitar-bot Bot commented Sep 15, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Implements S9401 to detect boxed primitive types used as keys in IdentityHashMap and Guava's Maps.newIdentityHashMap(), covering all eight wrapper types with focused semantic and no-bytecode tests. Resolved Guava classpath duplication by aligning test setup with existing patterns and corrected issue messaging to align with the rule description.

✅ 2 resolved
Quality: test_guava puts two Guava versions on analysis classpath

📄 java-checks/src/test/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheckTest.java:19-22 📄 java-checks/src/test/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheckTest.java:50-58
java-checks-test-sources/default already declares Guava (32.1.1-jre, provided scope), so TestClasspathUtils.DEFAULT_MODULE.getClassPath() alone resolves com.google.common.collect.Maps — this is how the existing Guava sample test works (BigDecimalEqualsCheckTest.test_guava uses no custom classpath). Appending Maps.class's code source here adds the java-checks test-scoped Guava (33.7.1-jre per the root pom dependencyManagement) on top of 32.1.1-jre, so ECJ analyses the sample against two Guava jars and silently resolves whichever comes first; the test only passes because both versions still declare newIdentityHashMap(). Drop the manual classpath (and the Guava import/URISyntaxException) to match the established pattern.

Quality: Issue message advises a fix the rule description does not recommend

📄 java-checks/src/main/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheck.java:33 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.html:7-19
The reported message tells the developer to "Use a non-boxed key type", but the rule description's remediation and its compliant example fix the defect by changing the map implementation (new HashMap<>() with the same Integer key). Since map keys can never be primitives, a user acting on the message alone is pointed away from the documented fix; align the message with the description, e.g. mention using a map that compares keys by value (the sample's {{...}} assertion on line 20 of IdentityHashMapBoxedKeyCheckSample.java must be updated too).

Review coverage

Functional validation 1 of 1 objectives covered

Rules No rules evaluated

Auto-approval Not enabled · Set up

Implementation Status ✅ 1 of 1 objectives covered
SONARJAVA-6777 - 1 of 1 objectives covered

This PR implements the S9401 rule checking that IdentityHashMap keys are not boxed primitive types.

✅ 1 covered here
  • ✅ Implement S9401 rule to check that IdentityHashMap keys are not boxed primitive types
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants