Fixed Defect 1451251

Added a new typeresolution rule - UnusedImports which finds unused on demand imports

Updated typeresolution to populate the type of ASTNames without VariableNameDeclarations - These are typically statically called methods from other classes such as Arrays.asList.


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@5031 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Allan Caplan
2007-02-03 12:48:12 +00:00
parent f3c562c3db
commit 8812185444
11 changed files with 390 additions and 49 deletions

View File

@ -8,6 +8,7 @@ Fixed bug 1634078 - StringToString now recognizes toString on a String Array, ra
Fixed bug 1631646 - UselessOperationOnImmutable doesn't throw on variable.method().variable.
Fixed bug 1627830 - UseLocaleWithCaseConversions now works with compound string operations
Fixed bug 1613807 - DontImportJavaLang rule allows import to Thread inner classes
Fixed bug 1451251 - A new UnusedImports rule, using typeresolution, finds unused import on demand rules
Applied patch 1612455 - RFE 1411022 CompareObjectsWithEquals now catches the case where comparison is against new Object
Implemented RFE 1562230 - Added migration rule to check for instantiation of Short/Byte/Long
Implemented RFE 1627581 - SuppressWarnings("unused") now suppresses all warnings in unusedcode.xml
@ -15,6 +16,7 @@ XPath rules are now chained together for an extra speedup in processing
PMD now requires JDK 1.5 to be compiled. Java 1.4 support is provided using Retroweaver
- PMD will still analyze code from earlier JDKs
- to run pmd with 1.4, use the files from the java14 directory (weaved pmd jar and support files)
TypeResolution now looks at some ASTName nodes.
December 19, 2006 - 3.9:
New rules:

View File

@ -0,0 +1,22 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.typeresolution.rules.imports;
import net.sourceforge.pmd.Rule;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
public class UnusedImportsTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() {
rule = findRule("typeresolution", "UnusedImports");
rule.setMessage("{0}");
rule.setUsesTypeResolution();
}
public void testAll() {
runTests(rule);
}
}

View File

