SONARJAVA-7213 Take inherited @Valid into account in S5128

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

@@ -0,0 +1,91 @@
+package checks;
+
+import checks.s5128.CrossPackageApi;
+import checks.s5128.CrossPackageDefinition;
+import checks.s5128.CrossPackageRequest;
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotBlank;
+
+class ValidatedBean {
+  @NotBlank
+  String title;
+}
+
+class CrossPackageController implements CrossPackageApi {
+  @Override
+  public void create(CrossPackageRequest body) { // Compliant, @Valid is inherited from an interface resolved from bytecode
+    System.out.println(body.definition());
+  }
+}
+
+class CrossPackageConverter {
+  public String convert(CrossPackageDefinition definition) { // Noncompliant {{Add missing "@Valid" on "definition" to validate it with "Bean Validation".}}
+//                      ^^^^^^^^^^^^^^^^^^^^^^
+    return definition.title();
+  }
+}
+
+interface LocalApi {
+  void create(@Valid ValidatedBean body);
+}
+
+class LocalApiImpl implements LocalApi {
+  @Override
+  public void create(ValidatedBean body) { // Compliant, @Valid is inherited from the interface
+  }
+}
+
+abstract class AbstractHandler {
+  public abstract void handle(@Valid ValidatedBean bean);
+}
+
+class ConcreteHandler extends AbstractHandler {
+  @Override
+  public void handle(ValidatedBean bean) { // Compliant, @Valid is inherited from the abstract class
+  }
+}
+
+interface BaseProcessApi {
+  void process(@Valid ValidatedBean bean);
+}
+
+abstract class MidProcessor implements BaseProcessApi {
+}
+
+class LeafProcessor extends MidProcessor {
+  @Override
+  public void process(ValidatedBean bean) { // Compliant, @Valid is inherited transitively
+  }
+}
+
+interface GenericApi<T> {
+  void handle(@Valid T payload);
+}
+
+class GenericApiImpl implements GenericApi<ValidatedBean> {
+  @Override
+  public void handle(ValidatedBean payload) { // Compliant, @Valid is inherited from the generic interface
+  }
+}
+
+interface PartiallyValidatedApi {
+  void handle(ValidatedBean unvalidated, @Valid ValidatedBean validated);
+}
+
+class PartiallyValidatedImpl implements PartiallyValidatedApi {
+  @Override
+  public void handle(ValidatedBean unvalidated, ValidatedBean validated) { // Noncompliant {{Add missing "@Valid" on "unvalidated" to validate it with "Bean Validation".}}
+//                   ^^^^^^^^^^^^^
+  }
+}
+
+interface PlainApi {
+  void store(ValidatedBean bean);
+}
+
+class PlainApiImpl implements PlainApi {
+  @Override
+  public void store(ValidatedBean bean) { // Noncompliant {{Add missing "@Valid" on "bean" to validate it with "Bean Validation".}}
+//                  ^^^^^^^^^^^^^
+  }
+}

java-checks-test-sources/default/src/main/java/checks/s5128/CrossPackageApi.java

@@ -0,0 +1,7 @@
+package checks.s5128;
+
+import jakarta.validation.Valid;
+
+public interface CrossPackageApi {
+  void create(@Valid CrossPackageRequest body);
+}

java-checks-test-sources/default/src/main/java/checks/s5128/CrossPackageDefinition.java

@@ -0,0 +1,6 @@
+package checks.s5128;
+
+import jakarta.validation.constraints.NotBlank;
+
+public record CrossPackageDefinition(@NotBlank String title) {
+}

java-checks-test-sources/default/src/main/java/checks/s5128/CrossPackageRequest.java

@@ -0,0 +1,7 @@
+package checks.s5128;
+
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotNull;
+
+public record CrossPackageRequest(@NotNull @Valid CrossPackageDefinition definition) {
+}

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

@@ -64,13 +64,30 @@ private void checkField(VariableTree field) {
   }
 
   private void checkMethod(MethodTree method) {
-    if (!isExcluded(method)) {
-      for (VariableTree parameter : method.parameters()) {
-        getIssueMessage(parameter).ifPresent(message -> reportIssue(parameter.type(), message));
+    if (isExcluded(method)) {
+      return;
+    }
+    List<VariableTree> parameters = method.parameters();
+    for (int index = 0; index < parameters.size(); index++) {
+      if (validationEnabledOnOverriddenParameter(method.symbol(), index)) {
+        continue;
       }
+      VariableTree parameter = parameters.get(index);
+      getIssueMessage(parameter).ifPresent(message -> reportIssue(parameter.type(), message));
     }
   }
 
+  /**
+   * Bean Validation providers resolve parameter annotations from the whole method hierarchy, so a parameter is
+   * already validated when the corresponding parameter of any overridden method carries {@code @Valid}. The
+   * overridden declaration may come from bytecode, where the annotation is read from the class file.
+   */
+  private static boolean validationEnabledOnOverriddenParameter(Symbol.MethodSymbol method, int index) {
+    return method.overriddenSymbols().stream()
+      .map(Symbol.MethodSymbol::declarationParameters)
+      .anyMatch(overriddenParameters -> isAnnotatedWithAny(overriddenParameters.get(index).metadata(), JSR380_VALID_ANNOTATIONS));
+  }
+
   private static boolean isExcluded(MethodTree methodTree) {
     return methodTree.symbol().isPrivate() || isInConstraintValidator(methodTree);
   }

java-checks/src/test/java/org/sonar/java/checks/MissingBeanValidationCheckTest.java

@@ -49,4 +49,17 @@ void testWithJakarta() {
       .verifyNoIssues();
   }
 
+  @Test
+  void testWithInheritance() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckInheritanceSample.java"))
+      .withCheck(new MissingBeanValidationCheck())
+      .verifyIssues();
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/MissingBeanValidationCheckInheritanceSample.java"))
+      .withCheck(new MissingBeanValidationCheck())
+      .withoutSemantic()
+      .verifyNoIssues();
+  }
+
 }