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

Merge branch 'master' into constructorcallsoverridablemethod-fix-IOOBE

Browse Source
This commit is contained in:
Clément Fournier authored and GitHub committed 2021-05-14 16:43:07 +02:00
2 parents 4fcbdbb248 9c7286bcf2
commit 2137cf4b80
29 files changed
+479 -120
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.github/workflows/build.yml
+3 -2
View File
@@ -29,12 +29,13 @@ jobs:
with:
path: |
~/.m2/repository
~/.gradle/caches
~/.cache
~/work/pmd/target/repositories
vendor/bundle
key: ${{ runner.os }}-${{ hashFiles('**/pom.xml') }}
key: v1-${{ runner.os }}-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-
v1-${{ runner.os }}-
- name: Set up Ruby 2.7
uses: actions/setup-ruby@v1
with:
.github/workflows/troubleshooting.yml
+7 -5
View File
@@ -16,12 +16,14 @@ jobs:
- uses: actions/cache@v2
with:
path: |
~/.m2/repository
~/.cache
vendor/bundle
key: push-${{ runner.os }}-${{ hashFiles('**/pom.xml') }}
~/.m2/repository
~/.gradle/caches
~/.cache
~/work/pmd/target/repositories
vendor/bundle
key: v1-${{ runner.os }}-${{ hashFiles('**/pom.xml') }}
restore-keys: |
push-${{ runner.os }}-
v1-${{ runner.os }}-
- name: Set up Ruby 2.7
uses: actions/setup-ruby@v1
with:
docs/pages/release_notes.md
+25
View File
@@ -14,10 +14,29 @@ This is a {{ site.pmd.release_type }} release.
### New and noteworthy
#### Modified rules
* The Java rule {% rule "java/errorprone/CompareObjectsWithEquals" %} has now a new property
`typesThatCompareByReference`. With that property, you can configure types, that should be whitelisted
for comparison by reference. By default, `java.lang.Enum` and `java.lang.Class` are allowed, but
you could add custom types here.
Additionally comparisons against constants are allowed now. This makes the rule less noisy when two constants
are compared. Constants are identified by looking for an all-caps identifier.
### Fixed Issues
* apex
* [#3243](https://github.com/pmd/pmd/pull/3243): \[apex] Correct findBoundary when traversing AST
* doc
* [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages
* dist
* [#2466](https://github.com/pmd/pmd/issues/2466): \[dist] Distribution archive doesn't include all batch scripts
* java
* [#3269](https://github.com/pmd/pmd/pull/3269): \[java] Fix NPE in MethodTypeResolution
* java-bestpractices
* [#1175](https://github.com/pmd/pmd/issues/1175): \[java] UnusedPrivateMethod FP with Junit 5 @MethodSource
* [#2737](https://github.com/pmd/pmd/issues/2737): \[java] Fix misleading rule message on rule SwitchStmtsShouldHaveDefault with non-exhaustive enum switch
* [#3236](https://github.com/pmd/pmd/issues/3236): \[java] LiteralsFirstInComparisons should consider constant fields (cont'd)
* java-codestyle
* [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports
* [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes
@@ -25,6 +44,12 @@ This is a {{ site.pmd.release_type }} release.
* [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes
* java-errorprone
* [#3268](https://github.com/pmd/pmd/pull/3268): \[java] ConstructorCallsOverridableMethod: IndexOutOfBoundsException with annotations
* java-design
* [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative
* java-errorprone
* [#3248](https://github.com/pmd/pmd/issues/3248): \[java] Documentation is wrong for SingletonClassReturningNewInstance rule
* [#3110](https://github.com/pmd/pmd/issues/3110): \[java] Enhance CompareObjectsWithEquals with list of exceptions
* [#3205](https://github.com/pmd/pmd/issues/3205): \[java] Make CompareObjectWithEquals allow comparing against constants
### API Changes
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexRootNode.java
+5
View File
@@ -50,4 +50,9 @@ public abstract class ApexRootNode<T extends AstNode> extends AbstractApexNode<T
public double getApexVersion() {
return node.getDefiningType().getCodeUnitDetails().getVersion().getExternal();
}
@Override
public boolean isFindBoundary() {
return true;
}
}
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexBadCryptoRule.java
+1
View File
@@ -35,6 +35,7 @@ public class ApexBadCryptoRule extends AbstractApexRule {
private final Set<String> potentiallyStaticBlob = new HashSet<>();
public ApexBadCryptoRule() {
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java
+1 -1
View File
@@ -40,7 +40,7 @@ public class ApexDangerousMethodsRule extends AbstractApexRule {
private final Set<String> whiteListedVariables = new HashSet<>();
public ApexDangerousMethodsRule() {
super.addRuleChainVisit(ASTUserClass.class);
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java
+1 -1
View File
@@ -31,7 +31,7 @@ public class ApexOpenRedirectRule extends AbstractApexRule {
private final Set<String> listOfStringLiteralVariables = new HashSet<>();
public ApexOpenRedirectRule() {
super.addRuleChainVisit(ASTUserClass.class);
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java
+1
View File
@@ -51,6 +51,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
private final Map<String, Boolean> selectContainingVariables = new HashMap<>();
public ApexSOQLInjectionRule() {
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSuggestUsingNamedCredRule.java
+1 -1
View File
@@ -34,7 +34,7 @@ public class ApexSuggestUsingNamedCredRule extends AbstractApexRule {
private final Set<String> listOfAuthorizationVariables = new HashSet<>();
public ApexSuggestUsingNamedCredRule() {
super.addRuleChainVisit(ASTUserClass.class);
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexXSSFromEscapeFalseRule.java
+1
View File
@@ -23,6 +23,7 @@ public class ApexXSSFromEscapeFalseRule extends AbstractApexRule {
private static final String ADD_ERROR = "addError";
public ApexXSSFromEscapeFalseRule() {
addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, "Security");
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml
+19
View File
@@ -7,6 +7,7 @@
<test-code>
<description>Apex Crypto hardcoded IV</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
public class Foo {
public Foo() {
@@ -76,6 +77,24 @@ public class Foo {
Blob data = Blob.valueOf('Data to be encrypted');
Blob encrypted = Crypto.encryptWithManagedIV('AES128', key, data);
}
}
]]></code>
</test-code>
<test-code>
<description>Apex Crypto hardcoded IV in inner class</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
public class Foo {
class MyInnerClass {
public MyInnerClass() {
Blob exampleIv = Blob.valueOf('0000000000000000');
Blob key = Crypto.generateAesKey(128);
Blob data = Blob.valueOf('Data to be encrypted');
Blob encrypted = Crypto.encrypt('AES128', key, exampleIv, data);
}
}
}
]]></code>
</test-code>
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml
+18
View File
@@ -7,6 +7,7 @@
<test-code>
<description>Potentially unsafe SOQL on concatenation of variables 1</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>5</expected-linenumbers>
<code><![CDATA[
public class Foo {
public void test1() {
@@ -304,6 +305,23 @@ public class Foo {
public void test1(String name) {
List<SObject> res = Database.query('Select Id,Name From ' + (name == 'Account' ? name : 'Cases'));
}
}
]]></code>
</test-code>
<test-code>
<description>Potentially unsafe SOQL on concatenation of variables in nested class</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
public class Foo {
class MyNestedClass {
public void test1() {
String field1 = getSomeID();
String field2 = 'SELECT Id FROM Account WHERE Id =';
Database.query(field2 + field1);
}
}
}
]]></code>
</test-code>
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexXSSFromEscapeFalse.xml
+16
View File
@@ -7,6 +7,7 @@
<test-code>
<description>Add error variable with escape false</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
public void test1(String bad) {
@@ -36,6 +37,21 @@ public class Foo {
public void test1() {
Trigger.new[0].addError('something else' + bad, false);
}
}
]]></code>
</test-code>
<test-code>
<description>Add error variable with escape false in nested class</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
class MyNestedClass {
public void test1(String bad) {
Trigger.new[0].addError(bad, false);
}
}
}
]]></code>
</test-code>
pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java
+28
View File
@@ -732,6 +732,34 @@ public final class StringUtil {
return sb.toString();
}
/**
* If the string starts and ends with the delimiter, returns the substring
* within the delimiters. Otherwise returns the original string. The
* start and end delimiter must be 2 separate instances.
* <pre>{@code
* removeSurrounding("", _ ) = ""
* removeSurrounding("q", 'q') = "q"
* removeSurrounding("qq", 'q') = ""
* removeSurrounding("q_q", 'q') = "_"
* }</pre>
*/
public static String removeSurrounding(String string, char delimiter) {
if (string.length() >= 2
&& string.charAt(0) == delimiter
&& string.charAt(string.length() - 1) == delimiter) {
return string.substring(1, string.length() - 1);
}
return string;
}
/**
* Like {@link #removeSurrounding(String, char) removeSurrounding} with
* a double quote as a delimiter.
*/
public static String removeDoubleQuotes(String string) {
return removeSurrounding(string, '"');
}
/**
* Returns an empty array of string
pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java
+10
View File
@@ -4,6 +4,8 @@
package net.sourceforge.pmd.util;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import org.junit.Test;
@@ -97,4 +99,12 @@ public class StringUtilTest {
StringUtil.appendXmlEscaped(sb, test, true);
assertEquals("é", sb.toString());
}
@Test
public void testRemoveSurrounding() {
assertThat(StringUtil.removeSurrounding("", 'q'), equalTo(""));
assertThat(StringUtil.removeSurrounding("q", 'q'), equalTo("q"));
assertThat(StringUtil.removeSurrounding("qq", 'q'), equalTo(""));
assertThat(StringUtil.removeSurrounding("qqq", 'q'), equalTo("q"));
}
}
pmd-dist/src/main/resources/assemblies/pmd-bin.xml
+1 -5
View File
@@ -12,11 +12,7 @@
<fileSets>
<fileSet>
<includes>
<include>bgastviewer.bat</include>
<include>cpd.bat</include>
<include>cpdgui.bat</include>
<include>designer.bat</include>
<include>pmd.bat</include>
<include>*.bat</include>
</includes>
<directory>target/extra-resources/scripts</directory>
<outputDirectory>bin</outputDirectory>
pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java
+1
View File
@@ -48,6 +48,7 @@ public class BinaryDistributionIT extends AbstractBinaryDistributionTest {
result.add(basedir + "bin/run.sh");
result.add(basedir + "bin/pmd.bat");
result.add(basedir + "bin/cpd.bat");
result.add(basedir + "bin/ast-dump.bat");
result.add(basedir + "lib/pmd-core-" + PMDVersion.VERSION + ".jar");
result.add(basedir + "lib/pmd-java-" + PMDVersion.VERSION + ".jar");
return result;
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java
+36 -43
View File
@@ -4,12 +4,8 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
@@ -21,9 +17,11 @@ 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.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
@@ -47,20 +45,22 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
private boolean hasStringLiteralFirst(ASTPrimaryExpression expression) {
ASTPrimaryPrefix primaryPrefix = expression.getFirstChildOfType(ASTPrimaryPrefix.class);
ASTLiteral firstLiteral = primaryPrefix.getFirstDescendantOfType(ASTLiteral.class);
ASTLiteral firstLiteral = primaryPrefix.getFirstChildOfType(ASTLiteral.class);
return firstLiteral != null && firstLiteral.isStringLiteral();
}
private boolean isNullableComparisonWithStringLiteral(ASTPrimaryExpression expression) {
String opName = getOperationName(expression);
ASTPrimarySuffix argsSuffix = getSuffixOfArguments(expression);
return opName != null && argsSuffix != null && isStringLiteralComparison(opName, argsSuffix)
&& isNotWithinNullComparison(expression);
return opName != null && argsSuffix != null
&& isStringLiteralComparison(opName, argsSuffix)
&& isNotWithinNullComparison(expression);
}
private String getOperationName(ASTPrimaryExpression primaryExpression) {
return isMethodsChain(primaryExpression) ? getOperationNameBySuffix(primaryExpression)
: getOperationNameByPrefix(primaryExpression);
return isMethodsChain(primaryExpression)
? getOperationNameBySuffix(primaryExpression)
: getOperationNameByPrefix(primaryExpression);
}
private boolean isMethodsChain(ASTPrimaryExpression primaryExpression) {
@@ -90,12 +90,11 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
}
private ASTPrimarySuffix getPrimarySuffixAtIndexFromEnd(ASTPrimaryExpression primaryExpression, int indexFromEnd) {
List<ASTPrimarySuffix> primarySuffixes = primaryExpression.findChildrenOfType(ASTPrimarySuffix.class);
if (!primarySuffixes.isEmpty()) {
int suffixIndex = primarySuffixes.size() - 1 - indexFromEnd;
return primarySuffixes.get(suffixIndex);
int index = primaryExpression.getNumChildren() - 1 - indexFromEnd;
if (index <= 0) {
return null;
}
return null;
return (ASTPrimarySuffix) primaryExpression.getChild(index);
}
private boolean isStringLiteralComparison(String opName, ASTPrimarySuffix argsSuffix) {
@@ -126,29 +125,27 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
}
private boolean isStringLiteralFirstArgumentOfSuffix(ASTPrimarySuffix primarySuffix) {
try {
JavaNode firstLiteralArg = getFirstLiteralArgument(primarySuffix);
JavaNode firstNameArg = getFirstNameArgument(primarySuffix);
return isStringLiteral(firstLiteralArg) || isConstantString(firstNameArg);
} catch (NullPointerException e) {
JavaNode argumentPrimaryPrefix = getArgumentPrimaryPrefix(primarySuffix);
if (argumentPrimaryPrefix == null) {
return false;
}
}
private JavaNode getFirstLiteralArgument(ASTPrimarySuffix primarySuffix) {
return getArgumentPrimaryPrefix(primarySuffix).getFirstChildOfType(ASTLiteral.class);
}
private JavaNode getFirstNameArgument(ASTPrimarySuffix primarySuffix) {
return getArgumentPrimaryPrefix(primarySuffix).getFirstChildOfType(ASTName.class);
JavaNode firstLiteralArg = argumentPrimaryPrefix.getFirstChildOfType(ASTLiteral.class);
JavaNode firstNameArg = argumentPrimaryPrefix.getFirstChildOfType(ASTName.class);
return isStringLiteral(firstLiteralArg) || isConstantString(firstNameArg);
}
private JavaNode getArgumentPrimaryPrefix(ASTPrimarySuffix primarySuffix) {
ASTArguments arguments = primarySuffix.getFirstChildOfType(ASTArguments.class);
ASTArgumentList argumentList = arguments.getFirstChildOfType(ASTArgumentList.class);
ASTExpression expression = argumentList.getFirstChildOfType(ASTExpression.class);
ASTExpression expression = primarySuffix.getFirstChildOfType(ASTArguments.class)
.getFirstChildOfType(ASTArgumentList.class)
.getFirstChildOfType(ASTExpression.class);
assert expression != null : "We checked before that we had exactly one argument, so this cannot fail";
ASTPrimaryExpression primaryExpression = expression.getFirstChildOfType(ASTPrimaryExpression.class);
return primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
if (primaryExpression != null) {
return primaryExpression.getChild(0);
}
return null;
}
private boolean isStringLiteral(JavaNode node) {
@@ -162,17 +159,13 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
private boolean isConstantString(JavaNode node) {
if (node instanceof ASTName) {
ASTName name = (ASTName) node;
ASTClassOrInterfaceBody classBody = name.getFirstParentOfType(ASTClassOrInterfaceBody.class);
ASTClassOrInterfaceBodyDeclaration classOrInterfaceBodyDeclaration = classBody.getFirstChildOfType(ASTClassOrInterfaceBodyDeclaration.class);
List<ASTFieldDeclaration> fieldDeclarations = classOrInterfaceBodyDeclaration.findChildrenOfType(ASTFieldDeclaration.class);
for (ASTFieldDeclaration fieldDeclaration : fieldDeclarations) {
ASTVariableDeclarator declaration = fieldDeclaration.getFirstChildOfType(ASTVariableDeclarator.class);
if (declaration.getName().equals(name.getImage())
&& String.class.equals(declaration.getType())
&& fieldDeclaration.isFinal()
&& fieldDeclaration.isStatic()) {
return true;
}
NameDeclaration resolved = name.getNameDeclaration();
if (resolved instanceof VariableNameDeclaration
&& resolved.getNode() instanceof ASTVariableDeclaratorId) {
ASTVariableDeclaratorId resolvedNode = (ASTVariableDeclaratorId) resolved.getNode();
return resolvedNode.isFinal()
&& resolvedNode.isField()
&& resolvedNode.getFirstParentOfType(ASTFieldDeclaration.class).isStatic();
}
}
return false;
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java
+34 -9
View File
@@ -13,15 +13,20 @@ import java.util.Map;
import java.util.Set;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.Annotatable;
import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.util.StringUtil;
/**
* This rule detects private methods, that are not used and can therefore be
@@ -41,25 +46,45 @@ public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule {
}
/**
* Visit each method declaration.
*
* @param node
* the method declaration
* @param data
* data - rule context
* @return data
* Return a set of method names which are considered used. Only the
* no-arg overload is considered used.
*/
private static Set<String> methodsUsedByAnnotations(ASTClassOrInterfaceDeclaration klassDecl) {
Set<String> result = Collections.emptySet();
for (ASTAnyTypeBodyDeclaration declaration : klassDecl.getDeclarations()) {
for (ASTAnnotation annot : declaration.findChildrenOfType(ASTAnnotation.class)) {
if (TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", annot)) {
// MethodSource#value() -> String[], there may be several of those methods
// todo this is not robust, revisit in pmd 7
for (ASTLiteral literal : annot.findDescendantsOfType(ASTLiteral.class)) {
if (literal.isStringLiteral()) {
if (result.isEmpty()) {
result = new HashSet<>(); // make writable
}
result.add(StringUtil.removeDoubleQuotes(literal.getImage()));
}
}
}
}
}
return result;
}
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (node.isInterface()) {
return data;
}
Set<String> methodsUsedByAnnotations = methodsUsedByAnnotations(node);
Map<MethodNameDeclaration, List<NameOccurrence>> methods = node.getScope().getEnclosingScope(ClassScope.class)
.getMethodDeclarations();
.getMethodDeclarations();
for (MethodNameDeclaration mnd : findUnique(methods)) {
List<NameOccurrence> occs = methods.get(mnd);
if (!privateAndNotExcluded(mnd) || hasIgnoredAnnotation((Annotatable) mnd.getNode().getParent())) {
if (!privateAndNotExcluded(mnd)
|| hasIgnoredAnnotation((Annotatable) mnd.getNode().getParent())
|| mnd.getParameterCount() == 0 && methodsUsedByAnnotations.contains(mnd.getName())) {
continue;
}
if (occs.isEmpty()) {
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java
+28 -10
View File
@@ -37,6 +37,11 @@ import net.sourceforge.pmd.lang.java.typeresolution.typeinference.Variable;
@Deprecated
@InternalApi
public final class MethodTypeResolution {
/**
*
*/
private static final String MESSAGE_INCOMPLETE_AUXCLASSPATH = "Possible incomplete auxclasspath: Error while processing methods";
private MethodTypeResolution() {}
private static final Logger LOG = Logger.getLogger(MethodTypeResolution.class.getName());
@@ -477,7 +482,7 @@ public final class MethodTypeResolution {
}
} catch (final LinkageError e) {
// This is an incomplete classpath, report the missing class
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e);
}
// search it's supertype
@@ -496,7 +501,7 @@ public final class MethodTypeResolution {
} catch (TypeNotPresentException | LinkageError e) {
// might be thrown by contextClass.getGenericSuperclass()
// This is an incomplete classpath, report the missing class
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e);
}
}
@@ -509,7 +514,7 @@ public final class MethodTypeResolution {
} catch (TypeNotPresentException | LinkageError e) {
// might be thrown by contextClass.getGenericInterface()
// This is an incomplete classpath, report the missing class
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e);
}
return result;
@@ -522,15 +527,22 @@ public final class MethodTypeResolution {
return MethodType.build(method);
}
JavaTypeDefinition returnType = context.resolveTypeDefinition(method.getGenericReturnType(),
method, typeArguments);
List<JavaTypeDefinition> argTypes = new ArrayList<>();
try {
JavaTypeDefinition returnType = context.resolveTypeDefinition(method.getGenericReturnType(),
method, typeArguments);
List<JavaTypeDefinition> argTypes = new ArrayList<>();
for (Type argType : method.getGenericParameterTypes()) {
argTypes.add(context.resolveTypeDefinition(argType, method, typeArguments));
for (Type argType : method.getGenericParameterTypes()) {
argTypes.add(context.resolveTypeDefinition(argType, method, typeArguments));
}
return MethodType.build(returnType, argTypes, method);
} catch (TypeNotPresentException | LinkageError e) {
// might be thrown by method.getGenericReturnType() and method.getGenericParameterTypes()
// This is an incomplete classpath, report the missing class
LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e);
return MethodType.build(method);
}
return MethodType.build(returnType, argTypes, method);
}
@@ -682,6 +694,12 @@ public final class MethodTypeResolution {
// example result: List<String>.getAsSuper(Collection) becomes Collection<String>
JavaTypeDefinition argSuper = argument.getAsSuper(parameter.getType());
// argSuper can't be null because isAssignableFrom check above returned true
// it might be null however, if the auxclasspath was not complete...
if (argSuper == null) {
// that's not really correct, because the generic type are ignored...
// be we can't compare the types
return true;
}
// right now we only check if generic arguments are the same
// TODO: add support for wildcard types
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java
+35 -28
View File
@@ -182,38 +182,45 @@ import java.util.logging.Logger;
return forClass(Object.class);
}
if (type instanceof Class) { // Raw types take this branch as well
return forClass((Class<?>) type);
} else if (type instanceof ParameterizedType) {
final ParameterizedType parameterizedType = (ParameterizedType) type;
try {
if (type instanceof Class) { // Raw types take this branch as well
return forClass((Class<?>) type);
} else if (type instanceof ParameterizedType) {
final ParameterizedType parameterizedType = (ParameterizedType) type;
// recursively determine each type argument's type def.
final Type[] typeArguments = parameterizedType.getActualTypeArguments();
final JavaTypeDefinition[] genericBounds = new JavaTypeDefinition[typeArguments.length];
for (int i = 0; i < typeArguments.length; i++) {
genericBounds[i] = resolveTypeDefinition(typeArguments[i], method, methodTypeArgs);
}
// recursively determine each type argument's type def.
final Type[] typeArguments = parameterizedType.getActualTypeArguments();
final JavaTypeDefinition[] genericBounds = new JavaTypeDefinition[typeArguments.length];
for (int i = 0; i < typeArguments.length; i++) {
genericBounds[i] = resolveTypeDefinition(typeArguments[i], method, methodTypeArgs);
}
// TODO : is this cast safe?
return forClass((Class<?>) parameterizedType.getRawType(), genericBounds);
} else if (type instanceof TypeVariable) {
return getGenericType(((TypeVariable<?>) type).getName(), method, methodTypeArgs);
} else if (type instanceof WildcardType) {
final Type[] wildcardLowerBounds = ((WildcardType) type).getLowerBounds();
// TODO : is this cast safe?
return forClass((Class<?>) parameterizedType.getRawType(), genericBounds);
} else if (type instanceof TypeVariable) {
return getGenericType(((TypeVariable<?>) type).getName(), method, methodTypeArgs);
} else if (type instanceof WildcardType) {
final Type[] wildcardLowerBounds = ((WildcardType) type).getLowerBounds();
if (wildcardLowerBounds.length != 0) { // lower bound wildcard
return forClass(LOWER_WILDCARD, resolveTypeDefinition(wildcardLowerBounds[0], method, methodTypeArgs));
} else { // upper bound wildcard
final Type[] wildcardUpperBounds = ((WildcardType) type).getUpperBounds();
return forClass(UPPER_WILDCARD, resolveTypeDefinition(wildcardUpperBounds[0], method, methodTypeArgs));
}
} else if (type instanceof GenericArrayType) {
JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType(), method, methodTypeArgs);
// only if we could determine the actual type
if (component != null) {
// TODO: retain the generic types of the array component...
return forClass(Array.newInstance(component.getType(), 0).getClass());
if (wildcardLowerBounds.length != 0) { // lower bound wildcard
return forClass(LOWER_WILDCARD, resolveTypeDefinition(wildcardLowerBounds[0], method, methodTypeArgs));
} else { // upper bound wildcard
final Type[] wildcardUpperBounds = ((WildcardType) type).getUpperBounds();
return forClass(UPPER_WILDCARD, resolveTypeDefinition(wildcardUpperBounds[0], method, methodTypeArgs));
}
} else if (type instanceof GenericArrayType) {
JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType(), method, methodTypeArgs);
// only if we could determine the actual type
if (component != null) {
// TODO: retain the generic types of the array component...
return forClass(Array.newInstance(component.getType(), 0).getClass());
}
}
} catch (TypeNotPresentException | LinkageError e) {
// might be thrown by parameterizedType.getActualTypeArguments(), type.getLowerBounds(),
// type.getUpperBounds(), type.getGenericComponentType()
// This is an incomplete classpath, report the missing class
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while resolving generic types", e);
}
// TODO : Shall we throw here?
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java
+8
View File
@@ -8,6 +8,7 @@ import java.lang.reflect.Modifier;
import java.util.List;
import net.sourceforge.pmd.internal.util.AssertionUtil;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
@@ -15,6 +16,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTImplementsList;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper;
@@ -221,6 +223,12 @@ public final class TypeTestUtil {
}
private static boolean fallbackIsA(TypeNode n, String canonicalName, boolean considerSubtype) {
if (n instanceof ASTAnnotation) {
// the annotation node has no image itself
n = n.getFirstDescendantOfType(ASTName.class);
assert n != null;
}
if (n.getImage() != null && !n.getImage().contains(".") && canonicalName.contains(".")) {
// simple name detected, check the imports to get the full name and use that for fallback
List<ASTImportDeclaration> imports = n.getRoot().findChildrenOfType(ASTImportDeclaration.class);
pmd-java/src/main/resources/category/java/bestpractices.xml
+8 -5
View File
@@ -1287,12 +1287,15 @@ public class Foo {
<rule name="SwitchStmtsShouldHaveDefault"
language="java"
since="1.0"
message="Switch statements should have a default label"
message="Switch statements should be exhaustive, add a default case (or missing enum branches)"
typeResolution="true"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#switchstmtsshouldhavedefault">
<description>
All switch statements should include a default option to catch any unspecified values.
Switch statements should be exhaustive, to make their control flow
easier to follow. This can be achieved by adding a `default` case, or,
if the switch is on an enum type, by ensuring there is one switch branch
for each enum constant.
</description>
<priority>3</priority>
<properties>
@@ -1305,14 +1308,14 @@ All switch statements should include a default option to catch any unspecified v
</properties>
<example>
<![CDATA[
public void bar() {
class Foo {{
int x = 2;
switch (x) {
case 1: int j = 6;
case 2: int j = 8;
// missing default: here
// missing default: here
}
}
}}
]]>
</example>
</rule>
pmd-java/src/main/resources/category/java/design.xml
+4
View File
@@ -512,10 +512,14 @@ into the former client classes.
<![CDATA[
public class DataClass {
// class exposes public attributes
public String name = "";
public int bar = 0;
public int na = 0;
private int bee = 0;
// and private ones through getters
public void setBee(int n) {
bee = n;
}
pmd-java/src/main/resources/category/java/errorprone.xml
+23 -9
View File
@@ -1100,24 +1100,38 @@ public class Bar {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#compareobjectswithequals">
<description>
Use equals() to compare object references; avoid comparing them with ==.
Use `equals()` to compare object references; avoid comparing them with `==`.
Since comparing objects with named constants is useful in some cases (eg, when
defining constants for sentinel values), the rule ignores comparisons against
fields with all-caps name (eg `this == SENTINEL`), which is a common naming
convention for constant fields.
You may allow some types to be compared by reference by listing the exceptions
in the `typesThatCompareByReference` property.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="typesThatCompareByReference" type="List[String]" delimiter="," description="List of canonical type names for which reference comparison is allowed.">
<value>java.lang.Enum,java.lang.Class</value>
</property>
<property name="xpath">
<value>
<![CDATA[
//EqualityExpression
[count(PrimaryExpression
[pmd-java:typeIs('java.lang.Object')]
[not(pmd-java:typeIs('java.lang.Enum'))]
[not(pmd-java:typeIs('java.lang.Class'))]) = 2
[count(
PrimaryExpression[pmd-java:typeIs('java.lang.Object')]
[not(some $t in $typesThatCompareByReference satisfies pmd-java:typeIs($t))]
) = 2
]
[not(PrimaryExpression[PrimaryPrefix/@ThisModifier = true()]
[not(PrimarySuffix)]
[ancestor::MethodDeclaration[@Name = 'equals']])
]
(: Is not a field access with an all-caps identifier :)
[not(PrimaryExpression[not(PrimarySuffix) and PrimaryPrefix/Name[upper-case(@Image)=@Image]
or PrimaryExpression/PrimarySuffix[last()][upper-case(@Image)=@Image]])]
]]>
</value>
</property>
@@ -2953,9 +2967,9 @@ public class Singleton {
class="net.sourceforge.pmd.lang.java.rule.errorprone.SingletonClassReturningNewInstanceRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#singletonclassreturningnewinstance">
<description>
Some classes contain overloaded getInstance. The problem with overloaded getInstance methods
is that the instance created using the overloaded method is not cached and so,
for each call and new objects will be created for every invocation.
A singleton class should only ever have one instance. Failure to check
whether an instance has already been created may result in multiple
instances being created.
</description>
<priority>2</priority>
<example>
@@ -2964,7 +2978,7 @@ class Singleton {
private static Singleton instance = null;
public static Singleton getInstance() {
synchronized(Singleton.class) {
return new Singleton();
return new Singleton(); // this should be assigned to the field
}
}
}
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/LiteralsFirstInComparisons.xml
+39
View File
@@ -370,4 +370,43 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>#3236 [java] LiteralsFirstInComparisons should consider constant fields (cont'd)</description>
<expected-problems>5</expected-problems>
<expected-linenumbers>6,8,17,24,26</expected-linenumbers>
<code><![CDATA[
class DT1 {
public static final String Q = "q";
public static final String T = "t";
public static int convert(String type) {
if (type.equals(Q)) { // 6
return 1;
} else if (type.equals(T)) { // 8
return 2;
} else {
return 3;
}
}
public static int convert2(String type) {
if (Q.equals(type)) { // 15
return 1;
} else if (type.equals(T)) { // 17
return 2;
} else {
return 3;
}
}
public static int convert3(String type) {
if (type.equals("q")) { // 24
return 1;
} else if (type.equals("t")) { // 26
return 2;
} else {
return 3;
}
}
}
]]></code>
</test-code>
</test-data>
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml
+27
View File
@@ -1697,6 +1697,33 @@ public class Outer {
Inner inner = new Inner();
inner.innerUsedByOuterMethod();
}
}
]]></code>
</test-code>
<test-code>
<description>#1175 False positive with Junit 5 MethodSource</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.stream.Stream;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
public class Outer {
private static Stream<Arguments> basenameKeyArguments() {
return Stream.of(
Arguments.of("simple", "simple"),
Arguments.of("simple", "one/two/many/simple"),
Arguments.of("simple", "//////an/////awful/key////simple")
);
}
@ParameterizedTest
@MethodSource("basenameKeyArguments")
void basenameKeyTest(final String expected, final String testString) {
assertEquals(expected, NetworkTable.basenameKey(testString));
}
}
]]></code>
</test-code>
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/DataClass.xml
+24
View File
@@ -220,4 +220,28 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>Example from the documentation</description>
<expected-problems>1</expected-problems>
<expected-messages>
<message>The class 'DataClass' is suspected to be a Data Class (WOC=0.000%, NOPA=3, NOAM=1, WMC=1)</message>
</expected-messages>
<code>
<![CDATA[
public class DataClass {
// class exposes public attributes
public String name = "";
public int bar = 0;
public int na = 0;
private int bee = 0;
// and private ones through getters
public void setBee(int n) {
bee = n;
}
}
]]></code>
</test-code>
</test-data>
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml
+74 -1
View File
@@ -112,7 +112,7 @@ package net.sourceforge.pmd.lang.java.rule.errorprone.compareobjectswithequals;
public class CompareObjectsWithEqualsSample {
void array(int[] a, String[] b) {
if (a[1] == b[1]) {} // int == String - this comparison doesn't make sense
if (a[1] == b[1]) {} // int == String - this comparison doesn't make sense (and doesn't compile...)
}
void array2(int[] c, int[] d) {
if (c[1] == d[1]) {}
@@ -365,4 +365,77 @@ public class EnumTest {
]]></code>
</test-code>
<test-code>
<description>static constant #3205</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class MyClass {
static final MyClass MISSING = new MyClass();
public static void isMissing(MyClass obj) {
return obj == MISSING; // no violation expected...
}
}
]]></code>
</test-code>
<test-code>
<description>static constant in other class #3205</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class MyClass {
static class Ts {
static final MyClass MISSING = new MyClass();
}
public static void isMissing(MyClass obj) {
return obj == Ts.MISSING; // no violation expected...
}
}
]]></code>
</test-code>
<test-code>
<description>constant field on some object #3205</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class MyClass {
static class Ts {
final MyClass MISSING = new MyClass();
}
public static void isMissing(MyClass obj, Ts ts) {
return obj == ts.MISSING; // no violation expected...
}
}
]]></code>
</test-code>
<test-code>
<description>constant field on some object, more complicated expr #3205</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class MyClass {
static class Ts {
final MyClass MISSING = new MyClass();
Ts id() { return this; }
}
public static void isMissing(MyClass obj, Ts ts) {
return obj == (ts.id()).id().MISSING; // no violation expected...
}
}
]]></code>
</test-code>
<test-code>
<description>Property typesThatCompareByReference #3110</description>
<rule-property name="typesThatCompareByReference">java.lang.String</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
class MyClass {
public static void isMissing(String obj, Object ts) {
return obj == ts;
}
}
]]></code>
</test-code>
</test-data>