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

Merge branch 'pr-2643'

Browse Source 
[java] AvoidCallingFinalize detects some false positives (2578) #2643
This commit is contained in:
Andreas Dangel
2020-07-17 20:30:01 +02:00
parent 585b9099ca 6b69964733
commit 25405eb870
3 changed files with 107 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/pages/release_notes.md
View File

@ -34,6 +34,7 @@ This is a {{ site.pmd.release_type }} release.
* [#2174](https://github.com/pmd/pmd/issues/2174): \[java] LawOfDemeter: False positive with 'this' pointer
* [#2189](https://github.com/pmd/pmd/issues/2189): \[java] LawOfDemeter: False positive when casting to derived class
* java-errorprone
* [#2578](https://github.com/pmd/pmd/issues/2578): \[java] AvoidCallingFinalize detects some false positives
* [#2634](https://github.com/pmd/pmd/issues/2634): \[java] NullPointerException in rule ProperCloneImplementation
* java-performance
* [#1736](https://github.com/pmd/pmd/issues/1736): \[java] UseStringBufferForStringAppends: False positive if only one concatenation
@ -58,6 +59,7 @@ This is a {{ site.pmd.release_type }} release.
* [#2597](https://github.com/pmd/pmd/pull/2597): \[dependencies] Fix issue #2594, update exec-maven-plugin everywhere - [Artem Krosheninnikov](https://github.com/KroArtem)
* [#2621](https://github.com/pmd/pmd/pull/2621): \[visualforce] add new safe resource for VfUnescapeEl - [Peter Chittum](https://github.com/pchittum)
* [#2640](https://github.com/pmd/pmd/pull/2640): \[java] NullPointerException in rule ProperCloneImplementation - [Mykhailo Palahuta](https://github.com/Drofff)
* [#2643](https://github.com/pmd/pmd/pull/2643): \[java] AvoidCallingFinalize detects some false positives (2578) - [Mykhailo Palahuta](https://github.com/Drofff)
{% endtocmaker %}

99
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidCallingFinalizeRule.java
View File

@ -4,68 +4,79 @@
package net.sourceforge.pmd.lang.java.rule.errorprone;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
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.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.MethodScope;
import net.sourceforge.pmd.lang.symboltable.ScopedNode;
public class AvoidCallingFinalizeRule extends AbstractJavaRule {
private Set<MethodScope> checked = new HashSet<>();
private static final Pattern FINALIZE_METHOD_PATTERN = Pattern.compile("^(.+\\.)?finalize$");
@Override
public Object visit(ASTCompilationUnit acu, Object ctx) {
checked.clear();
return super.visit(acu, ctx);
public AvoidCallingFinalizeRule() {
addRuleChainVisit(ASTPrimaryExpression.class);
}
@Override
public Object visit(ASTName name, Object ctx) {
if (name.getImage() == null || !name.getImage().endsWith("finalize")) {
return ctx;
public Object visit(ASTPrimaryExpression primaryExpression, Object data) {
if (isIncorrectFinalizeMethodCall(primaryExpression)) {
addViolation(data, primaryExpression);
}
if (!checkForViolation(name)) {
return ctx;
}
addViolation(ctx, name);
return ctx;
return data;
}
@Override
public Object visit(ASTPrimaryPrefix pp, Object ctx) {
List<ASTPrimarySuffix> primarySuffixes = pp.getParent().findChildrenOfType(ASTPrimarySuffix.class);
ASTPrimarySuffix firstSuffix = null;
private boolean isIncorrectFinalizeMethodCall(ASTPrimaryExpression primaryExpression) {
return isFinalizeMethodCall(primaryExpression)
&& (isNotInFinalizeMethod(primaryExpression) || isNotSuperMethodCall(primaryExpression));
}
private boolean isNotInFinalizeMethod(ASTPrimaryExpression primaryExpression) {
ASTMethodDeclaration methodDeclaration = primaryExpression.getFirstParentOfType(ASTMethodDeclaration.class);
return methodDeclaration == null || isNotFinalizeMethodDeclaration(methodDeclaration);
}
private boolean isNotFinalizeMethodDeclaration(ASTMethodDeclaration methodDeclaration) {
return !isFinalizeMethodDeclaration(methodDeclaration);
}
private boolean isFinalizeMethodDeclaration(ASTMethodDeclaration methodDeclaration) {
return "finalize".equals(methodDeclaration.getName()) && methodDeclaration.getArity() == 0;
}
private boolean isFinalizeMethodCall(ASTPrimaryExpression primaryExpression) {
return hasFinalizeName(primaryExpression) && getArgsCount(primaryExpression) == 0;
}
private boolean hasFinalizeName(ASTPrimaryExpression primaryExpression) {
List<JavaNode> expressionNodes = primaryExpression.findDescendantsOfType(JavaNode.class);
for (JavaNode expressionNode : expressionNodes) {
if (isFinalizeName(expressionNode.getImage())) {
return true;
}
}
return false;
}
private boolean isFinalizeName(String name) {
return name != null && FINALIZE_METHOD_PATTERN.matcher(name).find();
}
private int getArgsCount(ASTPrimaryExpression primaryExpression) {
List<ASTPrimarySuffix> primarySuffixes = primaryExpression.findChildrenOfType(ASTPrimarySuffix.class);
if (!primarySuffixes.isEmpty()) {
firstSuffix = primarySuffixes.get(0);
int lastSuffixIndex = primarySuffixes.size() - 1;
return primarySuffixes.get(lastSuffixIndex).getArgumentCount();
}
if (firstSuffix == null || firstSuffix.getImage() == null || !firstSuffix.getImage().endsWith("finalize")) {
return super.visit(pp, ctx);
}
if (!checkForViolation(pp)) {
return super.visit(pp, ctx);
}
addViolation(ctx, pp);
return super.visit(pp, ctx);
return -1;
}
private boolean checkForViolation(ScopedNode node) {
MethodScope meth = node.getScope().getEnclosingScope(MethodScope.class);
if (meth != null && "finalize".equals(meth.getName())) {
return false;
}
if (meth != null && checked.contains(meth)) {
return false;
}
if (meth != null) {
checked.add(meth);
}
return true;
private boolean isNotSuperMethodCall(ASTPrimaryExpression primaryExpression) {
ASTPrimaryPrefix primaryPrefix = primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
return primaryPrefix == null || !primaryPrefix.usesSuperModifier();
}
}

53
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidCallingFinalize.xml
View File

@ -1,12 +1,13 @@
<?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">
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>simple failure case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
void foo () {
@ -19,6 +20,7 @@ public class Foo {
<test-code>
<description>calling finalize on an object</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
void foo () {
@ -32,6 +34,7 @@ public class Foo {
<test-code>
<description>calling super.finalize</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
void foo () {
@ -83,11 +86,55 @@ public class Foo {
<test-code>
<description>#1440 NPE in AvoidCallingFinalize</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class InputFinalize {
{
super.finalize();
}
}
]]></code>
</test-code>
<test-code>
<description>overloaded finalize is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
finalize("hello", "world");
}
public void finalize(String ... args) {}
}
]]></code>
</test-code>
<test-code>
<description>variable name false-positive test</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
private int finalize;
public void bar() {
finalize++;
return finalize;
}
}
]]></code>
</test-code>
<test-code>
<description>super.finalize in constructor false-negative test</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
public Foo() throws Throwable {
super.equals(new String());
super.finalize();
}
}
]]></code>
</test-code>