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

Merge branch 'pr-969'

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero
2018-03-10 17:16:51 -03:00
parent 5ba5082ae2 20cddf5079
commit 05ab784d0b
4 changed files with 175 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
docs/pages/release_notes.md
View File

@@ -37,6 +37,8 @@ Both are bugfixing releases.
* [#907](https://github.com/pmd/pmd/issues/907): \[java] UnusedPrivateField false-positive with @FXML
* java-bestpracrtices
* [#963](https://github.com/pmd/pmd/issues/965): \[java] ArrayIsStoredDirectly not triggered from variadic functions
* java-design
* [#968](https://github.com/pmd/pmd/issues/968): \[java] UseUtilityClassRule reports false positive with lombok NoArgsConstructor
### API Changes
@@ -46,5 +48,6 @@ Both are bugfixing releases.
* [#943](https://github.com/pmd/pmd/pull/943): \[java] UnusedPrivateField false-positive with @FXML - [BBG](https://github.com/djydewang)
* [#965](https://github.com/pmd/pmd/pull/965): \[java] Make Varargs trigger ArrayIsStoredDirectly - [Stephen](https://github.com/pmd/pmd/issues/907)
* [#967](https://github.com/pmd/pmd/pull/967): \[doc] Issue 959: fixed broken link to XPath Rule Tutorial - [Andrey Mochalov](https://github.com/epidemia)
* [#969](https://github.com/pmd/pmd/pull/969): \[java] Issue 968 Add logic to handle lombok private constructors with utility classes - [Kirk Clemens](https://github.com/clem0110)
* [#970](https://github.com/pmd/pmd/pull/970): \[java] Fixed inefficient use of keySet iterator instead of entrySet iterator - [Andrey Mochalov](https://github.com/epidemia)

25
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractLombokAwareRule.java
View File

@@ -36,6 +36,7 @@ public class AbstractLombokAwareRule extends AbstractJavaRule {
LOMBOK_ANNOTATIONS.add("Value");
LOMBOK_ANNOTATIONS.add("RequiredArgsConstructor");
LOMBOK_ANNOTATIONS.add("AllArgsConstructor");
LOMBOK_ANNOTATIONS.add("NoArgsConstructor");
LOMBOK_ANNOTATIONS.add("Builder");
}
@@ -110,4 +111,28 @@ public class AbstractLombokAwareRule extends AbstractJavaRule {
}
return result;
}
protected ASTAnnotation getLombokAnnotation(Node node, String lombokAnnotation) {
Node parent = node.jjtGetParent();
List<ASTAnnotation> annotations = parent.findChildrenOfType(ASTAnnotation.class);
for (ASTAnnotation annotation : annotations) {
ASTName name = annotation.getFirstDescendantOfType(ASTName.class);
if (name != null) {
String annotationName = name.getImage();
if (lombokImported) {
if (lombokAnnotation.equals(annotationName)) {
return annotation;
}
} else {
if (annotationName.startsWith(LOMBOK_PACKAGE + ".")) {
String shortName = annotationName.substring(LOMBOK_PACKAGE.length() + 1);
if (lombokAnnotation.equals(shortName)) {
return annotation;
}
}
}
}
}
return null;
}
}

44
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java
View File

@@ -4,6 +4,8 @@
package net.sourceforge.pmd.lang.java.rule.design;
import java.util.List;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
@@ -12,11 +14,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTExtendsList;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule;
public class UseUtilityClassRule extends AbstractJavaRule {
public class UseUtilityClassRule extends AbstractLombokAwareRule {
@Override
public Object visit(ASTClassOrInterfaceBody decl, Object data) {
@@ -25,6 +29,11 @@ public class UseUtilityClassRule extends AbstractJavaRule {
if (parent.isAbstract() || parent.isInterface() || isExceptionType(parent)) {
return super.visit(decl, data);
}
if (isOkUsingLombok(parent)) {
return super.visit(decl, data);
}
int i = decl.jjtGetNumChildren();
int methodCount = 0;
boolean isOK = false;
@@ -63,7 +72,6 @@ public class UseUtilityClassRule extends AbstractJavaRule {
break;
}
}
}
}
if (!isOK && methodCount > 0) {
@@ -73,6 +81,35 @@ public class UseUtilityClassRule extends AbstractJavaRule {
return super.visit(decl, data);
}
private boolean isOkUsingLombok(ASTClassOrInterfaceDeclaration parent) {
// check if there's a lombok no arg private constructor, if so skip the rest of the rules
if (hasClassLombokAnnotation()) {
ASTAnnotation annotation = getLombokAnnotation(parent, "NoArgsConstructor");
if (annotation != null) {
List<ASTMemberValuePair> memberValuePairs = annotation.findDescendantsOfType(ASTMemberValuePair.class);
for (ASTMemberValuePair memberValuePair : memberValuePairs) {
// to set the access level of a constructor in lombok, you set the access property on the annotation
if ("access".equals(memberValuePair.getImage())) {
List<ASTName> names = memberValuePair.findDescendantsOfType(ASTName.class);
for (ASTName name : names) {
// check to see if the value of the member value pair ends PRIVATE. This is from the AccessLevel enum in Lombok
if (name.getImage().endsWith("PRIVATE")) {
// if the constructor is found and the accesslevel is private no need to check anything else
return true;
}
}
}
}
}
}
return false;
}
private Node skipAnnotations(Node p) {
int index = 0;
Node n = p.jjtGetChild(index++);
@@ -95,5 +132,4 @@ public class UseUtilityClassRule extends AbstractJavaRule {
}
return false;
}
}

107
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml
View File

@@ -238,4 +238,111 @@ public class AccountFragment extends Fragment {
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok NoArgsConstructor - ok
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import lombok.NoArgsConstructor;
import lombok.AccessLevel;
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok NoArgsConstructor static import- ok
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import static lombok.AccessLevel.PRIVATE;
import lombok.NoArgsConstructor;
@NoArgsConstructor(access = PRIVATE)
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok NoArgsConstructor no import- ok
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import lombok.NoArgsConstructor;
@NoArgsConstructor(access = lombok.AccessLevel.PRIVATE)
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok NoArgsConstructor with no access level- should be a utility class
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import lombok.NoArgsConstructor;
@NoArgsConstructor
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok NoArgsConstructor with wrong access level - should be a utility class
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import lombok.NoArgsConstructor;
@NoArgsConstructor(access = lombok.AccessLevel.PUBLIC)
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Lombok AllArgsConstructor - ok
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import lombok.AllArgsConstructor;
import lombok.AccessLevel;
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public class Foo {
public static Foo get() {
return null;
}
}
]]></code>
</test-code>
</test-data>