diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java new file mode 100644 index 0000000000..3f20b559d5 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java @@ -0,0 +1,50 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration.DeclarationKind; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; +import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class AbstractClassWithoutAbstractMethodRule extends AbstractJavaRule { + + public AbstractClassWithoutAbstractMethodRule() { + addRuleChainVisit(ASTClassOrInterfaceDeclaration.class); + } + + @Override + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (!node.isAbstract() || doesExtend(node) || doesImplement(node)) { + return data; + } + + int countOfAbstractMethods = 0; + for (ASTAnyTypeBodyDeclaration decl : node.getDeclarations()) { + if (decl.getKind() == DeclarationKind.METHOD) { + ASTMethodDeclaration methodDecl = (ASTMethodDeclaration) decl.getDeclarationNode(); + if (methodDecl.isAbstract()) { + countOfAbstractMethods++; + } + } + } + if (countOfAbstractMethods == 0) { + addViolation(data, node); + } + return data; + } + + private boolean doesExtend(ASTClassOrInterfaceDeclaration node) { + return node.getFirstChildOfType(ASTExtendsList.class) != null; + } + + private boolean doesImplement(ASTClassOrInterfaceDeclaration node) { + return node.getFirstChildOfType(ASTImplementsList.class) != null; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java new file mode 100644 index 0000000000..3b09462725 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java @@ -0,0 +1,119 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTArguments; +import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression; +import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression; +import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +class AbstractPositionLiteralsFirstInComparisons extends AbstractJavaRule { + + private final String equalsImage; + + AbstractPositionLiteralsFirstInComparisons(String equalsImage) { + addRuleChainVisit(ASTPrimaryExpression.class); + this.equalsImage = equalsImage; + } + + @Override + public Object visit(ASTPrimaryExpression node, Object data) { + ASTPrimaryPrefix primaryPrefix = node.getFirstChildOfType(ASTPrimaryPrefix.class); + ASTPrimarySuffix primarySuffix = node.getFirstChildOfType(ASTPrimarySuffix.class); + if (primaryPrefix != null && primarySuffix != null) { + ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class); + if (name == null || !name.getImage().endsWith(equalsImage)) { + return data; + } + if (!isSingleStringLiteralArgument(primarySuffix)) { + return data; + } + if (isWithinNullComparison(node)) { + return data; + } + addViolation(data, node); + } + return node; + } + + private boolean isWithinNullComparison(ASTPrimaryExpression node) { + for (ASTExpression parentExpr : node.getParentsOfType(ASTExpression.class)) { + if (isComparisonWithNull(parentExpr, "==", ASTConditionalOrExpression.class) + || isComparisonWithNull(parentExpr, "!=", ASTConditionalAndExpression.class)) { + return true; + } + } + return false; + } + + /* + * Expression/ConditionalAndExpression//EqualityExpression(@Image='!=']//NullLiteral + * Expression/ConditionalOrExpression//EqualityExpression(@Image='==']//NullLiteral + */ + private boolean isComparisonWithNull(ASTExpression parentExpr, String equalOperator, Class condition) { + Node condExpr = null; + ASTEqualityExpression eqExpr = null; + if (parentExpr != null) { + condExpr = parentExpr.getFirstChildOfType(condition); + } + if (condExpr != null) { + eqExpr = condExpr.getFirstDescendantOfType(ASTEqualityExpression.class); + } + if (eqExpr != null) { + return eqExpr.hasImageEqualTo(equalOperator) && eqExpr.hasDescendantOfType(ASTNullLiteral.class); + } + return false; + } + + /* + * This corresponds to the following XPath expression: + * (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()]) + * and + * ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 ) + */ + private boolean isSingleStringLiteralArgument(ASTPrimarySuffix primarySuffix) { + if (!primarySuffix.isArguments() || primarySuffix.getArgumentCount() != 1) { + return false; + } + Node node = primarySuffix; + node = node.getFirstChildOfType(ASTArguments.class); + if (node != null) { + node = node.getFirstChildOfType(ASTArgumentList.class); + if (node.getNumChildren() != 1) { + return false; + } + } + if (node != null) { + node = node.getFirstChildOfType(ASTExpression.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTPrimaryExpression.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTPrimaryPrefix.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTLiteral.class); + } + if (node != null) { + ASTLiteral literal = (ASTLiteral) node; + if (literal.isStringLiteral()) { + return true; + } + } + return false; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java new file mode 100644 index 0000000000..1c84769c01 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java @@ -0,0 +1,13 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +public class PositionLiteralsFirstInCaseInsensitiveComparisonsRule extends AbstractPositionLiteralsFirstInComparisons { + + public PositionLiteralsFirstInCaseInsensitiveComparisonsRule() { + super(".equalsIgnoreCase"); + } + +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java new file mode 100644 index 0000000000..2b9edd12c4 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java @@ -0,0 +1,13 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +public class PositionLiteralsFirstInComparisonsRule extends AbstractPositionLiteralsFirstInComparisons { + + public PositionLiteralsFirstInComparisonsRule() { + super(".equals"); + } + +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java new file mode 100644 index 0000000000..8fd2249133 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java @@ -0,0 +1,28 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.errorprone; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; + +public class JUnitSpellingRule extends AbstractJUnitRule { + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + if (node.getArity() != 0) { + return super.visit(node, data); + } + + String name = node.getName(); + if (!"setUp".equals(name) && "setup".equalsIgnoreCase(name)) { + addViolation(data, node); + } + if (!"tearDown".equals(name) && "teardown".equalsIgnoreCase(name)) { + addViolation(data, node); + } + return super.visit(node, data); + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java new file mode 100644 index 0000000000..de0f71ada6 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java @@ -0,0 +1,24 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.errorprone; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; + +public class JUnitStaticSuiteRule extends AbstractJUnitRule { + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + if (node.getArity() != 0) { + return super.visit(node, data); + } + String name = node.getName(); + if ("suite".equals(name) && (!node.isStatic() || !node.isPublic())) { + addViolation(data, node); + } + return super.visit(node, data); + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index aa0c9b6115..3264cd40a3 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -13,7 +13,7 @@ Rules which enforce generally accepted best practices. language="java" since="3.0" message="This abstract class does not have any abstract methods" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.bestpractices.AbstractClassWithoutAbstractMethodRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#abstractclasswithoutabstractmethod"> The abstract class does not contain any abstract methods. An abstract class suggests @@ -22,20 +22,6 @@ abstract methods. If the class is intended to be used as a base class only (not directly) a protected constructor can be provided prevent direct instantiation. 3 - - - - - - - - Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. 3 - - - - - - - - Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. 3 - - - - - - - - Some JUnit framework methods are easy to misspell. 3 - - - - - - - - The suite() method in a JUnit test needs to be both public and static. 3 - - - - - - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PositionLiteralsFirstInComparisons.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PositionLiteralsFirstInComparisons.xml index 8b6d7c5100..cfb8056dca 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PositionLiteralsFirstInComparisons.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PositionLiteralsFirstInComparisons.xml @@ -40,6 +40,15 @@ public class Foo { if((str == null) || (str.equals(""))) { str = "snafu"; } + if(str == null || str.equals("")) { + str = "snafu"; + } + if((str != null) && (str.equals(""))) { + str = "snafu"; + } + if(str != null && str.equals("")) { + str = "snafu"; + } } } ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml index cc94e27c55..6722f48456 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml @@ -3,10 +3,9 @@ xmlns="http://pmd.sourceforge.net/rule-tests" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> + - + ok 0 + - + failure case - two calls 2 + - + must be in a catch block 0 + - + bug 1626232, the rule should not be confused by inner classes 0 + - + bug 1626232, should work with a static block 1 + + + XPath problem: A sequence of more than one item is not allowed as the first argument of concat() + 0 + +