Merge branch 'pr-1265'

This commit is contained in:
Juan Martín Sotuyo Dodero
2018-08-04 15:18:32 -03:00
3 changed files with 94 additions and 6 deletions

View File

@ -30,6 +30,8 @@ This is a minor release.
* core
* [#1191](https://github.com/pmd/pmd/issues/1191): \[core] Test Framework: Sort violations by line/column
* java-codestyle
* [#1255](https://github.com/pmd/pmd/issues/1255): \[java] UnnecessaryFullyQualifiedName false positive: static method on shadowed implicitly imported class
* java-errorprone
* [#1078](https://github.com/pmd/pmd/issues/1078): \[java] MissingSerialVersionUID rule does not seem to catch inherited classes
* plsql

View File

@ -17,7 +17,11 @@ import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration;
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.AbstractJavaTypeNode;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.SourceFileScope;
@ -79,6 +83,16 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
&& name.lastIndexOf('.') == decl.getImportedName().length();
}
private boolean couldBeMethodCall(JavaNode node) {
if (node.getNthParent(2) instanceof ASTPrimaryExpression && node.getNthParent(1) instanceof ASTPrimaryPrefix) {
int nextSibling = node.jjtGetParent().jjtGetChildIndex() + 1;
if (node.getNthParent(2).jjtGetNumChildren() > nextSibling) {
return node.getNthParent(2).jjtGetChild(nextSibling) instanceof ASTPrimarySuffix;
}
}
return false;
}
private void checkImports(AbstractJavaTypeNode node, Object data) {
String name = node.getImage();
List<ASTImportDeclaration> matches = new ArrayList<>();
@ -127,7 +141,7 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
matches.add(importDeclaration);
}
} else {
// Last 2 parts match?
// Last 2 parts match? Class + Method name
if (nameParts[nameParts.length - 1].equals(importParts[importParts.length - 1])
&& nameParts[nameParts.length - 2].equals(importParts[importParts.length - 2])) {
matches.add(importDeclaration);
@ -137,6 +151,10 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
// last part matches?
if (nameParts[nameParts.length - 1].equals(importParts[importParts.length - 1])) {
matches.add(importDeclaration);
} else if (couldBeMethodCall(node)
&& nameParts.length > 1 && nameParts[nameParts.length - 2].equals(importParts[importParts.length - 1])) {
// maybe the Name is part of a method call, then the second two last part needs to match
matches.add(importDeclaration);
}
}
}
@ -147,7 +165,7 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
}
if (!matches.isEmpty()) {
ASTImportDeclaration firstMatch = matches.get(0);
ASTImportDeclaration firstMatch = findFirstMatch(matches);
// Could this done to avoid a conflict?
if (!isAvoidingConflict(node, name, firstMatch)) {
@ -159,13 +177,37 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
}
}
private ASTImportDeclaration findFirstMatch(List<ASTImportDeclaration> imports) {
// first search only static imports
ASTImportDeclaration result = null;
for (ASTImportDeclaration importDeclaration : imports) {
if (importDeclaration.isStatic()) {
result = importDeclaration;
break;
}
}
// then search all non-static, if needed
if (result == null) {
for (ASTImportDeclaration importDeclaration : imports) {
if (!importDeclaration.isStatic()) {
result = importDeclaration;
break;
}
}
}
return result;
}
private boolean isJavaLangImplicit(AbstractJavaTypeNode node) {
String name = node.getImage();
boolean isJavaLang = name != null && name.startsWith("java.lang.");
if (isJavaLang && node.getType() != null) {
if (isJavaLang && node.getType() != null && node.getType().getPackage() != null) {
// valid would be ProcessBuilder.Redirect.PIPE but not java.lang.ProcessBuilder.Redirect.PIPE
String packageName = node.getType().getPackage().getName();
String packageName = node.getType().getPackage() // package might be null, if type is an array type...
.getName();
return "java.lang".equals(packageName);
} else if (isJavaLang) {
// only java.lang.* is implicitly imported, but not e.g. java.lang.reflection.*
@ -231,15 +273,29 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
}
// There could be a conflict between an import of a class with the same name as the FQN
String importName = firstMatch.getImportedName();
String importUnqualified = importName.substring(importName.lastIndexOf('.') + 1);
if (!firstMatch.isImportOnDemand() && !firstMatch.isStatic()) {
String importName = firstMatch.getImportedName();
String importUnqualified = importName.substring(importName.lastIndexOf('.') + 1);
// the package is different, but the unqualified name is same
if (!firstMatch.getImportedName().equals(name) && importUnqualified.equals(unqualifiedName)) {
return true;
}
}
// There could be a conflict between an import of a class with the same name as the FQN, which
// could be a method call:
// import x.y.Thread;
// valid qualification (node): java.util.Thread.currentThread()
if (couldBeMethodCall(node)) {
String[] nameParts = name.split("\\.");
String fqnName = name.substring(0, name.lastIndexOf('.'));
// seems to be a static method call on a different FQN
if (!fqnName.equals(importName) && !firstMatch.isStatic() && !firstMatch.isImportOnDemand()
&& nameParts.length > 1 && nameParts[nameParts.length - 2].equals(importUnqualified)) {
return true;
}
}
// Is it a conflict with a class in the same file?
final Set<String> qualifiedTypes = node.getScope().getEnclosingScope(SourceFileScope.class)
.getQualifiedTypeNames().keySet();

View File

@ -458,4 +458,34 @@ public class JavaLang {
}
]]></code>
</test-code>
<test-code>
<description>#1255 [java] UnnecessaryFullyQualifiedName false positive: static method on shadowed implicitly imported class</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import x.y.Thread;
public class ThreadStuff {
public Thread stuff() {
return new Thread(java.lang.Thread.currentThread());
}
}
]]></code>
</test-code>
<test-code>
<description>Nullpointer in isJavaLangImplicit for java.lang.String[] arrays</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
public class TestArrayType {
String[] someArray = new String[0];
public void foo() {
boolean b1 = someArray instanceof String[];
boolean b2 = someArray instanceof java.lang.String[]; // unnecessary FQN
}
}
]]></code>
</test-code>
</test-data>