Merge pull request #3621 from adangel:java-rule-improvements-3

[java] Rule improvements part 3 #3621

* pr-3621:
  [doc] Update release notes (#3620)
  [java] CheckSkipResult - use rulechain
  [java] AvoidUsingOctalValues - use rulechain
  [java] AvoidMultipleUnaryOperators - remove unnecessary super
  [java] SwitchDensity - use super.visit
  [java] SingularField - fix false negative with anonymous classes
This commit is contained in:
Andreas Dangel
2021-11-26 08:47:58 +01:00
9 changed files with 73 additions and 18 deletions

View File

@ -25,6 +25,8 @@ This is a {{ site.pmd.release_type }} release.
* [#3614](https://github.com/pmd/pmd/issues/3614): \[java] JUnitTestsShouldIncludeAssert doesn't consider nested classes
* [#3618](https://github.com/pmd/pmd/issues/3618): \[java] UnusedFormalParameter doesn't consider anonymous classes
* [#3630](https://github.com/pmd/pmd/issues/3630): \[java] MethodReturnsInternalArray doesn't consider anonymous classes
* java-design
* [#3620](https://github.com/pmd/pmd/issues/3620): \[java] SingularField doesn't consider anonymous classes defined in non-private fields
* java-errorprone
* [#3624](https://github.com/pmd/pmd/issues/3624): \[java] TestClassWithoutTestCases reports wrong classes in a file
* java-performance

View File

@ -74,16 +74,16 @@ public class SingularFieldRule extends AbstractLombokAwareRule {
boolean disallowNotAssignment = getProperty(DISALLOW_NOT_ASSIGNMENT);
if (!node.isPrivate() || node.isStatic()) {
return data;
return super.visit(node, data);
}
if (hasClassLombokAnnotation() || hasIgnoredAnnotation(node)) {
return data;
return super.visit(node, data);
}
// lombok.EqualsAndHashCode is a class-level annotation
if (hasIgnoredAnnotation((Annotatable) node.getFirstParentOfType(ASTAnyTypeDeclaration.class))) {
return data;
return super.visit(node, data);
}
for (ASTVariableDeclarator declarator : node.findChildrenOfType(ASTVariableDeclarator.class)) {
@ -194,7 +194,7 @@ public class SingularFieldRule extends AbstractLombokAwareRule {
addViolation(data, node, new Object[] { declaration.getImage() });
}
}
return data;
return super.visit(node, data);
}
private boolean isInAssignment(Node potentialStatement) {

View File

@ -51,7 +51,6 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule {
}
public SwitchDensityRule() {
super();
setProperty(MINIMUM_DESCRIPTOR, 10d);
}
@ -65,7 +64,7 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule {
SwitchDensity density = new SwitchDensity();
node.childrenAccept(this, density);
super.visit(node, density);
DataPoint point = new DataPoint();
point.setNode(node);
@ -86,9 +85,7 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule {
((SwitchDensity) data).addStatement();
}
statement.childrenAccept(this, data);
return data;
return super.visit(statement, data);
}
@Override
@ -97,7 +94,6 @@ public class SwitchDensityRule extends AbstractStatisticalJavaRule {
((SwitchDensity) data).addSwitchLabel();
}
switchLabel.childrenAccept(this, data);
return data;
return super.visit(switchLabel, data);
}
}

View File

@ -15,8 +15,8 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
public class AvoidMultipleUnaryOperatorsRule extends AbstractJavaRule {
public AvoidMultipleUnaryOperatorsRule() {
super.addRuleChainVisit(ASTUnaryExpression.class);
super.addRuleChainVisit(ASTUnaryExpressionNotPlusMinus.class);
addRuleChainVisit(ASTUnaryExpression.class);
addRuleChainVisit(ASTUnaryExpressionNotPlusMinus.class);
}
@Override

View File

@ -27,6 +27,7 @@ public class AvoidUsingOctalValuesRule extends AbstractJavaRule {
public AvoidUsingOctalValuesRule() {
definePropertyDescriptor(STRICT_METHODS_DESCRIPTOR);
addRuleChainVisit(ASTLiteral.class);
}
@Override

View File

@ -19,6 +19,10 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
public class CheckSkipResultRule extends AbstractJavaRule {
public CheckSkipResultRule() {
addRuleChainVisit(ASTVariableDeclaratorId.class);
}
@Override
public Object visit(ASTVariableDeclaratorId node, Object data) {
if (!TypeTestUtil.isA(InputStream.class, node.getTypeNode())) {

View File

@ -7,16 +7,17 @@
<test-code>
<description>failure case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
public class Foo {
private int x;
private int x;
int bar(int y) {
int bar(int y) {
x = y + 5;
return x;
}
}
}
}
]]></code>
</test-code>
@ -190,6 +191,7 @@ public class Foo {
<test-code>
<description>Reuse variable name as params in method calls</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
public class Foo {
private Integer x = new Integer(1);
@ -244,6 +246,7 @@ public class Foo {
<test-code>
<description>failure, static</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int x;
@ -257,6 +260,7 @@ public class Foo {
<test-code>
<description>failure, second method re-uses class level name</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int x;
@ -308,6 +312,7 @@ public class Foo {
<test-code>
<description>1409944, fields not used to synchronize should trigger</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
private final Object sync = new Object();
@ -365,6 +370,7 @@ public class Foo {
<description>Not ok, since inner classes are checked</description>
<rule-property name="checkInnerClasses">true</rule-property>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
public class Foo {
private class Bar {
@ -387,6 +393,7 @@ public class Foo {
<description>Not ok, violation with first usage = non-assignment</description>
<rule-property name="disallowNotAssignment">true</rule-property>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int x;
@ -429,6 +436,7 @@ public class Foo {
<test-code>
<description>multiple fields on same line</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int x, foo;
@ -572,6 +580,7 @@ public class Outer {
<test-code>
<description>[java] SingularField: Lombok false positive with annotated inner class</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code><![CDATA[
import lombok.Data;
@ -751,4 +760,44 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>failure case with anonymous class</description>
<expected-problems>4</expected-problems>
<expected-linenumbers>4,13,20,23</expected-linenumbers>
<code><![CDATA[
public class Foo {
void myMethod() {
Object o = new Object() {
private int x;
int bar(int y) {
x = y + 5;
return x;
}
};
}
Object field = new Object() {
private int x;
int bar(int y) {
x = y + 5;
return x;
}
};
private Object doubleSingular;
Object doBar(final int y) {
doubleSingular = new Object() {
private int x;
int bar() {
x = y + 5;
return x;
}
};
return doubleSingular;
}
}
]]></code>
</test-code>
</test-data>

View File

@ -8,6 +8,7 @@
<description>Five stmts in one switch case, should be flagged</description>
<rule-property name="minimum">4</rule-property>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
// Switch Density = 5.0
public class SwitchDensity1 {

View File

@ -7,6 +7,7 @@
<test-code>
<description>failure case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
import java.io.FileInputStream;
@ -24,6 +25,7 @@ public class Foo {
<test-code>
<description>failure case but obfuscated</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
import java.io.FileInputStream;