diff --git a/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckGuavaSample.java b/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckGuavaSample.java new file mode 100644 index 00000000000..81c1bd54542 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckGuavaSample.java @@ -0,0 +1,29 @@ +package checks; + +import com.google.common.collect.Maps; +import java.util.Map; + +class IdentityHashMapBoxedKeyCheckGuavaSample { + + void boxedKeys() { + Map booleans = Maps.newIdentityHashMap(); // Noncompliant {{Use a map that compares keys by value because IdentityHashMap compares keys by reference.}} +// ^^^^^^^^^^^^^^^^^^^^^^^^^ + Map bytes = Maps.newIdentityHashMap(); // Noncompliant + Map characters = Maps.newIdentityHashMap(); // Noncompliant + Map shorts = Maps.newIdentityHashMap(); // Noncompliant + Map integers = Maps.newIdentityHashMap(); // Noncompliant + Map longs = Maps.newIdentityHashMap(); // Noncompliant + Map floats = Maps.newIdentityHashMap(); // Noncompliant + Map doubles = Maps.newIdentityHashMap(); // Noncompliant + } + + void compliant() { + Map boxedValue = Maps.newIdentityHashMap(); + Map strings = Maps.newIdentityHashMap(); + Map objects = Maps.newIdentityHashMap(); + } + + void genericKey() { + Map generic = Maps.newIdentityHashMap(); + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckSample.java new file mode 100644 index 00000000000..3d70858f1e1 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/IdentityHashMapBoxedKeyCheckSample.java @@ -0,0 +1,35 @@ +package checks; + +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.Map; + +class IdentityHashMapBoxedKeyCheckSample { + + void boxedKeys(Map source) { + Map booleans = new IdentityHashMap<>(); // Noncompliant + Map bytes = new IdentityHashMap<>(); // Noncompliant + Map characters = new IdentityHashMap<>(); // Noncompliant + Map shorts = new IdentityHashMap<>(); // Noncompliant + Map integers = new IdentityHashMap<>(); // Noncompliant + Map longs = new IdentityHashMap<>(); // Noncompliant + Map floats = new IdentityHashMap<>(); // Noncompliant + Map doubles = new IdentityHashMap<>(); // Noncompliant + Map explicit = new IdentityHashMap(); // Noncompliant + Map copied = new IdentityHashMap<>(source); // Noncompliant + var inferred = new IdentityHashMap(); // Noncompliant {{Use a map that compares keys by value because IdentityHashMap compares keys by reference.}} +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + } + + void compliant() { + Map boxedValue = new IdentityHashMap<>(); + Map strings = new IdentityHashMap<>(); + Map objects = new IdentityHashMap<>(); + Map hashMap = new HashMap<>(); + IdentityHashMap raw = new IdentityHashMap(); + } + + void genericKey() { + Map generic = new IdentityHashMap<>(); + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheck.java new file mode 100644 index 00000000000..260b6788c24 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheck.java @@ -0,0 +1,72 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.List; +import java.util.Set; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9401") +public class IdentityHashMapBoxedKeyCheck extends IssuableSubscriptionVisitor { + + private static final String IDENTITY_HASH_MAP = "java.util.IdentityHashMap"; + private static final String MESSAGE = "Use a map that compares keys by value because IdentityHashMap compares keys by reference."; + private static final Set BOXED_TYPES = Set.of( + "java.lang.Boolean", + "java.lang.Byte", + "java.lang.Character", + "java.lang.Short", + "java.lang.Integer", + "java.lang.Long", + "java.lang.Float", + "java.lang.Double"); + private static final MethodMatchers GUAVA_IDENTITY_HASH_MAP_FACTORY = MethodMatchers.create() + .ofTypes("com.google.common.collect.Maps") + .names("newIdentityHashMap") + .addWithoutParametersMatcher() + .build(); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.NEW_CLASS, Tree.Kind.METHOD_INVOCATION); + } + + @Override + public void visitNode(Tree tree) { + ExpressionTree expression = (ExpressionTree) tree; + if (isIdentityHashMapCreation(tree, expression.symbolType()) && hasBoxedKey(expression.symbolType())) { + reportIssue(tree, MESSAGE); + } + } + + private static boolean isIdentityHashMapCreation(Tree tree, Type resultType) { + return tree.is(Tree.Kind.NEW_CLASS) + ? resultType.is(IDENTITY_HASH_MAP) + : GUAVA_IDENTITY_HASH_MAP_FACTORY.matches((MethodInvocationTree) tree); + } + + private static boolean hasBoxedKey(Type resultType) { + List typeArguments = resultType.typeArguments(); + return typeArguments.size() == 2 && BOXED_TYPES.contains(typeArguments.get(0).fullyQualifiedName()); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheckTest.java new file mode 100644 index 00000000000..11127811070 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/IdentityHashMapBoxedKeyCheckTest.java @@ -0,0 +1,59 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class IdentityHashMapBoxedKeyCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/IdentityHashMapBoxedKeyCheckSample.java")) + .withCheck(new IdentityHashMapBoxedKeyCheck()) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/IdentityHashMapBoxedKeyCheckSample.java")) + .withCheck(new IdentityHashMapBoxedKeyCheck()) + .withoutSemantic() + .verifyIssues(); + } + + @Test + void test_guava() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/IdentityHashMapBoxedKeyCheckGuavaSample.java")) + .withCheck(new IdentityHashMapBoxedKeyCheck()) + .verifyIssues(); + } + + @Test + void test_guava_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/IdentityHashMapBoxedKeyCheckGuavaSample.java")) + .withCheck(new IdentityHashMapBoxedKeyCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.html new file mode 100644 index 00000000000..53383f5df18 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.html @@ -0,0 +1,34 @@ +

This rule raises an issue when an IdentityHashMap is created with a boxed primitive type as its key type.

+

Why is this an issue?

+

IdentityHashMap compares keys by reference instead of calling equals(). Boxed primitive types represent values as +objects, but boxing the same primitive value more than once does not always return the same object. For example, some Integer values are +cached while others may be represented by distinct instances.

+

Consequently, an IdentityHashMap with boxed primitive keys can appear to work for some values and fail for others. A lookup can return +null even when the map contains a key with the same primitive value.

+

How to fix it

+

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.

+

Code examples

+

Noncompliant code example

+
+Map<Integer, UserSession> sessions = new IdentityHashMap<>(); // Noncompliant
+
+sessions.put(500, session);
+UserSession retrieved = sessions.get(500); // May return null
+
+

Compliant solution

+
+Map<Integer, UserSession> sessions = new HashMap<>();
+
+sessions.put(500, session);
+UserSession retrieved = sessions.get(500);
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.json new file mode 100644 index 00000000000..cc5dd5b4ff0 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9401.json @@ -0,0 +1,23 @@ +{ + "title": "IdentityHashMap keys should not be boxed primitive types", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "suspicious" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9401", + "sqKey": "S9401", + "scope": "All", + "quickfix": "infeasible", + "code": { + "impacts": { + "RELIABILITY": "MEDIUM" + }, + "attribute": "LOGICAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9401 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9401 new file mode 100644 index 00000000000..e69de29bb2d