SONARJAVA-4169: S3553 should not report an issue for spring mvc optio…

…nal parameters (#4537)
SonarSource · Nov 17, 2023 · f287c64 · f287c64
1 parent cad794e
commit f287c64
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 8 deletions.
diff --git a/java-checks-test-sources/default/src/main/java/checks/OptionalAsParameterCheck.java b/java-checks-test-sources/default/src/main/java/checks/OptionalAsParameterCheck.java
@@ -4,9 +4,21 @@
 import java.util.OptionalDouble;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
+import javax.annotation.Nullable;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestParam;
 
 class OptionalAsParameterCheck {
-  void foo(OptionalAsParameterCheck a) {} // Compliant
+
+  @GetMapping("/{id}")
+  ResponseEntity<Foo> getFoo(@PathVariable Optional<Long> id, @RequestParam(value = "name") Optional<String> name, @RequestParam(value = "bar") Optional<Integer> bar) { // Compliant
+    return new ResponseEntity<>(new Foo(), HttpStatus.OK);
+  }
+
+  void foo(@Nullable OptionalAsParameterCheck a) {} // Compliant
 
   void foo(Optional<OptionalAsParameterCheck> a) {} // Noncompliant [[sc=12;ec=46]] {{Specify a "OptionalAsParameterCheck" parameter instead.}}
   void bar(Optional o) {} // Noncompliant [[sc=12;ec=20]] {{Specify a type instead.}}

diff --git a/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java b/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java
@@ -19,18 +19,18 @@
  */
 package org.sonar.java.checks;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.semantic.Type;
 import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.TypeTree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
-import java.util.Arrays;
-import java.util.List;
-import java.util.Optional;
-
 @Rule(key = "S3553")
 public class OptionalAsParameterCheck extends IssuableSubscriptionVisitor {
 
@@ -54,11 +54,15 @@ public void visitNode(Tree tree) {
     if (Boolean.FALSE.equals(methodTree.isOverriding())) {
 
       for (VariableTree parameter : methodTree.parameters()) {
+        SymbolMetadata parameterMetadata = parameter.symbol().metadata();
+        if (parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.RequestParam")
+          || parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.PathVariable")) {
+          continue;
+        }
+
         TypeTree typeTree = parameter.type();
         Optional<String> msg = expectedTypeInsteadOfOptional(typeTree.symbolType());
-        if (msg.isPresent()) {
-          reportIssue(typeTree, msg.get());
-        }
+        msg.ifPresent(s -> reportIssue(typeTree, s));
       }
     }
   }