SONARJAVA-4691 S6804 should not fail on named value annotation argument

Prevent crashes when processing a value annotation with a named argument

Author: dorian-burihabwa-sonarsource

its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json

@@ -2876,7 +2876,7 @@
   {
     "ruleKey": "S6804",
     "hasTruePositives": false,
-    "falseNegatives": 7,
+    "falseNegatives": 9,
     "falsePositives": 0
   },
   {

java-checks-test-sources/src/main/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSample.java

@@ -4,13 +4,28 @@
 import org.springframework.beans.factory.annotation.Value;
 
 class ValueAnnotationShouldInjectPropertyOrSpELCheckSample {
+  private static final String COMPLIANT_CONSTANT = "${catalog.name}";
+  private static final String NON_COMPLIANT_CONSTANT = "catalog.name";
 
   @Value("catalog.name") // Noncompliant [[sc=3;ec=25]] {{Either replace the "@Value" annotation with a standard field initialization, use "${propertyName}" to inject a property or use "#{expression}" to evaluate a SpEL expression.}}
   String catalogA;
 
   @Value("${catalog.name}") // Compliant
   String catalogB;
 
+  @Value(value = "catalog.name") // Noncompliant [[sc=3;ec=33]] {{Either replace the "@Value" annotation with a standard field initialization, use "${propertyName}" to inject a property or use "#{expression}" to evaluate a SpEL expression.}}
+  String catalogWithNamedArgumentA;
+
+  @Value(value = "${catalog.name}") // Compliant
+  String catalogWithNamedArgumentB;
+
+  @Value(value = NON_COMPLIANT_CONSTANT) // Noncompliant [[sc=3;ec=41]]
+  String catalogWithNamedArgumentC;
+
+  @Value(value = COMPLIANT_CONSTANT) // Compliant
+  String catalogWithNamedArgumentD;
+
+
   @Value("book.topics[0]") // Noncompliant, this will not evaluate the expression
   String topicA;
 
@@ -20,6 +35,7 @@ class ValueAnnotationShouldInjectPropertyOrSpELCheckSample {
   @Value("Hello, world!") // Noncompliant, this use of @Value is redundant
   String greetingA;
 
+
   String greetingB = "Hello, world!"; // Compliant
 
   @Value("") // Noncompliant

java-checks/src/main/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheck.java

@@ -22,11 +22,14 @@
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import javax.annotation.CheckForNull;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
+import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
-import org.sonar.plugins.java.api.tree.LiteralTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
@@ -61,21 +64,29 @@ public void visitNode(Tree tree) {
           "or use \"#{expression}\" to evaluate a SpEL expression."));
   }
 
-  private static boolean isSimpleSpringValue(AnnotationTree ann) {
-    if (ann.symbolType().is(SPRING_VALUE)) {
-      LiteralTree literal = (LiteralTree) ann.arguments().get(0);
-      String value = literal.value();
-      return !isPropertyName(value) && !isSpEL(value);
+  private static boolean isSimpleSpringValue(AnnotationTree annotation) {
+    if (annotation.symbolType().is(SPRING_VALUE)) {
+      String value = extractArgumentValue(annotation.arguments().get(0));
+      return value != null && !(isPropertyName(value) || isSpEL(value));
     }
     return false;
   }
 
+  @CheckForNull
+  private static String extractArgumentValue(ExpressionTree annotationArgument) {
+    if (annotationArgument.is(Tree.Kind.ASSIGNMENT)) {
+      ExpressionTree expression = ((AssignmentExpressionTree) annotationArgument).expression();
+      return ExpressionsHelper.getConstantValueAsString(expression).value();
+    }
+    return ExpressionsHelper.getConstantValueAsString(annotationArgument).value();
+  }
+
   private static boolean isPropertyName(String value) {
-    return value.startsWith("\"${") && value.endsWith("}\"");
+    return value.startsWith("${") && value.endsWith("}");
   }
 
   private static boolean isSpEL(String value) {
-    return value.startsWith("\"#{") && value.endsWith("}\"");
+    return value.startsWith("#{") && value.endsWith("}");
   }
 
 }

java-checks/src/test/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckTest.java

@@ -33,5 +33,4 @@ void test() {
       .withCheck(new ValueAnnotationShouldInjectPropertyOrSpELCheck())
       .verifyIssues();
   }
-
 }