diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/DontImportJavaLangRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/DontImportJavaLangRule.java index 5e1a5168f2..a5a4e0ed6c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/DontImportJavaLangRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/DontImportJavaLangRule.java @@ -8,8 +8,6 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; public class DontImportJavaLangRule extends AbstractJavaRule { - private static final Package JAVA_LANG_PACKAGE = Package.getPackage("java.lang"); - @Override public Object visit(ASTImportDeclaration node, Object data) { @@ -17,22 +15,16 @@ public class DontImportJavaLangRule extends AbstractJavaRule { return data; } - if (node.getPackage() != null) { - if (node.getPackage().equals(JAVA_LANG_PACKAGE)) { - addViolation(data, node); - } - } else { - String img = node.jjtGetChild(0).getImage(); - if (img.startsWith("java.lang")) { - if (img.startsWith("java.lang.ref") || img.startsWith("java.lang.reflect") - || img.startsWith("java.lang.annotation") || img.startsWith("java.lang.instrument") - || img.startsWith("java.lang.management") || img.startsWith("java.lang.Thread.") - || img.startsWith("java.lang.ProcessBuilder.")) { - return data; - } - addViolation(data, node); - } + String img = node.jjtGetChild(0).getImage(); + if (img.startsWith("java.lang")) { + if (img.startsWith("java.lang.ref") || img.startsWith("java.lang.reflect") + || img.startsWith("java.lang.annotation") || img.startsWith("java.lang.instrument") + || img.startsWith("java.lang.management") || img.startsWith("java.lang.Thread.") + || img.startsWith("java.lang.ProcessBuilder.")) { + return data; } + addViolation(data, node); + } return data; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnnecessaryFullyQualifiedNameRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnnecessaryFullyQualifiedNameRule.java index 26595dcbfb..f7461ba905 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnnecessaryFullyQualifiedNameRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnnecessaryFullyQualifiedNameRule.java @@ -18,6 +18,8 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule { private List imports = new ArrayList(); private List matches = new ArrayList(); + private List violations = new ArrayList(); + private List enumViolations = new ArrayList(); public UnnecessaryFullyQualifiedNameRule() { super.addRuleChainVisit(ASTCompilationUnit.class); @@ -29,6 +31,13 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule { @Override public Object visit(ASTCompilationUnit node, Object data) { imports.clear(); + violations.clear(); + enumViolations.clear(); + + super.visit(node, data); + + filterPotentialViolations(); + reportViolations(data); return data; } @@ -118,9 +127,49 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule { ASTImportDeclaration firstMatch = matches.get(0); String importStr = firstMatch.getImportedName() + (matches.get(0).isImportOnDemand() ? ".*" : ""); String type = firstMatch.isStatic() ? "static " : ""; - addViolation(data, node, new Object[] { node.getImage(), importStr, type }); + + PotentialViolation v = new PotentialViolation(node, importStr, type); + violations.add(v); + if (isEnum(firstMatch.getType())) { + enumViolations.add(v); + } } matches.clear(); } + + private static class PotentialViolation { + private JavaNode node; + private String importStr; + private String importType; + + public PotentialViolation(JavaNode node, String importStr, String importType) { + this.node = node; + this.importStr = importStr; + this.importType = importType; + } + + public void addViolation(UnnecessaryFullyQualifiedNameRule rule, Object data) { + rule.addViolation(data, node, new Object[] { node.getImage(), importStr, importType }); + } + } + + private void filterPotentialViolations() { + if (enumViolations.size() > 1) { + violations.removeAll(enumViolations); + } + } + + private void reportViolations(Object data) { + for (PotentialViolation v : violations) { + v.addViolation(this, data); + } + } + + private boolean isEnum(Class type) { + if (type != null && Enum.class.isAssignableFrom(type)) { + return true; + } + return false; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 5a3a0fbf30..49aba0791f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -662,6 +662,16 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } } + if (myType == null && qualifiedName != null && qualifiedName.contains(".")) { + // try if the last part defines a inner class + String qualifiedNameInner = qualifiedName.substring(0, qualifiedName.lastIndexOf('.')) + + "$" + qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1); + try { + myType = pmdClassLoader.loadClass(qualifiedNameInner); + } catch (Exception e) { + // ignored + } + } if (myType == null && qualifiedName != null && !qualifiedName.contains(".")) { // try again with java.lang.... try { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/imports/ImportsRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/imports/ImportsRulesTest.java index 4198bbbfbe..4c7d0c82a4 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/imports/ImportsRulesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/imports/ImportsRulesTest.java @@ -30,4 +30,14 @@ public class ImportsRulesTest extends SimpleAggregatorTst { System.out.println(message); } } + + // Do not delete these two enums - it is needed for a test case + // see: /pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/imports/xml/UnnecessaryFullyQualifiedName.xml + // #1436 UnnecessaryFullyQualifiedName false positive on clashing static imports with enums + public enum ENUM1 { + A, B; + } + public enum ENUM2 { + C, D; + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/imports/xml/UnnecessaryFullyQualifiedName.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/imports/xml/UnnecessaryFullyQualifiedName.xml index 5b67fe2faa..bb2036e9fb 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/imports/xml/UnnecessaryFullyQualifiedName.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/imports/xml/UnnecessaryFullyQualifiedName.xml @@ -315,6 +315,22 @@ public class InitialPageComponent extends BaseExecutionCourseComponent { .sorted(Comparator.comparing(Professorship::isResponsibleFor).reversed() .thenComparing(Professorship.COMPARATOR_BY_PERSON_NAME)).collect(Collectors.toList())); } +} + ]]> + + + + #1436 UnnecessaryFullyQualifiedName false positive on clashing static imports with enums + 0 + diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 779a5320c6..a090bc77e6 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -26,6 +26,8 @@ * [#1438](https://sourceforge.net/p/pmd/bugs/1438/): UseNotifyAllInsteadOfNotify gives false positive * java-finalizers/AvoidCallingFinalize * [#1440](https://sourceforge.net/p/pmd/bugs/1440/): NPE in AvoidCallingFinalize +* java-imports/UnnecessaryFullyQualifiedName + * [#1436](https://sourceforge.net/p/pmd/bugs/1436/): UnnecessaryFullyQualifiedName false positive on clashing static imports with enums * java-naming/SuspiciousEqualsMethodName * [#1431](https://sourceforge.net/p/pmd/bugs/1431/): SuspiciousEqualsMethodName false positive * java-optimizations/RedundantFieldInitializer