SONARJAVA-4901 S6856 does not raise when parent class defines model attribute (#5117)

Author: leonardo-pilastri-sonarsource

its/autoscan/src/test/resources/autoscan/diffs/diff_S6856.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S6856",
   "hasTruePositives": false,
-  "falseNegatives": 45,
+  "falseNegatives": 46,
   "falsePositives": 0
 }

java-checks-test-sources/default/src/main/java/checks/spring/MissingPathVariableAnnotationCheckSample.java

@@ -13,6 +13,26 @@
 
 public class MissingPathVariableAnnotationCheckSample {
 
+  class ParentController {
+    @ModelAttribute("viewCfg")
+    public String getView(@PathVariable("view") final String view){
+      return "";
+    }
+  }
+  class ChildController extends ParentController {
+    @GetMapping("/model/{view}") //Compliant, parent class defines 'view' path var in the model attribute
+    public String list(@ModelAttribute("viewCfg") final String viewConfig){
+      return "";
+    }
+  }
+  class MissingParentChildController extends MissingPathVariableParentInDifferentSample {
+    @GetMapping("/model/{view}") // Noncompliant
+    // FP: parent class in different file, cannot collect the model attribute
+    public String list(@ModelAttribute("parentView") final String viewConfig){
+      return "";
+    }
+  }
+
   @GetMapping("/{name:[a-z-]+}-{version:\\d\\.\\d\\.\\d}{ext:\\.[a-z]+}") // Noncompliant
   public void handleWithoutExt(@PathVariable String name, @PathVariable String version) {}
 

java-checks-test-sources/default/src/main/java/checks/spring/MissingPathVariableParentInDifferentSample.java

@@ -0,0 +1,11 @@
+package checks.spring;
+
+import org.springframework.web.bind.annotation.ModelAttribute;
+import org.springframework.web.bind.annotation.PathVariable;
+
+public class MissingPathVariableParentInDifferentSample {
+  @ModelAttribute("parentView")
+  public String getView(@PathVariable("view") final String view){
+    return "";
+  }
+}

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

@@ -77,12 +77,13 @@ public void visitNode(Tree tree) {
     }
 
     Set<String> modelAttributeMethodParameter = extractModelAttributeMethodParameter(methods);
+    modelAttributeMethodParameter.addAll(addInheritedModelAttributeMethodParameter(modelAttributeMethodParameter, clazzTree));
 
     checkParametersAndPathTemplate(methods, modelAttributeMethodParameter, requestMappingTemplateVariables);
   }
 
   private static Set<String> extractModelAttributeMethodParameter(List<MethodTree> methods){
-    Set<java.lang.String> modelAttributeMethodParameter = new HashSet<>();
+    Set<String> modelAttributeMethodParameter = new HashSet<>();
     for (var method : methods) {
       if (!method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) {
         continue;
@@ -98,6 +99,23 @@ private static Set<String> extractModelAttributeMethodParameter(List<MethodTree>
     return modelAttributeMethodParameter;
   }
 
+  private static Set<String> addInheritedModelAttributeMethodParameter(Set<String> modelAttributeMethodParameters, ClassTree clazz){
+    if(clazz.superClass() == null){
+      return modelAttributeMethodParameters;
+    }
+    Type superClass = clazz.superClass().symbolType();
+    ClassTree declaration = superClass.symbol().declaration();
+    if(declaration != null){
+      List<MethodTree> methods = declaration.members().stream()
+        .filter(member -> member.is(Tree.Kind.METHOD))
+        .map(MethodTree.class::cast)
+        .toList();
+      modelAttributeMethodParameters.addAll(extractModelAttributeMethodParameter(methods));
+      modelAttributeMethodParameters.addAll(addInheritedModelAttributeMethodParameter(modelAttributeMethodParameters, declaration));
+    }
+    return modelAttributeMethodParameters;
+  }
+
   private void checkParametersAndPathTemplate(List<MethodTree> methods, Set<String> modelAttributeMethodParameter, Set<String> requestMappingTemplateVariables) {
     for (var method : methods) {
       if (!method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) {