Make UnusedImports report duplicate imports

Refs #3128
This commit is contained in:
Clément Fournier committed 2021-04-05 21:19:12 +02:00
1 parent bd912f0cb6
commit b77f1d13bd
5 files changed
+175 -6

No files matched your search

@@ -15,7 +15,6 @@ import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
/**
@@ -117,7 +116,7 @@ public class ImportWrapper {
return fullname;
}
public Node getNode() {
public ASTImportDeclaration getNode() {
return node;
}
@@ -10,6 +10,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTRecordDeclaration;
@@ -77,4 +78,12 @@ public final class PrettyPrintingUtil {
return "class";
}
public static String prettyImport(ASTImportDeclaration importDecl) {
String name = importDecl.getImportedName();
if (importDecl.isImportOnDemand()) {
return name + ".*";
}
return name;
}
}
@@ -25,6 +25,7 @@ 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 UnusedImportsRule extends AbstractJavaRule {
@@ -68,7 +69,7 @@ public class UnusedImportsRule extends AbstractJavaRule {
visit((ASTPackageDeclaration) node.getChild(0), data);
}
for (ImportWrapper wrapper : imports) {
addViolation(data, wrapper.getNode(), wrapper.getFullName());
addViolation(data, wrapper.getNode(), PrettyPrintingUtil.prettyImport(wrapper.getNode()));
}
return data;
}
@@ -109,7 +110,10 @@ public class UnusedImportsRule extends AbstractJavaRule {
@Override
public Object visit(ASTImportDeclaration node, Object data) {
imports.add(new ImportWrapper(node));
if (!imports.add(new ImportWrapper(node))) {
// duplicate
addViolationWithMessage(data, node, "Duplicate import ''{0}''", new String[] {PrettyPrintingUtil.prettyImport(node)});
}
return data;
}
@@ -162,7 +166,6 @@ public class UnusedImportsRule extends AbstractJavaRule {
}
protected Pair<String, String> getImportWrapper(Node node) {
String fullName = node.getImage();
String name;
@@ -7,5 +7,16 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
public class UnusedImportsTest extends PmdRuleTst {
// no additional unit tests
// these 2 methods are used for a test case, do not delete
public static void assertTrue(String message, boolean condition) {
if (!condition) {
System.out.println(message);
}
}
public static void assertSomething(String message, boolean condition) {
if (!condition) {
System.out.println(message);
}
}
}
@@ -581,4 +581,151 @@ import static javax.swing.WindowConstants.*; //warn
class NPEImport {}
]]></code>
</test-code>
<!-- Test cases for duplicate imports -->
<test-code>
<description>duplicate single type imports</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>2,3</expected-linenumbers>
<expected-messages>
<message>Avoid unused imports such as 'java.util.*'</message>
<message>Duplicate import 'java.io.File'</message>
</expected-messages>
<code><![CDATA[
import java.io.File;
import java.util.*;
import java.io.File;
public class Foo {
File f;
}
]]></code>
</test-code>
<test-code>
<description>duplicate wildcard imports</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<expected-messages>
<message>Duplicate import 'java.io.*'</message>
</expected-messages>
<code><![CDATA[
import java.io.*;
import java.io.*;
public class Foo {
File f;
}
]]></code>
</test-code>
<test-code>
<description>single type import after wildcard import</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>1</expected-linenumbers>
<expected-messages>
<message>Avoid unused imports such as 'java.io.*'</message>
</expected-messages>
<code><![CDATA[
import java.io.*;
import java.io.File;
public class Foo {
File f;
}
]]></code>
</test-code>
<test-code>
<description>subpackage import, ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.*;
import java.util.logging.*;
public class Foo {
List c; Logger f;
}
]]></code>
</test-code>
<test-code>
<description>674394, disambiguation import should be allowed</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.awt.*;
import java.util.*;
import java.util.List; //False positive
class Foo{
Color color;
List list;
Set set;
}
]]></code>
</test-code>
<test-code>
<description>674394, disambiguation import because of conflict with java.lang</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>1</expected-linenumbers>
<code><![CDATA[
import foo.*;
import foo.System; //False positive
class Foo {
System system; //No, I do not mean java.lang.System
}
]]></code>
</test-code>
<test-code>
<description>#1306 False positive on duplicate when using static imports</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import static org.junit.Assert.*;
import static net.sourceforge.pmd.lang.java.rule.bestpractices.UnusedImportsTest.*;
import static org.junit.Assert.assertTrue;
// this import is needed for disambiguation - as DuplicateImportsTest
// defines assertTrue with the same signature, too.
public class DuplicateImports {
static {
assertTrue("", true); // the one from the disambiguation import
assertSomething("", true); // from UnusedImportsTest.*
assertFalse("", true); // from Assert.*
}
}
]]></code>
</test-code>
<test-code>
<description>Static on-demand import is used</description>
<expected-problems>0</expected-problems>
<!-- Technically we could report assertTrue, but for now we don't. -->
<code><![CDATA[
import static org.junit.Assert.*;
import static org.junit.Assert.assertTrue;
public class DuplicateImports {
static {
assertTrue("", true);
assertFalse("", true);
}
}
]]></code>
</test-code>
<test-code>
<description>[java] DuplicateImports reported for the same import... and import static... #2546</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>1</expected-linenumbers>
<code><![CDATA[
import java.util.Collections.*;
import static java.util.Collections.*;
public class DuplicateImports {
static {
emptyList();
}
}
]]></code>
</test-code>
</test-data>