SONARJAVA-5120 S1182 Super call that are not directly in the scope of…

… the method are wrongly taken into account (#4859)

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

@@ -19,69 +19,84 @@
  */
 package org.sonar.java.checks;
 
+import java.util.List;
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
+import org.sonar.plugins.java.api.tree.ClassTree;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodInvocationTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
+import org.sonar.plugins.java.api.tree.NewClassTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.Tree.Kind;
 
-import java.util.Arrays;
-import java.util.List;
-
 @Rule(key = "S1182")
 public class CloneMethodCallsSuperCloneCheck extends IssuableSubscriptionVisitor {
 
-  private boolean foundSuperClone;
-
   @Override
   public List<Kind> nodesToVisit() {
-    return Arrays.asList(Kind.METHOD, Kind.METHOD_INVOCATION);
+    return List.of(Kind.METHOD);
   }
 
   @Override
   public void visitNode(Tree tree) {
-    if (isCloneMethod(tree)) {
-      foundSuperClone = false;
-    } else if (isSuperCloneCall(tree)) {
-      foundSuperClone = true;
+    MethodTree methodTree = (MethodTree) tree;
+
+    if (isCloneMethod(methodTree)) {
+      var visitor = new CloneSuperCallVisitor();
+      tree.accept(visitor);
+
+      if (!visitor.foundSuperCall) {
+        reportIssue(((MethodTree) tree).simpleName(), "Use super.clone() to create and seed the cloned instance to be returned.");
+      }
     }
+  }
 
+  private static boolean isCloneMethod(MethodTree methodTree) {
+    return "clone".equals(methodTree.simpleName().name())
+      && methodTree.parameters().isEmpty()
+      && methodTree.block() != null;
   }
 
-  @Override
-  public void leaveNode(Tree tree) {
-    if (isCloneMethod(tree) && !foundSuperClone) {
-      reportIssue(((MethodTree) tree).simpleName(), "Use super.clone() to create and seed the cloned instance to be returned.");
+  private static class CloneSuperCallVisitor extends BaseTreeVisitor {
+
+    private boolean foundSuperCall;
+
+    @Override
+    public void visitClass(ClassTree tree) {
+      // Skip class definitions because super.clone() found in an inner-class is not the one we're looking for
     }
-  }
 
-  private static boolean isCloneMethod(Tree tree) {
-    if (!tree.is(Kind.METHOD)) {
-      return false;
+    @Override
+    public void visitLambdaExpression(LambdaExpressionTree lambdaExpressionTree) {
+      // Skip lambda expressions because super.clone() found in a lambda is not the one we're looking for
     }
-    MethodTree methodTree = (MethodTree) tree;
-    return "clone".equals(methodTree.simpleName().name()) && methodTree.parameters().isEmpty() && methodTree.block() != null;
-  }
 
-  private static boolean isSuperCloneCall(Tree tree) {
-    if (!tree.is(Kind.METHOD_INVOCATION)) {
-      return false;
+    @Override
+    public void visitNewClass(NewClassTree tree) {
+      // Skip new class expressions because super.clone() found in an anonymous class is not the one we're looking for
     }
 
-    MethodInvocationTree mit = (MethodInvocationTree) tree;
+    @Override
+    public void visitMethodInvocation(MethodInvocationTree tree) {
+      if (isSuperCloneCall(tree)) {
+        foundSuperCall = true;
+      }
+    }
 
-    return mit.arguments().isEmpty() &&
-      mit.methodSelect().is(Kind.MEMBER_SELECT) &&
-      isSuperClone((MemberSelectExpressionTree) mit.methodSelect());
-  }
+    private static boolean isSuperCloneCall(MethodInvocationTree mit) {
+      return mit.arguments().isEmpty()
+        && mit.methodSelect().is(Kind.MEMBER_SELECT)
+        && isSuperClone((MemberSelectExpressionTree) mit.methodSelect());
+    }
 
-  private static boolean isSuperClone(MemberSelectExpressionTree tree) {
-    return "clone".equals(tree.identifier().name()) &&
-      tree.expression().is(Kind.IDENTIFIER) &&
-      "super".equals(((IdentifierTree) tree.expression()).name());
+    private static boolean isSuperClone(MemberSelectExpressionTree tree) {
+      return "clone".equals(tree.identifier().name())
+        && tree.expression().is(Kind.IDENTIFIER)
+        && "super".equals(((IdentifierTree) tree.expression()).name());
+    }
   }
-
 }

java-checks/src/test/files/checks/CloneMethodCallsSuperCloneCheck.java

@@ -63,7 +63,8 @@ protected Object clone() throws CloneNotSupportedException { // Noncompliant {{U
 
 class H {
   @Override
-  protected Object clone() throws CloneNotSupportedException { // Compliant - limitation
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant {{Use super.clone() to create and seed the cloned instance to be returned.}}
+//                 ^^^^^
     class H2 {
       @Override
       protected Object clone() throws CloneNotSupportedException { // Compliant
@@ -72,3 +73,57 @@ protected Object clone() throws CloneNotSupportedException { // Compliant
     }
   }
 }
+
+abstract class I {
+  public abstract Object clone(); // Compliant
+}
+
+// Coverage
+
+class Cov1 {
+  @Override
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant {{Use super.clone() to create and seed the cloned instance to be returned.}}
+//                 ^^^^^
+    return super.toString();
+  }
+}
+
+class Cov2 {
+  @Override
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant {{Use super.clone() to create and seed the cloned instance to be returned.}}
+//                 ^^^^^
+    return this.clone();
+  }
+}
+
+class Cov3 {
+  @Override
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant {{Use super.clone() to create and seed the cloned instance to be returned.}}
+//                 ^^^^^
+    return new I4().clone();
+  }
+}
+
+class Cov4 {
+  @Override
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant {{Use super.clone() to create and seed the cloned instance to be returned.}}
+//                 ^^^^^
+    return toString();
+  }
+}
+
+// Limitation
+class J {
+  @Override
+  protected Object clone() throws CloneNotSupportedException { // Noncompliant
+    Supplier<Object> supplier = () -> {
+      try {
+        return J.super.clone();
+      } catch (CloneNotSupportedException e) {
+        throw Throwables.propagate(e);
+      }
+    };
+
+    return supplier.get();
+  }
+}