From 4759d18153e17415291849d788649bf822f2d303 Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 11:46:42 +0200 Subject: [PATCH 1/9] Implement S9350 --- .../EqualsMismatchedMembersCheckSample.java | 336 ++++++++++++++++++ .../checks/EqualsMismatchedMembersCheck.java | 236 ++++++++++++ .../EqualsMismatchedMembersCheckTest.java | 33 ++ .../org/sonar/l10n/java/rules/java/S9350.html | 200 +++++++++++ .../org/sonar/l10n/java/rules/java/S9350.json | 23 ++ .../main/resources/profiles/Sonar_way/S9350 | 0 6 files changed, 828 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9350 diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java new file mode 100644 index 00000000000..44f919e05e1 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -0,0 +1,336 @@ +package checks; + +import java.util.Arrays; + +class EqualsMismatchedMembersCheckSample { + + static class User { + private String firstName; + private String lastName; + + @Override + public boolean equals(Object obj) { + if (obj instanceof User other) { + return java.util.Objects.equals(this.firstName, other.firstName) + && java.util.Objects.equals(this.lastName, other.firstName); // Noncompliant {{This equals() implementation compares mismatched members; pairing "lastName" with "firstName" breaks the equality contract.}} + } + return false; + } + } + + static class CompliantUser { + private String firstName; + private String lastName; + + @Override + public boolean equals(Object obj) { + if (obj instanceof CompliantUser other) { + return java.util.Objects.equals(this.firstName, other.firstName) + && java.util.Objects.equals(this.lastName, other.lastName); + } + return false; + } + } + + static class Point { + private int x; + private int y; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Point that)) { + return false; + } + return x == that.x && y == that.x; // Noncompliant {{This equals() implementation compares mismatched members; pairing "y" with "x" breaks the equality contract.}} + } + } + + static class CompliantPoint { + private int x; + private int y; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CompliantPoint that)) { + return false; + } + return x == that.x && y == that.y; + } + } + + static class NotEqual { + private int x; + private int y; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NotEqual that)) { + return false; + } + return x != that.y; // Noncompliant {{This equals() implementation compares mismatched members; pairing "x" with "y" breaks the equality contract.}} + } + } + + static class Box { + private Object key; + private Object value; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Box that)) { + return false; + } + return key.equals(that.key) && value.equals(that.key); // Noncompliant {{This equals() implementation compares mismatched members; pairing "value" with "key" breaks the equality contract.}} + } + } + + static class CompliantBox { + private Object key; + private Object value; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CompliantBox that)) { + return false; + } + return key.equals(that.key) && value.equals(that.value); + } + } + + static class Person { + private String firstName; + private String lastName; + + String getFirstName() { + return firstName; + } + + String getLastName() { + return lastName; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Person that)) { + return false; + } + return getFirstName().equals(that.getFirstName()) + && getLastName().equals(that.getFirstName()); // Noncompliant {{This equals() implementation compares mismatched members; pairing "getLastName()" with "getFirstName()" breaks the equality contract.}} + } + } + + static class CompliantPerson { + private String firstName; + private String lastName; + + String getFirstName() { + return firstName; + } + + String getLastName() { + return lastName; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CompliantPerson that)) { + return false; + } + return getFirstName().equals(that.getFirstName()) + && getLastName().equals(that.getLastName()); + } + } + + static class PrimitiveGetters { + private int a; + private int b; + + int getA() { + return a; + } + + int getB() { + return b; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof PrimitiveGetters that)) { + return false; + } + return getA() == that.getB() && getB() == that.getB(); // Noncompliant {{This equals() implementation compares mismatched members; pairing "getA()" with "getB()" breaks the equality contract.}} + } + } + + record NamedPoint(int x, int y) { + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NamedPoint that)) { + return false; + } + return this.x() == that.y(); // Noncompliant {{This equals() implementation compares mismatched members; pairing "x()" with "y()" breaks the equality contract.}} + } + } + + static class ArraysHolder { + private int[] left; + private int[] right; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ArraysHolder that)) { + return false; + } + return Arrays.equals(this.left, that.right); // Noncompliant {{This equals() implementation compares mismatched members; pairing "left" with "right" breaks the equality contract.}} + } + } + + static class ArraysRangeHolder { + private int[] left; + private int[] right; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ArraysRangeHolder that)) { + return false; + } + return Arrays.equals(this.left, 0, 1, that.right, 0, 1); // Noncompliant {{This equals() implementation compares mismatched members; pairing "left" with "right" breaks the equality contract.}} + } + } + + static class GuavaHolder { + private Object a; + private Object b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof GuavaHolder that)) { + return false; + } + return com.google.common.base.Objects.equal(a, that.b) && com.google.common.base.Objects.equal(b, that.b); // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} + } + } + + static class MultipleMismatches { + private int a; + private int b; + private int c; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof MultipleMismatches that)) { + return false; + } + return a == that.b // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} + && b == that.c; // Noncompliant {{This equals() implementation compares mismatched members; pairing "b" with "c" breaks the equality contract.}} + } + } + + static class UnorderedPair { + private int a; + private int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof UnorderedPair that)) { + return false; + } + return (a == that.a && b == that.b) || (a == that.b && b == that.a); + } + } + + static class MixedFieldAndGetter { + private String name; + + String getName() { + return name; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof MixedFieldAndGetter that)) { + return false; + } + return this.name.equals(that.getName()); + } + } + + static class Parent { + int inherited; + } + + static class Child extends Parent { + int own; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Child that)) { + return false; + } + return this.inherited == that.own; + } + } + + static class Statics { + static int DEFAULT; + int value; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Statics that)) { + return false; + } + return this.value == DEFAULT && this.value == that.value; + } + } + + static class SuperCall { + int a; + int b; + + @Override + public boolean equals(Object obj) { + return super.equals(obj); + } + } + + static class NotEqualsMethod { + int a; + int b; + + public boolean equals(NotEqualsMethod other) { + return a == other.b; + } + + public int compareTo(Object other) { + return a == ((NotEqualsMethod) other).b ? 0 : 1; + } + } + + static class Locals { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Locals that)) { + return false; + } + int temp = that.a; + return this.a == temp && this.b == that.b; + } + } + + static class Parentheses { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Parentheses that)) { + return false; + } + return (a) == (that.b); // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java new file mode 100644 index 00000000000..5659c259733 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -0,0 +1,236 @@ +/* + * 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.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.MethodTreeUtils; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.BinaryExpressionTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9350") +public class EqualsMismatchedMembersCheck extends IssuableSubscriptionVisitor { + + private static final String ISSUE_MESSAGE = + "This equals() implementation compares mismatched members; pairing \"%s\" with \"%s\" breaks the equality contract."; + private static final String SECONDARY_THIS = "Compared member on this"; + private static final String SECONDARY_OTHER = "Compared member on the other instance"; + private static final String JAVA_LANG_OBJECT = "java.lang.Object"; + + private static final MethodMatchers OBJECTS_EQUALS = MethodMatchers.create() + .ofTypes("java.util.Objects") + .names("equals") + .addParametersMatcher(JAVA_LANG_OBJECT, JAVA_LANG_OBJECT) + .build(); + + private static final MethodMatchers GUAVA_OBJECTS_EQUAL = MethodMatchers.create() + .ofTypes("com.google.common.base.Objects") + .names("equal") + .withAnyParameters() + .build(); + + private static final MethodMatchers ARRAYS_EQUALS = MethodMatchers.create() + .ofTypes("java.util.Arrays") + .names("equals") + .withAnyParameters() + .build(); + + private static final MethodMatchers INSTANCE_EQUALS = MethodMatchers.create() + .ofAnyType() + .names("equals") + .addParametersMatcher(JAVA_LANG_OBJECT) + .build(); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.METHOD); + } + + @Override + public void visitNode(Tree tree) { + MethodTree methodTree = (MethodTree) tree; + if (!MethodTreeUtils.isEqualsMethod(methodTree) || methodTree.block() == null) { + return; + } + Symbol owner = methodTree.symbol().owner(); + if (owner == null || !owner.isTypeSymbol() || owner.isUnknown() || owner.type().isUnknown()) { + return; + } + ComparisonCollector collector = new ComparisonCollector(owner); + methodTree.block().accept(collector); + Set suspiciousPairs = collector.comparisons.stream() + .map(ComparisonSite::pair) + .collect(Collectors.toSet()); + for (ComparisonSite comparison : collector.comparisons) { + if (suspiciousPairs.contains(comparison.pair().reversed())) { + continue; + } + reportIssue( + comparison.tree, + String.format(ISSUE_MESSAGE, comparison.lhs.displayName, comparison.rhs.displayName), + List.of( + new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.lhs.tree), + new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.rhs.tree)), + null); + } + } + + private static final class ComparisonCollector extends BaseTreeVisitor { + private final Symbol enclosingClass; + private final List comparisons = new ArrayList<>(); + + private ComparisonCollector(Symbol enclosingClass) { + this.enclosingClass = enclosingClass; + } + + @Override + public void visitClass(ClassTree tree) { + // Nested types are visited independently through METHOD subscription. + } + + @Override + public void visitBinaryExpression(BinaryExpressionTree tree) { + if (tree.is(Tree.Kind.EQUAL_TO, Tree.Kind.NOT_EQUAL_TO)) { + addIfDubious(tree, tree.leftOperand(), tree.rightOperand()); + } + super.visitBinaryExpression(tree); + } + + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + List arguments = tree.arguments(); + if (isTwoArgEqualityHelper(tree)) { + addIfDubious(tree, arguments.get(0), arguments.get(1)); + } else if (ARRAYS_EQUALS.matches(tree) && arguments.size() >= 2) { + addIfDubious(tree, arguments.get(0), arguments.get(arguments.size() / 2)); + } else if (INSTANCE_EQUALS.matches(tree) && arguments.size() == 1) { + ExpressionTree receiver = receiver(tree); + if (receiver != null && !isSuper(receiver)) { + addIfDubious(tree, receiver, arguments.get(0)); + } + } + super.visitMethodInvocation(tree); + } + + private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTree rhs) { + Optional left = member(lhs); + Optional right = member(rhs); + if (left.isEmpty() || right.isEmpty()) { + return; + } + MemberRef leftMember = left.get(); + MemberRef rightMember = right.get(); + if (leftMember.kind != rightMember.kind || leftMember.symbol.equals(rightMember.symbol)) { + return; + } + comparisons.add(new ComparisonSite(comparisonTree, leftMember, rightMember)); + } + + private Optional member(ExpressionTree expression) { + ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + if (expr instanceof IdentifierTree identifierTree) { + return field(identifierTree.symbol(), expr); + } + if (expr instanceof MemberSelectExpressionTree memberSelect) { + return field(memberSelect.identifier().symbol(), expr); + } + if (expr instanceof MethodInvocationTree invocation && invocation.arguments().isEmpty()) { + return getter(invocation); + } + return Optional.empty(); + } + + private Optional field(Symbol symbol, ExpressionTree tree) { + if (symbol.isUnknown() || !symbol.isVariableSymbol() || symbol.isStatic() || !enclosingClass.equals(symbol.owner())) { + return Optional.empty(); + } + return Optional.of(new MemberRef(symbol, MemberKind.FIELD, symbol.name(), tree)); + } + + private Optional getter(MethodInvocationTree invocation) { + Symbol.MethodSymbol method = invocation.methodSymbol(); + if (method.isUnknown() || method.isStatic() || !enclosingClass.equals(method.owner())) { + return Optional.empty(); + } + Symbol.TypeSymbol returnType = method.returnType(); + if (returnType == null || returnType.isUnknown() || returnType.type().isVoid() || !method.parameterTypes().isEmpty()) { + return Optional.empty(); + } + return Optional.of(new MemberRef(method, MemberKind.METHOD, method.name() + "()", invocation)); + } + + private static boolean isTwoArgEqualityHelper(MethodInvocationTree tree) { + if (tree.arguments().size() != 2) { + return false; + } + if (OBJECTS_EQUALS.matches(tree) || GUAVA_OBJECTS_EQUAL.matches(tree)) { + return true; + } + Symbol.MethodSymbol method = tree.methodSymbol(); + return (method.isUnknown() || method.isStatic()) + && "equal".equals(ExpressionUtils.methodName(tree).name()); + } + + private static ExpressionTree receiver(MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect) { + return memberSelect.expression(); + } + return null; + } + + private static boolean isSuper(ExpressionTree expression) { + ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + return expr instanceof IdentifierTree identifierTree && "super".equals(identifierTree.name()); + } + } + + private enum MemberKind { + FIELD, + METHOD + } + + private record MemberRef(Symbol symbol, MemberKind kind, String displayName, ExpressionTree tree) { + } + + private record ComparisonSite(Tree tree, MemberRef lhs, MemberRef rhs) { + private MemberPair pair() { + return new MemberPair(lhs.symbol, rhs.symbol); + } + } + + private record MemberPair(Symbol lhs, Symbol rhs) { + private MemberPair reversed() { + return new MemberPair(rhs, lhs); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java new file mode 100644 index 00000000000..6c66e14530b --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java @@ -0,0 +1,33 @@ +/* + * 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 EqualsMismatchedMembersCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/EqualsMismatchedMembersCheckSample.java")) + .withCheck(new EqualsMismatchedMembersCheck()) + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html new file mode 100644 index 00000000000..848dd6c2a28 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html @@ -0,0 +1,200 @@ +

Comparing a field or getter of this with a different field or getter of the other instance inside equals is almost always +a copy-paste error. Replace the mismatched member with the corresponding one on the other instance.

+

Why is this an issue?

+

An equals implementation usually compares each piece of state on this with the same piece of state on the other object. +When the two operands name different members of the enclosing type, objects that represent the same value can compare as unequal, and objects that +differ can compare as equal. That defect is easy to miss in review because the surrounding method still looks like a normal field-by-field +comparison.

+

This rule raises an issue when equals compares two different instance fields, or two different instance getters, of the enclosing +type. The comparison may use operators such as == or helpers such as Objects.equals.

+

Code examples

+

Noncompliant code example

+
+class User {
+  private String firstName;
+  private String lastName;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof User other) {
+      return Objects.equals(this.firstName, other.firstName)
+        && Objects.equals(this.lastName, other.firstName); // Noncompliant: compares "lastName" with "firstName"
+    }
+    return false;
+  }
+}
+
+

Compliant solution

+
+class User {
+  private String firstName;
+  private String lastName;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof User other) {
+      return Objects.equals(this.firstName, other.firstName)
+        && Objects.equals(this.lastName, other.lastName);
+    }
+    return false;
+  }
+}
+
+

