diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java new file mode 100644 index 0000000000..e2f7d8e05d --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java @@ -0,0 +1,252 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.codestyle; + +import java.util.HashSet; +import java.util.Iterator; +import java.util.Objects; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.commons.lang3.tuple.Pair; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; +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.Comment; +import net.sourceforge.pmd.lang.java.ast.FormalComment; +import net.sourceforge.pmd.lang.java.ast.TypeNode; +import net.sourceforge.pmd.lang.java.ast.internal.ImportWrapper; +import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class UnnecessaryImportRule extends AbstractJavaRule { + + private static final String UNUSED_IMPORT_MESSAGE = "Unused import ''{0}''"; + private static final String DUPLICATE_IMPORT_MESSAGE = "Duplicate import ''{0}''"; + private static final String IMPORT_FROM_SAME_PACKAGE_MESSAGE = "Unnecessary import from the current package ''{0}''"; + private static final String IMPORT_FROM_JAVA_LANG_MESSAGE = "Unnecessary import from the java.lang package ''{0}''"; + + private final Set imports = new HashSet<>(); + private String thisPackageName; + + /* + * Patterns to match the following constructs: + * + * @see package.class#member(param, param) label {@linkplain + * package.class#member(param, param) label} {@link + * package.class#member(param, param) label} {@link package.class#field} + * {@value package.class#field} + * + * @throws package.class label + */ + private static final Pattern SEE_PATTERN = Pattern + .compile("@see\\s+((?:\\p{Alpha}\\w*\\.)*(?:\\p{Alpha}\\w*))?(?:#\\w*(?:\\(([.\\w\\s,\\[\\]]*)\\))?)?"); + + private static final Pattern LINK_PATTERNS = Pattern + .compile("\\{@link(?:plain)?\\s+((?:\\p{Alpha}\\w*\\.)*(?:\\p{Alpha}\\w*))?(?:#\\w*(?:\\(([.\\w\\s,\\[\\]]*)\\))?)?[\\s\\}]"); + + private static final Pattern VALUE_PATTERN = Pattern.compile("\\{@value\\s+(\\p{Alpha}\\w*)[\\s#\\}]"); + + private static final Pattern THROWS_PATTERN = Pattern.compile("@throws\\s+(\\p{Alpha}\\w*)"); + + private static final Pattern[] PATTERNS = { SEE_PATTERN, LINK_PATTERNS, VALUE_PATTERN, THROWS_PATTERN }; + + @Override + public Object visit(ASTCompilationUnit node, Object data) { + imports.clear(); + this.thisPackageName = node.getPackageName(); + super.visit(node, data); + visitComments(node); + + /* + * special handling for Bug 2606609 : False "UnusedImports" positive in + * package-info.java package annotations are processed before the import + * clauses so they need to be examined again later on. + */ + if (node.getNumChildren() > 0 && node.getChild(0) instanceof ASTPackageDeclaration) { + visit((ASTPackageDeclaration) node.getChild(0), data); + } + for (ImportWrapper wrapper : imports) { + addViolation(data, wrapper.getNode(), PrettyPrintingUtil.prettyImport(wrapper.getNode())); + } + return data; + } + + private void visitComments(ASTCompilationUnit node) { + if (imports.isEmpty()) { + return; + } + for (Comment comment : node.getComments()) { + if (!(comment instanceof FormalComment)) { + continue; + } + for (Pattern p : PATTERNS) { + Matcher m = p.matcher(comment.getImage()); + while (m.find()) { + String fullname = m.group(1); + + if (fullname != null) { // may be null for "@see #" and "@link #" + removeReferenceSingleImport(fullname); + } + + if (m.groupCount() > 1) { + fullname = m.group(2); + if (fullname != null) { + for (String param : fullname.split("\\s*,\\s*")) { + removeReferenceSingleImport(param); + } + } + } + + if (imports.isEmpty()) { + return; + } + } + } + } + } + + @Override + public Object visit(ASTImportDeclaration node, Object data) { + if (Objects.equals(node.getPackageName(), thisPackageName)) { + // import for the same package + addViolationWithMessage(data, node, IMPORT_FROM_SAME_PACKAGE_MESSAGE, + new String[] { PrettyPrintingUtil.prettyImport(node) }); + } else if (!imports.add(new ImportWrapper(node))) { + // duplicate + addViolationWithMessage(data, node, DUPLICATE_IMPORT_MESSAGE, + new String[] { PrettyPrintingUtil.prettyImport(node) }); + } + return data; + } + + @Override + public Object visit(ASTClassOrInterfaceType node, Object data) { + check(node); + return super.visit(node, data); + } + + @Override + public Object visit(ASTName node, Object data) { + check(node); + return data; + } + + /** + * Remove the import wrapper that imports the name referenced by the + * given node. + */ + protected void check(Node referenceNode) { + if (imports.isEmpty()) { + return; + } + Pair candidate = splitName(referenceNode); + String candFullName = candidate.getLeft(); + String candName = candidate.getRight(); + + // check exact imports + Iterator it = imports.iterator(); + while (it.hasNext()) { + ImportWrapper i = it.next(); + if (!i.isStaticOnDemand() && i.matches(candFullName, candName)) { + it.remove(); + return; + } + } + + // check static on-demand imports + it = imports.iterator(); + while (it.hasNext()) { + ImportWrapper i = it.next(); + if (i.isStaticOnDemand() && i.matches(candFullName, candName)) { + it.remove(); + return; + } + } + + if (referenceNode instanceof TypeNode && ((TypeNode) referenceNode).getType() != null) { + Class c = ((TypeNode) referenceNode).getType(); + if (c.getPackage() != null) { + removeOnDemandForPackageName(c.getPackage().getName()); + } + } + } + + + protected Pair splitName(Node node) { + String fullName = node.getImage(); + String name; + int firstDot = node.getImage().indexOf('.'); + if (firstDot == -1) { + name = node.getImage(); + } else { + // ASTName could be: MyClass.MyConstant + // name -> MyClass + // fullName -> MyClass.MyConstant + name = node.getImage().substring(0, firstDot); + if (isMethodCall(node)) { + // ASTName could be: MyClass.MyConstant.method(a, b) + // name -> MyClass + // fullName -> MyClass.MyConstant + fullName = node.getImage().substring(0, node.getImage().lastIndexOf('.')); + } + } + + return Pair.of(fullName, name); + } + + private boolean isMethodCall(Node node) { + // PrimaryExpression + // PrimaryPrefix + // Name + // PrimarySuffix + + if (node.getParent() instanceof ASTPrimaryPrefix && node.getNthParent(2) instanceof ASTPrimaryExpression) { + Node primaryPrefix = node.getParent(); + Node expression = primaryPrefix.getParent(); + + boolean hasNextSibling = expression.getNumChildren() > primaryPrefix.getIndexInParent() + 1; + if (hasNextSibling) { + Node nextSibling = expression.getChild(primaryPrefix.getIndexInParent() + 1); + if (nextSibling instanceof ASTPrimarySuffix) { + return true; + } + } + } + return false; + } + + /** We found a reference to the type given by the name. */ + private void removeReferenceSingleImport(String referenceName) { + int firstDot = referenceName.indexOf('.'); + String expectedImport = firstDot < 0 ? referenceName : referenceName.substring(0, firstDot); + Iterator iterator = imports.iterator(); + while (iterator.hasNext()) { + ImportWrapper anImport = iterator.next(); + if (!anImport.isOnDemand() && anImport.getName().equals(expectedImport)) { + iterator.remove(); + } + } + } + + private void removeOnDemandForPackageName(String fullName) { + Iterator iterator = imports.iterator(); + while (iterator.hasNext()) { + ImportWrapper anImport = iterator.next(); + if (anImport.isOnDemand() && anImport.getFullName().equals(fullName)) { + iterator.remove(); + break; + } + } + } +} diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 774b537977..5a774dd192 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -1951,6 +1951,29 @@ public class Foo { + + + Reports import statements that can be removed. They are either unused, + duplicated, or the members they import are already implicitly in scope, + because they're in java.lang, or the current package. + + 4 + + + + + + + + + simple unused single type import + 1 + + + + + one used single type import + 0 + + + + + 2 unused single-type imports + 2 + + + + + 1 used single type import + 0 + + + + + 1 import stmt, used only in throws clause + 0 + + + + + for loop + 0 + + + + + Generics + 0 + x = new ArrayList(); +} + ]]> + + + + Generics 2 + 0 + x = new ArrayList(); +} + ]]> + + + + Annotations + 0 + + + + + Annotations 2 + 0 + + + + + import from default package + 1 + + Unnecessary import from the current package 'Bar' + + + + + import from default package from somewhere else + 0 + + + + + import from default package + 1 + + + + + Used static import + 0 + + + + + Unused static import + 1 + + + + + On demand import + 0 + + + + + imports used in javadoc comment, see also bug #254 + 0 + + + + + #1280 False Positive in UnusedImports when import used in javadoc + 0 + + + + + #1720 False Positive in UnusedImports for Javadoc link with array type + 0 + + + + + Bug 2606609 : False "UnusedImports" positive in package-info.java + 0 + + + + + bug #254 False+ : UnusedImport with Javadoc @link + 0 + + + + + #1181 unused import false positive if used as parameter in javadoc only. + 0 + + + + + #1280 False Positive in UnusedImports when import used in javadoc + 0 + + + + + #914 False +ve from UnusedImports with wildcard static imports + 0 + + + + + #1465 False Positve UnusedImports with javadoc @link + 0 + + * An agent is active if it has not posted a {@link AgentStateChangeEvent} containing {@link AgentState#TERMINATED}. + * + * @return agent handles. + * @see OtherState#TERMINATED + */ + Iterable getAgentHandles(); +} + ]]> + + + + #1547 False Positve UnusedImports with javadoc for identifiers with underscores + 0 + + + + + #348 False Positive UnusedImports with javadoc for public static inner classes of imports + 0 + + + + + #925 [java] UnusedImports false positive for static import + 0 + + + + + #1404 [java] UnusedImports false positive for static import + 0 + + + + + #1209 [java] UnusedImports false positive for static import with package-private method usage + 0 + + + + + #1625 [java] UnusedImports false positive for method parameter type in @see Javadoc + 0 + + + + + #1720 False Positive in UnusedImports for Javadoc @see with array type + 0 + + + + + #2025 False Positive in UnusedImports for params when using @see with FQCN + 0 + + + + + #2025 False Positive in UnusedImports for params when using @link with FQCN + 0 + + + + + #2016 [java] UnusedImports: False positive if wildcard is used and only static methods + 0 + + + + + resolve ambiguous static on-demand imports (#2277) + 0 + + + + + [java] UnusedImports with static imports on subclasses #3132 + 0 + + + + + NPE with static import on interface + 1 + + + + + + + duplicate single type imports + 2 + 2,3 + + Unused import 'java.util.*' + Duplicate import 'java.io.File' + + + + + + duplicate wildcard imports + 1 + 2 + + Duplicate import 'java.io.*' + + + + + + single type import after wildcard import + 1 + 1 + + Unused import 'java.io.*' + + + + + + subpackage import, ok + 0 + + + + + 674394, disambiguation import should be allowed + 0 + + + + + 674394, disambiguation import because of conflict with java.lang + 1 + 1 + + + + + #1306 False positive on duplicate when using static imports + 0 + + + + + Static on-demand import is used + 0 + + + + + + [java] DuplicateImports reported for the same import... and import static... #2546 + 1 + 1 + + + + + + ImportFromSamePackage: simple failure + 1 + + + + + ImportFromSamePackage: class in default package importing from sub package + 0 + + + + + ImportFromSamePackage: importing all from same package + 1 + + Unnecessary import from the current package 'foo.bar.*' + + + +