From de924b50cf7b33aafa938df6a0d8c656dc11103d Mon Sep 17 00:00:00 2001 From: Xavier Le Vourch Date: Sun, 8 Feb 2009 19:19:33 +0000 Subject: [PATCH] Fixed bug 2142986 - UselessOverridingMethod doesn't consider annotations new property 'ignoreAnnotations' to get the old behavior git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6818 51baf565-9d33-0410-a72c-fc3788e3496d --- pmd/etc/changelog.txt | 1 + .../basic/xml/UselessOverridingMethod.xml | 63 +++++++++++++++++++ pmd/rulesets/basic.xml | 9 +++ .../pmd/rules/UselessOverridingMethod.java | 52 ++++++++++++--- 4 files changed, 117 insertions(+), 8 deletions(-) diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index b245979484..5a779d9b05 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -10,6 +10,7 @@ Fixed bug 2404700 - UseSingleton should not act on enums Fixed bug 2225474 - VariableNamingConventions does not work with nonprimitives Fixed bug 1609038 - Xslt report generators break if path contains "java" Fixed bug - JUnitTestsShouldIncludeAssert now detects Junit 4 Assert.assert... constructs +Fixed bug 2142986 - UselessOverridingMethod doesn't consider annotations New rule: StrictExceptions : AvoidThrowingNewInstanceOfSameException diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOverridingMethod.xml b/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOverridingMethod.xml index e61caae460..572a6f38cb 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOverridingMethod.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/basic/xml/UselessOverridingMethod.xml @@ -302,4 +302,67 @@ class Bar { } ]]> + + + + + true + 1 + + + + + + + + 0 + + + + + + + + 1 + + + + diff --git a/pmd/rulesets/basic.xml b/pmd/rulesets/basic.xml index 9a4e7b6721..8a01632c49 100644 --- a/pmd/rulesets/basic.xml +++ b/pmd/rulesets/basic.xml @@ -668,6 +668,9 @@ public class Foo { The overriding method merely calls the same method defined in a superclass 3 + + + + diff --git a/pmd/src/net/sourceforge/pmd/rules/UselessOverridingMethod.java b/pmd/src/net/sourceforge/pmd/rules/UselessOverridingMethod.java index d888fb083c..3a2e8c1ce6 100644 --- a/pmd/src/net/sourceforge/pmd/rules/UselessOverridingMethod.java +++ b/pmd/src/net/sourceforge/pmd/rules/UselessOverridingMethod.java @@ -5,8 +5,11 @@ package net.sourceforge.pmd.rules; import java.util.ArrayList; import java.util.List; +import java.util.Map; import net.sourceforge.pmd.AbstractRule; +import net.sourceforge.pmd.PropertyDescriptor; +import net.sourceforge.pmd.ast.ASTAnnotation; import net.sourceforge.pmd.ast.ASTArgumentList; import net.sourceforge.pmd.ast.ASTArguments; import net.sourceforge.pmd.ast.ASTBlock; @@ -15,6 +18,7 @@ import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.ast.ASTFormalParameter; import net.sourceforge.pmd.ast.ASTFormalParameters; import net.sourceforge.pmd.ast.ASTImplementsList; +import net.sourceforge.pmd.ast.ASTMarkerAnnotation; import net.sourceforge.pmd.ast.ASTMethodDeclaration; import net.sourceforge.pmd.ast.ASTMethodDeclarator; import net.sourceforge.pmd.ast.ASTName; @@ -27,6 +31,7 @@ import net.sourceforge.pmd.ast.ASTStatement; import net.sourceforge.pmd.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.ast.Node; import net.sourceforge.pmd.ast.SimpleNode; +import net.sourceforge.pmd.properties.BooleanProperty; import org.jaxen.JaxenException; @@ -34,25 +39,39 @@ import org.jaxen.JaxenException; * @author Romain Pelisse, bugfix for [ 1522517 ] False +: UselessOverridingMethod */ public class UselessOverridingMethod extends AbstractRule { - private List exceptions; - private static final String CLONE = "clone"; - private static final String OBJECT = "Object"; - public UselessOverridingMethod() - { + private final List exceptions; + private boolean ignoreAnnotations; + private static final String CLONE = "clone"; + private static final String OBJECT = "Object"; + + private static final PropertyDescriptor ignoreAnnotationsDescriptor = new BooleanProperty( + "ignoreAnnotations", "Ignore annotations", false, 1.0f); + + private static final Map propertyDescriptorsByName = asFixedMap(new PropertyDescriptor[] { ignoreAnnotationsDescriptor }); + + @Override + protected Map propertiesByName() { + return propertyDescriptorsByName; + } + + public UselessOverridingMethod() { exceptions = new ArrayList(1); exceptions.add("CloneNotSupportedException"); - } + } - public Object visit(ASTImplementsList clz, Object data) + @Override + public Object visit(ASTImplementsList clz, Object data) { return super.visit(clz,data); } + @Override public Object visit(ASTClassOrInterfaceDeclaration clz, Object data) { if (clz.isInterface()) { return data; } + ignoreAnnotations = getBooleanProperty(ignoreAnnotationsDescriptor); return super.visit(clz, data); } @@ -106,6 +125,7 @@ public class UselessOverridingMethod extends AbstractRule { return result; } + @Override public Object visit(ASTMethodDeclaration node, Object data) { // Can skip abstract methods and methods whose only purpose is to // guarantee that the inherited method is not changed by finalizing @@ -114,7 +134,7 @@ public class UselessOverridingMethod extends AbstractRule { return super.visit(node, data); } // We can also skip the 'clone' method as they are generally - // 'useless' but as it is considered a 'good practise' to + // 'useless' but as it is considered a 'good practice' to // implement them anyway ( see bug 1522517) if ( CLONE.equals(node.getMethodName()) && node.isPublic() && ! this.hasArguments(node) && @@ -168,6 +188,22 @@ public class UselessOverridingMethod extends AbstractRule { if (formalParameters.jjtGetNumChildren() != arguments.jjtGetNumChildren()) return super.visit(node, data); + if (!ignoreAnnotations) { + ASTClassOrInterfaceBodyDeclaration parent = (ASTClassOrInterfaceBodyDeclaration) node.jjtGetParent(); + for (int i = 0; i < parent.jjtGetNumChildren(); i++) { + Node n = parent.jjtGetChild(i); + if (n instanceof ASTAnnotation) { + if (n.jjtGetChild(0) instanceof ASTMarkerAnnotation) { + // @Override is ignored + if ("Override".equals(((ASTName) n.jjtGetChild(0).jjtGetChild(0)).getImage())) { + continue; + } + } + return super.visit(node, data); + } + } + } + if (arguments.jjtGetNumChildren() == 0) //No arguments to check addViolation(data, node, getMessage()); else {