Merge branch 'master' into metrics-prework

This commit is contained in:
Oowekyala
2017-05-05 14:52:39 +02:00
106 changed files with 2169 additions and 1433 deletions

View File

@ -7,7 +7,7 @@
<parent>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd</artifactId>
<version>5.6.0-SNAPSHOT</version>
<version>5.7.0-SNAPSHOT</version>
</parent>
<properties>
@ -111,7 +111,7 @@
<plugin>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-build</artifactId>
<artifactId>pmd-build-tools-plugin</artifactId>
</plugin>
</plugins>
</build>

View File

@ -72,19 +72,22 @@ public class AccessorClassGenerationRule extends AbstractJavaRule {
final List<ASTConstructorDeclaration> constructors = privateConstructors.get(type.getImage());
if (constructors != null) {
final ASTArguments callArguments = (ASTArguments) node.jjtGetChild(1);
final ClassScope enclosingScope = node.getScope().getEnclosingScope(ClassScope.class);
for (final ASTConstructorDeclaration cd : constructors) {
// Are we within the same class scope?
if (cd.getScope().getEnclosingScope(ClassScope.class) == enclosingScope) {
break;
}
if (cd.getParameterCount() == callArguments.getArgumentCount()) {
// TODO : Check types
addViolation(data, node);
break;
final ASTArguments callArguments = node.getFirstChildOfType(ASTArguments.class);
// Is this really a constructor call and not an array?
if (callArguments != null) {
final ClassScope enclosingScope = node.getScope().getEnclosingScope(ClassScope.class);
for (final ASTConstructorDeclaration cd : constructors) {
// Are we within the same class scope?
if (cd.getScope().getEnclosingScope(ClassScope.class) == enclosingScope) {
break;
}
if (cd.getParameterCount() == callArguments.getArgumentCount()) {
// TODO : Check types
addViolation(data, node);
break;
}
}
}
}

View File

@ -40,6 +40,8 @@ public class FieldDeclarationsShouldBeAtStartOfClassRule extends AbstractJavaRul
"Ignore Enum Declarations that precede fields.", true, 1.0f);
private BooleanProperty ignoreAnonymousClassDeclarations = new BooleanProperty("ignoreAnonymousClassDeclarations",
"Ignore Field Declarations, that are initialized with anonymous class declarations", true, 2.0f);
private BooleanProperty ignoreInterfaceDeclarations = new BooleanProperty("ignoreInterfaceDeclarations",
"Ignore Interface Declarations that precede fields.", false, 3.0f);
/**
* Initializes the rule {@link FieldDeclarationsShouldBeAtStartOfClassRule}.
@ -47,6 +49,7 @@ public class FieldDeclarationsShouldBeAtStartOfClassRule extends AbstractJavaRul
public FieldDeclarationsShouldBeAtStartOfClassRule() {
definePropertyDescriptor(ignoreEnumDeclarations);
definePropertyDescriptor(ignoreAnonymousClassDeclarations);
definePropertyDescriptor(ignoreInterfaceDeclarations);
}
@Override
@ -67,11 +70,20 @@ public class FieldDeclarationsShouldBeAtStartOfClassRule extends AbstractJavaRul
&& getProperty(ignoreAnonymousClassDeclarations).booleanValue()) {
continue;
}
if (child instanceof ASTClassOrInterfaceDeclaration || child instanceof ASTMethodDeclaration
|| child instanceof ASTConstructorDeclaration || child instanceof ASTAnnotationTypeDeclaration) {
if (child instanceof ASTMethodDeclaration || child instanceof ASTConstructorDeclaration
|| child instanceof ASTAnnotationTypeDeclaration) {
addViolation(data, node);
break;
}
if (child instanceof ASTClassOrInterfaceDeclaration) {
ASTClassOrInterfaceDeclaration declaration = (ASTClassOrInterfaceDeclaration) child;
if (declaration.isInterface() && getProperty(ignoreInterfaceDeclarations).booleanValue()) {
continue;
} else {
addViolation(data, node);
break;
}
}
if (child instanceof ASTEnumDeclaration && !getProperty(ignoreEnumDeclarations).booleanValue()) {
addViolation(data, node);
break;

View File

@ -8,6 +8,7 @@ import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
@ -18,12 +19,22 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
import net.sourceforge.pmd.lang.java.ast.AccessNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.rule.properties.BooleanProperty;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.lang.symboltable.Scope;
import net.sourceforge.pmd.lang.symboltable.ScopedNode;
public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
private static final BooleanProperty STATEMENT_ORDER_MATTERS = new BooleanProperty("statementOrderMatters",
"If set to false this rule no longer requires the variable declaration and return statement to be "
+ "on consecutive lines. Any variable that is used solely in a return statement will be reported.",
true, 1.0f);
public UnnecessaryLocalBeforeReturnRule() {
definePropertyDescriptor(STATEMENT_ORDER_MATTERS);
}
@Override
public Object visit(ASTMethodDeclaration meth, Object data) {
// skip void/abstract/native method
@ -61,8 +72,10 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
if (var.indexOf('.') != -1) {
var = var.substring(0, var.indexOf('.'));
}
// Is the variable initialized with another member that is later used?
if (!isInitDataModifiedAfterInit(variableDeclaration, rtn)) {
if (!isInitDataModifiedAfterInit(variableDeclaration, rtn)
&& !statementsBeforeReturn(variableDeclaration, rtn)) {
addViolation(data, rtn, var);
}
}
@ -71,6 +84,21 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
return data;
}
private boolean statementsBeforeReturn(VariableNameDeclaration variableDeclaration, ASTReturnStatement returnStatement) {
if (!getProperty(STATEMENT_ORDER_MATTERS)) {
return false;
}
ASTBlockStatement declarationStatement = variableDeclaration.getAccessNodeParent().getFirstParentOfType(ASTBlockStatement.class);
ASTBlockStatement returnBlockStatement = returnStatement.getFirstParentOfType(ASTBlockStatement.class);
// double check: we should now be at the same level in the AST - both block statements are children of the same parent
if (declarationStatement.jjtGetParent() == returnBlockStatement.jjtGetParent()) {
return returnBlockStatement.jjtGetChildIndex() - declarationStatement.jjtGetChildIndex() > 1;
}
return false;
}
private boolean isInitDataModifiedAfterInit(final VariableNameDeclaration variableDeclaration,
final ASTReturnStatement rtn) {
final ASTVariableInitializer initializer = variableDeclaration.getAccessNodeParent()

View File

@ -8,6 +8,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
public class DontImportJavaLangRule extends AbstractJavaRule {
private static final String IMPORT_JAVA_LANG = "java.lang";
@Override
public Object visit(ASTImportDeclaration node, Object data) {
@ -17,12 +18,12 @@ public class DontImportJavaLangRule extends AbstractJavaRule {
}
String img = node.jjtGetChild(0).getImage();
if (img.startsWith("java.lang")) {
if (img.startsWith("java.lang.ref") || img.startsWith("java.lang.reflect")
|| img.startsWith("java.lang.annotation") || img.startsWith("java.lang.instrument")
|| img.startsWith("java.lang.management") || img.startsWith("java.lang.Thread.")
|| img.startsWith("java.lang.ProcessBuilder.")) {
return data;
if (img.startsWith(IMPORT_JAVA_LANG)) {
if (!IMPORT_JAVA_LANG.equals(img)) {
if (img.indexOf('.', IMPORT_JAVA_LANG.length() + 1) != -1 || node.isImportOnDemand()) {
// Importing from a subpackage / inner class
return data;
}
}
addViolation(data, node);
}

View File

@ -124,10 +124,13 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
if (expression != null) {
ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class);
if (pe != null) {
String img = pe.jjtGetChild(0).jjtGetChild(0).getImage();
if (img != null && (img.startsWith("assert") || img.startsWith("fail")
|| img.startsWith("Assert.assert") || img.startsWith("Assert.fail"))) {
return true;
Node name = pe.getFirstDescendantOfType(ASTName.class);
if (name != null) {
String img = name.getImage();
if (img != null && (img.startsWith("assert") || img.startsWith("fail")
|| img.startsWith("Assert.assert") || img.startsWith("Assert.fail"))) {
return true;
}
}
}
}
@ -138,9 +141,16 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
Map<String, List<NameOccurrence>> expectables) {
if (expression != null) {
ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class);
if (pe != null) {
String img = pe.jjtGetChild(0).jjtGetChild(0).getImage();
Node name = pe.getFirstDescendantOfType(ASTName.class);
// case of an AllocationExpression
if (name == null) {
return false;
}
String img = name.getImage();
if (img.indexOf(".") == -1) {
return false;
}
@ -151,7 +161,7 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
}
for (NameOccurrence occ : expectables.get(varname)) {
if (occ.getLocation() == pe.jjtGetChild(0).jjtGetChild(0) && img.startsWith(varname + ".expect")) {
if (occ.getLocation() == name && img.startsWith(varname + ".expect")) {
return true;
}
}
@ -160,4 +170,3 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule {
return false;
}
}

View File

@ -2,7 +2,7 @@
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.unusedcode;
package net.sourceforge.pmd.lang.java.rule.unnecessary;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotationMethodDeclaration;
@ -13,7 +13,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
public class UnusedModifierRule extends AbstractJavaRule {
public class UnnecessaryModifierRule extends AbstractJavaRule {
@Override
public Object visit(ASTEnumDeclaration node, Object data) {

View File

@ -26,6 +26,7 @@ public class SourceFileScope extends AbstractJavaScope {
private final String packageImage;
private final TypeSet types;
private Map<String, Node> qualifiedTypeNames;
public SourceFileScope(final ClassLoader classLoader) {
this(classLoader, "");
@ -136,7 +137,12 @@ public class SourceFileScope extends AbstractJavaScope {
* @return set of all types in this source file.
*/
public Map<String, Node> getQualifiedTypeNames() {
return getSubTypes(null, this);
if (qualifiedTypeNames != null) {
return qualifiedTypeNames;
}
qualifiedTypeNames = getSubTypes(null, this);
return qualifiedTypeNames;
}
private Map<String, Node> getSubTypes(String qualifyingName, Scope subType) {

View File

@ -342,4 +342,41 @@ public class Foo {
</example>
</rule>
<rule name="UnnecessaryModifier"
language="java"
since="1.02"
message="Avoid modifiers which are implied by the context"
class="net.sourceforge.pmd.lang.java.rule.unnecessary.UnnecessaryModifierRule"
externalInfoUrl="${pmd.website.baseurl}/rules/java/unnecessary.html#UnnecessaryModifier">
<description>
Fields in interfaces and annotations are automatically `public static final`, and methods are `public abstract`.
Classes, interfaces or annotations nested in an interface or annotation are automatically `public static`
(all nested interfaces and annotations are automatically static).
Nested enums are automatically `static`.
For historical reasons, modifiers which are implied by the context are accepted by the compiler, but are superfluous.
</description>
<priority>3</priority>
<example>
<![CDATA[
public @interface Annotation {
public abstract void bar(); // both abstract and public are ignored by the compiler
public static final int X = 0; // public, static, and final all ignored
public static class Bar {} // public, static ignored
public static interface Baz {} // ditto
}
public interface Foo {
public abstract void bar(); // both abstract and public are ignored by the compiler
public static final int X = 0; // public, static, and final all ignored
public static class Bar {} // public, static ignored
public static interface Baz {} // ditto
}
public class Bar {
public static interface Baz {} // static ignored
public static enum FoorBar { // static ignored
FOO;
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -95,42 +95,4 @@ public class Foo {
]]>
</example>
</rule>
<rule name="UnusedModifier"
language="java"
since="1.02"
message="Avoid modifiers which are implied by the context"
class="net.sourceforge.pmd.lang.java.rule.unusedcode.UnusedModifierRule"
externalInfoUrl="${pmd.website.baseurl}/rules/java/unusedcode.html#UnusedModifier">
<description>
Fields in interfaces and annotations are automatically `public static final`, and methods are `public abstract`.
Classes, interfaces or annotations nested in an interface or annotation are automatically `public static`
(all nested interfaces and annotations are automatically static).
Nested enums are automatically `static`.
For historical reasons, modifiers which are implied by the context are accepted by the compiler, but are superfluous.
</description>
<priority>3</priority>
<example>
<![CDATA[
public @interface Annotation {
public abstract void bar(); // both abstract and public are ignored by the compiler
public static final int X = 0; // public, static, and final all ignored
public static class Bar {} // public, static ignored
public static interface Baz {} // ditto
}
public interface Foo {
public abstract void bar(); // both abstract and public are ignored by the compiler
public static final int X = 0; // public, static, and final all ignored
public static class Bar {} // public, static ignored
public static interface Baz {} // ditto
}
public class Bar {
public static interface Baz {} // static ignored
public static enum FoorBar { // static ignored
FOO;
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -18,6 +18,7 @@ public class UnnecessaryRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "UnnecessaryConversionTemporary");
addRule(RULESET, "UnnecessaryReturn");
addRule(RULESET, "UnnecessaryFinalModifier");
addRule(RULESET, "UnnecessaryModifier");
addRule(RULESET, "UnusedNullCheckInEquals");
addRule(RULESET, "UselessOverridingMethod");
addRule(RULESET, "UselessOperationOnImmutable");

View File

@ -22,6 +22,5 @@ public class UnusedCodeRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "UnusedLocalVariable");
addRule(RULESET, "UnusedPrivateField");
addRule(RULESET, "UnusedPrivateMethod");
addRule(RULESET, "UnusedModifier");
}
}

View File

@ -128,6 +128,26 @@ public class Example extends View {
}
});
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Array initializer is not a class body
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
private class Bar {
private int size;
private Bar() {
}
void bar() {
new Bar[size];
}
}
}
]]></code>
</test-code>

View File

@ -194,6 +194,36 @@ public class MyEntity {
public MyEntity() {
}
}
]]></code>
</test-code>
<test-code>
<description>#345 [java] FieldDeclarationsShouldBeAtStartOfClass: Add ability to ignore interfaces - default case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
public class TestFieldDeclarations {
public interface StartCaptureListener {
void onStartCapture(boolean shouldUploadMediaToTba);
}
private static final int TAKE_PHOTO_RC = 334; // line 6
}
]]></code>
</test-code>
<test-code>
<description>#345 [java] FieldDeclarationsShouldBeAtStartOfClass: Add ability to ignore interfaces - ignore interfaces</description>
<expected-problems>0</expected-problems>
<rule-property name="ignoreInterfaceDeclarations">true</rule-property>
<code><![CDATA[
public class TestFieldDeclarations {
public interface StartCaptureListener {
void onStartCapture(boolean shouldUploadMediaToTba);
}
private static final int TAKE_PHOTO_RC = 334; // line 6
}
]]></code>
</test-code>

