Skip to content

Commit

Permalink
Make unused rules ignore some names, fix #2957
Browse files Browse the repository at this point in the history
  • Loading branch information
oowekyala committed Jan 16, 2021
1 parent 9158c9f commit 27d98fa
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 13 deletions.
1 change: 1 addition & 0 deletions docs/pages/release_notes.md
Expand Up @@ -20,6 +20,7 @@ This is a {{ site.pmd.release_type }} release.
* [#2994](https://github.com/pmd/pmd/pull/2994): \[core] Fix code climate severity strings
* java-bestpractices
* [#575](https://github.com/pmd/pmd/issues/575): \[java] LiteralsFirstInComparisons should consider constant fields
* [#2957](https://github.com/pmd/pmd/issues/2957): \[java] Ignore unused declarations that have special name

### API Changes

Expand Down
Expand Up @@ -78,6 +78,7 @@
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.Scope;
Expand Down Expand Up @@ -206,7 +207,7 @@ private void reportFinished(GlobalAlgoState result, RuleContext ruleCtx) {
} else {
reason = joinLines("overwritten on lines ", killers);
}
if (reason == null && hasExplicitIgnorableName(entry.var.getName())) {
if (reason == null && JavaRuleUtil.isExplicitUnusedVarName(entry.var.getName())) {
// Then the variable is never used (cf UnusedVariable)
// We ignore those that start with "ignored", as that is standard
// practice for exceptions, and may be useful for resources/foreach vars
Expand All @@ -217,11 +218,6 @@ private void reportFinished(GlobalAlgoState result, RuleContext ruleCtx) {
}
}

private boolean hasExplicitIgnorableName(String name) {
return name.startsWith("ignored")
|| "_".equals(name); // before java 9 it's ok
}

private boolean suppressUnusedVariableRuleOverlap(AssignmentEntry entry) {
return !getProperty(REPORT_UNUSED_VARS) && (entry.rhs instanceof ASTVariableInitializer
|| entry.rhs instanceof ASTVariableDeclaratorId);
Expand Down
Expand Up @@ -24,6 +24,7 @@
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.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
Expand Down Expand Up @@ -96,7 +97,8 @@ private void check(Node node, Object data) {
continue;
}

if (actuallyUsed(nameDecl, entry.getValue())) {
if (actuallyUsed(nameDecl, entry.getValue())
|| JavaRuleUtil.isExplicitUnusedVarName(nameDecl.getName())) {
continue;
}
addViolation(data, nameDecl.getNode(), new Object[] {
Expand Down
Expand Up @@ -10,6 +10,7 @@
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;

Expand All @@ -29,7 +30,9 @@ public Object visit(ASTLocalVariableDeclaration decl, Object data) {
// TODO this isArray() check misses some cases
// need to add DFAish code to determine if an array
// is initialized locally or gotten from somewhere else
if (!node.getNameDeclaration().isArray() && !actuallyUsed(node.getUsages())) {
if (!node.getNameDeclaration().isArray()
&& !actuallyUsed(node.getUsages())
&& !JavaRuleUtil.isExplicitUnusedVarName(node.getName())) {
addViolation(data, node, node.getNameDeclaration().getImage());
}
}
Expand Down
@@ -0,0 +1,25 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/

package net.sourceforge.pmd.lang.java.rule.internal;

/**
* Utilities shared between rules (note: in pmd 7 this is used much more extensively).
*/
public final class JavaRuleUtil {

private JavaRuleUtil() {
// utility class
}


/**
* Whether the name may be ignored by unused rules like UnusedAssignment.
*/
public static boolean isExplicitUnusedVarName(String name) {
return name.startsWith("ignored")
|| name.startsWith("unused")
|| "_".equals(name); // before java 9 it's ok
}
}
8 changes: 6 additions & 2 deletions pmd-java/src/main/resources/category/java/bestpractices.xml
Expand Up @@ -1365,7 +1365,7 @@ class Foo{
The rule subsumes {% rule "UnusedLocalVariable" %}, and {% rule "UnusedFormalParameter" %}.
Those violations are filtered
out by default, in case you already have enabled those rules, but may be enabled with the property
`reportUnusedVariables`. Variables whose name starts with `ignored` are filtered out, as
`reportUnusedVariables`. Variables whose name starts with `ignored` or `unused` are filtered out, as
is standard practice for exceptions.

Limitations:
Expand Down Expand Up @@ -1476,10 +1476,13 @@ class C {
class="net.sourceforge.pmd.lang.java.rule.bestpractices.UnusedFormalParameterRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#unusedformalparameter">
<description>
Avoid passing parameters to methods or constructors without actually referencing them in the method body.
Reports parameters of methods and constructors that are not referenced them in the method body.
Parameters whose name starts with `ignored` or `unused` are filtered out.

Removing unused formal parameters from public methods could cause a ripple effect through the code base.
Hence, by default, this rule only considers private methods. To include non-private methods, set the
`checkAll` property to `true`.

</description>
<priority>3</priority>
<example>
Expand Down Expand Up @@ -1523,6 +1526,7 @@ public class Foo {}
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#unusedlocalvariable">
<description>
Detects when a local variable is declared and/or assigned, but not used.
Variables whose name starts with `ignored` or `unused` are filtered out.
</description>
<priority>3</priority>
<example>
Expand Down
Expand Up @@ -2797,10 +2797,10 @@ class Foo {
<test-code>
<description>Test ignored name 0</description>
<rule-property name="reportUnusedVariables">true</rule-property>
<expected-problems>1</expected-problems>
<expected-problems>2</expected-problems>
<code><![CDATA[
class Foo {
int method(int param) {
int method(int param, int other) {
return 2;
}
}
Expand All @@ -2814,7 +2814,7 @@ class Foo {
<expected-problems>0</expected-problems>
<code><![CDATA[
class Foo {
int method(int ignored) {
int method(int ignored, int unused) {
return 2;
}
}
Expand Down
Expand Up @@ -287,6 +287,21 @@ class Imp2 extends Base {
public int badMethod(int arg1, String arg2) {
return arg2.length() + arg1;
}
}
]]></code>
</test-code>
<test-code>
<description>#2957 Unused rules should ignore some names</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>2,3</expected-linenumbers>
<code><![CDATA[
class Imp1 {
private void m(int arg1,
String arg2,
int unusedArg1,
int ignoredArg2,
int unused,
String ignored) {}
}
]]></code>
</test-code>
Expand Down
Expand Up @@ -378,6 +378,19 @@ public class UnusedLocalVariable {
System.out.println("foo!");
}
}
}
]]></code>
</test-code>
<test-code>
<description>#2957 Unused rules should ignore some names</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class UnusedLocalVariable {
public void testSomething() {
int ignored, unused = 0;
int notok = 0;
}
}
]]></code>
</test-code>
Expand Down

0 comments on commit 27d98fa

Please sign in to comment.