From 1972368a61beca667b53824ab367ec3d42d33616 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 11:29:06 +0200 Subject: [PATCH 01/14] SONARJAVA-6824: Implemented rule S9358 - Conditional expressions should not duplicate operations in both branches This rule detects when a ternary operator applies the same operation (method invocation, object creation, or array access) to different arguments in both branches. Such patterns can be refactored by moving the condition inside the operation for better readability. --- ...rnaryOperatorSameOperationCheckSample.java | 67 +++++++ .../TernaryOperatorSameOperationCheck.java | 187 ++++++++++++++++++ ...TernaryOperatorSameOperationCheckTest.java | 42 ++++ 3 files changed, 296 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java new file mode 100644 index 00000000000..ea579180301 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -0,0 +1,67 @@ +package checks; + +class TernaryOperatorSameOperationCheckSample { + + boolean condition; + String result; + String[] array; + + // Method invocations - Noncompliant + String m1 = condition ? foo(a) : foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + + String m2 = condition ? this.foo(a) : this.foo(b); // Noncompliant + String m3 = condition ? obj.foo(a) : obj.foo(b); // Noncompliant + String m4 = condition ? StaticClass.foo(a) : StaticClass.foo(b); // Noncompliant + + // Method invocations - Compliant (different operations or same arguments) + String c1 = condition ? foo(a) : bar(b); // Compliant + String c2 = condition ? foo(a) : foo(a); // Compliant (same arguments) + String c3 = condition ? foo(a) : foo(a, b); // Compliant (different number of arguments) + + // New class - Noncompliant + Object n1 = condition ? new Foo(a) : new Foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + + // New class - Compliant + Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant (different classes) + Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant (same arguments) + Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant (different arguments) + + // Array access - Noncompliant + String[] arr = new String[10]; + String a1 = condition ? arr[a] : arr[b]; // Noncompliant {{Move the conditional expression inside this operation.}} + + // Array access - Compliant + String c7 = condition ? arr[a] : arr[a]; // Compliant (same index) + String c8 = condition ? arr[a] : otherArr[b]; // Compliant (different arrays) + + // Nested ternary - Noncompliant (outer ternary) + String n3 = condition ? (other ? foo(x) : foo(y)) : foo(z); // Noncompliant {{Move the conditional expression inside this operation.}} + + // Nested ternary - Compliant (inner ternary is not same operation) + String c9 = condition ? (other ? foo(x) : bar(x)) : foo(z); // Compliant + + // Method reference - Compliant (different method references) + java.util.function.Function f1 = condition ? this::foo : this::method; // Compliant + + // Multiple different operations - Compliant + String c10 = condition ? foo(a) : bar(b); // Compliant + String c11 = condition ? new Foo(a) : new Bar(b); // Compliant + + // Private methods used in ternary + private String foo(String s) { return s; } + private String foo(String s, String t) { return s + t; } + private void method(String a, String b) {} + + private static class StaticClass { + static String foo(String s) { return s; } + } + + private static class Foo { + Foo(String s) {} + Foo(String s, String t) {} + } + + private static class Bar { + Bar(String s) {} + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java new file mode 100644 index 00000000000..90a0675d802 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -0,0 +1,187 @@ +/* + * 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 org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; +import org.sonar.plugins.java.api.tree.ArrayDimensionTree; +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.NewClassTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.TypeArguments; +import org.sonar.plugins.java.api.tree.TypeTree; + +@Rule(key = "S9358") +public class TernaryOperatorSameOperationCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CONDITIONAL_EXPRESSION); + } + + @Override + public void visitNode(Tree tree) { + var conditional = (org.sonar.plugins.java.api.tree.ConditionalExpressionTree) tree; + var trueExpr = conditional.trueExpression(); + var falseExpr = conditional.falseExpression(); + + if (hasSameOperationStructure(trueExpr, falseExpr)) { + reportIssue(conditional, "Move the conditional expression inside this operation."); + } + } + + private static boolean hasSameOperationStructure(ExpressionTree left, ExpressionTree right) { + if (left == null || right == null) { + return false; + } + + if (left.is(Tree.Kind.METHOD_INVOCATION) && right.is(Tree.Kind.METHOD_INVOCATION)) { + return sameMethodInvocation((MethodInvocationTree) left, (MethodInvocationTree) right); + } + if (left.is(Tree.Kind.NEW_CLASS) && right.is(Tree.Kind.NEW_CLASS)) { + return sameNewClass((NewClassTree) left, (NewClassTree) right); + } + if (left.is(Tree.Kind.ARRAY_ACCESS_EXPRESSION) && right.is(Tree.Kind.ARRAY_ACCESS_EXPRESSION)) { + return sameArrayAccess((ArrayAccessExpressionTree) left, (ArrayAccessExpressionTree) right); + } + return false; + } + + private static boolean sameMethodInvocation(MethodInvocationTree left, MethodInvocationTree right) { + if (!sameMethodSelect(left.methodSelect(), right.methodSelect())) { + return false; + } + + var leftArgs = (List) left.arguments(); + var rightArgs = (List) right.arguments(); + if (leftArgs.size() != rightArgs.size()) { + return false; + } + + for (int i = 0; i < leftArgs.size(); i++) { + if (sameExpression(leftArgs.get(i), rightArgs.get(i))) { + return false; + } + } + return true; + } + + private static boolean sameMethodSelect(ExpressionTree left, ExpressionTree right) { + if (!left.is(right.kind())) { + return false; + } + if (left.is(Tree.Kind.MEMBER_SELECT)) { + var leftMember = (MemberSelectExpressionTree) left; + var rightMember = (MemberSelectExpressionTree) right; + return sameMethodSelect(leftMember.expression(), rightMember.expression()) + && sameIdentifier(leftMember.identifier(), rightMember.identifier()); + } + if (left.is(Tree.Kind.IDENTIFIER)) { + return sameIdentifier((IdentifierTree) left, (IdentifierTree) right); + } + return left.toString().equals(right.toString()); + } + + private static boolean sameIdentifier(IdentifierTree left, IdentifierTree right) { + return left.name().equals(right.name()); + } + + private static boolean sameNewClass(NewClassTree left, NewClassTree right) { + if (!sameTree(left.identifier(), right.identifier())) { + return false; + } + + var leftTypeArgs = left.typeArguments(); + var rightTypeArgs = right.typeArguments(); + if ((leftTypeArgs == null) != (rightTypeArgs == null)) { + return false; + } + if (leftTypeArgs != null && !sameTypeArguments(leftTypeArgs, rightTypeArgs)) { + return false; + } + + var leftArgs = (List) left.arguments(); + var rightArgs = (List) right.arguments(); + if (leftArgs.size() != rightArgs.size()) { + return false; + } + + for (int i = 0; i < leftArgs.size(); i++) { + if (sameExpression(leftArgs.get(i), rightArgs.get(i))) { + return false; + } + } + return true; + } + + private static boolean sameArrayAccess(ArrayAccessExpressionTree left, ArrayAccessExpressionTree right) { + if (!sameExpression(left.expression(), right.expression())) { + return false; + } + var leftIndex = getArrayIndex(left); + var rightIndex = getArrayIndex(right); + if (leftIndex == null || rightIndex == null) { + return false; + } + return !sameExpression(leftIndex, rightIndex); + } + + private static ExpressionTree getArrayIndex(ArrayAccessExpressionTree arrayAccess) { + var dimension = arrayAccess.dimension(); + if (dimension != null && dimension.expression() != null) { + return dimension.expression(); + } + return null; + } + + private static boolean sameExpression(ExpressionTree left, ExpressionTree right) { + if (left == null || right == null) { + return false; + } + if (!left.is(right.kind())) { + return false; + } + return left.toString().equals(right.toString()); + } + + private static boolean sameTree(Tree left, Tree right) { + if (left == null || right == null) { + return false; + } + if (!left.is(right.kind())) { + return false; + } + return left.toString().equals(right.toString()); + } + + private static boolean sameTypeArguments(TypeArguments left, TypeArguments right) { + if (left.size() != right.size()) { + return false; + } + for (int i = 0; i < left.size(); i++) { + if (!left.get(i).toString().equals(right.get(i).toString())) { + return false; + } + } + return true; + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java new file mode 100644 index 00000000000..096911d4b11 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java @@ -0,0 +1,42 @@ +/* + * 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 TernaryOperatorSameOperationCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/TernaryOperatorSameOperationCheckSample.java")) + .withCheck(new TernaryOperatorSameOperationCheck()) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/TernaryOperatorSameOperationCheckSample.java")) + .withCheck(new TernaryOperatorSameOperationCheck()) + .withoutSemantic() + .verifyIssues(); + } +} From cef165e448a0c60a878667211eccbae734d345b2 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:36:16 +0200 Subject: [PATCH 02/14] SONARJAVA-6824: Add metadata for rule S9358 Co-Authored-By: Claude Opus 4.6 --- .../org/sonar/l10n/java/rules/java/S9358.html | 51 +++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S9358.json | 25 +++++++++ .../main/resources/profiles/Sonar_way/S9358 | 0 3 files changed, 76 insertions(+) create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9358 diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.html new file mode 100644 index 00000000000..6cfc3cd3764 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.html @@ -0,0 +1,51 @@ +

An issue is raised when a conditional expression (with a condition determining which of two alternative expressions to evaluate) performs the same +operation on both branches, differing only in the arguments.

+

In Java, this refers to the ternary operator with the syntax condition ? trueExpr : falseExpr.

+

Why is this an issue?

+

When both branches of a ternary operator apply the same function call or operation to different values, the code contains unnecessary duplication. +This pattern makes the code harder to read because the common operation is stated twice instead of once.

+

Consider this example:

+
+message = isError ? formatMessage(errorText) : formatMessage(warningText)
+
+

Here, the message formatting function appears in both branches. This duplication obscures the actual difference between the branches (the input +value) and makes the code longer than necessary.

+

By moving the condition inside the function call, you highlight what actually varies:

+
+message = formatMessage(isError ? errorText : warningText)
+
+

This refactored version:

+
    +
  • Eliminates repetition of the function call
    • +
  • Makes the variable part (the condition) more visible
    • +
  • Reduces the overall line length
    • +
  • Improves readability by following the DRY (Don’t Repeat Yourself) principle
    • +
+

This pattern applies to function calls, object creation, array access, and other operations that are identical in both branches.

+

What is the potential impact?

+

This issue has a minor impact on code maintainability:

+
    +
  • Readability: Duplicated operations make code harder to scan and understand quickly
    • +
  • Maintenance burden: If the operation needs to change, developers must update it in two places instead of one
    • +
  • Code size: Unnecessary duplication increases the code footprint without adding value
    • +
+

How to fix it

+

Move the ternary operator inside the common method call or operation. The condition should determine which argument is passed, not which method is +called.

+

Code examples

+

Noncompliant code example

+
+String result = condition ? foo(a) : foo(b); // Noncompliant
+
+

Compliant solution

+
+String result = foo(condition ? a : b);
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.json new file mode 100644 index 00000000000..2967f9aa388 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9358.json @@ -0,0 +1,25 @@ +{ + "title": "Conditional expressions should not duplicate operations in both branches", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "clumsy", + "redundant", + "confusing" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-9358", + "sqKey": "S9358", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9358 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9358 new file mode 100644 index 00000000000..e69de29bb2d From 950bdb652d408f4a35c1d83d67e486015ce3537f Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 13:48:02 +0200 Subject: [PATCH 03/14] SONARJAVA-6824: Fix compilation errors and bugs in rule S9358 - Move test cases into methods with properly typed local variables to fix compilation errors in TernaryOperatorSameOperationCheckSample - Use ExpressionUtils.skipParentheses() to handle parenthesized expressions in ternary branches - Fix argument comparison logic to flag cases where at least one argument differs (not only when all arguments differ) - Add test cases for multi-argument scenarios Co-Authored-By: Claude Opus 4.6 --- ...rnaryOperatorSameOperationCheckSample.java | 103 ++++++++++++------ .../TernaryOperatorSameOperationCheck.java | 19 ++-- 2 files changed, 81 insertions(+), 41 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index ea579180301..0c2d11d12f3 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -3,54 +3,91 @@ class TernaryOperatorSameOperationCheckSample { boolean condition; - String result; - String[] array; + boolean other; + TernaryOperatorSameOperationCheckSample obj; + + void testMethodInvocations() { + String a = "a"; + String b = "b"; + String x = "x"; + String y = "y"; + String z = "z"; + + // Method invocations - Noncompliant + String m1 = condition ? foo(a) : foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + + String m2 = condition ? this.foo(a) : this.foo(b); // Noncompliant + String m3 = condition ? obj.foo(a) : obj.foo(b); // Noncompliant + String m4 = condition ? StaticClass.foo(a) : StaticClass.foo(b); // Noncompliant + + // Method invocations with multiple args where some differ - Noncompliant + String m5 = condition ? foo(a, x) : foo(b, x); // Noncompliant + + // Method invocations - Compliant (different operations or same arguments) + String c1 = condition ? foo(a) : bar(b); // Compliant + String c2 = condition ? foo(a) : foo(a); // Compliant (same arguments) + String c3 = condition ? foo(a) : foo(a, b); // Compliant (different number of arguments) + } - // Method invocations - Noncompliant - String m1 = condition ? foo(a) : foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + void testNewClass() { + String a = "a"; + String b = "b"; - String m2 = condition ? this.foo(a) : this.foo(b); // Noncompliant - String m3 = condition ? obj.foo(a) : obj.foo(b); // Noncompliant - String m4 = condition ? StaticClass.foo(a) : StaticClass.foo(b); // Noncompliant + // New class - Noncompliant + Object n1 = condition ? new Foo(a) : new Foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} - // Method invocations - Compliant (different operations or same arguments) - String c1 = condition ? foo(a) : bar(b); // Compliant - String c2 = condition ? foo(a) : foo(a); // Compliant (same arguments) - String c3 = condition ? foo(a) : foo(a, b); // Compliant (different number of arguments) + // New class with multiple args where some differ - Noncompliant + Object n2 = condition ? new Foo(a, b) : new Foo(b, b); // Noncompliant - // New class - Noncompliant - Object n1 = condition ? new Foo(a) : new Foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + // New class - Compliant + Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant (different classes) + Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant (same arguments) + Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant (different arguments count) + } - // New class - Compliant - Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant (different classes) - Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant (same arguments) - Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant (different arguments) + void testArrayAccess() { + String[] arr = new String[10]; + String[] otherArr = new String[10]; + int i = 0; + int j = 1; - // Array access - Noncompliant - String[] arr = new String[10]; - String a1 = condition ? arr[a] : arr[b]; // Noncompliant {{Move the conditional expression inside this operation.}} + // Array access - Noncompliant + String a1 = condition ? arr[i] : arr[j]; // Noncompliant {{Move the conditional expression inside this operation.}} - // Array access - Compliant - String c7 = condition ? arr[a] : arr[a]; // Compliant (same index) - String c8 = condition ? arr[a] : otherArr[b]; // Compliant (different arrays) + // Array access - Compliant + String c7 = condition ? arr[i] : arr[i]; // Compliant (same index) + String c8 = condition ? arr[i] : otherArr[j]; // Compliant (different arrays) + } - // Nested ternary - Noncompliant (outer ternary) - String n3 = condition ? (other ? foo(x) : foo(y)) : foo(z); // Noncompliant {{Move the conditional expression inside this operation.}} + void testNestedTernary() { + String x = "x"; + String y = "y"; + String z = "z"; - // Nested ternary - Compliant (inner ternary is not same operation) - String c9 = condition ? (other ? foo(x) : bar(x)) : foo(z); // Compliant + // Nested ternary - Noncompliant (outer ternary) + String n3 = condition ? (other ? foo(x) : foo(y)) : foo(z); // Noncompliant {{Move the conditional expression inside this operation.}} - // Method reference - Compliant (different method references) - java.util.function.Function f1 = condition ? this::foo : this::method; // Compliant + // Nested ternary - Compliant (inner ternary is not same operation) + String c9 = condition ? (other ? foo(x) : bar(x)) : foo(z); // Compliant + } - // Multiple different operations - Compliant - String c10 = condition ? foo(a) : bar(b); // Compliant - String c11 = condition ? new Foo(a) : new Bar(b); // Compliant + void testOther() { + String a = "a"; + String b = "b"; + + // Method reference - Compliant (different method references) + java.util.function.Function f1 = condition ? this::foo : this::method; // Compliant + + // Multiple different operations - Compliant + String c10 = condition ? foo(a) : bar(b); // Compliant + String c11 = condition ? new Foo(a) : new Bar(b); // Compliant + } // Private methods used in ternary private String foo(String s) { return s; } private String foo(String s, String t) { return s + t; } - private void method(String a, String b) {} + private String bar(String s) { return s; } + private String method(String s) { return s; } private static class StaticClass { static String foo(String s) { return s; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index 90a0675d802..bdce6b0a5de 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -18,6 +18,7 @@ import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; import org.sonar.plugins.java.api.tree.ArrayDimensionTree; @@ -41,8 +42,8 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { var conditional = (org.sonar.plugins.java.api.tree.ConditionalExpressionTree) tree; - var trueExpr = conditional.trueExpression(); - var falseExpr = conditional.falseExpression(); + var trueExpr = ExpressionUtils.skipParentheses(conditional.trueExpression()); + var falseExpr = ExpressionUtils.skipParentheses(conditional.falseExpression()); if (hasSameOperationStructure(trueExpr, falseExpr)) { reportIssue(conditional, "Move the conditional expression inside this operation."); @@ -77,12 +78,13 @@ private static boolean sameMethodInvocation(MethodInvocationTree left, MethodInv return false; } + boolean anyDifferent = false; for (int i = 0; i < leftArgs.size(); i++) { - if (sameExpression(leftArgs.get(i), rightArgs.get(i))) { - return false; + if (!sameExpression(leftArgs.get(i), rightArgs.get(i))) { + anyDifferent = true; } } - return true; + return anyDifferent; } private static boolean sameMethodSelect(ExpressionTree left, ExpressionTree right) { @@ -125,12 +127,13 @@ private static boolean sameNewClass(NewClassTree left, NewClassTree right) { return false; } + boolean anyDifferent = false; for (int i = 0; i < leftArgs.size(); i++) { - if (sameExpression(leftArgs.get(i), rightArgs.get(i))) { - return false; + if (!sameExpression(leftArgs.get(i), rightArgs.get(i))) { + anyDifferent = true; } } - return true; + return anyDifferent; } private static boolean sameArrayAccess(ArrayAccessExpressionTree left, ArrayAccessExpressionTree right) { From d42704517bd206d9ef18c4a5613c58b9b50032a4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 24 Aug 2026 11:50:29 +0000 Subject: [PATCH 04/14] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- .../src/test/resources/guava/java-S9358.json | 15 +++++++++++++++ .../test/resources/sonar-server/java-S9358.json | 17 +++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 its/ruling/src/test/resources/guava/java-S9358.json create mode 100644 its/ruling/src/test/resources/sonar-server/java-S9358.json diff --git a/its/ruling/src/test/resources/guava/java-S9358.json b/its/ruling/src/test/resources/guava/java-S9358.json new file mode 100644 index 00000000000..cc9c5e76286 --- /dev/null +++ b/its/ruling/src/test/resources/guava/java-S9358.json @@ -0,0 +1,15 @@ +{ +"com.google.guava:guava:src/com/google/common/collect/ImmutableList.java": [ +209 +], +"com.google.guava:guava:src/com/google/common/collect/ImmutableSet.java": [ +263 +], +"com.google.guava:guava:src/com/google/common/collect/RegularImmutableTable.java": [ +151, +155 +], +"com.google.guava:guava:src/com/google/common/hash/MessageDigestHashFunction.java": [ +156 +] +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S9358.json b/its/ruling/src/test/resources/sonar-server/java-S9358.json new file mode 100644 index 00000000000..22b7952bded --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S9358.json @@ -0,0 +1,17 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/ConditionEvaluator.java": [ +113 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java": [ +175 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/ws/ShowAction.java": [ +66 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualityprofile/RuleActivator.java": [ +200 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/setting/ws/ValuesAction.java": [ +247 +] +} From a3a988c0224359c2f1e0c5cfc18fcedf8b51f5fb Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 14:02:14 +0200 Subject: [PATCH 05/14] SONARJAVA-6824: Fix compilation error and merge ruling expectations - Change type from String to Object for ternary with Foo/Bar constructors - Merge ruling expectation updates from PR #6012 Co-Authored-By: Claude Opus 4.6 --- .../java/checks/TernaryOperatorSameOperationCheckSample.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index 0c2d11d12f3..42282da920f 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -80,7 +80,7 @@ void testOther() { // Multiple different operations - Compliant String c10 = condition ? foo(a) : bar(b); // Compliant - String c11 = condition ? new Foo(a) : new Bar(b); // Compliant + Object c11 = condition ? new Foo(a) : new Bar(b); // Compliant } // Private methods used in ternary From bc724780ebb8ab4a7ee8fc81c4ea2ccbf9872a4d Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 14:24:52 +0200 Subject: [PATCH 06/14] SONARJAVA-6824: Fix QG issues in rule S9358 - Remove unused imports (ArrayDimensionTree, TypeTree) - Remove dead code: null checks that always evaluate to false - Reduce duplication by extracting hasExactlyOneArgumentDifference() and consolidating sameExpression/sameTree into a single method - Remove unused typeArguments handling in sameNewClass - Add more test cases for edge cases (no-arg methods, different receivers, mixed expression kinds, member select edge cases) Co-Authored-By: Claude Opus 4.6 --- ...rnaryOperatorSameOperationCheckSample.java | 66 ++++++++++--- .../TernaryOperatorSameOperationCheck.java | 92 +++---------------- 2 files changed, 66 insertions(+), 92 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index 42282da920f..e661e5d67b2 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -1,10 +1,13 @@ package checks; +import java.util.function.Function; + class TernaryOperatorSameOperationCheckSample { boolean condition; boolean other; TernaryOperatorSameOperationCheckSample obj; + TernaryOperatorSameOperationCheckSample obj2; void testMethodInvocations() { String a = "a"; @@ -24,9 +27,30 @@ void testMethodInvocations() { String m5 = condition ? foo(a, x) : foo(b, x); // Noncompliant // Method invocations - Compliant (different operations or same arguments) - String c1 = condition ? foo(a) : bar(b); // Compliant - String c2 = condition ? foo(a) : foo(a); // Compliant (same arguments) - String c3 = condition ? foo(a) : foo(a, b); // Compliant (different number of arguments) + String c1 = condition ? foo(a) : bar(b); // Compliant - different methods + String c2 = condition ? foo(a) : foo(a); // Compliant - same arguments + String c3 = condition ? foo(a) : foo(a, b); // Compliant - different number of arguments + } + + void testMethodInvocationsEdgeCases() { + String a = "a"; + String b = "b"; + + // No-arg methods - Compliant (no arguments to differ) + String e1 = condition ? noArg() : noArg(); // Compliant + + // Different receivers - Compliant + String e2 = condition ? obj.foo(a) : obj2.foo(b); // Compliant - different receiver objects + + // Different kinds in true/false - Compliant + Object e3 = condition ? foo(a) : new Foo(b); // Compliant - method vs constructor + Object e4 = condition ? a : b; // Compliant - simple identifiers, not method/new/array + + // String literal - Compliant + String e5 = condition ? "hello" : "world"; // Compliant + + // Numeric literal - Compliant + int e6 = condition ? 1 : 2; // Compliant } void testNewClass() { @@ -40,9 +64,9 @@ void testNewClass() { Object n2 = condition ? new Foo(a, b) : new Foo(b, b); // Noncompliant // New class - Compliant - Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant (different classes) - Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant (same arguments) - Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant (different arguments count) + Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant - different classes + Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant - same arguments + Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant - different arguments count } void testArrayAccess() { @@ -55,8 +79,8 @@ void testArrayAccess() { String a1 = condition ? arr[i] : arr[j]; // Noncompliant {{Move the conditional expression inside this operation.}} // Array access - Compliant - String c7 = condition ? arr[i] : arr[i]; // Compliant (same index) - String c8 = condition ? arr[i] : otherArr[j]; // Compliant (different arrays) + String c7 = condition ? arr[i] : arr[i]; // Compliant - same index + String c8 = condition ? arr[i] : otherArr[j]; // Compliant - different arrays } void testNestedTernary() { @@ -64,23 +88,41 @@ void testNestedTernary() { String y = "y"; String z = "z"; - // Nested ternary - Noncompliant (outer ternary) + // Nested ternary - Noncompliant (outer ternary has same operation after parentheses skip) String n3 = condition ? (other ? foo(x) : foo(y)) : foo(z); // Noncompliant {{Move the conditional expression inside this operation.}} // Nested ternary - Compliant (inner ternary is not same operation) String c9 = condition ? (other ? foo(x) : bar(x)) : foo(z); // Compliant } + void testMemberSelectEdgeCases() { + String a = "a"; + String b = "b"; + + // Same receiver, different method names - Compliant + String ms1 = condition ? obj.foo(a) : obj.bar(b); // Compliant - different method names + + // Method invocation vs member select method invocation - Compliant + String ms2 = condition ? foo(a) : obj.foo(b); // Compliant - identifier vs member select + } + void testOther() { String a = "a"; String b = "b"; - // Method reference - Compliant (different method references) - java.util.function.Function f1 = condition ? this::foo : this::method; // Compliant + // Method reference - Compliant + Function f1 = condition ? this::foo : this::method; // Compliant // Multiple different operations - Compliant String c10 = condition ? foo(a) : bar(b); // Compliant Object c11 = condition ? new Foo(a) : new Bar(b); // Compliant + + // Array access vs method invocation - Compliant + String[] arr = {a, b}; + Object o1 = condition ? arr[0] : foo(b); // Compliant - different expression kinds + + // New class vs array access - Compliant + Object o2 = condition ? new Foo(a) : arr[0]; // Compliant - different expression kinds } // Private methods used in ternary @@ -88,6 +130,7 @@ void testOther() { private String foo(String s, String t) { return s + t; } private String bar(String s) { return s; } private String method(String s) { return s; } + private String noArg() { return ""; } private static class StaticClass { static String foo(String s) { return s; } @@ -101,4 +144,5 @@ private static class Foo { private static class Bar { Bar(String s) {} } + } diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index bdce6b0a5de..620fb125eb6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -21,15 +21,13 @@ import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; -import org.sonar.plugins.java.api.tree.ArrayDimensionTree; +import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; 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.NewClassTree; import org.sonar.plugins.java.api.tree.Tree; -import org.sonar.plugins.java.api.tree.TypeArguments; -import org.sonar.plugins.java.api.tree.TypeTree; @Rule(key = "S9358") public class TernaryOperatorSameOperationCheck extends IssuableSubscriptionVisitor { @@ -41,7 +39,7 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { - var conditional = (org.sonar.plugins.java.api.tree.ConditionalExpressionTree) tree; + var conditional = (ConditionalExpressionTree) tree; var trueExpr = ExpressionUtils.skipParentheses(conditional.trueExpression()); var falseExpr = ExpressionUtils.skipParentheses(conditional.falseExpression()); @@ -51,10 +49,6 @@ public void visitNode(Tree tree) { } private static boolean hasSameOperationStructure(ExpressionTree left, ExpressionTree right) { - if (left == null || right == null) { - return false; - } - if (left.is(Tree.Kind.METHOD_INVOCATION) && right.is(Tree.Kind.METHOD_INVOCATION)) { return sameMethodInvocation((MethodInvocationTree) left, (MethodInvocationTree) right); } @@ -68,23 +62,8 @@ private static boolean hasSameOperationStructure(ExpressionTree left, Expression } private static boolean sameMethodInvocation(MethodInvocationTree left, MethodInvocationTree right) { - if (!sameMethodSelect(left.methodSelect(), right.methodSelect())) { - return false; - } - - var leftArgs = (List) left.arguments(); - var rightArgs = (List) right.arguments(); - if (leftArgs.size() != rightArgs.size()) { - return false; - } - - boolean anyDifferent = false; - for (int i = 0; i < leftArgs.size(); i++) { - if (!sameExpression(leftArgs.get(i), rightArgs.get(i))) { - anyDifferent = true; - } - } - return anyDifferent; + return sameMethodSelect(left.methodSelect(), right.methodSelect()) + && hasExactlyOneArgumentDifference(left.arguments(), right.arguments()); } private static boolean sameMethodSelect(ExpressionTree left, ExpressionTree right) { @@ -100,7 +79,7 @@ private static boolean sameMethodSelect(ExpressionTree left, ExpressionTree righ if (left.is(Tree.Kind.IDENTIFIER)) { return sameIdentifier((IdentifierTree) left, (IdentifierTree) right); } - return left.toString().equals(right.toString()); + return sameTree(left, right); } private static boolean sameIdentifier(IdentifierTree left, IdentifierTree right) { @@ -111,25 +90,16 @@ private static boolean sameNewClass(NewClassTree left, NewClassTree right) { if (!sameTree(left.identifier(), right.identifier())) { return false; } + return hasExactlyOneArgumentDifference(left.arguments(), right.arguments()); + } - var leftTypeArgs = left.typeArguments(); - var rightTypeArgs = right.typeArguments(); - if ((leftTypeArgs == null) != (rightTypeArgs == null)) { - return false; - } - if (leftTypeArgs != null && !sameTypeArguments(leftTypeArgs, rightTypeArgs)) { - return false; - } - - var leftArgs = (List) left.arguments(); - var rightArgs = (List) right.arguments(); + private static boolean hasExactlyOneArgumentDifference(List leftArgs, List rightArgs) { if (leftArgs.size() != rightArgs.size()) { return false; } - boolean anyDifferent = false; for (int i = 0; i < leftArgs.size(); i++) { - if (!sameExpression(leftArgs.get(i), rightArgs.get(i))) { + if (!sameTree(leftArgs.get(i), rightArgs.get(i))) { anyDifferent = true; } } @@ -137,54 +107,14 @@ private static boolean sameNewClass(NewClassTree left, NewClassTree right) { } private static boolean sameArrayAccess(ArrayAccessExpressionTree left, ArrayAccessExpressionTree right) { - if (!sameExpression(left.expression(), right.expression())) { - return false; - } - var leftIndex = getArrayIndex(left); - var rightIndex = getArrayIndex(right); - if (leftIndex == null || rightIndex == null) { - return false; - } - return !sameExpression(leftIndex, rightIndex); - } - - private static ExpressionTree getArrayIndex(ArrayAccessExpressionTree arrayAccess) { - var dimension = arrayAccess.dimension(); - if (dimension != null && dimension.expression() != null) { - return dimension.expression(); - } - return null; - } - - private static boolean sameExpression(ExpressionTree left, ExpressionTree right) { - if (left == null || right == null) { - return false; - } - if (!left.is(right.kind())) { - return false; - } - return left.toString().equals(right.toString()); + return sameTree(left.expression(), right.expression()) + && !sameTree(left.dimension().expression(), right.dimension().expression()); } private static boolean sameTree(Tree left, Tree right) { - if (left == null || right == null) { - return false; - } if (!left.is(right.kind())) { return false; } return left.toString().equals(right.toString()); } - - private static boolean sameTypeArguments(TypeArguments left, TypeArguments right) { - if (left.size() != right.size()) { - return false; - } - for (int i = 0; i < left.size(); i++) { - if (!left.get(i).toString().equals(right.get(i).toString())) { - return false; - } - } - return true; - } } From f8a3c0d92d0cafd311294b3a22402cc1a7957bf5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 24 Aug 2026 12:40:39 +0000 Subject: [PATCH 07/14] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- .../resources/eclipse-jetty/java-S9358.json | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 its/ruling/src/test/resources/eclipse-jetty/java-S9358.json diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json new file mode 100644 index 00000000000..37fd915561c --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json @@ -0,0 +1,19 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java": [ +129 +], +"org.eclipse.jetty:jetty-project:jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java": [ +362, +373 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/JavaVersion.java": [ +58 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java": [ +178, +192 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/statistic/CounterStatisticTest.java": [ +91 +] +} From de8e95b62feec08c5c1c0051ee237b2feedb8cfe Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 14:47:41 +0200 Subject: [PATCH 08/14] SONARJAVA-6824: Fix hasExactlyOneArgumentDifference to count differences and add ruling expectations The method was using a boolean flag that returned true when any arguments differed, causing false positives for cases with multiple differing arguments. Now uses an integer counter to ensure exactly one argument differs. Also adds eclipse-jetty ruling expectations and test cases for multiple argument differences. Co-Authored-By: Claude Opus 4.6 --- .../resources/eclipse-jetty/java-S9358.json | 19 +++++++++++++++++++ ...rnaryOperatorSameOperationCheckSample.java | 6 ++++++ .../TernaryOperatorSameOperationCheck.java | 6 +++--- 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 its/ruling/src/test/resources/eclipse-jetty/java-S9358.json diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json new file mode 100644 index 00000000000..37fd915561c --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json @@ -0,0 +1,19 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java": [ +129 +], +"org.eclipse.jetty:jetty-project:jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java": [ +362, +373 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/JavaVersion.java": [ +58 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java": [ +178, +192 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/statistic/CounterStatisticTest.java": [ +91 +] +} diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index e661e5d67b2..308f2ca19b8 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -26,6 +26,9 @@ void testMethodInvocations() { // Method invocations with multiple args where some differ - Noncompliant String m5 = condition ? foo(a, x) : foo(b, x); // Noncompliant + // Method invocations with multiple args where all differ - Compliant + String m6 = condition ? foo(a, x) : foo(b, y); // Compliant - more than one argument differs + // Method invocations - Compliant (different operations or same arguments) String c1 = condition ? foo(a) : bar(b); // Compliant - different methods String c2 = condition ? foo(a) : foo(a); // Compliant - same arguments @@ -63,6 +66,9 @@ void testNewClass() { // New class with multiple args where some differ - Noncompliant Object n2 = condition ? new Foo(a, b) : new Foo(b, b); // Noncompliant + // New class with multiple args where all differ - Compliant + Object n3 = condition ? new Foo(a, a) : new Foo(b, b); // Compliant - more than one argument differs + // New class - Compliant Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant - different classes Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant - same arguments diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index 620fb125eb6..2b952044130 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -97,13 +97,13 @@ private static boolean hasExactlyOneArgumentDifference(List Date: Mon, 24 Aug 2026 13:03:10 +0000 Subject: [PATCH 09/14] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- its/ruling/src/test/resources/eclipse-jetty/java-S9358.json | 4 ---- its/ruling/src/test/resources/sonar-server/java-S9358.json | 6 ------ 2 files changed, 10 deletions(-) diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json index 37fd915561c..da3fc3fea25 100644 --- a/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9358.json @@ -9,10 +9,6 @@ "org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/JavaVersion.java": [ 58 ], -"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java": [ -178, -192 -], "org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/statistic/CounterStatisticTest.java": [ 91 ] diff --git a/its/ruling/src/test/resources/sonar-server/java-S9358.json b/its/ruling/src/test/resources/sonar-server/java-S9358.json index 22b7952bded..9745bb780f8 100644 --- a/its/ruling/src/test/resources/sonar-server/java-S9358.json +++ b/its/ruling/src/test/resources/sonar-server/java-S9358.json @@ -2,15 +2,9 @@ "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/ConditionEvaluator.java": [ 113 ], -"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java": [ -175 -], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/ws/ShowAction.java": [ 66 ], -"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualityprofile/RuleActivator.java": [ -200 -], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/setting/ws/ValuesAction.java": [ 247 ] From 4c652adfa54648bd84152de9b7311e057ed9ad2e Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 24 Aug 2026 15:47:18 +0200 Subject: [PATCH 10/14] SONARJAVA-6824: Add S9358 ruling expectations for eclipse-jetty-similar-to-main Co-Authored-By: Claude Opus 4.6 --- .../eclipse-jetty-similar-to-main/java-S9358.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9358.json diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9358.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9358.json new file mode 100644 index 00000000000..a0499db7273 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9358.json @@ -0,0 +1,9 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java": [ +129 +], +"org.eclipse.jetty:jetty-project:jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java": [ +362, +373 +] +} From 0e2ef4b45c84f740aa2d8b280753df922d26f59b Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 25 Aug 2026 10:41:00 +0200 Subject: [PATCH 11/14] SONARJAVA-6824: Compare resolved method symbols to avoid false positives on overloaded methods Use resolved method/constructor symbols instead of syntactic name comparison to correctly distinguish overloaded methods and constructors. When semantic info is unavailable, fall back to syntactic matching. Also exclude new expressions with anonymous class bodies or differing enclosing expressions. Update ruling expectations to remove identified false positives. Co-Authored-By: Claude Opus 4.6 --- .../src/test/resources/guava/java-S9358.json | 6 -- .../resources/sonar-server/java-S9358.json | 3 - ...torSameOperationCheckNoSemanticSample.java | 78 +++++++++++++++++++ ...rnaryOperatorSameOperationCheckSample.java | 31 ++++++++ .../TernaryOperatorSameOperationCheck.java | 38 ++++++++- ...TernaryOperatorSameOperationCheckTest.java | 2 +- 6 files changed, 145 insertions(+), 13 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java diff --git a/its/ruling/src/test/resources/guava/java-S9358.json b/its/ruling/src/test/resources/guava/java-S9358.json index cc9c5e76286..312877bb2d2 100644 --- a/its/ruling/src/test/resources/guava/java-S9358.json +++ b/its/ruling/src/test/resources/guava/java-S9358.json @@ -1,10 +1,4 @@ { -"com.google.guava:guava:src/com/google/common/collect/ImmutableList.java": [ -209 -], -"com.google.guava:guava:src/com/google/common/collect/ImmutableSet.java": [ -263 -], "com.google.guava:guava:src/com/google/common/collect/RegularImmutableTable.java": [ 151, 155 diff --git a/its/ruling/src/test/resources/sonar-server/java-S9358.json b/its/ruling/src/test/resources/sonar-server/java-S9358.json index 9745bb780f8..3c769011709 100644 --- a/its/ruling/src/test/resources/sonar-server/java-S9358.json +++ b/its/ruling/src/test/resources/sonar-server/java-S9358.json @@ -2,9 +2,6 @@ "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/ConditionEvaluator.java": [ 113 ], -"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/ws/ShowAction.java": [ -66 -], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/setting/ws/ValuesAction.java": [ 247 ] diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java new file mode 100644 index 00000000000..503a619e095 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java @@ -0,0 +1,78 @@ +package checks; + +import java.util.function.Function; + +class TernaryOperatorSameOperationCheckNoSemanticSample { + + boolean condition; + boolean other; + TernaryOperatorSameOperationCheckNoSemanticSample obj; + TernaryOperatorSameOperationCheckNoSemanticSample obj2; + + void testMethodInvocations() { + String a = "a"; + String b = "b"; + String x = "x"; + String y = "y"; + + // Method invocations - Noncompliant + String m1 = condition ? foo(a) : foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + + String m2 = condition ? this.foo(a) : this.foo(b); // Noncompliant + String m3 = condition ? obj.foo(a) : obj.foo(b); // Noncompliant + + // Method invocations with multiple args where some differ - Noncompliant + String m5 = condition ? foo(a, x) : foo(b, x); // Noncompliant + + // Method invocations with multiple args where all differ - Compliant + String m6 = condition ? foo(a, x) : foo(b, y); // Compliant - more than one argument differs + + // Method invocations - Compliant (different operations or same arguments) + String c1 = condition ? foo(a) : bar(b); // Compliant - different methods + String c2 = condition ? foo(a) : foo(a); // Compliant - same arguments + String c3 = condition ? foo(a) : foo(a, b); // Compliant - different number of arguments + } + + void testNewClass() { + String a = "a"; + String b = "b"; + + // New class - Noncompliant + Object n1 = condition ? new Foo(a) : new Foo(b); // Noncompliant {{Move the conditional expression inside this operation.}} + + // New class - Compliant + Object c4 = condition ? new Foo(a) : new Bar(b); // Compliant - different classes + Object c5 = condition ? new Foo(a) : new Foo(a); // Compliant - same arguments + Object c6 = condition ? new Foo(a) : new Foo(a, b); // Compliant - different arguments count + } + + void testArrayAccess() { + String[] arr = new String[10]; + String[] otherArr = new String[10]; + int i = 0; + int j = 1; + + // Array access - Noncompliant + String a1 = condition ? arr[i] : arr[j]; // Noncompliant {{Move the conditional expression inside this operation.}} + + // Array access - Compliant + String c7 = condition ? arr[i] : arr[i]; // Compliant - same index + String c8 = condition ? arr[i] : otherArr[j]; // Compliant - different arrays + } + + // Private methods used in ternary + private String foo(String s) { return s; } + private String foo(String s, String t) { return s + t; } + private String bar(String s) { return s; } + private String noArg() { return ""; } + + private static class Foo { + Foo(String s) {} + Foo(String s, String t) {} + } + + private static class Bar { + Bar(String s) {} + } + +} diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index 308f2ca19b8..0dd8dd03d27 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -131,12 +131,38 @@ void testOther() { Object o2 = condition ? new Foo(a) : arr[0]; // Compliant - different expression kinds } + void testOverloadedMethods() { + // Overloaded methods with different parameter types - Compliant (different method symbols) + Object ov1 = condition ? overloaded(1) : overloaded("x"); // Compliant - different overloads + Object ov2 = condition ? this.overloaded(1) : this.overloaded("x"); // Compliant - different overloads + + // Same overload, different arguments - Noncompliant + Object ov3 = condition ? overloaded(1) : overloaded(2); // Noncompliant + Object ov4 = condition ? overloaded("a") : overloaded("b"); // Noncompliant + } + + void testOverloadedConstructors() { + // Overloaded constructors with different parameter types - Compliant (different constructor symbols) + Object oc1 = condition ? new OverloadedCtor(1) : new OverloadedCtor("x"); // Compliant - different constructors + } + + void testNewClassWithClassBody() { + String a = "a"; + String b = "b"; + + // Anonymous class body - Compliant (class bodies make each instantiation unique) + Object ac1 = condition ? new Foo(a) { } : new Foo(b) { }; // Compliant - anonymous class bodies + Object ac2 = condition ? new Foo(a) { } : new Foo(b); // Compliant - one has class body + } + // Private methods used in ternary private String foo(String s) { return s; } private String foo(String s, String t) { return s + t; } private String bar(String s) { return s; } private String method(String s) { return s; } private String noArg() { return ""; } + private Object overloaded(int i) { return i; } + private Object overloaded(String s) { return s; } private static class StaticClass { static String foo(String s) { return s; } @@ -151,4 +177,9 @@ private static class Bar { Bar(String s) {} } + private static class OverloadedCtor { + OverloadedCtor(int i) {} + OverloadedCtor(String s) {} + } + } diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index 2b952044130..2ce8d07438d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -20,6 +20,7 @@ import org.sonar.check.Rule; import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; @@ -62,8 +63,20 @@ private static boolean hasSameOperationStructure(ExpressionTree left, Expression } private static boolean sameMethodInvocation(MethodInvocationTree left, MethodInvocationTree right) { - return sameMethodSelect(left.methodSelect(), right.methodSelect()) - && hasExactlyOneArgumentDifference(left.arguments(), right.arguments()); + if (!sameMethodSelect(left.methodSelect(), right.methodSelect())) { + return false; + } + if (!hasExactlyOneArgumentDifference(left.arguments(), right.arguments())) { + return false; + } + return sameMethodSymbol(left.methodSymbol(), right.methodSymbol()); + } + + private static boolean sameMethodSymbol(Symbol.MethodSymbol left, Symbol.MethodSymbol right) { + if (left.isUnknown() || right.isUnknown()) { + return true; + } + return left.equals(right); } private static boolean sameMethodSelect(ExpressionTree left, ExpressionTree right) { @@ -90,7 +103,26 @@ private static boolean sameNewClass(NewClassTree left, NewClassTree right) { if (!sameTree(left.identifier(), right.identifier())) { return false; } - return hasExactlyOneArgumentDifference(left.arguments(), right.arguments()); + if (!hasExactlyOneArgumentDifference(left.arguments(), right.arguments())) { + return false; + } + if (!sameMethodSymbol(left.methodSymbol(), right.methodSymbol())) { + return false; + } + if (left.classBody() != null || right.classBody() != null) { + return false; + } + return sameNullableTree(left.enclosingExpression(), right.enclosingExpression()); + } + + private static boolean sameNullableTree(Tree left, Tree right) { + if (left == null && right == null) { + return true; + } + if (left == null || right == null) { + return false; + } + return sameTree(left, right); } private static boolean hasExactlyOneArgumentDifference(List leftArgs, List rightArgs) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java index 096911d4b11..2dac4ea8a55 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/TernaryOperatorSameOperationCheckTest.java @@ -34,7 +34,7 @@ void test() { @Test void test_without_semantic() { CheckVerifier.newVerifier() - .onFile(mainCodeSourcesPath("checks/TernaryOperatorSameOperationCheckSample.java")) + .onFile(mainCodeSourcesPath("checks/TernaryOperatorSameOperationCheckNoSemanticSample.java")) .withCheck(new TernaryOperatorSameOperationCheck()) .withoutSemantic() .verifyIssues(); From 4ebbb37904eed02919cab0336604de9631241ef6 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 25 Aug 2026 11:20:18 +0200 Subject: [PATCH 12/14] SONARJAVA-6824: Use SyntacticEquivalence for tree comparison and add test coverage Replace toString()-based tree comparison with SyntacticEquivalence.areEquivalent() to correctly handle non-identifier receivers (e.g. getObj().foo(a) vs getObj().foo(b)). Add test cases for qualified instantiations, mixed enclosing expressions, non-identifier receivers, and anonymous class bodies in without-semantic mode. Co-Authored-By: Claude Opus 4.6 --- ...torSameOperationCheckNoSemanticSample.java | 9 +++++++ ...rnaryOperatorSameOperationCheckSample.java | 27 +++++++++++++++++++ .../TernaryOperatorSameOperationCheck.java | 6 ++--- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java index 503a619e095..83e49878722 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckNoSemanticSample.java @@ -60,6 +60,15 @@ void testArrayAccess() { String c8 = condition ? arr[i] : otherArr[j]; // Compliant - different arrays } + void testNewClassWithClassBody() { + String a = "a"; + String b = "b"; + + // Anonymous class body - Compliant even without semantics + Object ac1 = condition ? new Foo(a) { } : new Foo(b) { }; // Compliant - anonymous class bodies + Object ac2 = condition ? new Foo(a) { } : new Foo(b); // Compliant - one has class body + } + // Private methods used in ternary private String foo(String s) { return s; } private String foo(String s, String t) { return s + t; } diff --git a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java index 0dd8dd03d27..f0e3dc02894 100644 --- a/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/TernaryOperatorSameOperationCheckSample.java @@ -155,6 +155,28 @@ void testNewClassWithClassBody() { Object ac2 = condition ? new Foo(a) { } : new Foo(b); // Compliant - one has class body } + void testQualifiedInstantiations() { + String a = "a"; + String b = "b"; + + // Qualified instantiation with same enclosing expression - Noncompliant + Object qi1 = condition ? obj.new Inner(a) : obj.new Inner(b); // Noncompliant + + // Qualified instantiation with different enclosing expression - Compliant + Object qi2 = condition ? obj.new Inner(a) : obj2.new Inner(b); // Compliant - different enclosing expressions + + // Mixed: unqualified vs qualified - Compliant + Object qi3 = condition ? new Inner(a) : obj.new Inner(b); // Compliant - one has enclosing, other doesn't + } + + void testNonIdentifierReceiver() { + String a = "a"; + String b = "b"; + + // Non-identifier receiver (method call as receiver) - Noncompliant + Object nir1 = condition ? getObj().foo(a) : getObj().foo(b); // Noncompliant + } + // Private methods used in ternary private String foo(String s) { return s; } private String foo(String s, String t) { return s + t; } @@ -163,6 +185,7 @@ void testNewClassWithClassBody() { private String noArg() { return ""; } private Object overloaded(int i) { return i; } private Object overloaded(String s) { return s; } + private TernaryOperatorSameOperationCheckSample getObj() { return this; } private static class StaticClass { static String foo(String s) { return s; } @@ -182,4 +205,8 @@ private static class OverloadedCtor { OverloadedCtor(String s) {} } + class Inner { + Inner(String s) {} + } + } diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index 2ce8d07438d..16c709c4360 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -19,6 +19,7 @@ import java.util.List; import org.sonar.check.Rule; import org.sonar.java.model.ExpressionUtils; +import org.sonar.java.model.SyntacticEquivalence; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; @@ -144,9 +145,6 @@ private static boolean sameArrayAccess(ArrayAccessExpressionTree left, ArrayAcce } private static boolean sameTree(Tree left, Tree right) { - if (!left.is(right.kind())) { - return false; - } - return left.toString().equals(right.toString()); + return SyntacticEquivalence.areEquivalent(left, right); } } From 74f544af3f9487618b70d2ecbff095cf1557159c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 25 Aug 2026 09:36:18 +0000 Subject: [PATCH 13/14] Update ruling results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with GitHub Actions --- its/ruling/src/test/resources/guava/java-S9358.json | 3 +++ its/ruling/src/test/resources/sonar-server/java-S9358.json | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/its/ruling/src/test/resources/guava/java-S9358.json b/its/ruling/src/test/resources/guava/java-S9358.json index 312877bb2d2..05f6667d541 100644 --- a/its/ruling/src/test/resources/guava/java-S9358.json +++ b/its/ruling/src/test/resources/guava/java-S9358.json @@ -3,6 +3,9 @@ 151, 155 ], +"com.google.guava:guava:src/com/google/common/collect/TreeMultiset.java": [ +91 +], "com.google.guava:guava:src/com/google/common/hash/MessageDigestHashFunction.java": [ 156 ] diff --git a/its/ruling/src/test/resources/sonar-server/java-S9358.json b/its/ruling/src/test/resources/sonar-server/java-S9358.json index 3c769011709..550f9758876 100644 --- a/its/ruling/src/test/resources/sonar-server/java-S9358.json +++ b/its/ruling/src/test/resources/sonar-server/java-S9358.json @@ -2,6 +2,12 @@ "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/ConditionEvaluator.java": [ 113 ], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java": [ +175 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualityprofile/RuleActivator.java": [ +200 +], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/setting/ws/ValuesAction.java": [ 247 ] From ab586989b451f5841b664bf3fafc457d907c0933 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Tue, 25 Aug 2026 11:38:57 +0200 Subject: [PATCH 14/14] SONARJAVA-6824: Inline sameNullableTree to fix SonarQube always-false condition findings Inline the null checks for enclosingExpression() directly in sameNewClass so SonarQube can properly track nullability from the @Nullable return type. Co-Authored-By: Claude Opus 4.6 --- .../checks/TernaryOperatorSameOperationCheck.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java index 16c709c4360..0861e01ad11 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TernaryOperatorSameOperationCheck.java @@ -113,17 +113,15 @@ private static boolean sameNewClass(NewClassTree left, NewClassTree right) { if (left.classBody() != null || right.classBody() != null) { return false; } - return sameNullableTree(left.enclosingExpression(), right.enclosingExpression()); - } - - private static boolean sameNullableTree(Tree left, Tree right) { - if (left == null && right == null) { + var leftEnclosing = left.enclosingExpression(); + var rightEnclosing = right.enclosingExpression(); + if (leftEnclosing == null && rightEnclosing == null) { return true; } - if (left == null || right == null) { + if (leftEnclosing == null || rightEnclosing == null) { return false; } - return sameTree(left, right); + return sameTree(leftEnclosing, rightEnclosing); } private static boolean hasExactlyOneArgumentDifference(List leftArgs, List rightArgs) {