phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'master' into pr/3409

Browse Source
This commit is contained in:
Clément Fournier
2021-07-29 13:20:42 +02:00
parent d9adde3d96 0bc7b4fb8c
commit ff805ae7c5
22 changed files with 1706 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
.ci/build.sh
View File

@ -85,6 +85,23 @@ function build() {
pmd_ci_log_group_end
if pmd_ci_maven_isSnapshotBuild; then
if [ "${PMD_CI_MAVEN_PROJECT_VERSION}" != "7.0.0-SNAPSHOT" ]; then
pmd_ci_log_group_start "Executing PMD dogfood test with ${PMD_CI_MAVEN_PROJECT_VERSION}"
./mvnw versions:set -DnewVersion=${PMD_CI_MAVEN_PROJECT_VERSION}-dogfood -DgenerateBackupPoms=false
./mvnw verify --show-version --errors --batch-mode --no-transfer-progress "${PMD_MAVEN_EXTRA_OPTS[@]}" \
-DskipTests \
-Dmaven.javadoc.skip=true \
-Dmaven.source.skip=true \
-Dcheckstyle.skip=true \
-Ppmd-dogfood \
-Dpmd.dogfood.version=${PMD_CI_MAVEN_PROJECT_VERSION}
./mvnw versions:set -DnewVersion=${PMD_CI_MAVEN_PROJECT_VERSION} -DgenerateBackupPoms=false
pmd_ci_log_group_end
else
# current maven-pmd-plugin is not compatible with PMD 7 yet.
pmd_ci_log_info "Skipping PMD dogfood test with ${PMD_CI_MAVEN_PROJECT_VERSION}"
fi
pmd_ci_log_group_start "Executing build with sonar"
# Note: Sonar also needs GITHUB_TOKEN (!)
./mvnw \

28
.ci/inc/regression-tester.inc
View File

@ -70,19 +70,21 @@ function regression_tester_uploadBaseline() {
function regression_tester_executeDanger() {
pmd_ci_log_debug "${FUNCNAME[0]}"
# Create a corresponding remote branch locally
if ! git show-ref --verify --quiet "refs/heads/${PMD_CI_BRANCH}"; then
git fetch --no-tags --depth=1 origin "+refs/heads/${PMD_CI_BRANCH}:refs/remotes/origin/${PMD_CI_BRANCH}"
git branch "${PMD_CI_BRANCH}" "origin/${PMD_CI_BRANCH}"
pmd_ci_log_debug "Created local branch ${PMD_CI_BRANCH}"
fi
# Fetch more commits of the PR for danger and regression tester
git fetch --no-tags --depth=50 origin "+$(git rev-parse HEAD^2):"
# Fetch more commits from master branch for regression tester
if [[ "${PMD_CI_BRANCH}" != "master" ]]; then
git fetch --no-tags --depth=50 origin +master:
git branch master origin/master
fi
# git clone initially only fetched with depth 2. Danger and regression tester
# need more history, so we'll fetch more here
# and create local branches as well (${PMD_CI_BRANCH} and pr-fetch)
pmd_ci_log_info "Fetching 25 commits for ${PMD_CI_BRANCH} and pull/${PMD_CI_PULL_REQUEST_NUMBER}/head"
git fetch --no-tags --depth=25 origin "${PMD_CI_BRANCH}:${PMD_CI_BRANCH}" "pull/${PMD_CI_PULL_REQUEST_NUMBER}/head:pr-fetch"
# if the PR is older, base might have advanced more than 25 commits... fetch more, up to 150
for i in $(seq 1 3); do
if [ -z "$( git merge-base "${PMD_CI_BRANCH}" "pr-fetch" )" ]; then
pmd_ci_log_info "No merge-base yet - fetching more commits... (try $i)"
git fetch --no-tags --deepen=50 origin "${PMD_CI_BRANCH}:" "pull/${PMD_CI_PULL_REQUEST_NUMBER}/head:pr-fetch"
fi
done
pmd_ci_log_info "Merge base is: $( git merge-base "${PMD_CI_BRANCH}" "pr-fetch" )"
pmd_ci_log_info "Running danger on branch ${PMD_CI_BRANCH}"
bundle exec danger --verbose

54
docs/pages/release_notes.md
View File

@ -14,14 +14,68 @@ This is a {{ site.pmd.release_type }} release.
### New and noteworthy
#### New rules
This release ships with 2 new Java rules.
* {% rule java/bestpractices/SimplifiableTestAssertion %} suggests rewriting
some test assertions to be more readable.
```xml
<rule ref="category/java/bestpractices.xml/SimplifiableTestAssertion" />
```
The rule is part of the quickstart.xml ruleset.
* {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} suggests returning empty collections / arrays
instead of null.
```xml
<rule ref="category/java/errorprone.xml/ReturnEmptyCollectionRatherThanNull" />
```
The rule is part of the quickstart.xml ruleset.
#### Renamed rules
* The Java rule {% rule java/errorprone/MissingBreakInSwitch %} has been renamed to
{% rule java/errorprone/ImplicitSwitchFallThrough %} (category error prone) to better reflect the rule's
purpose: The rule finds implicit fall-through cases in switch statements, which are most
likely unexpected. The old rule name described only one way how to avoid a fall-through,
namely using `break` but `continue`, `throw` and `return` avoid a fall-through
as well. This enables us to improve this rule in the future.
#### Deprecated rules
The following Java rules are deprecated and removed from the quickstart ruleset,
as the new rule {% rule java/bestpractices/SimplifiableTestAssertion %} merges
their functionality:
* {% rule java/bestpractices/UseAssertEqualsInsteadOfAssertTrue %}
* {% rule java/bestpractices/UseAssertNullInsteadOfAssertTrue %}
* {% rule java/bestpractices/UseAssertSameInsteadOfAssertTrue %}
* {% rule java/bestpractices/UseAssertTrueInsteadOfAssertEquals %}
* {% rule java/design/SimplifyBooleanAssertion %}
The rule {% rule java/errorprone/ReturnEmptyArrayRatherThanNull %} is deprecated and removed from
the quickstart ruleset, as the new rule {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %}
supersedes it.
### Fixed Issues
* apex
* [#3201](https://github.com/pmd/pmd/issues/3201): \[apex] ApexCRUDViolation doesn't report Database class DMLs, inline no-arg object instantiations and inline list initialization
* [#3329](https://github.com/pmd/pmd/issues/3329): \[apex] ApexCRUDViolation doesn't report SOQL for loops
* core
* [#1603](https://github.com/pmd/pmd/issues/1603): \[core] Language version comparison
* [#3377](https://github.com/pmd/pmd/issues/3377): \[core] NPE when specifying report file in current directory in PMD CLI
* [#3387](https://github.com/pmd/pmd/issues/3387): \[core] CPD should avoid unnecessary copies when running with --skip-lexical-errors
* java-bestpractices
* [#2908](https://github.com/pmd/pmd/issues/2908): \[java] Merge Junit assertion simplification rules
* [#3235](https://github.com/pmd/pmd/issues/3235): \[java] UseTryWithResources false positive when closeable is provided as a method argument or class field
* java-errorprone
* [#3361](https://github.com/pmd/pmd/issues/3361): \[java] Rename rule MissingBreakInSwitch to ImplicitSwitchFallThrough
* [#3382](https://github.com/pmd/pmd/pull/3382): \[java] New rule ReturnEmptyCollectionRatherThanNull
### API Changes

41
pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java
View File

@ -4,6 +4,8 @@
package net.sourceforge.pmd.lang;
import java.util.List;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.annotation.InternalApi;
@ -102,27 +104,28 @@ public class LanguageVersion implements Comparable<LanguageVersion> {
@Override
public int compareTo(LanguageVersion o) {
if (o == null) {
return 1;
}
List<LanguageVersion> versions = language.getVersions();
int thisPosition = versions.indexOf(this);
int otherPosition = versions.indexOf(o);
return Integer.compare(thisPosition, otherPosition);
}
int comp = getName().compareTo(o.getName());
if (comp != 0) {
return comp;
}
String[] vals1 = getName().split("\\.");
String[] vals2 = o.getName().split("\\.");
int i = 0;
while (i < vals1.length && i < vals2.length && vals1[i].equals(vals2[i])) {
i++;
}
if (i < vals1.length && i < vals2.length) {
int diff = Integer.valueOf(vals1[i]).compareTo(Integer.valueOf(vals2[i]));
return Integer.signum(diff);
} else {
return Integer.signum(vals1.length - vals2.length);
/**
* Compare this version to another version of the same language identified
* by the given version string.
*
* @param versionString The version with which to compare
*
* @throws IllegalArgumentException If the argument is not a valid version
* string for the parent language
*/
public int compareToVersion(String versionString) {
LanguageVersion otherVersion = language.getVersion(versionString);
if (otherVersion == null) {
throw new IllegalArgumentException(
"No such version '" + versionString + "' for language " + language.getName());
}
return this.compareTo(otherVersion);
}
@Override

327
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionRule.java Normal file
View File

File diff suppressed because it is too large Load Diff

121
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseTryWithResourcesRule.java Normal file
View File

@ -0,0 +1,121 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
import net.sourceforge.pmd.lang.java.ast.ASTFinallyStatement;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.MethodScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.properties.PropertyDescriptor;
public final class UseTryWithResourcesRule extends AbstractJavaRule {
private static final PropertyDescriptor<List<String>> CLOSE_METHODS =
stringListProperty("closeMethods")
.desc("Method names in finally block, which trigger this rule")
.defaultValues("close", "closeQuietly")
.delim(',')
.build();
public UseTryWithResourcesRule() {
addRuleChainVisit(ASTTryStatement.class);
definePropertyDescriptor(CLOSE_METHODS);
}
@Override
public Object visit(ASTTryStatement node, Object data) {
boolean isJava9OrLater = isJava9OrLater((RuleContext) data);
ASTFinallyStatement finallyClause = node.getFinallyClause();
if (finallyClause != null) {
List<ASTName> methods = findCloseMethods(finallyClause.findDescendantsOfType(ASTName.class));
for (ASTName method : methods) {
ASTName closeTarget = getCloseTarget(method);
if (TypeTestUtil.isA(AutoCloseable.class, closeTarget)
&& (isJava9OrLater || isLocalVar(closeTarget))) {
addViolation(data, node);
break; // only report the first closeable
}
}
}
return data;
}
private boolean isJava9OrLater(RuleContext ruleContext) {
String currentVersion = ruleContext.getLanguageVersion().getVersion();
currentVersion = currentVersion.replace("-preview", "");
return Double.parseDouble(currentVersion) >= 9;
}
private boolean isLocalVar(ASTName closeTarget) {
NameDeclaration nameDeclaration = closeTarget.getNameDeclaration();
if (nameDeclaration instanceof VariableNameDeclaration) {
ASTVariableDeclaratorId id = ((VariableNameDeclaration) nameDeclaration).getDeclaratorId();
return id.isLocalVariable();
} else if (closeTarget.getImage().contains(".")) {
// this is a workaround for a bug in the symbol table:
// the name might be resolved to a wrong method
int lastDot = closeTarget.getImage().lastIndexOf('.');
String varName = closeTarget.getImage().substring(0, lastDot);
Map<VariableNameDeclaration, List<NameOccurrence>> vars = closeTarget.getScope()
.getEnclosingScope(MethodScope.class)
.getDeclarations(VariableNameDeclaration.class);
for (VariableNameDeclaration varDecl : vars.keySet()) {
if (varDecl.getName().equals(varName)) {
return varDecl.getDeclaratorId().isLocalVariable();
}
}
}
return false;
}
private ASTName getCloseTarget(ASTName method) {
ASTArguments arguments = method.getNthParent(2).getFirstDescendantOfType(ASTArguments.class);
if (arguments.size() > 0) {
ASTName firstArgument = arguments.getChild(0).getChild(0).getFirstDescendantOfType(ASTName.class);
if (firstArgument != null) {
return firstArgument;
}
}
return method;
}
private List<ASTName> findCloseMethods(List<ASTName> names) {
List<ASTName> potentialCloses = new ArrayList<>();
for (ASTName name : names) {
String image = name.getImage();
int lastDot = image.lastIndexOf('.');
if (lastDot > -1) {
image = image.substring(lastDot + 1);
}
if (getProperty(CLOSE_METHODS).contains(image) && isMethodCall(name)) {
potentialCloses.add(name);
}
}
return potentialCloses;
}
private boolean isMethodCall(ASTName potentialMethodCall) {
return potentialMethodCall.getNthParent(2) instanceof ASTPrimaryExpression
&& !potentialMethodCall.getNthParent(2).findChildrenOfType(ASTPrimarySuffix.class).isEmpty();
}
}

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BigIntegerInstantiationRule.java
View File

@ -8,9 +8,7 @@ import java.math.BigDecimal;
import java.math.BigInteger;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits;
@ -34,8 +32,7 @@ public class BigIntegerInstantiationRule extends AbstractJavaRule {
return super.visit(node, data);
}
boolean jdk15 = ((RuleContext) data).getLanguageVersion()
.compareTo(LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion("1.5")) >= 0;
boolean jdk15 = ((RuleContext) data).getLanguageVersion().compareToVersion("1.5") >= 0;
if ((TypeTestUtil.isA(BigInteger.class, (ASTClassOrInterfaceType) type)
|| jdk15 && TypeTestUtil.isA(BigDecimal.class, (ASTClassOrInterfaceType) type))
&& !node.hasDescendantOfType(ASTArrayDimsAndInits.class)) {

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationRule.java
View File

@ -7,9 +7,7 @@ package net.sourceforge.pmd.lang.java.rule.performance;
import java.util.Set;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
@ -36,8 +34,7 @@ public class UnnecessaryWrapperObjectCreationRule extends AbstractJavaRule {
image = image.substring(10);
}
boolean checkBoolean = ((RuleContext) data).getLanguageVersion()
.compareTo(LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion("1.5")) >= 0;
boolean checkBoolean = ((RuleContext) data).getLanguageVersion().compareToVersion("1.5") >= 0;
if (PREFIX_SET.contains(image) || checkBoolean && "Boolean.valueOf".equals(image)) {
ASTPrimaryExpression parent = (ASTPrimaryExpression) node.getParent();

75
pmd-java/src/main/resources/category/java/bestpractices.xml
View File

@ -1364,6 +1364,50 @@ public class Foo {
</example>
</rule>
<rule name="SimplifiableTestAssertion"
language="java"
since="6.37.0"
message="Assertion may be simplified using {0}"
typeResolution="true"
class="net.sourceforge.pmd.lang.java.rule.bestpractices.SimplifiableTestAssertionRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#simplifiabletestassertion">
<description>
Reports test assertions that may be simplified using a more specific
assertion method. This enables better error messages, and makes the
assertions more readable.
The rule only applies within test classes for the moment. It replaces
the deprecated rules {% rule UseAssertEqualsInsteadOfAssertTrue %},
{% rule UseAssertNullInsteadOfAssertTrue %}, {% rule UseAssertSameInsteadOfAssertTrue %},
{% rule UseAssertTrueInsteadOfAssertEquals %}, and {% rule java/design/SimplifyBooleanAssertion %}.
</description>
<priority>3</priority>
<example>
<![CDATA[
import org.junit.Test;
import static org.junit.Assert.*;
class SomeTestClass {
Object a,b;
@Test
void testMethod() {
assertTrue(a.equals(b)); // could be assertEquals(a, b);
assertTrue(!a.equals(b)); // could be assertNotEquals(a, b);
assertTrue(!something); // could be assertFalse(something);
assertFalse(!something); // could be assertTrue(something);
assertTrue(a == b); // could be assertSame(a, b);
assertTrue(a != b); // could be assertNotSame(a, b);
assertTrue(a == null); // could be assertNull(a);
assertTrue(a != null); // could be assertNotNull(a);
}
}
]]>
</example>
</rule>
<rule name="SwitchStmtsShouldHaveDefault"
language="java"
since="1.0"
@ -1690,9 +1734,12 @@ public class Something {
message="Use assertEquals(x, y) instead of assertTrue(x.equals(y))"
class="net.sourceforge.pmd.lang.rule.XPathRule"
typeResolution="true"
deprecated="true"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useassertequalsinsteadofasserttrue">
<description>
This rule detects JUnit assertions in object equality. These assertions should be made by more specific methods, like assertEquals.
Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead.
</description>
<priority>3</priority>
<properties>
@ -1740,10 +1787,13 @@ public class FooTest extends TestCase {
message="Use assertNull(x) instead of assertTrue(x==null), or assertNotNull(x) vs assertFalse(x==null)"
class="net.sourceforge.pmd.lang.rule.XPathRule"
typeResolution="true"
deprecated="true"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useassertnullinsteadofasserttrue">
<description>
This rule detects JUnit assertions in object references equality. These assertions should be made by
more specific methods, like assertNull, assertNotNull.
Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead.
</description>
<priority>3</priority>
<properties>
@ -1794,10 +1844,13 @@ public class FooTest extends TestCase {
message="Use assertSame(x, y) instead of assertTrue(x==y), or assertNotSame(x,y) vs assertFalse(x==y)"
class="net.sourceforge.pmd.lang.rule.XPathRule"
typeResolution="true"
deprecated="true"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useassertsameinsteadofasserttrue">
<description>
This rule detects JUnit assertions in object references equality. These assertions should be made
by more specific methods, like assertSame, assertNotSame.
Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead.
</description>
<priority>3</priority>
<properties>
@ -1845,9 +1898,12 @@ public class FooTest extends TestCase {
since="5.0"
message="Use assertTrue(x)/assertFalse(x) instead of assertEquals(true, x)/assertEquals(false, x) or assertEquals(Boolean.TRUE, x)/assertEquals(Boolean.FALSE, x)."
class="net.sourceforge.pmd.lang.rule.XPathRule"
deprecated="true"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useasserttrueinsteadofassertequals">
<description>
When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals.
Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead.
</description>
<priority>3</priority>
<properties>
@ -1970,7 +2026,7 @@ public class UseStandardCharsets {
since="6.12.0"
typeResolution="true"
message="Consider using a try-with-resources statement instead of explicitly closing the resource"
class="net.sourceforge.pmd.lang.rule.XPathRule"
class="net.sourceforge.pmd.lang.java.rule.bestpractices.UseTryWithResourcesRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#usetrywithresources">
<description>
Java 7 introduced the try-with-resources statement. This statement ensures that each resource is closed at the end
@ -1980,23 +2036,6 @@ the try block was suppressed. With the `try`-with-resources statement, the excep
preserved.
</description>
<priority>3</priority>
<properties>
<property name="closeMethods" type="List[String]" delimiter="," description="Method names in finally block, which trigger this rule" value="close,closeQuietly"/>
<property name="version" value="2.0" />
<property name="xpath">
<value>
<![CDATA[
//TryStatement[FinallyStatement//Name[
tokenize(@Image, '\.')[last()] = $closeMethods
][
pmd-java:typeIs('java.lang.AutoCloseable')
or
../../PrimarySuffix/Arguments//PrimaryPrefix[pmd-java:typeIs('java.lang.AutoCloseable')]
]]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public class TryWithResources {

2
pmd-java/src/main/resources/category/java/design.xml
View File

@ -1327,6 +1327,7 @@ public class Foo {
message="assertTrue(!expr) can be replaced by assertFalse(expr)"
class="net.sourceforge.pmd.lang.rule.XPathRule"
typeResolution="true"
deprecated="true"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#simplifybooleanassertion">
<description>
Avoid negation in an assertTrue or assertFalse test.
@ -1339,6 +1340,7 @@ as:
assertFalse(expr);
Deprecated since PMD 6.37.0, use {% rule java/bestpractices/SimplifiableTestAssertion %} instead.
</description>
<priority>3</priority>
<properties>

157
pmd-java/src/main/resources/category/java/errorprone.xml
View File

@ -2130,6 +2130,60 @@ public class Foo {
</example>
</rule>
<rule name="ImplicitSwitchFallThrough"
language="java"
since="3.0"
message="A switch statement does not contain a break"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#implicitswitchfallthrough">
<description>
Switch statements without break or return statements for each case option
may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.
This rule has been renamed from "MissingBreakInSwitch" with PMD 6.37.0.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//SwitchStatement
[(count(.//BreakStatement)
+ count(BlockStatement//Statement/ReturnStatement)
+ count(BlockStatement//Statement/ContinueStatement)
+ count(BlockStatement//Statement/ThrowStatement)
+ count(BlockStatement//Statement/IfStatement[@Else= true() and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement])
+ count(SwitchLabel[ following-sibling::node()[1][self::SwitchLabel] ])
+ count(SwitchLabel[count(following-sibling::node()) = 0])
< count (SwitchLabel))]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public void bar(int status) {
switch(status) {
case CANCELLED:
doCancelled();
// break; hm, should this be commented out?
case NEW:
doNew();
// is this really a fall-through?
case REMOVED:
doRemoved();
// what happens if you add another case after this one?
case OTHER: // empty case - this is interpreted as an intentional fall-through
case ERROR:
doErrorHandling();
break;
}
}
]]>
</example>
</rule>
<rule name="ImportFromSamePackage"
language="java"
since="1.02"
@ -2434,57 +2488,7 @@ public class Foo {
</example>
</rule>
<rule name="MissingBreakInSwitch"
language="java"
since="3.0"
message="A switch statement does not contain a break"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#missingbreakinswitch">
<description>
Switch statements without break or return statements for each case option
may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//SwitchStatement
[(count(.//BreakStatement)
+ count(BlockStatement//Statement/ReturnStatement)
+ count(BlockStatement//Statement/ContinueStatement)
+ count(BlockStatement//Statement/ThrowStatement)
+ count(BlockStatement//Statement/IfStatement[@Else= true() and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement])
+ count(SwitchLabel[ following-sibling::node()[1][self::SwitchLabel] ])
+ count(SwitchLabel[count(following-sibling::node()) = 0])
< count (SwitchLabel))]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public void bar(int status) {
switch(status) {
case CANCELLED:
doCancelled();
// break; hm, should this be commented out?
case NEW:
doNew();
// is this really a fall-through?
case REMOVED:
doRemoved();
// what happens if you add another case after this one?
case OTHER: // empty case - this is interpreted as an intentional fall-through
case ERROR:
doErrorHandling();
break;
}
}
]]>
</example>
</rule>
<rule name="MissingBreakInSwitch" ref="ImplicitSwitchFallThrough" deprecated="true"/>
<rule name="MissingSerialVersionUID"
language="java"
@ -2856,12 +2860,15 @@ public class Foo {
language="java"
since="4.2"
class="net.sourceforge.pmd.lang.rule.XPathRule"
deprecated="true"
message="Return an empty array rather than 'null'."
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#returnemptyarrayratherthannull">
<description>
For any method that returns an array, it is a better to return an empty array rather than a
null reference. This removes the need for null checking all results and avoids inadvertent
NullPointerExceptions.
Deprecated since PMD 6.37.0, use {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} instead.
</description>
<priority>1</priority>
<properties>
@ -2898,6 +2905,54 @@ public class Example {
</example>
</rule>
<rule name="ReturnEmptyCollectionRatherThanNull"
language="java"
since="6.37.0"
class="net.sourceforge.pmd.lang.rule.XPathRule"
message="Return an empty collection rather than 'null'."
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#returnemptycollectionratherthannull">
<description>
For any method that returns an collection (such as an array, Collection or Map), it is better to return
an empty one rather than a null reference. This removes the need for null checking all results and avoids
inadvertent NullPointerExceptions.
See Effective Java, 3rd Edition, Item 54: Return empty collections or arrays instead of null
</description>
<priority>1</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//MethodDeclaration
[
(./ResultType/Type[pmd-java:typeIs('java.util.Collection') or pmd-java:typeIs('java.util.Map') or @ArrayType=true()])
and
(./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral)
]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public class Example {
// Not a good idea...
public int[] badBehavior() {
// ...
return null;
}
// Good behavior
public String[] bonnePratique() {
//...
return new String[0];
}
}
]]>
</example>
</rule>
<rule name="ReturnFromFinallyBlock"
language="java"
since="1.05"

10
pmd-java/src/main/resources/rulesets/java/quickstart.xml
View File

@ -41,6 +41,7 @@
<!-- <rule ref="category/java/bestpractices.xml/ReplaceEnumerationWithIterator" /> -->
<!-- <rule ref="category/java/bestpractices.xml/ReplaceHashtableWithMap" /> -->
<!-- <rule ref="category/java/bestpractices.xml/ReplaceVectorWithList" /> -->
<rule ref="category/java/bestpractices.xml/SimplifiableTestAssertion"/>
<rule ref="category/java/bestpractices.xml/SwitchStmtsShouldHaveDefault"/>
<!-- <rule ref="category/java/bestpractices.xml/SystemPrintln" /> -->
<!-- <rule ref="category/java/bestpractices.xml/UnusedAssignment"/> -->
@ -48,10 +49,6 @@
<rule ref="category/java/bestpractices.xml/UnusedLocalVariable"/>
<rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
<rule ref="category/java/bestpractices.xml/UseAssertEqualsInsteadOfAssertTrue"/>
<rule ref="category/java/bestpractices.xml/UseAssertNullInsteadOfAssertTrue"/>
<rule ref="category/java/bestpractices.xml/UseAssertSameInsteadOfAssertTrue"/>
<rule ref="category/java/bestpractices.xml/UseAssertTrueInsteadOfAssertEquals"/>
<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
<rule ref="category/java/bestpractices.xml/UseStandardCharsets" />
<!-- <rule ref="category/java/bestpractices.xml/UseTryWithResources" /> -->
@ -155,7 +152,6 @@
<!-- <rule ref="category/java/design.xml/NPathComplexity" /> -->
<!-- <rule ref="category/java/design.xml/SignatureDeclareThrowsException" /> -->
<rule ref="category/java/design.xml/SimplifiedTernary"/>
<!-- <rule ref="category/java/design.xml/SimplifyBooleanAssertion" /> -->
<!-- <rule ref="category/java/design.xml/SimplifyBooleanExpressions" /> -->
<rule ref="category/java/design.xml/SimplifyBooleanReturns"/>
<rule ref="category/java/design.xml/SimplifyConditional"/>
@ -227,6 +223,7 @@
<!-- <rule ref="category/java/errorprone.xml/FinalizeOverloaded" /> -->
<!-- <rule ref="category/java/errorprone.xml/FinalizeShouldBeProtected" /> -->
<rule ref="category/java/errorprone.xml/IdempotentOperations"/>
<rule ref="category/java/errorprone.xml/ImplicitSwitchFallThrough"/>
<rule ref="category/java/errorprone.xml/InstantiationToGetClass"/>
<!-- <rule ref="category/java/errorprone.xml/InvalidLogMessageFormat" /> -->
<rule ref="category/java/errorprone.xml/JumbledIncrementer"/>
@ -235,7 +232,6 @@
<!-- <rule ref="category/java/errorprone.xml/LoggerIsNotStaticFinal" /> -->
<!-- <rule ref="category/java/errorprone.xml/MethodWithSameNameAsEnclosingClass" /> -->
<rule ref="category/java/errorprone.xml/MisplacedNullCheck"/>
<rule ref="category/java/errorprone.xml/MissingBreakInSwitch"/>
<!-- <rule ref="category/java/errorprone.xml/MissingSerialVersionUID" /> -->
<rule ref="category/java/errorprone.xml/MissingStaticMethodInNonInstantiatableClass"/>
<!-- <rule ref="category/java/errorprone.xml/MoreThanOneLogger" /> -->
@ -245,7 +241,7 @@
<rule ref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode"/>
<rule ref="category/java/errorprone.xml/ProperCloneImplementation"/>
<rule ref="category/java/errorprone.xml/ProperLogger"/>
<rule ref="category/java/errorprone.xml/ReturnEmptyArrayRatherThanNull"/>
<rule ref="category/java/errorprone.xml/ReturnEmptyCollectionRatherThanNull"/>
<rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>
<!-- <rule ref="category/java/errorprone.xml/SimpleDateFormatNeedsLocale" /> -->
<rule ref="category/java/errorprone.xml/SingleMethodSingleton"/>

51
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/JavaLanguageModuleTest.java Normal file
View File

@ -0,0 +1,51 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java;
import java.util.List;
import org.junit.Assert;
import org.junit.Test;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
public class JavaLanguageModuleTest {
private Language javaLanguage = LanguageRegistry.getLanguage(JavaLanguageModule.NAME);
@Test
public void java9IsSmallerThanJava10() {
LanguageVersion java9 = javaLanguage.getVersion("9");
LanguageVersion java10 = javaLanguage.getVersion("10");
Assert.assertTrue("java9 should be smaller than java10", java9.compareTo(java10) < 0);
}
@Test
public void previewVersionShouldBeGreaterThanNonPreview() {
LanguageVersion java16 = javaLanguage.getVersion("16");
LanguageVersion java16p = javaLanguage.getVersion("16-preview");
Assert.assertTrue("java16-preview should be greater than java16", java16p.compareTo(java16) > 0);
}
@Test
public void testCompareToVersion() {
LanguageVersion java9 = javaLanguage.getVersion("9");
Assert.assertTrue("java9 should be smaller than java10", java9.compareToVersion("10") < 0);
}
@Test
public void allVersions() {
List<LanguageVersion> versions = javaLanguage.getVersions();
for (int i = 1; i < versions.size(); i++) {
LanguageVersion previous = versions.get(i - 1);
LanguageVersion current = versions.get(i);
Assert.assertTrue("Version " + previous + " should be smaller than " + current,
previous.compareTo(current) < 0);
}
}
}

11
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionTest.java Normal file
View File

@ -0,0 +1,11 @@
/**
* 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.testframework.PmdRuleTst;
public class SimplifiableTestAssertionTest extends PmdRuleTst {
// no additional unit tests
}

2
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingBreakInSwitchTest.java → pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ImplicitSwitchFallThroughTest.java
View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
import net.sourceforge.pmd.testframework.PmdRuleTst;
public class MissingBreakInSwitchTest extends PmdRuleTst {
public class ImplicitSwitchFallThroughTest extends PmdRuleTst {
// no additional unit tests
}

12
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyCollectionRatherThanNullTest.java Normal file
View File

@ -0,0 +1,12 @@
/**
* 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.testframework.PmdRuleTst;
public class ReturnEmptyCollectionRatherThanNullTest extends PmdRuleTst {
// no additional unit tests
}

662
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SimplifiableTestAssertion.xml Normal file
View File

File diff suppressed because it is too large Load Diff

2
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertEqualsInsteadOfAssertTrue.xml
View File

@ -5,7 +5,7 @@
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>TEST1</description>
<description>use assert equals</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import junit.framework.TestCase;

114
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseTryWithResources.xml
View File

@ -212,6 +212,120 @@ public class TryWithResources {
myClose(in, in2);
}
}
}
]]></code>
</test-code>
<code-fragment id="issue-3235"><![CDATA[
import java.io.IOException;
import java.io.Reader;
class Scratch {
public static int count(AutoCloseable iterator) {
int count = 0;
try {
count++;
} finally {
iterator.close();
}
return count;
}
}
class Holder implements AutoCloseable {
private Reader reader;
public void close() throws IOException {
try {
someOtherActivity();
} finally {
reader.close();
}
}
private void someOtherActivity() throws IOException {
// do stuff
}
}
]]></code-fragment>
<test-code>
<description>[java] UseTryWithResources false positive when closeable is provided as a method argument or class field #3235 before java 9</description>
<expected-problems>0</expected-problems>
<code-ref id="issue-3235"/>
<source-type>java 1.8</source-type>
</test-code>
<test-code>
<description>[java] UseTryWithResources false positive when closeable is provided as a method argument or class field #3235</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>7,19</expected-linenumbers>
<code-ref id="issue-3235"/>
</test-code>
<code-fragment id="issue-3235-with-local-var"><![CDATA[
import java.io.IOException;
import java.io.Reader;
class Scratch {
public static int count() {
AutoCloseable iterator;
int count = 0;
try {
count++;
} finally {
iterator.close();
}
return count;
}
}
class Holder implements AutoCloseable {
public void close() throws IOException {
Reader reader;
try {
someOtherActivity();
} finally {
reader.close();
}
}
private void someOtherActivity() throws IOException {
// do stuff
}
}
]]></code-fragment>
<test-code>
<description>[java] UseTryWithResources with local var and before java 9 #3235</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>8,20</expected-linenumbers>
<code-ref id="issue-3235-with-local-var"/>
<source-type>java 1.8</source-type>
</test-code>
<test-code>
<description>[java] UseTryWithResources with local var and latest java #3235</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>8,20</expected-linenumbers>
<code-ref id="issue-3235-with-local-var"/>
</test-code>
<test-code>
<description>NPE when determining closeTarget</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
import java.io.InputStream;
import java.io.IOException;
public class UseTryWithResources {
public void read(InputStream is, boolean close) throws IOException {
try {
is.read();
} finally {
if (close) {
is.close();
}
}
}
}
]]></code>
</test-code>

0
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MissingBreakInSwitch.xml → pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ImplicitSwitchFallThrough.xml
View File

101
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ReturnEmptyCollectionRatherThanNull.xml Normal file
View File

@ -0,0 +1,101 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data 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">
<test-code>
<description>Returning null array</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
// Not a good idea...
public int []bar()
{
// ...
return null;
}
}
]]></code>
</test-code>
<test-code>
<description>Nonnull empty array</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
// Good behavior
public String[] bar()
{
//...
return new String[0];
}
}
]]></code>
</test-code>
<test-code>
<description>Returning null instead of collection (List)</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>5</expected-linenumbers>
<code><![CDATA[
import java.util.List;
public class Foo {
// Not a good idea...
public List<String> bar()
{
// ...
return null;
}
}
]]></code>
</test-code>
<test-code>
<description>Returning proper empty collection</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.List;
import java.util.Collections;
public class Foo {
// Not a good idea...
public List<String> bar()
{
// ...
return Collections.emptyList();
}
}
]]></code>
</test-code>
<test-code>
<description>Returning null instead of collection (Set)</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>5</expected-linenumbers>
<code><![CDATA[
import java.util.Set;
public class Foo {
// Not a good idea...
public Set<String> bar()
{
// ...
return null;
}
}
]]></code>
</test-code>
<test-code>
<description>Returning null instead of collection (Map)</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>5</expected-linenumbers>
<code><![CDATA[
import java.util.Map;
public class Foo {
// Not a good idea...
public Map<String, String> bar()
{
// ...
return null;
}
}
]]></code>
</test-code>
</test-data>

27
pom.xml
View File

@ -1048,6 +1048,33 @@
</plugins>
</build>
</profile>
<profile>
<id>pmd-dogfood</id>
<properties>
<pmd.dogfood.version>${project.version}</pmd.dogfood.version>
</properties>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<dependencies>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-core</artifactId>
<version>${pmd.dogfood.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-java</artifactId>
<version>${pmd.dogfood.version}</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>
</profile>
</profiles>
<modules>