From 45fa2368cc6af5a7664eb9fcfc0666c3d242e8dd Mon Sep 17 00:00:00 2001 From: rombirli Date: Wed, 14 Oct 2026 15:59:41 +0200 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 +] +} From 0087b5947171ed82a788bca9dd008c5aaf9bb423 Mon Sep 17 00:00:00 2001 From: rombirli Date: Mon, 19 Oct 2026 17:50:02 +0200 Subject: [PATCH 05/10] Do not report negated comparisons --- .../NegatedTernaryConditionCheckSample.java | 17 +++++++++++++ .../checks/NegatedTernaryConditionCheck.java | 24 ++++++++++--------- 2 files changed, 30 insertions(+), 11 deletions(-) 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 index 28033d74163..b266d8096fb 100644 --- a/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java @@ -91,6 +91,23 @@ String negatedNullEqualityIsExempt() { return !(value == null) ? value.toString() : "unknown"; } + // S1940 already reports a negation wrapped around a comparison, so this rule stays silent. + String negatedRelationalIsCoveredByS1940() { + return !(count > 0) ? "nonpositive" : "positive"; + } + + String negatedEqualityIsCoveredByS1940() { + return !(count == 0) ? "some" : "none"; + } + + String negatedInequalityIsCoveredByS1940() { + return !(count != 0) ? "none" : "some"; + } + + String parenthesizedNegatedComparisonIsCoveredByS1940() { + return (!(count <= 0)) ? "positive" : "nonpositive"; + } + String relationalComparison() { return count > 0 ? "some" : "none"; } 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 0b3c196af84..a1e7e04ad4e 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 @@ -31,6 +31,15 @@ public class NegatedTernaryConditionCheck extends IssuableSubscriptionVisitor { private static final String MESSAGE = "Invert this negated condition and swap the two branches of the ternary."; + private static final Tree.Kind[] COMPARISONS = { + Tree.Kind.EQUAL_TO, + Tree.Kind.NOT_EQUAL_TO, + Tree.Kind.LESS_THAN, + Tree.Kind.GREATER_THAN, + Tree.Kind.LESS_THAN_OR_EQUAL_TO, + Tree.Kind.GREATER_THAN_OR_EQUAL_TO + }; + @Override public List nodesToVisit() { return List.of(Tree.Kind.CONDITIONAL_EXPRESSION); @@ -47,20 +56,13 @@ public void visitNode(Tree tree) { 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); + // A negated comparison such as "!(count > 0)" is already reported by S1940. + return !negated.is(COMPARISONS); } - return isNotEqualButNotNullCheck(condition); - } - - private static boolean isNotEqualButNotNullCheck(ExpressionTree condition) { - return condition.is(Tree.Kind.NOT_EQUAL_TO) && !isNullComparison(condition, Tree.Kind.NOT_EQUAL_TO); + return condition.is(Tree.Kind.NOT_EQUAL_TO) && !isNullComparison(condition); } - private static boolean isNullComparison(ExpressionTree expression, Tree.Kind kind) { - if (!expression.is(kind)) { - return false; - } + private static boolean isNullComparison(ExpressionTree expression) { BinaryExpressionTree comparison = (BinaryExpressionTree) expression; return isNullLiteral(comparison.leftOperand()) || isNullLiteral(comparison.rightOperand()); } From dc5acba90dfd885510078d84a7ff2ee719e16086 Mon Sep 17 00:00:00 2001 From: rombirli Date: Mon, 19 Oct 2026 17:51:41 +0200 Subject: [PATCH 06/10] Add quick fix --- .../NegatedTernaryConditionCheckSample.java | 30 +++++++++++++++---- .../checks/NegatedTernaryConditionCheck.java | 29 ++++++++++++++++-- 2 files changed, 52 insertions(+), 7 deletions(-) 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 index b266d8096fb..b5add8f4276 100644 --- a/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java @@ -12,18 +12,30 @@ public class NegatedTernaryConditionCheckSample { private List items; String negatedBooleanVariable() { - return !verbose ? "short" : "long"; // Noncompliant {{Invert this negated condition and swap the two branches of the ternary.}} + return !verbose ? "short" : "long"; // Noncompliant {{Invert this negated condition and swap the two branches of the ternary.}} [[quickfixes=qf1]] // ^^^^^^^^ + // fix@qf1 {{Invert the condition and swap the branches}} + // edit@qf1 [[sc=12;ec=20]] {{verbose}} + // edit@qf1 [[sc=23;ec=30]] {{"long"}} + // edit@qf1 [[sc=33;ec=39]] {{"short"}} } String notEqualComparison() { - return count != 0 ? "some" : "none"; // Noncompliant + return count != 0 ? "some" : "none"; // Noncompliant [[quickfixes=qf2]] // ^^^^^^^^^^ + // fix@qf2 {{Invert the condition and swap the branches}} + // edit@qf2 [[sc=18;ec=20]] {{==}} + // edit@qf2 [[sc=25;ec=31]] {{"none"}} + // edit@qf2 [[sc=34;ec=40]] {{"some"}} } String negatedMethodCall() { - return !items.isEmpty() ? items.get(0) : "none"; // Noncompliant + return !items.isEmpty() ? items.get(0) : "none"; // Noncompliant [[quickfixes=qf3]] // ^^^^^^^^^^^^^^^^ + // fix@qf3 {{Invert the condition and swap the branches}} + // edit@qf3 [[sc=12;ec=28]] {{items.isEmpty()}} + // edit@qf3 [[sc=31;ec=43]] {{"none"}} + // edit@qf3 [[sc=46;ec=52]] {{items.get(0)}} } String negatedEqualsCall() { @@ -32,13 +44,21 @@ String negatedEqualsCall() { } String parenthesizedCondition() { - return (!verbose) ? "short" : "long"; // Noncompliant + return (!verbose) ? "short" : "long"; // Noncompliant [[quickfixes=qf4]] // ^^^^^^^^^^ + // fix@qf4 {{Invert the condition and swap the branches}} + // edit@qf4 [[sc=12;ec=22]] {{verbose}} + // edit@qf4 [[sc=25;ec=32]] {{"long"}} + // edit@qf4 [[sc=35;ec=41]] {{"short"}} } String negatedConjunction() { - return !(verbose && flag) ? "short" : "long"; // Noncompliant + return !(verbose && flag) ? "short" : "long"; // Noncompliant [[quickfixes=qf5]] // ^^^^^^^^^^^^^^^^^^ + // fix@qf5 {{Invert the condition and swap the branches}} + // edit@qf5 [[sc=12;ec=30]] {{(verbose && flag)}} + // edit@qf5 [[sc=33;ec=40]] {{"long"}} + // edit@qf5 [[sc=43;ec=49]] {{"short"}} } String inLocalVariable() { 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 a1e7e04ad4e..824df60baf8 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,7 +18,10 @@ import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.model.ExpressionUtils; +import org.sonar.java.reporting.JavaQuickFix; +import org.sonar.java.reporting.JavaTextEdit; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; @@ -30,6 +33,7 @@ public class NegatedTernaryConditionCheck extends IssuableSubscriptionVisitor { private static final String MESSAGE = "Invert this negated condition and swap the two branches of the ternary."; + private static final String QUICK_FIX = "Invert the condition and swap the branches"; private static final Tree.Kind[] COMPARISONS = { Tree.Kind.EQUAL_TO, @@ -47,12 +51,33 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { - ExpressionTree condition = ((ConditionalExpressionTree) tree).condition(); + ConditionalExpressionTree ternary = (ConditionalExpressionTree) tree; + ExpressionTree condition = ternary.condition(); if (isNegated(ExpressionUtils.skipParentheses(condition))) { - reportIssue(condition, MESSAGE); + QuickFixHelper.newIssue(context) + .forRule(this) + .onTree(condition) + .withMessage(MESSAGE) + .withQuickFix(() -> invertAndSwap(ternary)) + .report(); } } + private JavaQuickFix invertAndSwap(ConditionalExpressionTree ternary) { + ExpressionTree condition = ternary.condition(); + ExpressionTree stripped = ExpressionUtils.skipParentheses(condition); + JavaQuickFix.Builder fix = JavaQuickFix.newQuickFix(QUICK_FIX); + if (stripped.is(Tree.Kind.LOGICAL_COMPLEMENT)) { + ExpressionTree positive = ((UnaryExpressionTree) stripped).expression(); + fix.addTextEdit(JavaTextEdit.replaceTree(condition, QuickFixHelper.contentForTree(positive, context))); + } else { + fix.addTextEdit(JavaTextEdit.replaceTree(((BinaryExpressionTree) stripped).operatorToken(), "==")); + } + fix.addTextEdit(JavaTextEdit.replaceTree(ternary.trueExpression(), QuickFixHelper.contentForTree(ternary.falseExpression(), context))); + fix.addTextEdit(JavaTextEdit.replaceTree(ternary.falseExpression(), QuickFixHelper.contentForTree(ternary.trueExpression(), context))); + return fix.build(); + } + private static boolean isNegated(ExpressionTree condition) { if (condition.is(Tree.Kind.LOGICAL_COMPLEMENT)) { ExpressionTree negated = ExpressionUtils.skipParentheses(((UnaryExpressionTree) condition).expression()); From 88d30634d14997ed2435cd3827d8e747344dea6f Mon Sep 17 00:00:00 2001 From: rombirli Date: Mon, 19 Oct 2026 17:52:33 +0200 Subject: [PATCH 07/10] Mark quick fix covered --- .../main/resources/org/sonar/l10n/java/rules/java/S9435.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 396bb79d3d2..3da96460d6d 100644 --- 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 @@ -19,5 +19,5 @@ }, "attribute": "CLEAR" }, - "quickfix": "targeted" + "quickfix": "covered" } From 862e8fdd3f348d6703809307138f9f2f4273ca2c Mon Sep 17 00:00:00 2001 From: rombirli Date: Tue, 20 Oct 2026 08:07:07 +0200 Subject: [PATCH 08/10] Parenthesize assignments moved to the false branch --- .../NegatedTernaryConditionCheckSample.java | 27 +++++++++++++++++++ .../checks/NegatedTernaryConditionCheck.java | 15 ++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) 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 index b5add8f4276..2381708c2c8 100644 --- a/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java @@ -85,6 +85,33 @@ String bothTernariesNegated() { // ^^^^^^^^^^ } + int assignmentMovedToFalseBranch(int x, int y) { + return !flag ? x = 1 : y; // Noncompliant [[quickfixes=qf6]] +// ^^^^^ + // fix@qf6 {{Invert the condition and swap the branches}} + // edit@qf6 [[sc=12;ec=17]] {{flag}} + // edit@qf6 [[sc=20;ec=25]] {{y}} + // edit@qf6 [[sc=28;ec=29]] {{(x = 1)}} + } + + int compoundAssignmentMovedToFalseBranch(int x, int y) { + return !flag ? x += 1 : y; // Noncompliant [[quickfixes=qf7]] +// ^^^^^ + // fix@qf7 {{Invert the condition and swap the branches}} + // edit@qf7 [[sc=12;ec=17]] {{flag}} + // edit@qf7 [[sc=20;ec=26]] {{y}} + // edit@qf7 [[sc=29;ec=30]] {{(x += 1)}} + } + + int parenthesizedAssignmentStaysAsIs(int x, int y) { + return !flag ? (x = 1) : y; // Noncompliant [[quickfixes=qf8]] +// ^^^^^ + // fix@qf8 {{Invert the condition and swap the branches}} + // edit@qf8 [[sc=12;ec=17]] {{flag}} + // edit@qf8 [[sc=20;ec=27]] {{y}} + // edit@qf8 [[sc=30;ec=31]] {{(x = 1)}} + } + // Compliant cases String positiveCondition() { 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 824df60baf8..ea8e2230f65 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 @@ -23,6 +23,7 @@ import org.sonar.java.reporting.JavaQuickFix; import org.sonar.java.reporting.JavaTextEdit; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; @@ -74,10 +75,22 @@ private JavaQuickFix invertAndSwap(ConditionalExpressionTree ternary) { fix.addTextEdit(JavaTextEdit.replaceTree(((BinaryExpressionTree) stripped).operatorToken(), "==")); } fix.addTextEdit(JavaTextEdit.replaceTree(ternary.trueExpression(), QuickFixHelper.contentForTree(ternary.falseExpression(), context))); - fix.addTextEdit(JavaTextEdit.replaceTree(ternary.falseExpression(), QuickFixHelper.contentForTree(ternary.trueExpression(), context))); + fix.addTextEdit(JavaTextEdit.replaceTree(ternary.falseExpression(), falseBranchText(ternary.trueExpression()))); return fix.build(); } + /** + * The third operand of {@code ?:} cannot be an assignment, so an assignment moved out of the true branch + * has to be parenthesized or the rewritten expression does not compile. + */ + private String falseBranchText(ExpressionTree formerTrueBranch) { + String text = QuickFixHelper.contentForTree(formerTrueBranch, context); + if (formerTrueBranch instanceof AssignmentExpressionTree) { + return "(" + text + ")"; + } + return text; + } + private static boolean isNegated(ExpressionTree condition) { if (condition.is(Tree.Kind.LOGICAL_COMPLEMENT)) { ExpressionTree negated = ExpressionUtils.skipParentheses(((UnaryExpressionTree) condition).expression()); From c5b98f2bed5842aca0baa47a31f35451375807be Mon Sep 17 00:00:00 2001 From: rombirli Date: Tue, 20 Oct 2026 11:16:13 +0200 Subject: [PATCH 09/10] Keep a space when a negation is glued to the previous word --- .../NegatedTernaryConditionCheckSample.java | 18 ++++++++++++++++++ .../checks/NegatedTernaryConditionCheck.java | 19 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) 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 index 2381708c2c8..d72d5029f3c 100644 --- a/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/NegatedTernaryConditionCheckSample.java @@ -112,6 +112,24 @@ int parenthesizedAssignmentStaysAsIs(int x, int y) { // edit@qf8 [[sc=30;ec=31]] {{(x = 1)}} } + int negationGluedToReturn() { + return!flag ? 1 : 2; // Noncompliant [[quickfixes=qf9]] +// ^^^^^ + // fix@qf9 {{Invert the condition and swap the branches}} + // edit@qf9 [[sc=11;ec=16]] {{ flag}} + // edit@qf9 [[sc=19;ec=20]] {{2}} + // edit@qf9 [[sc=23;ec=24]] {{1}} + } + + int parenthesizedNegationGluedToReturn() { + return!(verbose && flag) ? 1 : 2; // Noncompliant [[quickfixes=qf10]] +// ^^^^^^^^^^^^^^^^^^ + // fix@qf10 {{Invert the condition and swap the branches}} + // edit@qf10 [[sc=11;ec=29]] {{ (verbose && flag)}} + // edit@qf10 [[sc=32;ec=33]] {{2}} + // edit@qf10 [[sc=36;ec=37]] {{1}} + } + // Compliant cases String positiveCondition() { 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 ea8e2230f65..35d3ccb49de 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 @@ -23,6 +23,7 @@ import org.sonar.java.reporting.JavaQuickFix; import org.sonar.java.reporting.JavaTextEdit; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.location.Position; import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; @@ -70,7 +71,8 @@ private JavaQuickFix invertAndSwap(ConditionalExpressionTree ternary) { JavaQuickFix.Builder fix = JavaQuickFix.newQuickFix(QUICK_FIX); if (stripped.is(Tree.Kind.LOGICAL_COMPLEMENT)) { ExpressionTree positive = ((UnaryExpressionTree) stripped).expression(); - fix.addTextEdit(JavaTextEdit.replaceTree(condition, QuickFixHelper.contentForTree(positive, context))); + String replacement = withSeparator(condition, QuickFixHelper.contentForTree(positive, context)); + fix.addTextEdit(JavaTextEdit.replaceTree(condition, replacement)); } else { fix.addTextEdit(JavaTextEdit.replaceTree(((BinaryExpressionTree) stripped).operatorToken(), "==")); } @@ -83,6 +85,21 @@ private JavaQuickFix invertAndSwap(ConditionalExpressionTree ternary) { * The third operand of {@code ?:} cannot be an assignment, so an assignment moved out of the true branch * has to be parenthesized or the rewritten expression does not compile. */ + private String withSeparator(Tree replaced, String replacement) { + if (replacement.isEmpty() || Character.isWhitespace(replacement.charAt(0))) { + return replacement; + } + int column = Position.startOf(replaced).columnOffset(); + if (column == 0) { + return replacement; + } + String line = context.getFileLines().get(Position.startOf(replaced).lineOffset()); + if (Character.isJavaIdentifierPart(line.charAt(column - 1))) { + return " " + replacement; + } + return replacement; + } + private String falseBranchText(ExpressionTree formerTrueBranch) { String text = QuickFixHelper.contentForTree(formerTrueBranch, context); if (formerTrueBranch instanceof AssignmentExpressionTree) { From 95c89a2e0982e84c79fbc0d6220b092dc6e24abd Mon Sep 17 00:00:00 2001 From: rombirli Date: Tue, 20 Oct 2026 14:53:13 +0200 Subject: [PATCH 10/10] Move the assignment Javadoc back onto its method --- .../sonar/java/checks/NegatedTernaryConditionCheck.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 35d3ccb49de..2c7456a61ed 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 @@ -81,10 +81,6 @@ private JavaQuickFix invertAndSwap(ConditionalExpressionTree ternary) { return fix.build(); } - /** - * The third operand of {@code ?:} cannot be an assignment, so an assignment moved out of the true branch - * has to be parenthesized or the rewritten expression does not compile. - */ private String withSeparator(Tree replaced, String replacement) { if (replacement.isEmpty() || Character.isWhitespace(replacement.charAt(0))) { return replacement; @@ -100,6 +96,10 @@ private String withSeparator(Tree replaced, String replacement) { return replacement; } + /** + * The third operand of {@code ?:} cannot be an assignment, so an assignment moved out of the true branch + * has to be parenthesized or the rewritten expression does not compile. + */ private String falseBranchText(ExpressionTree formerTrueBranch) { String text = QuickFixHelper.contentForTree(formerTrueBranch, context); if (formerTrueBranch instanceof AssignmentExpressionTree) {