Noncompliant code example

+
+class Point {
+  private int x;
+  private int y;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Point that)) {
+      return false;
+    }
+    return x == that.x && y == that.x; // Noncompliant: compares "y" with "x"
+  }
+}
+
+

Compliant solution

+
+class Point {
+  private int x;
+  private int y;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Point that)) {
+      return false;
+    }
+    return x == that.x && y == that.y;
+  }
+}
+
+

Noncompliant code example

+
+class Box {
+  private Object key;
+  private Object value;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Box that)) {
+      return false;
+    }
+    return key.equals(that.key) && value.equals(that.key); // Noncompliant: compares "value" with "key"
+  }
+}
+
+

Compliant solution

+
+class Box {
+  private Object key;
+  private Object value;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Box that)) {
+      return false;
+    }
+    return key.equals(that.key) && value.equals(that.value);
+  }
+}
+
+

Noncompliant code example

+
+class Person {
+  private String firstName;
+  private String lastName;
+
+  String getFirstName() {
+    return firstName;
+  }
+
+  String getLastName() {
+    return lastName;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Person that)) {
+      return false;
+    }
+    return getFirstName().equals(that.getFirstName())
+      && getLastName().equals(that.getFirstName()); // Noncompliant: compares "getLastName()" with "getFirstName()"
+  }
+}
+
+

