From 45fa2368cc6af5a7664eb9fcfc0666c3d242e8dd Mon Sep 17 00:00:00 2001 From: rombirli Date: Wed, 14 Oct 2026 15:59:41 +0200 Subject: [PATCH 1/4] Add rule metadata --- .../org/sonar/l10n/java/rules/java/S9435.html | 59 +++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S9435.json | 23 ++++++++ .../main/resources/profiles/Sonar_way/S9435 | 0 3 files changed, 82 insertions(+) create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9435 diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.html new file mode 100644 index 00000000000..1aa739a9cbb --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.html @@ -0,0 +1,59 @@ +

Why is this an issue?

+

A ternary expression packs both outcomes of a choice into a single line, so the reader takes in the condition and the two results at once. When the +condition is negated, that reading no longer works in one pass: the reader has to invert the condition mentally before pairing each result with the +case it belongs to. In order.status() != Status.SHIPPED ? "Pending" : "Shipped", the value that goes with "shipped" is the second one, +even though shipping is what the condition talks about.

+

Flipping the condition and swapping the two results expresses exactly the same logic without that double negative. It also settles the ordering +question in a consistent way: the positive case, which is usually the common one, comes first.

+

This rule raises an issue on a ternary expression whose condition is negated. A condition counts as negated when it is a ! expression +such as !verbose or !list.isEmpty(), or a != comparison.

+

The rule is only about the readability of the two branches of a ternary. Negated conditions elsewhere, such as in if statements, are +out of scope.

+

Exceptions

+

Null checks do not raise an issue. value != null ? value.name() : "unknown" reads naturally as a positive guard, "if there is a +value", so inverting it would not help the reader.

+

How to fix it

+

Replace the condition with its positive form and swap the two result expressions, keeping each result with the case it handles. For +!x, use x. For x != y, use x == y. For a negated call such as !list.isEmpty(), use +list.isEmpty().

+

The rewritten expression must produce the same value as before, so make sure you move both results and not just the condition. Evaluation order +matters when a result expression has a side effect or can throw, because only the selected branch is evaluated, and swapping the branches must not +change which one that is for a given input.

+

If the negated form is genuinely the clearer one, for example because the negative case is the one the surrounding code exists to handle, this is a +trade-off you may decide to keep rather than a false positive.

+

Code examples

+

Noncompliant code example

+
+public class OrderFormatter {
+
+  public String statusLabel(Order order) {
+    return order.status() != Status.SHIPPED ? "Pending" : "Shipped"; // Noncompliant: negated condition
+  }
+
+  public String summarize(Order order, boolean verbose) {
+    return !verbose ? order.id() : order.id() + " for " + order.customer(); // Noncompliant: negated condition
+  }
+}
+
+

Compliant solution

+
+public class OrderFormatter {
+
+  public String statusLabel(Order order) {
+    return order.status() == Status.SHIPPED ? "Shipped" : "Pending";
+  }
+
+  public String summarize(Order order, boolean verbose) {
+    return verbose ? order.id() + " for " + order.customer() : order.id();
+  }
+}
+
+

Resources

+