@ -0,0 +1,145 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data>
<test-code>
<description><![CDATA[
simple unused single type import
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import java.io.File;
public class Foo {}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
one used single type import
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.io.File;
public class Foo {
private File file;
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
2 unused single-type imports
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
import java.io.File;
import java.util.List;
public class Foo {
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
1 used single type import
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.security.AccessController;
public class Foo {
public void foo() {
AccessController.doPrivileged(null);
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
1 import stmt, used only in throws clause
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.rmi.RemoteException;
public class Foo {
public void foo() throws RemoteException {}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
for loop
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
public class Foo {
public void foo(ArrayList list) {
for (String s : list) {}
}
}
]]></code>
<source-type>java 1.5</source-type>
</test-code>
<test-code>
<description><![CDATA[
Generics
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.Collection;
import java.util.List;
import java.util.ArrayList;
public class Foo {
private List<Collection> x = new ArrayList<Collection>();
}
]]></code>
<source-type>java 1.5</source-type>
</test-code>
<test-code>
<description><![CDATA[
Annotations
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import foo.annotation.Retention;
import foo.annotation.RetentionPolicy;
@Retention(RetentionPolicy.RUNTIME)
public @interface Foo {
}
]]></code>
<source-type>java 1.5</source-type>
</test-code>
<test-code>
<description><![CDATA[
Annotations 2
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import foo.FooAnnotation1;
import foo.FooAnnotation2;
@FooAnnotation1
@FooAnnotation2
public class Foo {}
]]></code>
<source-type>java 1.5</source-type>
</test-code>
<test-code>
<description><![CDATA[
import from default package
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import Bar;
public class Foo {
public Bar foo() {}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
import from default package
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import Bar;
public class Foo {
public void foo() {}
}
]]></code>
</test-code>
</test-data>

View File

@ -61,5 +61,22 @@ public class MyClass {
</example>
</rule>
<rule name="UnusedImports"
message="Avoid unused imports such as ''{0}''"
class="net.sourceforge.pmd.typeresolution.rules.imports.UnusedImports"
typeResolution="true"
externalInfoUrl="http://pmd.sourceforge.net/rules/typeresolution.html#UnusedImports">
<description>
Avoid unused import statements.
</description>
<priority>4</priority>
<example>
<![CDATA[
// this is bad
import java.io.*;
public class Foo {}
]]>
</example>
</rule>
</ruleset>

View File

@ -2,7 +2,7 @@
package net.sourceforge.pmd.ast;
public class ASTClassOrInterfaceType extends SimpleJavaNode {
public class ASTClassOrInterfaceType extends SimpleJavaNode implements TypeNode {
public ASTClassOrInterfaceType(int id) {
super(id);
}

View File

@ -4,7 +4,7 @@ package net.sourceforge.pmd.ast;
import net.sourceforge.pmd.symboltable.NameDeclaration;
public class ASTName extends SimpleJavaNode {
public class ASTName extends SimpleJavaNode implements TypeNode {
public ASTName(int id) {
super(id);
}
@ -31,4 +31,12 @@ public class ASTName extends SimpleJavaNode {
return visitor.visit(this, data);
}
private Class type;
public void setType(Class type){
this.type = type;
}
public Class getType(){
return type;
}
}

View File

@ -0,0 +1,8 @@
package net.sourceforge.pmd.ast;
public interface TypeNode {
public Class getType();
public void setType(Class type);
}

View File

@ -19,11 +19,17 @@ public class ImportWrapper {
public boolean equals(Object other) {
ImportWrapper i = (ImportWrapper) other;
if(name == null && i.getName() == null){
return i.getFullName().equals(fullname);
}
return i.getName().equals(name);
}
public int hashCode() {
return getName().hashCode();
if(name == null){
return fullname.hashCode();
}
return name.hashCode();
}
public String getName() {

View File

@ -17,7 +17,7 @@ import java.util.Set;
public class UnusedImportsRule extends AbstractRule {
private Set<ImportWrapper> imports = new HashSet<ImportWrapper>();
protected Set<ImportWrapper> imports = new HashSet<ImportWrapper>();
public Object visit(ASTCompilationUnit node, Object data) {
imports.clear();
@ -54,7 +54,17 @@ public class UnusedImportsRule extends AbstractRule {
return data;
}
private void check(SimpleNode node) {
protected void check(SimpleNode node) {
if (imports.isEmpty()) {
return;
}
ImportWrapper candidate = getImportWrapper(node);
if (imports.contains(candidate)) {
imports.remove(candidate);
}
}
protected ImportWrapper getImportWrapper(SimpleNode node) {
String name;
if (!isQualifiedName(node)) {
name = node.getImage();
@ -62,8 +72,6 @@ public class UnusedImportsRule extends AbstractRule {
name = node.getImage().substring(0, node.getImage().indexOf('.'));
}
ImportWrapper candidate = new ImportWrapper(node.getImage(), name, new SimpleJavaNode(-1));
if (imports.contains(candidate)) {
imports.remove(candidate);
}
return candidate;
}
}

View File

@ -10,7 +10,9 @@ import net.sourceforge.pmd.ast.ASTImportDeclaration;
import net.sourceforge.pmd.ast.ASTName;
import net.sourceforge.pmd.ast.ASTPackageDeclaration;
import net.sourceforge.pmd.ast.JavaParserVisitorAdapter;
import net.sourceforge.pmd.ast.TypeNode;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@ -19,6 +21,8 @@ import java.util.Map;
public class ClassTypeResolver extends JavaParserVisitorAdapter {
private static Map<String, Class> myPrimitiveTypes;
private static Map<String, String> myJavaLang;
private static PMDASMClassLoader pmdClassLoader = new PMDASMClassLoader();
@ -34,19 +38,60 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
thePrimitiveTypes.put("boolean", Boolean.TYPE);
thePrimitiveTypes.put("void", Void.TYPE);
myPrimitiveTypes = Collections.unmodifiableMap(thePrimitiveTypes);
myJavaLang = new HashMap<String, String>();
myJavaLang.put("Boolean","java.lang.Boolean");
myJavaLang.put("Byte","java.lang.Byte");
myJavaLang.put("Character","java.lang.Character");
myJavaLang.put("CharSequence","java.lang.CharSequence");
myJavaLang.put("Class","java.lang.Class");
myJavaLang.put("ClassLoader","java.lang.ClassLoader");
myJavaLang.put("Cloneable","java.lang.Cloneable");
myJavaLang.put("Comparable","java.lang.Comparable");
myJavaLang.put("Compiler","java.lang.Compiler");
myJavaLang.put("Double","java.lang.Double");
myJavaLang.put("Float","java.lang.Float");
myJavaLang.put("InheritableThreadLocal","java.lang.InheritableThreadLocal");
myJavaLang.put("Integer","java.lang.Integer");
myJavaLang.put("Long","java.lang.Long");
myJavaLang.put("Math","java.lang.Math");
myJavaLang.put("Number","java.lang.Number");
myJavaLang.put("Object","java.lang.Object");
myJavaLang.put("Package","java.lang.Package");
myJavaLang.put("Process","java.lang.Process");
myJavaLang.put("Runnable","java.lang.Runnable");
myJavaLang.put("Runtime","java.lang.Runtime");
myJavaLang.put("RuntimePermission","java.lang.RuntimePermission");
myJavaLang.put("SecurityManager","java.lang.SecurityManager");
myJavaLang.put("Short","java.lang.Short");
myJavaLang.put("StackTraceElement","java.lang.StackTraceElement");
myJavaLang.put("StrictMath","java.lang.StrictMath");
myJavaLang.put("String","java.lang.String");
myJavaLang.put("StringBuffer","java.lang.StringBuffer");
myJavaLang.put("System","java.lang.System");
myJavaLang.put("Thread","java.lang.Thread");
myJavaLang.put("ThreadGroup","java.lang.ThreadGroup");
myJavaLang.put("ThreadLocal","java.lang.ThreadLocal");
myJavaLang.put("Throwable","java.lang.Throwable");
myJavaLang.put("Void","java.lang.Void");
}
private Map<String, String> importedClasses;
private Map<String, String> importedClasses;
private List<String> importedOnDemand;
private String className;
public Object visit(ASTCompilationUnit node, Object data) {
try {
importedOnDemand = new ArrayList<String>();
populateClassName(node);
populateImports(node, true);
} catch (ClassNotFoundException e) {
populateImports(node);
populateImports(node, false);
} catch (NoClassDefFoundError e) {
populateImports(node);
populateImports(node, false);
}
return super.visit(node, data);
}
@ -56,30 +101,35 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
*
* @param node
*/
private void populateImports(ASTCompilationUnit node) {
private void populateImports(ASTCompilationUnit node, boolean processOnDemand){
List<ASTImportDeclaration> theImportDeclarations = node.findChildrenOfType(ASTImportDeclaration.class);
importedClasses = new HashMap<String, String>();
importedClasses = new HashMap<String, String>();
// go through the imports
for (ASTImportDeclaration anImportDeclaration : theImportDeclarations) {
if (!anImportDeclaration.isImportOnDemand()) {
String strPackage = anImportDeclaration.getPackageName();
String strName = anImportDeclaration.getImportedName();
importedClasses.put(strName, strName);
importedClasses.put(strName.substring(strPackage.length() + 1), strName);
}
}
String strPackage = anImportDeclaration.getPackageName();
if (processOnDemand && anImportDeclaration.isImportOnDemand()) {
importedOnDemand.add(strPackage);
} else if (!anImportDeclaration.isImportOnDemand()) {
String strName = anImportDeclaration.getImportedName();
importedClasses.put(strName, strName);
importedClasses.put(strName.substring(strPackage.length() + 1), strName);
}
}
importedClasses.put("String", "java.lang.String");
importedClasses.put("Object", "java.lang.Object");
importedClasses.putAll(myJavaLang);
}
private void populateClassName(ASTCompilationUnit node) throws ClassNotFoundException {
ASTClassOrInterfaceDeclaration decl = node.getFirstChildOfType(ASTClassOrInterfaceDeclaration.class);
if (decl != null) {
ASTPackageDeclaration pkgDecl = node.getFirstChildOfType(ASTPackageDeclaration.class);
className = pkgDecl == null ? decl.getImage() : ((ASTName) pkgDecl.jjtGetChild(0)).getImage() + "."
+ decl.getImage();
if (pkgDecl == null) {
className = decl.getImage();
} else {
importedOnDemand.add(((ASTName) pkgDecl.jjtGetChild(0)).getImage());
className = ((ASTName) pkgDecl.jjtGetChild(0)).getImage() + "." + decl.getImage();
}
pmdClassLoader.loadClass(className);
importedClasses = pmdClassLoader.getImportedClasses(className);
}
@ -87,31 +137,63 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
public Object visit(ASTClassOrInterfaceType node, Object data) {
String className = node.getImage();
String qualifiedName = className;
Class myType = myPrimitiveTypes.get(className);
if (myType == null && importedClasses != null) {
if (importedClasses.containsKey(className)) {
qualifiedName = importedClasses.get(className);
} else if (importedClasses.containsValue(className)) {
qualifiedName = className;
}
if (qualifiedName != null) {
try {
/*
* TODO - the map right now contains just class names. if we use a map of
* classname/class then we don't have to hit the class loader for every type -
* much faster
*/
myType = pmdClassLoader.loadClass(qualifiedName);
} catch (ClassNotFoundException e) {
//@TODO What should we do if it's not found?
}
}
}
if (myType != null) {
node.setType(myType);
}
populateType(node, node.getImage());
return data;
}
public Object visit(ASTName node, Object data) {
/*
* Only doing this for nodes where getNameDeclaration is null this
* means it's not a named node, i.e. Static reference or Annotation
* Doing this for memory - TODO: Investigate if there is a valid memory
* concern or not
*/
if (node.getNameDeclaration() == null) {
String name = node.getImage();
if (name.indexOf('.') != -1) {
name = name.substring(0, name.indexOf('.'));
}
populateType(node, name);
}
return super.visit(node, data);
}
private void populateType(TypeNode node, String className) {
String qualifiedName = className;
Class myType = myPrimitiveTypes.get(className);
if (myType == null && importedClasses != null) {
if (importedClasses.containsKey(className)) {
qualifiedName = importedClasses.get(className);
} else if (importedClasses.containsValue(className)) {
qualifiedName = className;
}
if (qualifiedName != null) {
try {
/*
* TODO - the map right now contains just class names. if we
* use a map of classname/class then we don't have to hit
* the class loader for every type - much faster
*/
myType = pmdClassLoader.loadClass(qualifiedName);
} catch (ClassNotFoundException e) {
myType = processOnDemand(qualifiedName);
}
}
}
if (myType != null) {
node.setType(myType);
}
}
private Class processOnDemand(String qualifiedName) {
for(String entry : importedOnDemand){
try {
return pmdClassLoader.loadClass(entry + "." + qualifiedName);
} catch (Throwable e) {
}
}
return null;
}
}

View File

@ -0,0 +1,43 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.typeresolution.rules.imports;
import net.sourceforge.pmd.ast.ASTImportDeclaration;
import net.sourceforge.pmd.ast.ASTName;
import net.sourceforge.pmd.ast.SimpleJavaNode;
import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.ast.TypeNode;
import net.sourceforge.pmd.rules.ImportWrapper;
import net.sourceforge.pmd.rules.imports.UnusedImportsRule;
public class UnusedImports extends UnusedImportsRule {
public Object visit(ASTImportDeclaration node, Object data) {
if (node.isImportOnDemand()) {
ASTName importedType = (ASTName) node.jjtGetChild(0);
imports.add(new ImportWrapper(importedType.getImage(), null, node));
} else {
super.visit(node, data);
}
return data;
}
protected void check(SimpleNode node) {
if (imports.isEmpty()) {
return;
}
ImportWrapper candidate = getImportWrapper(node);
if (imports.contains(candidate)) {
imports.remove(candidate);
return;
}
if (TypeNode.class.isAssignableFrom(node.getClass()) && ((TypeNode) node).getType() != null) {
Class c = ((TypeNode) node).getType();
candidate = new ImportWrapper(c.getPackage().getName(), null, new SimpleJavaNode(-1));
if (imports.contains(candidate)) {
imports.remove(candidate);
}
}
}
}