SONARJAVA-6825: Implemented rule S9360 - Comparisons should not use Yoda conditions

its/ruling/src/test/resources/commons-beanutils/java-S9360.json

@@ -0,0 +1,8 @@
+{
+"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/ConstructorUtils.java": [
+106,
+154,
+218,
+267
+]
+}

its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9360.json

@@ -0,0 +1,59 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpStatus.java": [
+343,
+358,
+373,
+388,
+403
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java": [
+1028
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java": [
+431,
+436,
+445,
+453,
+455,
+459,
+465,
+471,
+477,
+483,
+489,
+495,
+497,
+501,
+503,
+514,
+514,
+515,
+575,
+615,
+617,
+622,
+624,
+628,
+634,
+640,
+646,
+652,
+658,
+664,
+666,
+670,
+677,
+684
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java": [
+93,
+414
+],
+"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java": [
+94
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java": [
+744,
+835
+]
+}

its/ruling/src/test/resources/eclipse-jetty/java-S9360.json

@@ -0,0 +1,75 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpStatus.java": [
+343,
+358,
+373,
+388,
+403
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java": [
+1028
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java": [
+431,
+436,
+445,
+453,
+455,
+459,
+465,
+471,
+477,
+483,
+489,
+495,
+497,
+501,
+503,
+514,
+514,
+515,
+575,
+615,
+617,
+622,
+624,
+628,
+634,
+640,
+646,
+652,
+658,
+664,
+666,
+670,
+677,
+684
+],
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java": [
+93,
+414
+],
+"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java": [
+94
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java": [
+744,
+835
+],
+"org.eclipse.jetty:jetty-project:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/AsyncJSON.java": [
+1275
+],
+"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java": [
+901,
+907,
+925,
+942,
+957,
+962,
+978
+],
+"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java": [
+117,
+132
+]
+}

its/ruling/src/test/resources/guava/java-S9360.json

@@ -0,0 +1,13 @@
+{
+"com.google.guava:guava:src/com/google/common/base/SmallCharMatcher.java": [
+61
+],
+"com.google.guava:guava:src/com/google/common/io/LittleEndianDataInputStream.java": [
+82
+],
+"com.google.guava:guava:src/com/google/common/net/MediaType.java": [
+628,
+631,
+632
+]
+}

java-checks-test-sources/default/src/main/files/non-compiling/checks/YodaConditionCheckSample.java

@@ -0,0 +1,10 @@
+package checks;
+
+class YodaConditionCheckSample {
+
+  void unknownLiteralType() {
+    Object x = new Object();
+    if (UNKNOWN_LITERAL == x) { } // Compliant - UNKNOWN_LITERAL is not a valid literal
+  }
+
+}

java-checks-test-sources/default/src/main/java/checks/YodaConditionCheckSample.java

