Merge branch 'master' into pmd/7.0.x
This commit is contained in:
commit
ab489832fe
@ -19,27 +19,66 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
### New and noteworthy
|
||||
|
||||
#### Changes in how tab characters are handled
|
||||
|
||||
In the past, tab characters in source files has been handled differently in different languages by PMD.
|
||||
For instance in Java, tab characters had a width of 8 columns, while C# used only 1 column. Visualforce instead
|
||||
used 4 columns.
|
||||
|
||||
This has been unified now so that tab characters are consistently now always 1 column wide.
|
||||
|
||||
This however might be a **incompatible** change, if you're using the properties "BeginColumn" or "EndColumn"
|
||||
additionally to "BeginLine" and "EndLine" of a Token/AST node in order to highlight
|
||||
where a rule violation occurred in the source file. If you have logic there that deals with tab characters,
|
||||
you most likely can remove this logic now, since tab characters are now just "normal" characters
|
||||
in terms of string processing.
|
||||
|
||||
See also [[all] Ensure PMD/CPD uses tab width of 1 for tabs consistently #2656](https://github.com/pmd/pmd/pull/2656).
|
||||
|
||||
#### New Rules
|
||||
|
||||
* The new Java rule {% rule "java/bestpractices/AvoidReassigningCatchVariables" %} (`java-bestpractices`) finds
|
||||
cases where the variable of the caught exception is reassigned. This practice is surprising and prevents
|
||||
further evolution of the code like multi-catch.
|
||||
|
||||
#### Modified Rules
|
||||
|
||||
* The Java rule {% rule "java/errorprone/CloseResource" %} (`java-errorprone`) has a new property
|
||||
`closeNotInFinally`. With this property set to `true` the rule will also find calls to close a
|
||||
resource, which are not in a finally-block of a try-statement. If a resource is not closed within a
|
||||
finally block, it might not be closed at all in case of exceptions.
|
||||
|
||||
As this new detection would yield many new violations, it is disabled by default. It might be
|
||||
enabled in a later version of PMD.
|
||||
|
||||
#### Deprecated Rules
|
||||
|
||||
* The Java rule {% rule "java/errorprone/DataflowAnomalyAnalysis" %} (`java-errorprone`)
|
||||
is deprecated in favour of {% rule "java/bestpractices/UnusedAssignment" %} (`java-bestpractices`),
|
||||
which was introduced in PMD 6.26.0.
|
||||
|
||||
### Fixed Issues
|
||||
|
||||
* core
|
||||
* [#724](https://github.com/pmd/pmd/issues/724): \[core] Avoid parsing rulesets multiple times
|
||||
* [#1962](https://github.com/pmd/pmd/issues/1962): \[core] Simplify Report API
|
||||
* [#2653](https://github.com/pmd/pmd/issues/2653): \[lang-test] Upgrade kotlintest to Kotest
|
||||
* [#2656](https://github.com/pmd/pmd/pull/2656): \[all] Ensure PMD/CPD uses tab width of 1 for tabs consistently
|
||||
* [#2690](https://github.com/pmd/pmd/pull/2690): \[core] Fix java7 compatibility
|
||||
* java-bestpractices
|
||||
* [#2471](https://github.com/pmd/pmd/issues/2471): \[java] New Rule: AvoidReassigningCatchVariables
|
||||
* [#2663](https://github.com/pmd/pmd/issues/2663): \[java] NoClassDefFoundError on upgrade from 6.25.0 to 6.26.0
|
||||
* [#2668](https://github.com/pmd/pmd/issues/2668): \[java] UnusedAssignment false positives
|
||||
* [#2673](https://github.com/pmd/pmd/issues/2673): \[java] UnusedPrivateField and SingularField false positive with lombok annotation EqualsAndHashCode
|
||||
* [#2684](https://github.com/pmd/pmd/issues/2684): \[java] UnusedAssignment FP in try/catch
|
||||
* [#2686](https://github.com/pmd/pmd/issues/2686): \[java] UnusedAssignment must not flag abstract method parameters in interfaces and abstract classes
|
||||
* java-design
|
||||
* [#2461](https://github.com/pmd/pmd/issues/2461): \[java] ExcessiveParameterListRule must ignore a private constructor
|
||||
* java-errorprone
|
||||
* [#2410](https://github.com/pmd/pmd/issues/2410): \[java] ProperCloneImplementation not valid for final class
|
||||
* [#2431](https://github.com/pmd/pmd/issues/2431): \[java] InvalidLogMessageFormatRule throws IndexOutOfBoundsException when only logging exception message
|
||||
* [#2439](https://github.com/pmd/pmd/issues/2439): \[java] AvoidCatchingThrowable can not detect the case: catch (java.lang.Throwable t)
|
||||
* [#2470](https://github.com/pmd/pmd/issues/2470): \[java] CloseResource false positive when resource included in return value
|
||||
* [#2531](https://github.com/pmd/pmd/issues/2531): \[java] UnnecessaryCaseChange can not detect the case like: foo.equals(bar.toLowerCase())
|
||||
* [#2647](https://github.com/pmd/pmd/issues/2647): \[java] Deprecate rule DataFlowAnomalyAnalysis
|
||||
* java-performance
|
||||
@ -56,6 +95,8 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
#### Deprecated API
|
||||
|
||||
##### For removal
|
||||
|
||||
* {% jdoc !!core::Rule#getParserOptions() %}
|
||||
* {% jdoc !!core::lang.Parser#getParserOptions() %}
|
||||
* {% jdoc !!core::lang.AbstractParser %}
|
||||
@ -83,14 +124,22 @@ This is a {{ site.pmd.release_type }} release.
|
||||
* {% jdoc_package core::lang.dfa %}
|
||||
* and the class {% jdoc plsql::lang.plsql.PLSQLDataFlowHandler %}
|
||||
|
||||
* {% jdoc !!visualforce::lang.vf.VfSimpleCharStream %}
|
||||
|
||||
### External Contributions
|
||||
|
||||
* [#2656](https://github.com/pmd/pmd/pull/2656): \[all] Ensure PMD/CPD uses tab width of 1 for tabs consistently - [Maikel Steneker](https://github.com/maikelsteneker)
|
||||
* [#2659](https://github.com/pmd/pmd/pull/2659): \[java] StringToString can not detect the case: getStringMethod().toString() - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2662](https://github.com/pmd/pmd/pull/2662): \[java] UnnecessaryCaseChange can not detect the case like: foo.equals(bar.toLowerCase()) - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2671](https://github.com/pmd/pmd/pull/2671): \[java] CloseResource false positive when resource included in return value - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2674](https://github.com/pmd/pmd/pull/2674): \[java] add lombok.EqualsAndHashCode in AbstractLombokAwareRule - [berkam](https://github.com/berkam)
|
||||
* [#2677](https://github.com/pmd/pmd/pull/2677): \[java] RedundantFieldInitializer can not detect a special case for char initialize: `char foo = '\0';` - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2678](https://github.com/pmd/pmd/pull/2678): \[java] AvoidCatchingThrowable can not detect the case: catch (java.lang.Throwable t) - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2679](https://github.com/pmd/pmd/pull/2679): \[java] InvalidLogMessageFormatRule throws IndexOutOfBoundsException when only logging exception message - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2682](https://github.com/pmd/pmd/pull/2682): \[java] New Rule: AvoidReassigningCatchVariables - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2697](https://github.com/pmd/pmd/pull/2697): \[java] ExcessiveParameterListRule must ignore a private constructor - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2699](https://github.com/pmd/pmd/pull/2699): \[java] ProperCloneImplementation not valid for final class - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2700](https://github.com/pmd/pmd/pull/2700): \[java] Fix OnlyOneReturn code example - [Jan-Lukas Else](https://github.com/jlelse)
|
||||
|
||||
|
||||
{% endtocmaker %}
|
||||
|
@ -47,6 +47,11 @@ public class ApexTokenizerTest extends CpdTextComparisonTest {
|
||||
doTest("comments");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTabWidth() {
|
||||
doTest("tabWidth");
|
||||
}
|
||||
|
||||
private Properties caseSensitive() {
|
||||
return properties(true);
|
||||
}
|
||||
|
10
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/cpd/testdata/tabWidth.cls
vendored
Normal file
10
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/cpd/testdata/tabWidth.cls
vendored
Normal file
@ -0,0 +1,10 @@
|
||||
/*
|
||||
* Some comment
|
||||
*/
|
||||
public with sharing class Simple {
|
||||
public String someParam { get; set; }
|
||||
|
||||
public void getInit() {
|
||||
someParam = "test";
|
||||
}
|
||||
}
|
35
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/cpd/testdata/tabWidth.txt
vendored
Normal file
35
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/cpd/testdata/tabWidth.txt
vendored
Normal file
@ -0,0 +1,35 @@
|
||||
[Image] or [Truncated image[ Bcol Ecol
|
||||
L4
|
||||
[public] 1 6
|
||||
[with] 8 11
|
||||
[sharing] 13 19
|
||||
[class] 21 25
|
||||
[simple] 27 32
|
||||
[{] 34 34
|
||||
L5
|
||||
[public] 2 7
|
||||
[string] 9 14
|
||||
[someparam] 16 24
|
||||
[{] 26 26
|
||||
[get] 28 30
|
||||
[;] 31 31
|
||||
[set] 33 35
|
||||
[;] 36 36
|
||||
[}] 38 38
|
||||
L7
|
||||
[public] 2 7
|
||||
[void] 9 12
|
||||
[getinit] 14 20
|
||||
[(] 21 21
|
||||
[)] 22 22
|
||||
[{] 24 24
|
||||
L8
|
||||
[someparam] 3 11
|
||||
[=] 13 13
|
||||
[test] 16 19
|
||||
[;] 21 21
|
||||
L9
|
||||
[}] 2 2
|
||||
L10
|
||||
[}] 1 1
|
||||
EOF
|
@ -101,6 +101,10 @@
|
||||
<replace file="${tmp-package.dir}/${cs.prefix}CharStream.java"
|
||||
token="throw new Error(t.getMessage())"
|
||||
value="throw t" />
|
||||
<!-- Set tab size of JavaCC to 1 -->
|
||||
<replace file="${target}/net/sourceforge/pmd/lang/ast/dummy/SimpleCharStream.java"
|
||||
token="int tabSize = 8;"
|
||||
value="int tabSize = 1;" />
|
||||
|
||||
<replaceregexp file="${tmp-package.dir}/${cs.prefix}CharStream.java">
|
||||
<regexp pattern='throw new Error\("Invalid escape character at line " \+ line \+\s+" column " \+ column \+ "."\);' />
|
||||
|
@ -124,6 +124,11 @@ public class CPPTokenizerTest extends CpdTextComparisonTest {
|
||||
doTest("issue-1784");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTabWidth() {
|
||||
doTest("tabWidth");
|
||||
}
|
||||
|
||||
|
||||
private static Properties skipBlocks(String skipPattern) {
|
||||
return properties(true, skipPattern);
|
||||
|
1
pmd-cpp/src/test/resources/net/sourceforge/pmd/lang/cpp/cpd/testdata/tabWidth.cpp
vendored
Normal file
1
pmd-cpp/src/test/resources/net/sourceforge/pmd/lang/cpp/cpd/testdata/tabWidth.cpp
vendored
Normal file
@ -0,0 +1 @@
|
||||
int i = 0;
|
8
pmd-cpp/src/test/resources/net/sourceforge/pmd/lang/cpp/cpd/testdata/tabWidth.txt
vendored
Normal file
8
pmd-cpp/src/test/resources/net/sourceforge/pmd/lang/cpp/cpd/testdata/tabWidth.txt
vendored
Normal file
@ -0,0 +1,8 @@
|
||||
[Image] or [Truncated image[ Bcol Ecol
|
||||
L1
|
||||
[int] 2 4
|
||||
[i] 6 6
|
||||
[=] 8 8
|
||||
[0] 10 10
|
||||
[;] 11 11
|
||||
EOF
|
@ -85,6 +85,11 @@ public class CsTokenizerTest extends CpdTextComparisonTest {
|
||||
doTest("usingDirectives", "_ignored", ignoreUsings());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTabWidth() {
|
||||
doTest("tabWidth");
|
||||
}
|
||||
|
||||
private Properties ignoreUsings() {
|
||||
return properties(true);
|
||||
}
|
||||
|
1
pmd-cs/src/test/resources/net/sourceforge/pmd/lang/cs/cpd/testdata/tabWidth.cs
vendored
Normal file
1
pmd-cs/src/test/resources/net/sourceforge/pmd/lang/cs/cpd/testdata/tabWidth.cs
vendored
Normal file
@ -0,0 +1 @@
|
||||
int i = 0;
|
8
pmd-cs/src/test/resources/net/sourceforge/pmd/lang/cs/cpd/testdata/tabWidth.txt
vendored
Normal file
8
pmd-cs/src/test/resources/net/sourceforge/pmd/lang/cs/cpd/testdata/tabWidth.txt
vendored
Normal file
@ -0,0 +1,8 @@
|
||||
[Image] or [Truncated image[ Bcol Ecol
|
||||
L1
|
||||
[int] 2 4
|
||||
[i] 6 6
|
||||
[=] 8 8
|
||||
[0] 10 10
|
||||
[;] 11 11
|
||||
EOF
|
@ -82,4 +82,9 @@ public class DartTokenizerTest extends CpdTextComparisonTest {
|
||||
doTest("string_multiline");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTabWidth() {
|
||||
doTest("tabWidth");
|
||||
}
|
||||
|
||||
}
|
||||
|
3
pmd-dart/src/test/resources/net/sourceforge/pmd/cpd/testdata/tabWidth.dart
vendored
Normal file
3
pmd-dart/src/test/resources/net/sourceforge/pmd/cpd/testdata/tabWidth.dart
vendored
Normal file
@ -0,0 +1,3 @@
|
||||
class MyClass {
|
||||
var s1 = 'test';
|
||||
}
|
13
pmd-dart/src/test/resources/net/sourceforge/pmd/cpd/testdata/tabWidth.txt
vendored
Normal file
13
pmd-dart/src/test/resources/net/sourceforge/pmd/cpd/testdata/tabWidth.txt
vendored
Normal file
@ -0,0 +1,13 @@
|
||||
[Image] or [Truncated image[ Bcol Ecol
|
||||
L1
|
||||
[class] 1 5
|
||||
[MyClass] 7 13
|
||||
[{] 15 15
|
||||
L2
|
||||
[var] 2 4
|
||||
[s1] 6 7
|
||||
[=] 9 9
|
||||
['test'] 11 16
|
||||
L3
|
||||
[}] 1 1
|
||||
EOF
|
@ -8,9 +8,12 @@ package net.sourceforge.pmd.lang.java.ast.internal;
|
||||
import java.lang.reflect.Field;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
|
||||
@ -19,11 +22,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
|
||||
* Helper class to analyze {@link ASTImportDeclaration}s.
|
||||
*/
|
||||
public class ImportWrapper {
|
||||
private ASTImportDeclaration node;
|
||||
private String name;
|
||||
private String fullname;
|
||||
private boolean isStaticDemand;
|
||||
private Set<String> allDemands = new HashSet<>();
|
||||
private static final Logger LOG = Logger.getLogger(ImportWrapper.class.getName());
|
||||
|
||||
private final ASTImportDeclaration node;
|
||||
private final String name;
|
||||
private final String fullname;
|
||||
private final boolean isStaticDemand;
|
||||
private final Set<String> allStaticDemands;
|
||||
|
||||
public ImportWrapper(String fullname, String name) {
|
||||
this(fullname, name, null);
|
||||
@ -38,27 +43,38 @@ public class ImportWrapper {
|
||||
this.name = name;
|
||||
this.node = node;
|
||||
this.isStaticDemand = isStaticDemand;
|
||||
this.allStaticDemands = collectStaticFieldsAndMethods(node);
|
||||
|
||||
if (node != null && node.getType() != null) {
|
||||
}
|
||||
|
||||
/**
|
||||
* @param node
|
||||
*/
|
||||
private Set<String> collectStaticFieldsAndMethods(ASTImportDeclaration node) {
|
||||
if (!this.isStaticDemand || node == null || node.getType() == null) {
|
||||
return Collections.emptySet();
|
||||
}
|
||||
|
||||
try {
|
||||
Set<String> names = new HashSet<>();
|
||||
Class<?> type = node.getType();
|
||||
for (Method m : type.getMethods()) {
|
||||
allDemands.add(m.getName());
|
||||
}
|
||||
for (Field f : type.getFields()) {
|
||||
allDemands.add(f.getName());
|
||||
}
|
||||
// also consider static fields, that are not public
|
||||
// consider static fields, public and non-public
|
||||
for (Field f : type.getDeclaredFields()) {
|
||||
if (Modifier.isStatic(f.getModifiers())) {
|
||||
allDemands.add(f.getName());
|
||||
names.add(f.getName());
|
||||
}
|
||||
}
|
||||
// and methods, too
|
||||
for (Method m : type.getDeclaredMethods()) {
|
||||
if (Modifier.isStatic(m.getModifiers())) {
|
||||
allDemands.add(m.getName());
|
||||
names.add(m.getName());
|
||||
}
|
||||
}
|
||||
return names;
|
||||
} catch (LinkageError e) {
|
||||
// This is an incomplete classpath, report the missing class
|
||||
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing imports", e);
|
||||
return Collections.emptySet();
|
||||
}
|
||||
}
|
||||
|
||||
@ -86,7 +102,7 @@ public class ImportWrapper {
|
||||
|
||||
public boolean matches(ImportWrapper i) {
|
||||
if (isStaticDemand) {
|
||||
if (allDemands.contains(i.fullname)) {
|
||||
if (allStaticDemands.contains(i.fullname)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -15,6 +15,7 @@ import java.util.Map;
|
||||
import java.util.Map.Entry;
|
||||
import java.util.Set;
|
||||
import java.util.Stack;
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
|
||||
@ -131,7 +132,8 @@ public class MissingOverrideRule extends AbstractJavaRule {
|
||||
|
||||
return new MethodLookup(result, overridden);
|
||||
} catch (final LinkageError e) {
|
||||
// we may have an incomplete auxclasspath
|
||||
// This is an incomplete classpath, report the missing class
|
||||
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
@ -37,6 +37,7 @@ public class UnusedPrivateFieldRule extends AbstractLombokAwareRule {
|
||||
defaultValues.add("java.lang.Deprecated");
|
||||
defaultValues.add("javafx.fxml.FXML");
|
||||
defaultValues.add("lombok.experimental.Delegate");
|
||||
defaultValues.add("lombok.EqualsAndHashCode");
|
||||
return defaultValues;
|
||||
}
|
||||
|
||||
@ -50,7 +51,8 @@ public class UnusedPrivateFieldRule extends AbstractLombokAwareRule {
|
||||
VariableNameDeclaration decl = entry.getKey();
|
||||
AccessNode accessNodeParent = decl.getAccessNodeParent();
|
||||
if (!accessNodeParent.isPrivate() || isOK(decl.getImage()) || classHasLombok
|
||||
|| hasIgnoredAnnotation((Annotatable) accessNodeParent)) {
|
||||
|| hasIgnoredAnnotation((Annotatable) accessNodeParent)
|
||||
|| hasIgnoredAnnotation(node)) {
|
||||
continue;
|
||||
}
|
||||
if (!actuallyUsed(entry.getValue())) {
|
||||
|
@ -8,6 +8,8 @@ import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
|
||||
@ -15,6 +17,7 @@ import net.sourceforge.pmd.lang.java.ast.internal.ImportWrapper;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
|
||||
|
||||
public class DuplicateImportsRule extends AbstractJavaRule {
|
||||
private static final Logger LOG = Logger.getLogger(DuplicateImportsRule.class.getName());
|
||||
|
||||
private Set<ImportWrapper> singleTypeImports;
|
||||
private Set<ImportWrapper> importOnDemandImports;
|
||||
@ -68,11 +71,16 @@ public class DuplicateImportsRule extends AbstractJavaRule {
|
||||
} else {
|
||||
Class<?> importClass = node.getClassTypeResolver().loadClassOrNull(thisImportOnDemand.getName());
|
||||
if (importClass != null) {
|
||||
for (Method m : importClass.getMethods()) {
|
||||
if (Modifier.isStatic(m.getModifiers()) && m.getName().equals(singleTypeName)) {
|
||||
// static method in another imported class
|
||||
return true;
|
||||
try {
|
||||
for (Method m : importClass.getMethods()) {
|
||||
if (Modifier.isStatic(m.getModifiers()) && m.getName().equals(singleTypeName)) {
|
||||
// static method in another imported class
|
||||
return true;
|
||||
}
|
||||
}
|
||||
} catch (LinkageError e) {
|
||||
// This is an incomplete classpath, report the missing class
|
||||
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -11,6 +11,8 @@ import java.util.List;
|
||||
import java.util.Map.Entry;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
|
||||
@ -32,6 +34,7 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
|
||||
import net.sourceforge.pmd.lang.symboltable.Scope;
|
||||
|
||||
public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
|
||||
private static final Logger LOG = Logger.getLogger(UnnecessaryFullyQualifiedNameRule.class.getName());
|
||||
|
||||
private List<ASTImportDeclaration> imports = new ArrayList<>();
|
||||
private String currentPackage;
|
||||
@ -335,10 +338,15 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
|
||||
// We need type resolution to make sure there is a
|
||||
// conflicting method
|
||||
if (importDeclaration.getType() != null) {
|
||||
for (final Method m : importDeclaration.getType().getMethods()) {
|
||||
if (m.getName().equals(methodCalled)) {
|
||||
return true;
|
||||
try {
|
||||
for (final Method m : importDeclaration.getType().getMethods()) {
|
||||
if (m.getName().equals(methodCalled)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
} catch (LinkageError e) {
|
||||
// This is an incomplete classpath, report the missing class
|
||||
LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e);
|
||||
}
|
||||
}
|
||||
} else if (importDeclaration.getImportedName().endsWith(methodCalled)) {
|
||||
|
@ -4,6 +4,8 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.java.rule.design;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
|
||||
import net.sourceforge.pmd.lang.java.rule.internal.AbstractJavaCounterCheckRule;
|
||||
|
||||
@ -13,6 +15,7 @@ import net.sourceforge.pmd.lang.java.rule.internal.AbstractJavaCounterCheckRule;
|
||||
* topcount and sigma should work.)
|
||||
*/
|
||||
public class ExcessiveParameterListRule extends AbstractJavaCounterCheckRule<ASTFormalParameters> {
|
||||
|
||||
public ExcessiveParameterListRule() {
|
||||
super(ASTFormalParameters.class);
|
||||
}
|
||||
@ -22,9 +25,25 @@ public class ExcessiveParameterListRule extends AbstractJavaCounterCheckRule<AST
|
||||
return 10;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean isIgnored(ASTFormalParameters node) {
|
||||
if (areParametersOfPrivateConstructor(node)) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean areParametersOfPrivateConstructor(ASTFormalParameters params) {
|
||||
Node parent = params.getParent();
|
||||
if (parent instanceof ASTConstructorDeclaration) {
|
||||
ASTConstructorDeclaration constructor = (ASTConstructorDeclaration) parent;
|
||||
return constructor.isPrivate();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean isViolation(ASTFormalParameters node, int reportLevel) {
|
||||
return node.getParameterCount() > reportLevel;
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -11,6 +11,7 @@ import java.util.Collection;
|
||||
import java.util.List;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
|
||||
@ -24,6 +25,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTSynchronizedStatement;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
|
||||
import net.sourceforge.pmd.lang.java.ast.Annotatable;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule;
|
||||
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
|
||||
import net.sourceforge.pmd.properties.PropertyDescriptor;
|
||||
@ -62,6 +64,7 @@ public class SingularFieldRule extends AbstractLombokAwareRule {
|
||||
Collection<String> defaultValues = new ArrayList<>();
|
||||
defaultValues.addAll(super.defaultSuppressionAnnotations());
|
||||
defaultValues.add("lombok.experimental.Delegate");
|
||||
defaultValues.add("lombok.EqualsAndHashCode");
|
||||
return defaultValues;
|
||||
}
|
||||
|
||||
@ -71,107 +74,118 @@ public class SingularFieldRule extends AbstractLombokAwareRule {
|
||||
boolean checkInnerClasses = getProperty(CHECK_INNER_CLASSES);
|
||||
boolean disallowNotAssignment = getProperty(DISALLOW_NOT_ASSIGNMENT);
|
||||
|
||||
if (node.isPrivate() && !node.isStatic() && !hasClassLombokAnnotation() && !hasIgnoredAnnotation(node)) {
|
||||
for (ASTVariableDeclarator declarator : node.findChildrenOfType(ASTVariableDeclarator.class)) {
|
||||
ASTVariableDeclaratorId declaration = (ASTVariableDeclaratorId) declarator.getChild(0);
|
||||
List<NameOccurrence> usages = declaration.getUsages();
|
||||
Node decl = null;
|
||||
boolean violation = true;
|
||||
for (int ix = 0; ix < usages.size(); ix++) {
|
||||
NameOccurrence no = usages.get(ix);
|
||||
Node location = no.getLocation();
|
||||
if (!node.isPrivate() || node.isStatic()) {
|
||||
return data;
|
||||
}
|
||||
|
||||
ASTPrimaryExpression primaryExpressionParent = location
|
||||
.getFirstParentOfType(ASTPrimaryExpression.class);
|
||||
if (ix == 0 && !disallowNotAssignment) {
|
||||
if (primaryExpressionParent.getFirstParentOfType(ASTIfStatement.class) != null) {
|
||||
// the first usage is in an if, so it may be skipped
|
||||
// on
|
||||
// later calls to the method. So this might be legit
|
||||
// code
|
||||
// that simply stores an object for later use.
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
if (hasClassLombokAnnotation() || hasIgnoredAnnotation(node)) {
|
||||
return data;
|
||||
}
|
||||
|
||||
// Is the first usage in an assignment?
|
||||
Node potentialStatement = primaryExpressionParent.getParent();
|
||||
// Check that the assignment is not to a field inside
|
||||
// the field object
|
||||
boolean assignmentToField = no.getImage().equals(location.getImage());
|
||||
if (!assignmentToField || !isInAssignment(potentialStatement)) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
} else {
|
||||
if (usages.size() > ix + 1) {
|
||||
Node secondUsageLocation = usages.get(ix + 1).getLocation();
|
||||
// lombok.EqualsAndHashCode is a class-level annotation
|
||||
if (hasIgnoredAnnotation((Annotatable) node.getFirstParentOfType(ASTAnyTypeDeclaration.class))) {
|
||||
return data;
|
||||
}
|
||||
|
||||
List<ASTStatementExpression> parentStatements = secondUsageLocation
|
||||
.getParentsOfType(ASTStatementExpression.class);
|
||||
for (ASTStatementExpression statementExpression : parentStatements) {
|
||||
if (statementExpression != null && statementExpression.equals(potentialStatement)) {
|
||||
// The second usage is in the assignment
|
||||
// of the first usage, which is allowed
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
for (ASTVariableDeclarator declarator : node.findChildrenOfType(ASTVariableDeclarator.class)) {
|
||||
ASTVariableDeclaratorId declaration = (ASTVariableDeclaratorId) declarator.getChild(0);
|
||||
List<NameOccurrence> usages = declaration.getUsages();
|
||||
Node decl = null;
|
||||
boolean violation = true;
|
||||
for (int ix = 0; ix < usages.size(); ix++) {
|
||||
NameOccurrence no = usages.get(ix);
|
||||
Node location = no.getLocation();
|
||||
|
||||
ASTPrimaryExpression primaryExpressionParent = location
|
||||
.getFirstParentOfType(ASTPrimaryExpression.class);
|
||||
if (ix == 0 && !disallowNotAssignment) {
|
||||
if (primaryExpressionParent.getFirstParentOfType(ASTIfStatement.class) != null) {
|
||||
// the first usage is in an if, so it may be skipped
|
||||
// on
|
||||
// later calls to the method. So this might be legit
|
||||
// code
|
||||
// that simply stores an object for later use.
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
// Is the first usage in an assignment?
|
||||
Node potentialStatement = primaryExpressionParent.getParent();
|
||||
// Check that the assignment is not to a field inside
|
||||
// the field object
|
||||
boolean assignmentToField = no.getImage().equals(location.getImage());
|
||||
if (!assignmentToField || !isInAssignment(potentialStatement)) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
} else {
|
||||
if (usages.size() > ix + 1) {
|
||||
Node secondUsageLocation = usages.get(ix + 1).getLocation();
|
||||
|
||||
List<ASTStatementExpression> parentStatements = secondUsageLocation
|
||||
.getParentsOfType(ASTStatementExpression.class);
|
||||
for (ASTStatementExpression statementExpression : parentStatements) {
|
||||
if (statementExpression != null && statementExpression.equals(potentialStatement)) {
|
||||
// The second usage is in the assignment
|
||||
// of the first usage, which is allowed
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!checkInnerClasses) {
|
||||
// Skip inner classes because the field can be used in
|
||||
// the outer class and checking this is too difficult
|
||||
ASTClassOrInterfaceDeclaration clazz = location
|
||||
.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
|
||||
if (clazz != null && clazz.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) != null) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
}
|
||||
|
||||
if (primaryExpressionParent.getParent() instanceof ASTSynchronizedStatement) {
|
||||
// This usage is directly in an expression of a
|
||||
// synchronized block
|
||||
if (!checkInnerClasses) {
|
||||
// Skip inner classes because the field can be used in
|
||||
// the outer class and checking this is too difficult
|
||||
ASTClassOrInterfaceDeclaration clazz = location
|
||||
.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
|
||||
if (clazz != null && clazz.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) != null) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
}
|
||||
|
||||
if (location.getFirstParentOfType(ASTLambdaExpression.class) != null) {
|
||||
// This usage is inside a lambda expression
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
if (primaryExpressionParent.getParent() instanceof ASTSynchronizedStatement) {
|
||||
// This usage is directly in an expression of a
|
||||
// synchronized block
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
Node method = location.getFirstParentOfType(ASTMethodDeclaration.class);
|
||||
if (location.getFirstParentOfType(ASTLambdaExpression.class) != null) {
|
||||
// This usage is inside a lambda expression
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
Node method = location.getFirstParentOfType(ASTMethodDeclaration.class);
|
||||
if (method == null) {
|
||||
method = location.getFirstParentOfType(ASTConstructorDeclaration.class);
|
||||
if (method == null) {
|
||||
method = location.getFirstParentOfType(ASTConstructorDeclaration.class);
|
||||
method = location.getFirstParentOfType(ASTInitializer.class);
|
||||
if (method == null) {
|
||||
method = location.getFirstParentOfType(ASTInitializer.class);
|
||||
if (method == null) {
|
||||
continue;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
if (decl == null) {
|
||||
decl = method;
|
||||
continue;
|
||||
} else if (decl != method
|
||||
// handle inner classes
|
||||
&& decl.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) == method
|
||||
.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
if (violation && !usages.isEmpty()) {
|
||||
addViolation(data, node, new Object[] { declaration.getImage() });
|
||||
if (decl == null) {
|
||||
decl = method;
|
||||
continue;
|
||||
} else if (decl != method
|
||||
// handle inner classes
|
||||
&& decl.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class) == method
|
||||
.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)) {
|
||||
violation = false;
|
||||
break; // Optimization
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
if (violation && !usages.isEmpty()) {
|
||||
addViolation(data, node, new Object[] { declaration.getImage() });
|
||||
}
|
||||
}
|
||||
return data;
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -8,7 +8,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
|
||||
import java.util.List;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
@ -21,36 +20,40 @@ public class ProperCloneImplementationRule extends AbstractJavaRule {
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
if (!"clone".equals(node.getName()) || node.getArity() > 0) {
|
||||
return data;
|
||||
public Object visit(ASTMethodDeclaration method, Object data) {
|
||||
if (isCloneMethod(method) && isNotAbstractMethod(method)) {
|
||||
ASTClassOrInterfaceDeclaration classDecl = method.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
|
||||
if (isNotFinal(classDecl) && hasAnyAllocationOfClass(method, classDecl.getSimpleName())) {
|
||||
addViolation(data, method);
|
||||
}
|
||||
}
|
||||
|
||||
ASTBlock block = node.getFirstChildOfType(ASTBlock.class);
|
||||
if (block == null) {
|
||||
return data;
|
||||
}
|
||||
|
||||
String enclosingClassName = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class).getSimpleName();
|
||||
if (blockHasAllocations(block, enclosingClassName)) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
|
||||
return data;
|
||||
}
|
||||
|
||||
private boolean blockHasAllocations(ASTBlock block, String enclosingClassName) {
|
||||
List<ASTAllocationExpression> allocations = block.findDescendantsOfType(ASTAllocationExpression.class);
|
||||
for (ASTAllocationExpression alloc : allocations) {
|
||||
ASTClassOrInterfaceType type = alloc.getFirstChildOfType(ASTClassOrInterfaceType.class);
|
||||
if (typeHasImage(type, enclosingClassName)) {
|
||||
private boolean isCloneMethod(ASTMethodDeclaration method) {
|
||||
return "clone".equals(method.getName()) && method.getArity() == 0;
|
||||
}
|
||||
|
||||
private boolean isNotAbstractMethod(ASTMethodDeclaration method) {
|
||||
return !method.isAbstract();
|
||||
}
|
||||
|
||||
private boolean isNotFinal(ASTClassOrInterfaceDeclaration classOrInterfaceDecl) {
|
||||
return !classOrInterfaceDecl.isFinal();
|
||||
}
|
||||
|
||||
private boolean hasAnyAllocationOfClass(ASTMethodDeclaration method, String className) {
|
||||
List<ASTAllocationExpression> allocations = method.findDescendantsOfType(ASTAllocationExpression.class);
|
||||
for (ASTAllocationExpression allocation : allocations) {
|
||||
ASTClassOrInterfaceType allocatedType = allocation.getFirstChildOfType(ASTClassOrInterfaceType.class);
|
||||
if (isSimpleNameOfType(className, allocatedType)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean typeHasImage(ASTClassOrInterfaceType type, String image) {
|
||||
return type != null && type.hasImageEqualTo(image);
|
||||
private boolean isSimpleNameOfType(String simpleName, ASTClassOrInterfaceType type) {
|
||||
return type != null && type.hasImageEqualTo(simpleName);
|
||||
}
|
||||
}
|
||||
|
@ -475,8 +475,9 @@ public final class MethodTypeResolution {
|
||||
result.add(getTypeDefOfMethod(context, method, typeArguments));
|
||||
}
|
||||
}
|
||||
} catch (final LinkageError ignored) {
|
||||
// TODO : This is an incomplete classpath, report the missing class
|
||||
} 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);
|
||||
}
|
||||
|
||||
// search it's supertype
|
||||
|
@ -1197,7 +1197,7 @@ A method should have only one exit point, and that should be the last statement
|
||||
<example>
|
||||
<![CDATA[
|
||||
public class OneReturnOnly1 {
|
||||
public void foo(int x) {
|
||||
public String foo(int x) {
|
||||
if (x > 0) {
|
||||
return "hey"; // first exit
|
||||
}
|
||||
|
@ -24,30 +24,50 @@ gets it.
|
||||
<properties>
|
||||
<property name="version" value="2.0"/>
|
||||
<property name="xpath">
|
||||
<value>//MethodDeclaration[@Synchronized= true()]</value>
|
||||
<value>//MethodDeclaration[@Synchronized = true()]</value>
|
||||
</property>
|
||||
</properties>
|
||||
<example>
|
||||
<![CDATA[
|
||||
public class Foo {
|
||||
// Try to avoid this:
|
||||
synchronized void foo() {
|
||||
}
|
||||
// Prefer this:
|
||||
void bar() {
|
||||
synchronized(this) {
|
||||
// Try to avoid this:
|
||||
synchronized void foo() {
|
||||
// code, that doesn't need synchronization
|
||||
// ...
|
||||
// code, that requires synchronization
|
||||
if (!sharedData.has("bar")) {
|
||||
sharedData.add("bar");
|
||||
}
|
||||
// more code, that doesn't need synchronization
|
||||
// ...
|
||||
}
|
||||
}
|
||||
|
||||
// Try to avoid this for static methods:
|
||||
static synchronized void fooStatic() {
|
||||
}
|
||||
|
||||
// Prefer this:
|
||||
static void barStatic() {
|
||||
synchronized(Foo.class) {
|
||||
// Prefer this:
|
||||
void bar() {
|
||||
// code, that doesn't need synchronization
|
||||
// ...
|
||||
synchronized(this) {
|
||||
if (!sharedData.has("bar")) {
|
||||
sharedData.add("bar");
|
||||
}
|
||||
}
|
||||
// more code, that doesn't need synchronization
|
||||
// ...
|
||||
}
|
||||
|
||||
// Try to avoid this for static methods:
|
||||
static synchronized void fooStatic() {
|
||||
}
|
||||
|
||||
// Prefer this:
|
||||
static void barStatic() {
|
||||
// code, that doesn't need synchronization
|
||||
// ...
|
||||
synchronized(Foo.class) {
|
||||
// code, that requires synchronization
|
||||
}
|
||||
// more code, that doesn't need synchronization
|
||||
// ...
|
||||
}
|
||||
}
|
||||
}
|
||||
]]>
|
||||
</example>
|
||||
|
@ -83,6 +83,11 @@ public class JavaTokenizerTest extends CpdTextComparisonTest {
|
||||
doTest("ignoreLiterals", "_noignore");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTabWidth() {
|
||||
doTest("tabWidth");
|
||||
}
|
||||
|
||||
|
||||
private static Properties ignoreAnnotations() {
|
||||
return properties(true, false, false);
|
||||
|
1
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/cpd/testdata/tabWidth.java
vendored
Normal file
1
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/cpd/testdata/tabWidth.java
vendored
Normal file
@ -0,0 +1 @@
|
||||
int i = 0;
|
7
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/cpd/testdata/tabWidth.txt
vendored
Normal file
7
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/cpd/testdata/tabWidth.txt
vendored
Normal file
@ -0,0 +1,7 @@
|
||||
[Image] or [Truncated image[ Bcol Ecol
|
||||
L1
|
||||
[int] 2 4
|
||||
[i] 6 6
|
||||
[=] 8 8
|
||||
[0] 10 10
|
||||
EOF
|
@ -626,6 +626,18 @@ public class Foo {
|
||||
|
||||
@Value
|
||||
private String bar;
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>#2673 UnusedPrivateField false positive with lombok annotation EqualsAndHashCode</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import lombok.EqualsAndHashCode;
|
||||
@EqualsAndHashCode
|
||||
public class Foo {
|
||||
private String bar;
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
Some files were not shown because too many files have changed in this diff Show More
Loading…
x
Reference in New Issue
Block a user