Compliant solution

+
+class Person {
+  private String firstName;
+  private String lastName;
+
+  String getFirstName() {
+    return firstName;
+  }
+
+  String getLastName() {
+    return lastName;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Person that)) {
+      return false;
+    }
+    return getFirstName().equals(that.getFirstName())
+      && getLastName().equals(that.getLastName());
+  }
+}
+
+

Exceptions

+

The rule does not raise an issue when both swapped pairings appear, because that pattern can express order-independent equality:

+
+class Pair {
+  private int a;
+  private int b;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Pair that)) {
+      return false;
+    }
+    return (a == that.a && b == that.b) || (a == that.b && b == that.a);
+  }
+}
+
+

The rule also does not raise an issue when a field is compared with a getter, because the two operands are different kinds of members:

+
+class Named {
+  private String name;
+
+  String getName() {
+    return name;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof Named that)) {
+      return false;
+    }
+    return this.name.equals(that.getName());
+  }
+}
+
+

Resources

+

Documentation

+ +

Related rules

+
    +
  • {rule:java:S2162} - "equals" methods should be symmetric and work for subclasses
    • +
  • {rule:java:S1206} - "equals(Object)" and "hashCode()" should be overridden in pairs
    • +
  • {rule:java:S2160} - Subclasses that add fields to classes that override "equals" should also override "equals"
    • +