@@ -0,0 +1,131 @@
+package checks;
+
+class YodaConditionCheckSample {
+
+  void testIntLiteral() {
+    int count = 0;
+    int x = 5;
+    if (0 == count) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^
+    if (5 != x) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^
+    if (count == 0) { } // Compliant
+    if (x != 5) { } // Compliant
+  }
+
+  void testNullLiteral() {
+    Object obj = null;
+    Object myObject = null;
+    if (null == obj) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^^
+    if (null != myObject) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^^
+    if (obj == null) { } // Compliant
+    if (myObject != null) { } // Compliant
+    if (null == null) { } // Compliant
+  }
+
+  void testBooleanLiteral() {
+    boolean flag = true;
+    boolean result = false;
+    if (true == flag) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^^
+    if (false != result) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^^^
+    if (flag == true) { } // Compliant
+    if (result != false) { } // Compliant
+  }
+
+  void testStringLiteral() {
+    String str = "hello";
+    String value = "";
+    if ("hello" == str) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^^^^^
+    if ("" != value) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^
+    if (str == "hello") { } // Compliant
+    if (value != "") { } // Compliant
+  }
+
+  void testCharLiteral() {
+    char ch = 'a';
+    if ('a' == ch) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^
+    if (ch == 'a') { } // Compliant
+  }
+
+  void testFloatingPointLiteral() {
+    double doubleValue = 0.0;
+    if (0.0 == doubleValue) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^^^
+    if (doubleValue == 0.0) { } // Compliant
+  }
+
+  void testNestedParentheses() {
+    int count = 0;
+    Object obj = null;
+    if ((0) == count) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+    if (((null)) == obj) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//        ^^^^
+  }
+
+  void testLessThanGreaterThan() {
+    int count = 0;
+    int x = 5;
+    if (0 < count) { } // Noncompliant {{Put the variable on the left side of this comparison and reverse the operator.}}
+//      ^
+    if (5 > x) { } // Noncompliant {{Put the variable on the left side of this comparison and reverse the operator.}}
+//      ^
+    if (count > 0) { } // Compliant
+    if (x < 5) { } // Compliant
+  }
+
+  void testLessThanOrEqualGreaterThanOrEqual() {
+    int count = 0;
+    int x = 5;
+    if (0 <= count) { } // Noncompliant {{Put the variable on the left side of this comparison and reverse the operator.}}
+//      ^
+    if (5 >= x) { } // Noncompliant {{Put the variable on the left side of this comparison and reverse the operator.}}
+//      ^
+    if (count >= 0) { } // Compliant
+    if (x <= 5) { } // Compliant
+  }
+
+  void testNonComparisonContexts() {
+    int count = 0;
+    int a = 1;
+    int b = 2;
+    count = 0; // Compliant - assignment
+    int sum = a + 5; // Compliant - arithmetic
+    int product = 5 * b; // Compliant - arithmetic
+    Object obj = Math.max(5, 10); // Compliant - method call
+  }
+
+  void testTernaryOperator() {
+    boolean condition = true;
+    int result = condition ? 5 : 10; // Compliant
+    if (condition) { } // Compliant
+  }
+
+  void testArrayAccess() {
+    int[] array = {1, 2, 3};
+    if (0 == array[0]) { } // Noncompliant {{Put the variable on the left side of this comparison.}}
+//      ^
+    if (array[0] == 0) { } // Compliant
+  }
+
+  void testBothLiterals() {
+    if (0 == 0) { } // Compliant - both sides are literals
+    if (5 != 10) { } // Compliant - both sides are literals
+    if (true == false) { } // Compliant - both sides are literals
+  }
+
+  void testBothVariables() {
+    int count = 0;
+    int otherCount = 0;
+    Object obj1 = null;
+    Object obj2 = null;
+    if (count == otherCount) { } // Compliant - both are variables
+    if (obj1 == obj2) { } // Compliant - both are variables
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/YodaConditionCheck.java

@@ -0,0 +1,77 @@
+/*
+ * 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.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.ExpressionTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S9360")
+public class YodaConditionCheck extends IssuableSubscriptionVisitor {
+
+  private static final String MESSAGE = "Put the variable on the left side of this comparison.";
+  private static final String MESSAGE_WITH_REVERSE = "Put the variable on the left side of this comparison and reverse the operator.";
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(
+      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 void visitNode(Tree tree) {
+    BinaryExpressionTree binaryExpression = (BinaryExpressionTree) tree;
+    ExpressionTree left = ExpressionUtils.skipParentheses(binaryExpression.leftOperand());
+    ExpressionTree right = ExpressionUtils.skipParentheses(binaryExpression.rightOperand());
+
+    if (isLiteral(left) && !isLiteral(right)) {
+      reportIssue(left, isRelationalOperator(tree) ? MESSAGE_WITH_REVERSE : MESSAGE);
+    }
+  }
+
+  private static boolean isRelationalOperator(Tree tree) {
+    return tree.is(
+      Tree.Kind.LESS_THAN,
+      Tree.Kind.GREATER_THAN,
+      Tree.Kind.LESS_THAN_OR_EQUAL_TO,
+      Tree.Kind.GREATER_THAN_OR_EQUAL_TO
+    );
+  }
+
+  private static boolean isLiteral(ExpressionTree tree) {
+    return tree.is(
+      Tree.Kind.INT_LITERAL,
+      Tree.Kind.LONG_LITERAL,
+      Tree.Kind.FLOAT_LITERAL,
+      Tree.Kind.DOUBLE_LITERAL,
+      Tree.Kind.BOOLEAN_LITERAL,
+      Tree.Kind.CHAR_LITERAL,
+      Tree.Kind.STRING_LITERAL,
+      Tree.Kind.NULL_LITERAL
+    );
+  }
+}