SONARJAVA-4653 Fix ClassCastException and FPs (#4495)

SonarSource · Oct 20, 2023 · f106053 · f106053
1 parent 6dbbe3f
commit f106053
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 7 deletions.
diff --git a/...hecks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java b/...hecks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java
@@ -56,11 +56,26 @@ public Article getArticleButSettingADifferentValue(@PathVariable(name = "article
     return new Article(articleId);
   }
 
+  @GetMapping(value = "{/article/id")
+  public Article getArticleRequestParamWithDefaultValue(@RequestParam(required = false, defaultValue = "1") int articleId) { // Compliant
+    return new Article(articleId);
+  }
+
   @GetMapping(value = {"/article/{id}"})
   public Article getArticleButDifferentlyAnnotated(@Nullable int articleId) { // Compliant
     return new Article(articleId);
   }
 
+  @GetMapping(value = {"/article/{id}"})
+  public Article getArticlePathVariableWithValue(@PathVariable("id") int articleId) { // Compliant
+    return new Article(articleId);
+  }
+
+  @GetMapping(value = {"/article/{id}"})
+  public Article getArticleRequestParamWithValue(@RequestParam("id") int articleId) { // Compliant
+    return new Article(articleId);
+  }
+
   record Article(int id) {
   }
 }
diff --git a/...rc/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java b/...rc/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java
@@ -20,6 +20,7 @@
 package org.sonar.java.checks.spring;
 
 import java.util.List;
+import java.util.stream.Stream;
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
@@ -31,10 +32,11 @@
 
 @Rule(key = "S6814")
 public class OptionalRestParametersShouldBeObjectsCheck extends IssuableSubscriptionVisitor {
-
+  private static final String PATH_VARIABLE_ANNOTATION = "org.springframework.web.bind.annotation.PathVariable";
+  private static final String REQUEST_PARAM_ANNOTATION = "org.springframework.web.bind.annotation.RequestParam";
   private static final List<String> PARAMETER_ANNOTATIONS = List.of(
-    "org.springframework.web.bind.annotation.PathVariable",
-    "org.springframework.web.bind.annotation.RequestParam"
+    PATH_VARIABLE_ANNOTATION,
+    REQUEST_PARAM_ANNOTATION
   );
 
   @Override
@@ -53,17 +55,29 @@ public void visitNode(Tree tree) {
   private static boolean isOptionalPrimitive(VariableTree parameter) {
     return parameter.type().symbolType().isPrimitive() &&
       parameter.modifiers().annotations().stream()
-        .anyMatch(OptionalRestParametersShouldBeObjectsCheck::isMarkingAsOptional);
+        .anyMatch(annotation -> isMarkingAsOptional(annotation) && !hasDefaultValue(annotation));
   }
 
   private static boolean isMarkingAsOptional(AnnotationTree annotation) {
     return PARAMETER_ANNOTATIONS.stream().anyMatch(candidate -> annotation.annotationType().symbolType().is(candidate)) &&
-      annotation.arguments().stream().anyMatch(expressionTree -> {
-        // Because all parameters of the supported annotations are assignments, we can cast safely here.
-        AssignmentExpressionTree assignment = (AssignmentExpressionTree) expressionTree;
+      streamAllNamedArguments(annotation).anyMatch(assignment -> {
         IdentifierTree variable = (IdentifierTree) assignment.variable();
         Boolean constant = assignment.expression().asConstant(Boolean.class).orElse(Boolean.TRUE);
         return "required".equals(variable.name()) && Boolean.FALSE.equals(constant);
       });
   }
+
+  private static boolean hasDefaultValue(AnnotationTree annotation) {
+    return annotation.annotationType().symbolType().is(REQUEST_PARAM_ANNOTATION) &&
+      streamAllNamedArguments(annotation).anyMatch(assignment -> {
+        IdentifierTree variable = (IdentifierTree) assignment.variable();
+        return "defaultValue".equals(variable.name());
+      });
+  }
+
+  private static Stream<AssignmentExpressionTree> streamAllNamedArguments(AnnotationTree annotation) {
+    return annotation.arguments().stream()
+      .filter(expression -> expression.is(Tree.Kind.ASSIGNMENT))
+      .map(AssignmentExpressionTree.class::cast);
+  }
 }