+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.json new file mode 100644 index 00000000000..32959cd6c0e --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.json @@ -0,0 +1,23 @@ +{ + "title": "equals() implementations should not compare mismatched members", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "suspicious" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9350", + "sqKey": "S9350", + "scope": "Main", + "quickfix": "targeted", + "code": { + "impacts": { + "RELIABILITY": "MEDIUM" + }, + "attribute": "LOGICAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9350 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9350 new file mode 100644 index 00000000000..e69de29bb2d From f3f978a6f2a996526e9bc43c2f2988b711123be9 Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 11:59:55 +0200 Subject: [PATCH 2/9] Require this-versus-other receivers in S9350 --- .../EqualsMismatchedMembersCheckSample.java | 72 ++++++++++++ .../checks/EqualsMismatchedMembersCheck.java | 105 ++++++++++++++---- 2 files changed, 155 insertions(+), 22 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java index 44f919e05e1..dca0efb0c62 100644 --- a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -333,4 +333,76 @@ public boolean equals(Object obj) { return (a) == (that.b); // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} } } + + static class SameObjectFields { + int start; + int end; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof SameObjectFields that)) { + return false; + } + if (this.start == this.end && that.start == that.end) { + return true; + } + return this.start == that.start && this.end == that.end; + } + } + + static class OtherFirst { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof OtherFirst that)) { + return false; + } + return that.a == this.b; // Noncompliant {{This equals() implementation compares mismatched members; pairing "b" with "a" breaks the equality contract.}} + } + } + + static class Holder { + private T left; + private T right; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Holder that)) { + return false; + } + return java.util.Objects.equals(this.left, that.right); // Noncompliant {{This equals() implementation compares mismatched members; pairing "left" with "right" breaks the equality contract.}} + } + } + + static class StaticSingleton { + static final StaticSingleton EMPTY = new StaticSingleton(); + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof StaticSingleton that)) { + return false; + } + return this.a == EMPTY.b && this.a == that.a && this.b == that.b; + } + } + + static class DistinctStatements { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof DistinctStatements that)) { + return false; + } + if (a == that.b) { // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} + return true; + } + return b == that.a; // Noncompliant {{This equals() implementation compares mismatched members; pairing "b" with "a" breaks the equality contract.}} + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 5659c259733..2f62920bf6d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -17,10 +17,12 @@ package org.sonar.java.checks; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.MethodTreeUtils; import org.sonar.java.model.ExpressionUtils; @@ -36,6 +38,7 @@ import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.StatementTree; import org.sonar.plugins.java.api.tree.Tree; @Rule(key = "S9350") @@ -88,19 +91,20 @@ public void visitNode(Tree tree) { } ComparisonCollector collector = new ComparisonCollector(owner); methodTree.block().accept(collector); - Set suspiciousPairs = collector.comparisons.stream() - .map(ComparisonSite::pair) - .collect(Collectors.toSet()); + Map> pairsByStatement = new HashMap<>(); for (ComparisonSite comparison : collector.comparisons) { - if (suspiciousPairs.contains(comparison.pair().reversed())) { + pairsByStatement.computeIfAbsent(comparison.statement, key -> new HashSet<>()).add(comparison.pair()); + } + for (ComparisonSite comparison : collector.comparisons) { + if (pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { continue; } reportIssue( comparison.tree, - String.format(ISSUE_MESSAGE, comparison.lhs.displayName, comparison.rhs.displayName), + String.format(ISSUE_MESSAGE, comparison.thisMember.displayName, comparison.otherMember.displayName), List.of( - new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.lhs.tree), - new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.rhs.tree)), + new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.thisMember.tree), + new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.otherMember.tree)), null); } } @@ -150,43 +154,92 @@ private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTre } MemberRef leftMember = left.get(); MemberRef rightMember = right.get(); - if (leftMember.kind != rightMember.kind || leftMember.symbol.equals(rightMember.symbol)) { + if (leftMember.kind != rightMember.kind || leftMember.symbol.equals(rightMember.symbol) || leftMember.onThis == rightMember.onThis) { return; } - comparisons.add(new ComparisonSite(comparisonTree, leftMember, rightMember)); + MemberRef thisMember = leftMember.onThis ? leftMember : rightMember; + MemberRef otherMember = leftMember.onThis ? rightMember : leftMember; + comparisons.add(new ComparisonSite(comparisonTree, enclosingStatement(comparisonTree), thisMember, otherMember)); } private Optional member(ExpressionTree expression) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + Optional onThis = receiverIsThis(expr); + if (onThis.isEmpty()) { + return Optional.empty(); + } if (expr instanceof IdentifierTree identifierTree) { - return field(identifierTree.symbol(), expr); + return field(identifierTree.symbol(), expr, onThis.get()); } if (expr instanceof MemberSelectExpressionTree memberSelect) { - return field(memberSelect.identifier().symbol(), expr); + return field(memberSelect.identifier().symbol(), expr, onThis.get()); } if (expr instanceof MethodInvocationTree invocation && invocation.arguments().isEmpty()) { - return getter(invocation); + return getter(invocation, onThis.get()); } return Optional.empty(); } - private Optional field(Symbol symbol, ExpressionTree tree) { - if (symbol.isUnknown() || !symbol.isVariableSymbol() || symbol.isStatic() || !enclosingClass.equals(symbol.owner())) { + private Optional field(Symbol symbol, ExpressionTree tree, boolean onThis) { + if (symbol.isUnknown() || !symbol.isVariableSymbol() || symbol.isStatic() || !ownedByEnclosing(symbol)) { return Optional.empty(); } - return Optional.of(new MemberRef(symbol, MemberKind.FIELD, symbol.name(), tree)); + return Optional.of(new MemberRef(symbol, MemberKind.FIELD, symbol.name(), tree, onThis)); } - private Optional getter(MethodInvocationTree invocation) { + private Optional getter(MethodInvocationTree invocation, boolean onThis) { Symbol.MethodSymbol method = invocation.methodSymbol(); - if (method.isUnknown() || method.isStatic() || !enclosingClass.equals(method.owner())) { + if (method.isUnknown() || method.isStatic() || !ownedByEnclosing(method)) { return Optional.empty(); } Symbol.TypeSymbol returnType = method.returnType(); if (returnType == null || returnType.isUnknown() || returnType.type().isVoid() || !method.parameterTypes().isEmpty()) { return Optional.empty(); } - return Optional.of(new MemberRef(method, MemberKind.METHOD, method.name() + "()", invocation)); + return Optional.of(new MemberRef(method, MemberKind.METHOD, method.name() + "()", invocation, onThis)); + } + + private boolean ownedByEnclosing(Symbol symbol) { + Symbol owner = symbol.owner(); + return owner != null && !owner.isUnknown() && owner.isTypeSymbol() + && enclosingClass.type().erasure().equals(owner.type().erasure()); + } + + private static Optional receiverIsThis(ExpressionTree access) { + if (access instanceof IdentifierTree) { + return Optional.of(true); + } + if (access instanceof MemberSelectExpressionTree memberSelect) { + return classifyReceiver(memberSelect.expression()); + } + if (access instanceof MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof IdentifierTree) { + return Optional.of(true); + } + if (invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect) { + return classifyReceiver(memberSelect.expression()); + } + } + return Optional.empty(); + } + + private static Optional classifyReceiver(ExpressionTree receiverExpr) { + ExpressionTree expr = ExpressionUtils.skipParentheses(receiverExpr); + if (!(expr instanceof IdentifierTree identifierTree)) { + return Optional.empty(); + } + if ("this".equals(identifierTree.name())) { + return Optional.of(true); + } + Symbol symbol = identifierTree.symbol(); + if (symbol.isUnknown() || !symbol.isVariableSymbol() || symbol.isStatic()) { + return Optional.empty(); + } + Symbol owner = symbol.owner(); + if (owner == null || !owner.isMethodSymbol()) { + return Optional.empty(); + } + return Optional.of(false); } private static boolean isTwoArgEqualityHelper(MethodInvocationTree tree) { @@ -212,6 +265,14 @@ private static boolean isSuper(ExpressionTree expression) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); return expr instanceof IdentifierTree identifierTree && "super".equals(identifierTree.name()); } + + private static Tree enclosingStatement(Tree tree) { + Tree current = tree; + while (current != null && !(current instanceof StatementTree)) { + current = current.parent(); + } + return current != null ? current : tree; + } } private enum MemberKind { @@ -219,12 +280,12 @@ private enum MemberKind { METHOD } - private record MemberRef(Symbol symbol, MemberKind kind, String displayName, ExpressionTree tree) { + private record MemberRef(Symbol symbol, MemberKind kind, String displayName, ExpressionTree tree, boolean onThis) { } - private record ComparisonSite(Tree tree, MemberRef lhs, MemberRef rhs) { + private record ComparisonSite(Tree tree, Tree statement, MemberRef thisMember, MemberRef otherMember) { private MemberPair pair() { - return new MemberPair(lhs.symbol, rhs.symbol); + return new MemberPair(thisMember.symbol, otherMember.symbol); } } From 5f430726574615d78c02a011877b672c658ab427 Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 12:17:41 +0200 Subject: [PATCH 3/9] Identify S9350 members by name, not symbol equality Ruling flagged this.foo.equals(that.foo) on generic types because JSymbol equality includes type arguments, so the same field looked mismatched. --- .../EqualsMismatchedMembersCheckSample.java | 28 +++++++++++++++++++ .../checks/EqualsMismatchedMembersCheck.java | 10 ++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java index dca0efb0c62..b17fe9b5bd6 100644 --- a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -376,6 +376,34 @@ public boolean equals(Object obj) { } } + static class GenericSameMember { + private T left; + private T right; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof GenericSameMember that)) { + return false; + } + return java.util.Objects.equals(this.left, that.left) + && java.util.Objects.equals(this.right, that.right); + } + } + + static class GenericUnordered { + private T a; + private T b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof GenericUnordered that)) { + return false; + } + return (java.util.Objects.equals(a, that.a) && java.util.Objects.equals(b, that.b)) + || (java.util.Objects.equals(a, that.b) && java.util.Objects.equals(b, that.a)); + } + } + static class StaticSingleton { static final StaticSingleton EMPTY = new StaticSingleton(); int a; diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 2f62920bf6d..46b2e40bf1c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -154,7 +154,9 @@ private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTre } MemberRef leftMember = left.get(); MemberRef rightMember = right.get(); - if (leftMember.kind != rightMember.kind || leftMember.symbol.equals(rightMember.symbol) || leftMember.onThis == rightMember.onThis) { + if (leftMember.kind != rightMember.kind + || leftMember.displayName.equals(rightMember.displayName) + || leftMember.onThis == rightMember.onThis) { return; } MemberRef thisMember = leftMember.onThis ? leftMember : rightMember; @@ -285,13 +287,13 @@ private record MemberRef(Symbol symbol, MemberKind kind, String displayName, Exp private record ComparisonSite(Tree tree, Tree statement, MemberRef thisMember, MemberRef otherMember) { private MemberPair pair() { - return new MemberPair(thisMember.symbol, otherMember.symbol); + return new MemberPair(thisMember.displayName, otherMember.displayName); } } - private record MemberPair(Symbol lhs, Symbol rhs) { + private record MemberPair(String thisName, String otherName) { private MemberPair reversed() { - return new MemberPair(rhs, lhs); + return new MemberPair(otherName, thisName); } } } From c5408190fd242f449a0e618c5cc240cd49b8121b Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 12:39:36 +0200 Subject: [PATCH 4/9] Clear S9350 quality-gate findings and cover defensive branches --- .../EqualsMismatchedMembersCheckSample.java | 46 ++++++++++ .../EqualsMismatchedMembersCheckSample.java | 92 +++++++++++++++++++ .../checks/EqualsMismatchedMembersCheck.java | 24 ++--- .../EqualsMismatchedMembersCheckTest.java | 9 ++ 4 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/EqualsMismatchedMembersCheckSample.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/EqualsMismatchedMembersCheckSample.java new file mode 100644 index 00000000000..48b3d0ec919 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/EqualsMismatchedMembersCheckSample.java @@ -0,0 +1,46 @@ +package checks; + +class EqualsMismatchedMembersCheckSample { + + class UnknownMembers { + UnknownType a; + UnknownType b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof UnknownMembers that)) { + return false; + } + return a == that.b && Unknown.equal(a, that.a); // Noncompliant + } + } + + class UnknownReceiver { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof UnknownReceiver that)) { + return false; + } + return this.a == unknown.b && this.a == that.a; + } + } + + class VoidGetter { + int a; + int b; + + void getA() { + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof VoidGetter that)) { + return false; + } + return getA() == that.b; + } + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java index b17fe9b5bd6..e00f0ea8be7 100644 --- a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -433,4 +433,96 @@ public boolean equals(Object obj) { return b == that.a; // Noncompliant {{This equals() implementation compares mismatched members; pairing "b" with "a" breaks the equality contract.}} } } + + static class NestedTypeInEquals { + int a; + int b; + + @Override + public boolean equals(Object obj) { + class Local { + } + if (!(obj instanceof NestedTypeInEquals that)) { + return false; + } + return a == that.a && b == that.b; + } + } + + static class NonMemberOperand { + int a; + int b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NonMemberOperand that)) { + return false; + } + return a == (that.b + 0) && a == that.a; + } + } + + static class UnqualifiedEquals { + Object a; + Object b; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof UnqualifiedEquals that)) { + return false; + } + return equals(that.a) && a.equals(that.a); + } + } + + static class NestedReceiver { + int a; + NestedReceiver child; + + NestedReceiver child() { + return child; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NestedReceiver that)) { + return false; + } + return this.a == that.child().a && this.a == that.a; + } + } + + static class CustomEqualHelper { + Object a; + Object b; + + static boolean equal(Object x, Object y) { + return x == y; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CustomEqualHelper that)) { + return false; + } + return equal(a, that.b); // Noncompliant {{This equals() implementation compares mismatched members; pairing "a" with "b" breaks the equality contract.}} + } + } + + static class StaticGetter { + int a; + int b; + + static int getA() { + return 0; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof StaticGetter that)) { + return false; + } + return getA() == that.b && a == that.a; + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 46b2e40bf1c..80c26d267e7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -49,10 +49,11 @@ public class EqualsMismatchedMembersCheck extends IssuableSubscriptionVisitor { private static final String SECONDARY_THIS = "Compared member on this"; private static final String SECONDARY_OTHER = "Compared member on the other instance"; private static final String JAVA_LANG_OBJECT = "java.lang.Object"; + private static final String EQUALS_METHOD_NAME = "equals"; private static final MethodMatchers OBJECTS_EQUALS = MethodMatchers.create() .ofTypes("java.util.Objects") - .names("equals") + .names(EQUALS_METHOD_NAME) .addParametersMatcher(JAVA_LANG_OBJECT, JAVA_LANG_OBJECT) .build(); @@ -64,13 +65,13 @@ public class EqualsMismatchedMembersCheck extends IssuableSubscriptionVisitor { private static final MethodMatchers ARRAYS_EQUALS = MethodMatchers.create() .ofTypes("java.util.Arrays") - .names("equals") + .names(EQUALS_METHOD_NAME) .withAnyParameters() .build(); private static final MethodMatchers INSTANCE_EQUALS = MethodMatchers.create() .ofAnyType() - .names("equals") + .names(EQUALS_METHOD_NAME) .addParametersMatcher(JAVA_LANG_OBJECT) .build(); @@ -91,12 +92,8 @@ public void visitNode(Tree tree) { } ComparisonCollector collector = new ComparisonCollector(owner); methodTree.block().accept(collector); - Map> pairsByStatement = new HashMap<>(); for (ComparisonSite comparison : collector.comparisons) { - pairsByStatement.computeIfAbsent(comparison.statement, key -> new HashSet<>()).add(comparison.pair()); - } - for (ComparisonSite comparison : collector.comparisons) { - if (pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { + if (collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { continue; } reportIssue( @@ -112,6 +109,7 @@ public void visitNode(Tree tree) { private static final class ComparisonCollector extends BaseTreeVisitor { private final Symbol enclosingClass; private final List comparisons = new ArrayList<>(); + private final Map> pairsByStatement = new HashMap<>(); private ComparisonCollector(Symbol enclosingClass) { this.enclosingClass = enclosingClass; @@ -161,7 +159,9 @@ private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTre } MemberRef thisMember = leftMember.onThis ? leftMember : rightMember; MemberRef otherMember = leftMember.onThis ? rightMember : leftMember; - comparisons.add(new ComparisonSite(comparisonTree, enclosingStatement(comparisonTree), thisMember, otherMember)); + ComparisonSite comparison = new ComparisonSite(comparisonTree, enclosingStatement(comparisonTree), thisMember, otherMember); + comparisons.add(comparison); + pairsByStatement.computeIfAbsent(comparison.statement, key -> new HashSet<>()).add(comparison.pair()); } private Optional member(ExpressionTree expression) { @@ -186,7 +186,7 @@ private Optional field(Symbol symbol, ExpressionTree tree, boolean on if (symbol.isUnknown() || !symbol.isVariableSymbol() || symbol.isStatic() || !ownedByEnclosing(symbol)) { return Optional.empty(); } - return Optional.of(new MemberRef(symbol, MemberKind.FIELD, symbol.name(), tree, onThis)); + return Optional.of(new MemberRef(MemberKind.FIELD, symbol.name(), tree, onThis)); } private Optional getter(MethodInvocationTree invocation, boolean onThis) { @@ -198,7 +198,7 @@ private Optional getter(MethodInvocationTree invocation, boolean onTh if (returnType == null || returnType.isUnknown() || returnType.type().isVoid() || !method.parameterTypes().isEmpty()) { return Optional.empty(); } - return Optional.of(new MemberRef(method, MemberKind.METHOD, method.name() + "()", invocation, onThis)); + return Optional.of(new MemberRef(MemberKind.METHOD, method.name() + "()", invocation, onThis)); } private boolean ownedByEnclosing(Symbol symbol) { @@ -282,7 +282,7 @@ private enum MemberKind { METHOD } - private record MemberRef(Symbol symbol, MemberKind kind, String displayName, ExpressionTree tree, boolean onThis) { + private record MemberRef(MemberKind kind, String displayName, ExpressionTree tree, boolean onThis) { } private record ComparisonSite(Tree tree, Tree statement, MemberRef thisMember, MemberRef otherMember) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java index 6c66e14530b..5075eb763b3 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java @@ -20,6 +20,7 @@ import org.sonar.java.checks.verifier.CheckVerifier; import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; class EqualsMismatchedMembersCheckTest { @@ -30,4 +31,12 @@ void test() { .withCheck(new EqualsMismatchedMembersCheck()) .verifyIssues(); } + + @Test + void test_non_compiling() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/EqualsMismatchedMembersCheckSample.java")) + .withCheck(new EqualsMismatchedMembersCheck()) + .verifyIssues(); + } } From 2bad6eb8701380cb11d696fdb2533ff9513b6539 Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 12:55:38 +0200 Subject: [PATCH 5/9] Cover remaining S9350 receiver and non-member branches --- .../EqualsMismatchedMembersCheckSample.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java index e00f0ea8be7..e45e1506a28 100644 --- a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -525,4 +525,35 @@ public boolean equals(Object obj) { return getA() == that.b && a == that.a; } } + + static class FieldReceiver { + int a; + int b; + FieldReceiver other; + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof FieldReceiver that)) { + return false; + } + return this.a == other.b && this.a == that.a; + } + } + + static class GetterWithArgument { + int a; + int b; + + int value(int ignored) { + return a; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof GetterWithArgument that)) { + return false; + } + return value(0) == that.b && a == that.a; + } + } } From 1151208080e3a0cf7455056ea457717e121a8eaa Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 14:22:34 +0200 Subject: [PATCH 6/9] Refresh S9350 HTML with NOSONAR and undirected-edge exception --- .../org/sonar/l10n/java/rules/java/S9350.html | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html index 848dd6c2a28..6e3d577c10d 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9350.html @@ -149,18 +149,20 @@

Compliant solution

}

Exceptions

-

The rule does not raise an issue when both swapped pairings appear, because that pattern can express order-independent equality:

+

The rule does not raise an issue when both swapped pairings appear in the same statement, because that pattern can express order-independent +equality. An undirected connection is the same in either direction:

-class Pair {
-  private int a;
-  private int b;
+class UndirectedEdge {
+  private String from;
+  private String to;
 
   @Override
   public boolean equals(Object obj) {
-    if (!(obj instanceof Pair that)) {
+    if (!(obj instanceof UndirectedEdge that)) {
       return false;
     }
-    return (a == that.a && b == that.b) || (a == that.b && b == that.a);
+    return (from.equals(that.from) && to.equals(that.to))
+      || (from.equals(that.to) && to.equals(that.from));
   }
 }
@@ -182,6 +184,26 @@

Exceptions

} } +

When a mismatched comparison is intentional and those automatic exceptions do not apply, suppress the issue with a NOSONAR comment and +a short rationale on that line. Keep both swapped pairings in one statement when you can; use NOSONAR when the reverse pairing lives in +another statement, or when the domain truly requires a one-sided comparison:

+
+class UndirectedEdge {
+  private String from;
+  private String to;
+
+  @Override
+  public boolean equals(Object obj) {
+    if (!(obj instanceof UndirectedEdge that)) {
+      return false;
+    }
+    if (from.equals(that.from) && to.equals(that.to)) {
+      return true;
+    }
+    return from.equals(that.to) && to.equals(that.from); // NOSONAR: undirected edge, reverse pairing is on the previous return
+  }
+}
+

Resources

Documentation

    From ea9cac64a13c6d81e3b55fd5109fed09a0207935 Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 14:25:50 +0200 Subject: [PATCH 7/9] Comment the S9350 matching rules for readability --- .../checks/EqualsMismatchedMembersCheck.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 80c26d267e7..a7feab81936 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -41,6 +41,10 @@ import org.sonar.plugins.java.api.tree.StatementTree; import org.sonar.plugins.java.api.tree.Tree; +/** + * Flags {@code equals} implementations that compare a field or getter of {@code this} + * with a different field or getter of the other instance. + */ @Rule(key = "S9350") public class EqualsMismatchedMembersCheck extends IssuableSubscriptionVisitor { @@ -93,6 +97,7 @@ public void visitNode(Tree tree) { ComparisonCollector collector = new ComparisonCollector(owner); methodTree.block().accept(collector); for (ComparisonSite comparison : collector.comparisons) { + // Order-independent equality: (a, b) || (b, a) in the same statement is not a mismatch. if (collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { continue; } @@ -134,6 +139,7 @@ public void visitMethodInvocation(MethodInvocationTree tree) { if (isTwoArgEqualityHelper(tree)) { addIfDubious(tree, arguments.get(0), arguments.get(1)); } else if (ARRAYS_EQUALS.matches(tree) && arguments.size() >= 2) { + // 2-arg compares the two arrays; 6-arg subrange overloads compare arguments 0 and 3. addIfDubious(tree, arguments.get(0), arguments.get(arguments.size() / 2)); } else if (INSTANCE_EQUALS.matches(tree) && arguments.size() == 1) { ExpressionTree receiver = receiver(tree); @@ -144,6 +150,10 @@ public void visitMethodInvocation(MethodInvocationTree tree) { super.visitMethodInvocation(tree); } + /** + * Records a candidate when one operand is a member of {@code this} and the other is a + * differently named member of the same kind on a local or parameter (the other instance). + */ private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTree rhs) { Optional left = member(lhs); Optional right = member(rhs); @@ -152,6 +162,7 @@ private void addIfDubious(Tree comparisonTree, ExpressionTree lhs, ExpressionTre } MemberRef leftMember = left.get(); MemberRef rightMember = right.get(); + // Skip field-vs-getter, same member names, and same-object or two-foreign-object pairings. if (leftMember.kind != rightMember.kind || leftMember.displayName.equals(rightMember.displayName) || leftMember.onThis == rightMember.onThis) { @@ -168,6 +179,7 @@ private Optional member(ExpressionTree expression) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); Optional onThis = receiverIsThis(expr); if (onThis.isEmpty()) { + // Not reached through this or a local/parameter (for example a static or a nested selection). return Optional.empty(); } if (expr instanceof IdentifierTree identifierTree) { @@ -203,10 +215,15 @@ private Optional getter(MethodInvocationTree invocation, boolean onTh private boolean ownedByEnclosing(Symbol symbol) { Symbol owner = symbol.owner(); + // Compare erasures so a field of Holder still belongs to Holder. return owner != null && !owner.isUnknown() && owner.isTypeSymbol() && enclosingClass.type().erasure().equals(owner.type().erasure()); } + /** + * {@code Optional.of(true)} for {@code this} (implicit or explicit), {@code Optional.of(false)} + * for a local or parameter, empty when the receiver is neither. + */ private static Optional receiverIsThis(ExpressionTree access) { if (access instanceof IdentifierTree) { return Optional.of(true); @@ -239,6 +256,7 @@ private static Optional classifyReceiver(ExpressionTree receiverExpr) { } Symbol owner = symbol.owner(); if (owner == null || !owner.isMethodSymbol()) { + // A field of this type used as receiver is not the other instance. return Optional.empty(); } return Optional.of(false); @@ -252,6 +270,7 @@ private static boolean isTwoArgEqualityHelper(MethodInvocationTree tree) { return true; } Symbol.MethodSymbol method = tree.methodSymbol(); + // Fallback when Guava is absent from the classpath: a static or unresolved method named equal. return (method.isUnknown() || method.isStatic()) && "equal".equals(ExpressionUtils.methodName(tree).name()); } From 95bb53d6b21203eff23810608bd38b7c0b0f446f Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 16:56:36 +0200 Subject: [PATCH 8/9] Add a withoutSemantic verifier test for S9350 --- .../java/checks/EqualsMismatchedMembersCheckTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java index 5075eb763b3..a8603418767 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/EqualsMismatchedMembersCheckTest.java @@ -39,4 +39,13 @@ void test_non_compiling() { .withCheck(new EqualsMismatchedMembersCheck()) .verifyIssues(); } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/EqualsMismatchedMembersCheckSample.java")) + .withCheck(new EqualsMismatchedMembersCheck()) + .withoutSemantic() + .verifyIssues(); + } } From 8f5c43938c33d8c3c5f86ca91066f8a05672f8ff Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 18 Aug 2026 17:00:58 +0200 Subject: [PATCH 9/9] Require boolean return for resolved static equal helpers --- .../EqualsMismatchedMembersCheckSample.java | 17 +++++++++++++++++ .../checks/EqualsMismatchedMembersCheck.java | 17 ++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java index e45e1506a28..06df481f31e 100644 --- a/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/EqualsMismatchedMembersCheckSample.java @@ -509,6 +509,23 @@ public boolean equals(Object obj) { } } + static class NonBooleanEqualHelper { + int a; + int b; + + static void equal(Object x, Object y) { + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof NonBooleanEqualHelper that)) { + return false; + } + equal(a, that.b); + return a == that.a; + } + } + static class StaticGetter { int a; int b; diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index a7feab81936..47657662d3a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -30,6 +30,7 @@ import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ClassTree; @@ -269,10 +270,20 @@ private static boolean isTwoArgEqualityHelper(MethodInvocationTree tree) { if (OBJECTS_EQUALS.matches(tree) || GUAVA_OBJECTS_EQUAL.matches(tree)) { return true; } + if (!"equal".equals(ExpressionUtils.methodName(tree).name())) { + return false; + } Symbol.MethodSymbol method = tree.methodSymbol(); - // Fallback when Guava is absent from the classpath: a static or unresolved method named equal. - return (method.isUnknown() || method.isStatic()) - && "equal".equals(ExpressionUtils.methodName(tree).name()); + // Unresolved `equal` covers Guava when it is absent from the classpath. + if (method.isUnknown()) { + return true; + } + Symbol.TypeSymbol returnType = method.returnType(); + // Resolved user helpers are equality comparisons only when they return boolean. + return method.isStatic() + && returnType != null + && !returnType.isUnknown() + && returnType.type().isPrimitive(Type.Primitives.BOOLEAN); } private static ExpressionTree receiver(MethodInvocationTree invocation) {