Merge branch 'master' into pr/3393

This commit is contained in:
Clément Fournier
2021-07-29 13:19:46 +02:00
27 changed files with 1776 additions and 125 deletions

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)) {

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();

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>

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>

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"

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"/>

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);
}
}
}

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
}

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
}

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
}

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;

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>