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

Merge branch 'pr-1141'

Browse Source
This commit is contained in:
Andreas Dangel
2018-05-26 22:36:56 +02:00
parent e13c9f3fff 929c2dfba2
commit ba55164b2f
4 changed files with 157 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
docs/pages/release_notes.md
View File

@@ -50,6 +50,7 @@ This is a minor release.
* java-codestyle
* [#720](https://github.com/pmd/pmd/issues/720): \[java] ShortVariable should whitelist lambdas
* [#955](https://github.com/pmd/pmd/issues/955): \[java] Detect identical catch statements
* [#1114](https://github.com/pmd/pmd/issues/1114): \[java] Star import overwritten by explicit import is not correctly handled
* [#1064](https://github.com/pmd/pmd/issues/1064): \[java] ClassNamingConventions suggests to add Util suffix for simple exception wrappers
* [#1065](https://github.com/pmd/pmd/issues/1065): \[java] ClassNamingConventions shouldn't prohibit numbers in class names
* [#1067](https://github.com/pmd/pmd/issues/1067): \[java] [6.3.0] PrematureDeclaration false-positive

45
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java
View File

@@ -5,6 +5,14 @@
package net.sourceforge.pmd.lang.java.ast;
/**
* Represents an import declaration in a Java file.
*
* <pre>
* ImportDeclaration ::= "import" [ "static" ] Name [ "." "*" ] ";"
* </pre>
*
*/
public class ASTImportDeclaration extends AbstractJavaTypeNode {
private boolean isImportOnDemand;
@@ -41,10 +49,35 @@ public class ASTImportDeclaration extends AbstractJavaTypeNode {
return (ASTName) jjtGetChild(0);
}
/**
* Returns the full name of the import. For on-demand imports, this is the name without
* the final asterisk.
*/
public String getImportedName() {
return ((ASTName) jjtGetChild(0)).getImage();
return jjtGetChild(0).getImage();
}
/**
* Returns the simple name of the type or method imported by this declaration.
* For on-demand imports, returns {@code null}.
*/
public String getImportedSimpleName() {
if (isImportOnDemand) {
return null;
}
String importName = getImportedName();
return importName.substring(importName.lastIndexOf('.') + 1);
}
/**
* Returns the "package" prefix of the imported name. For type imports, including on-demand
* imports, this is really the package name of the imported type(s). For static imports,
* this is actually the qualified name of the enclosing type, including the type name.
*/
public String getPackageName() {
String importName = getImportedName();
if (isImportOnDemand) {
@@ -57,9 +90,6 @@ public class ASTImportDeclaration extends AbstractJavaTypeNode {
return importName.substring(0, lastDot);
}
/**
* Accept the visitor. *
*/
@Override
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
@@ -69,6 +99,13 @@ public class ASTImportDeclaration extends AbstractJavaTypeNode {
this.pkg = packge;
}
/**
* Returns the {@link Package} instance representing the package of the
* type or method imported by this declaration. This may be null if the
* auxclasspath is not correctly set, as this method depends on correct
* type resolution.
*/
public Package getPackage() {
return this.pkg;
}

79
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java
View File

@@ -22,7 +22,6 @@ import net.sourceforge.pmd.lang.java.symboltable.SourceFileScope;
public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
private List<ASTImportDeclaration> imports = new ArrayList<>();
private List<ASTImportDeclaration> matches = new ArrayList<>();
public UnnecessaryFullyQualifiedNameRule() {
super.addRuleChainVisit(ASTCompilationUnit.class);
@@ -45,6 +44,10 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
@Override
public Object visit(ASTClassOrInterfaceType node, Object data) {
// This name has no qualification, it can't be unnecessarily qualified
if (node.getImage().indexOf('.') < 0) {
return data;
}
checkImports(node, data);
return data;
}
@@ -53,38 +56,44 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
public Object visit(ASTName node, Object data) {
if (!(node.jjtGetParent() instanceof ASTImportDeclaration)
&& !(node.jjtGetParent() instanceof ASTPackageDeclaration)) {
// This name has no qualification, it can't be unnecessarily qualified
if (node.getImage().indexOf('.') < 0) {
return data;
}
checkImports(node, data);
}
return data;
}
/**
* Returns true if the name could be imported by this declaration.
* The name must be fully qualified, the import is either on-demand
* or static, that is its {@link ASTImportDeclaration#getImportedName()}
* is the enclosing package or type name of the imported type or static member.
*/
private boolean declarationMatches(ASTImportDeclaration decl, String name) {
return name.startsWith(decl.getImportedName())
&& name.lastIndexOf('.') == decl.getImportedName().length();
}
private void checkImports(JavaNode node, Object data) {
String name = node.getImage();
matches.clear();
List<ASTImportDeclaration> matches = new ArrayList<>();
// Find all "matching" import declarations
for (ASTImportDeclaration importDeclaration : imports) {
if (importDeclaration.isImportOnDemand()) {
// On demand import exactly matches the package of the type
if (name.startsWith(importDeclaration.getImportedName())) {
if (name.lastIndexOf('.') == importDeclaration.getImportedName().length()) {
matches.add(importDeclaration);
continue;
}
}
} else {
if (!importDeclaration.isImportOnDemand()) {
// Exact match of imported class
if (name.equals(importDeclaration.getImportedName())) {
matches.add(importDeclaration);
continue;
}
// Match of static method call on imported class
if (name.startsWith(importDeclaration.getImportedName())) {
if (name.lastIndexOf('.') == importDeclaration.getImportedName().length()) {
matches.add(importDeclaration);
continue;
}
}
}
// On demand import exactly matches the package of the type
// Or match of static method call on imported class
if (declarationMatches(importDeclaration, name)) {
matches.add(importDeclaration);
}
}
@@ -98,7 +107,7 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
// List list1 = Arrays.asList("foo"); // Array class name not needed!
// List list2 = asList("foo"); // Preferred, used static import
// }
if (matches.isEmpty() && name.indexOf('.') >= 0) {
if (matches.isEmpty()) {
for (ASTImportDeclaration importDeclaration : imports) {
if (importDeclaration.isStatic()) {
String[] importParts = importDeclaration.getImportedName().split("\\.");
@@ -130,22 +139,18 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
addViolation(data, node, new Object[] { node.getImage(), importStr, type });
}
}
matches.clear();
}
private boolean isAvoidingConflict(final JavaNode node, final String name,
final ASTImportDeclaration firstMatch) {
// is it a conflict between different imports?
if (firstMatch.isImportOnDemand() && firstMatch.isStatic() && name.indexOf('.') != -1) {
if (firstMatch.isImportOnDemand() && firstMatch.isStatic()) {
final String methodCalled = name.substring(name.indexOf('.') + 1);
// Is there any other static import conflictive?
for (final ASTImportDeclaration importDeclaration : imports) {
if (!Objects.equals(importDeclaration, firstMatch) && importDeclaration.isStatic()) {
if (importDeclaration.getImportedName().startsWith(firstMatch.getImportedName())
&& importDeclaration.getImportedName().lastIndexOf('.') == firstMatch.getImportedName()
.length()) {
if (declarationMatches(firstMatch, importDeclaration.getImportedName())) {
// A conflict against the same class is not an excuse,
// ie:
// import java.util.Arrays;
@@ -170,11 +175,31 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
}
}
// Is it a conflict with a class in the same file?
final String unqualifiedName = name.substring(name.lastIndexOf('.') + 1);
final int unqualifiedNameLength = unqualifiedName.length();
// There could be a conflict between an import on demand and another import, e.g.
// import One.*;
// import Two.Problem;
// Where One.Problem is a legitimate qualification
if (firstMatch.isImportOnDemand() && !firstMatch.isStatic()) {
for (ASTImportDeclaration importDeclaration : imports) {
if (importDeclaration != firstMatch // NOPMD
&& !importDeclaration.isStatic()
&& !importDeclaration.isImportOnDemand()) {
// Duplicate imports are legal
if (!importDeclaration.getPackageName().equals(firstMatch.getPackageName())
&& importDeclaration.getImportedSimpleName().equals(unqualifiedName)) {
return true;
}
}
}
}
// Is it a conflict with a class in the same file?
final Set<String> qualifiedTypes = node.getScope().getEnclosingScope(SourceFileScope.class)
.getQualifiedTypeNames().keySet();
.getQualifiedTypeNames().keySet();
for (final String qualified : qualifiedTypes) {
int fullLength = qualified.length();
if (qualified.endsWith(unqualifiedName)

132
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml
View File

File diff suppressed because it is too large Load Diff