View File

@ -94,10 +94,11 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>Detect violation even if not on consecutive lines</description>
<expected-problems>1</expected-problems>
<rule-property name="statementOrderMatters">false</rule-property>
<code><![CDATA[
public class Foo {
public int bar() {
@ -110,7 +111,7 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>No violations on multiple uses of the variable</description>
<expected-problems>0</expected-problems>
@ -127,6 +128,23 @@ public class Foo {
]]></code>
</test-code>
<test-code>
<description>No violations on multiple uses of the variable - statement order does not matter</description>
<expected-problems>0</expected-problems>
<rule-property name="statementOrderMatters">false</rule-property>
<code><![CDATA[
public class Foo {
public int bar() {
int res = 2;
doSomething(res);
return res;
}
public void doSomething(int x) { }
}
]]></code>
</test-code>
<test-code>
<description>#933 UnnecessaryLocalBeforeReturn false positive for SuppressWarnings annotation</description>
<expected-problems>0</expected-problems>
@ -164,6 +182,50 @@ public class CustomerErrorCollector {
customerErrors.clear();
return copy; // PMD complains that variable could be avoided
}
}
]]></code>
</test-code>
<test-code>
<description>#310 UnnecessaryLocalBeforeReturn enhancement is overly restrictive -- method order matters</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class UnnecessaryLocalBeforeReturnFP {
public int example1() {
int i = compute(); // might throw
markComputationDone();
return i; // PMD complains that variable could be avoided
}
public int example2() {
Mutable m = new Mutable();
int i = compute(m);
sideEffect(m);
return i;
}
}
]]></code>
</test-code>
<test-code>
<description>#310 UnnecessaryLocalBeforeReturn statement order does not matter</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>5</expected-linenumbers>
<rule-property name="statementOrderMatters">false</rule-property>
<code><![CDATA[
public class UnnecessaryLocalBeforeReturnFP {
public int example1() {
int i = compute(); // might throw
markComputationDone();
return i; // PMD complains that variable could be avoided
}
public int example2() {
Mutable m = new Mutable();
int i = compute(m);
sideEffect(m);
return i;
}
}
]]></code>
</test-code>

View File

@ -31,6 +31,7 @@ import java.lang.reflect.*;
import java.lang.annotation.*;
import java.lang.instrument.*;
import java.lang.management.*;
import java.lang.ProcessBuilder.*;
public class Foo {}
]]></code>
</test-code>
@ -61,6 +62,15 @@ public class Foo {}
<code><![CDATA[
import java.lang.ProcessBuilder.Redirect;
public class Foo {}
]]></code>
</test-code>
<test-code>
<description>import java.lang.invoke.MethodHandles: #339 false DontImportJavaLang</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.lang.invoke.MethodHandles;
public class Foo {}
]]></code>
</test-code>

View File

@ -312,4 +312,32 @@ public class SimpleExpectedExceptionTest {
}
]]></code>
</test-code>
<test-code>
<description>#330 Rule treats AllocationExpression correctly</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import org.junit.*;
import javafx.embed.swing.JFXPanel;
public class BaseConsoleTest extends UART {
@Test
public void testInitialize() throws InterruptedException {
new JFXPanel(); // AllocationExpression
}
}
]]></code>
</test-code>
<test-code>
<description>#330 NPE accessing local field / method with this.XXX</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
@Test
public void testName() {
this.field = null;
}
}
]]></code>
</test-code>
</test-data>