Related rules

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.json new file mode 100644 index 00000000000..396bb79d3d2 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9435.json @@ -0,0 +1,23 @@ +{ + "title": "Ternary operator conditions should not be negated", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "clumsy" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-9435", + "sqKey": "S9435", + "scope": "All", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "CLEAR" + }, + "quickfix": "targeted" +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9435 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9435 new file mode 100644 index 00000000000..e69de29bb2d From 14e8ca98417cb9fc453bf960eda75756bd3376e6 Mon Sep 17 00:00:00 2001 From: rombirli Date: Wed, 14 Oct 2026 16:02:45 +0200 Subject: [PATCH 2/4] Add failing reproducer --- .../NegatedTernaryConditionCheckSample.java | 121 ++++++++++++++++++ .../checks/NegatedTernaryConditionCheck.java | 36 ++++++ .../NegatedTernaryConditionCheckTest.java | 45 +++++++ 3 files changed, 202 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/NegatedTernaryConditionCheckTest.java diff --git a/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java new file mode 100644 index 00000000000..28033d74163 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java @@ -0,0 +1,121 @@ +package checks; + +import java.util.List; + +public class NegatedTernaryConditionCheckSample { + + private boolean verbose; + private boolean flag; + private int count; + private String state; + private Object value; + private List items; + + String negatedBooleanVariable() { + return !verbose ? "short" : "long"; // Noncompliant {{Invert this negated condition and swap the two branches of the ternary.}} +// ^^^^^^^^ + } + + String notEqualComparison() { + return count != 0 ? "some" : "none"; // Noncompliant +// ^^^^^^^^^^ + } + + String negatedMethodCall() { + return !items.isEmpty() ? items.get(0) : "none"; // Noncompliant +// ^^^^^^^^^^^^^^^^ + } + + String negatedEqualsCall() { + return !"SHIPPED".equals(state) ? "Pending" : "Shipped"; // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^ + } + + String parenthesizedCondition() { + return (!verbose) ? "short" : "long"; // Noncompliant +// ^^^^^^^^^^ + } + + String negatedConjunction() { + return !(verbose && flag) ? "short" : "long"; // Noncompliant +// ^^^^^^^^^^^^^^^^^^ + } + + String inLocalVariable() { + String label = !flag ? "off" : "on"; // Noncompliant +// ^^^^^ + return label; + } + + String asArgument() { + return String.valueOf(!flag ? "off" : "on"); // Noncompliant +// ^^^^^ + } + + String nestedTernaryInBranch() { + return verbose ? (!flag ? "off" : "on") : "none"; // Noncompliant +// ^^^^^ + } + + String bothTernariesNegated() { + return !verbose // Noncompliant +// ^^^^^^^^ + ? "short" + : (count != 0 ? "some" : "none"); // Noncompliant +// ^^^^^^^^^^ + } + + // Compliant cases + + String positiveCondition() { + return verbose ? "long" : "short"; + } + + String equalComparison() { + return count == 0 ? "none" : "some"; + } + + String positiveMethodCall() { + return items.isEmpty() ? "none" : items.get(0); + } + + String nullCheckIsExempt() { + return value != null ? value.toString() : "unknown"; + } + + String reversedNullCheckIsExempt() { + return null != value ? value.toString() : "unknown"; + } + + String negatedNullEqualityIsExempt() { + return !(value == null) ? value.toString() : "unknown"; + } + + String relationalComparison() { + return count > 0 ? "some" : "none"; + } + + String conjunctionIsNotNegatedAtTopLevel() { + return verbose && flag ? "both" : "other"; + } + + String nestedNotEqualIsNotNegatedAtTopLevel() { + return value != null && count != 0 ? "both" : "other"; + } + + String negatedConditionInIfStatementIsOutOfScope() { + if (!verbose) { + return "short"; + } else { + return "long"; + } + } + + boolean negationOutsideTernary() { + return !verbose; + } + + boolean notEqualOutsideTernary() { + return count != 0; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java new file mode 100644 index 00000000000..e049146a3e2 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java @@ -0,0 +1,36 @@ +/* + * 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.Tree; + +@Rule(key = "S9435") +public class NegatedTernaryConditionCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CONDITIONAL_EXPRESSION); + } + + @Override + public void visitNode(Tree tree) { + // FIXME implemented in the next step + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/NegatedTernaryConditionCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/NegatedTernaryConditionCheckTest.java new file mode 100644 index 00000000000..51efde1ed21 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/NegatedTernaryConditionCheckTest.java @@ -0,0 +1,45 @@ +/* + * 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 NegatedTernaryConditionCheckTest { + + private static final String SAMPLE = "checks/NegatedTernaryConditionCheckSample.java"; + private static final NegatedTernaryConditionCheck CHECK = new NegatedTernaryConditionCheck(); + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(CHECK) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(CHECK) + .withoutSemantic() + .verifyIssues(); + } +} From 42beb00256567e9baf03a84fc10db56bf9a678c6 Mon Sep 17 00:00:00 2001 From: rombirli Date: Wed, 14 Oct 2026 16:04:46 +0200 Subject: [PATCH 3/4] Implement rule --- .../checks/NegatedTernaryConditionCheck.java | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java index e049146a3e2..0b3c196af84 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/NegatedTernaryConditionCheck.java @@ -18,12 +18,19 @@ 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.BinaryExpressionTree; +import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.UnaryExpressionTree; @Rule(key = "S9435") public class NegatedTernaryConditionCheck extends IssuableSubscriptionVisitor { + private static final String MESSAGE = "Invert this negated condition and swap the two branches of the ternary."; + @Override public List nodesToVisit() { return List.of(Tree.Kind.CONDITIONAL_EXPRESSION); @@ -31,6 +38,34 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { - // FIXME implemented in the next step + ExpressionTree condition = ((ConditionalExpressionTree) tree).condition(); + if (isNegated(ExpressionUtils.skipParentheses(condition))) { + reportIssue(condition, MESSAGE); + } + } + + private static boolean isNegated(ExpressionTree condition) { + if (condition.is(Tree.Kind.LOGICAL_COMPLEMENT)) { + ExpressionTree negated = ExpressionUtils.skipParentheses(((UnaryExpressionTree) condition).expression()); + // "!(value == null)" is a null check spelled the long way, and is exempt like "value != null". + return !isNullComparison(negated, Tree.Kind.EQUAL_TO); + } + return isNotEqualButNotNullCheck(condition); + } + + private static boolean isNotEqualButNotNullCheck(ExpressionTree condition) { + return condition.is(Tree.Kind.NOT_EQUAL_TO) && !isNullComparison(condition, Tree.Kind.NOT_EQUAL_TO); + } + + private static boolean isNullComparison(ExpressionTree expression, Tree.Kind kind) { + if (!expression.is(kind)) { + return false; + } + BinaryExpressionTree comparison = (BinaryExpressionTree) expression; + return isNullLiteral(comparison.leftOperand()) || isNullLiteral(comparison.rightOperand()); + } + + private static boolean isNullLiteral(ExpressionTree expression) { + return ExpressionUtils.skipParentheses(expression).is(Tree.Kind.NULL_LITERAL); } } From 93691f822f9608e0bff3d308c44a2c2fbf5eb604 Mon Sep 17 00:00:00 2001 From: rombirli Date: Wed, 14 Oct 2026 16:32:06 +0200 Subject: [PATCH 4/4] Update ruling expectations --- .../resources/expected/sonarqube-6.5/java-S9435.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9435.json diff --git a/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9435.json b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9435.json new file mode 100644 index 00000000000..eb0cfe9c4f3 --- /dev/null +++ b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9435.json @@ -0,0 +1,9 @@ +{ +"sonarqube-6.5:src/main/java/org/sonar/server/debt/DebtRulesXMLImporter.java": [ +254 +], +"sonarqube-6.5:src/test/java/org/sonar/server/issue/IssueDocTesting.java": [ +69, +74 +] +}