diff --git a/java-checks/src/main/java/org/sonar/java/checks/CloneMethodCallsSuperCloneCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CloneMethodCallsSuperCloneCheck.java index 2799f005575..a792c759fcd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CloneMethodCallsSuperCloneCheck.java +++ b/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 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()); + } } - } diff --git a/java-checks/src/test/files/checks/CloneMethodCallsSuperCloneCheck.java b/java-checks/src/test/files/checks/CloneMethodCallsSuperCloneCheck.java index 1fb70366364..12abc84a775 100644 --- a/java-checks/src/test/files/checks/CloneMethodCallsSuperCloneCheck.java +++ b/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 supplier = () -> { + try { + return J.super.clone(); + } catch (CloneNotSupportedException e) { + throw Throwables.propagate(e); + } + }; + + return supplier.get(); + } +}