IDEA-126841 ("Unused imports" makes mistakes)

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/imports/ImportStatementComparator.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2000-2014 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.imports;
+
+import com.intellij.psi.PsiImportStatementBase;
+import com.siyeh.ig.psiutils.PsiElementOrderComparator;
+
+import java.util.Comparator;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class ImportStatementComparator implements Comparator<PsiImportStatementBase> {
+
+  public static final ImportStatementComparator INSTANCE = new ImportStatementComparator();
+
+  private ImportStatementComparator() {}
+
+  public static ImportStatementComparator getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public int compare(PsiImportStatementBase importStatementBase1, PsiImportStatementBase importStatementBase2) {
+    final boolean onDemand = importStatementBase1.isOnDemand();
+    if (onDemand != importStatementBase2.isOnDemand()) {
+      return onDemand ? -1 : 1;
+    }
+    // just sort on demand imports first, and sort the rest in reverse file order.
+    return -PsiElementOrderComparator.getInstance().compare(importStatementBase1, importStatementBase2);
+  }
+}

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/imports/ImportsAreUsedVisitor.java

@@ -40,7 +40,7 @@ class ImportsAreUsedVisitor extends JavaRecursiveElementVisitor {
     } else {
       final PsiImportStatementBase[] importStatements = importList.getAllImportStatements();
       this.importStatements = new ArrayList<PsiImportStatementBase>(Arrays.asList(importStatements));
-      Collections.reverse(this.importStatements);
+      Collections.sort(this.importStatements, ImportStatementComparator.getInstance());
     }
   }
 
@@ -101,33 +101,33 @@ class ImportsAreUsedVisitor extends JavaRecursiveElementVisitor {
       return null;
     }
     final boolean hasOnDemandImportConflict = ImportUtils.hasOnDemandImportConflict(qualifiedName, myFile);
-    for (PsiImportStatementBase importStatementBase : importStatements) {
-      if (!importStatementBase.isOnDemand()) {
-        if (member.equals(importStatementBase.resolve())) {
-          return importStatementBase;
+    for (PsiImportStatementBase importStatement : importStatements) {
+      if (!importStatement.isOnDemand()) {
+        if (member.equals(importStatement.resolve())) {
+          return importStatement;
         }
       }
       else {
         if (hasOnDemandImportConflict) {
           continue;
         }
-        final PsiElement target = importStatementBase.resolve();
+        final PsiElement target = importStatement.resolve();
         if (target instanceof PsiPackage) {
           final PsiPackage aPackage = (PsiPackage)target;
           if (packageName.equals(aPackage.getQualifiedName())) {
-            return importStatementBase;
+            return importStatement;
           }
         }
         else if (target instanceof PsiClass) {
           final PsiClass aClass = (PsiClass)target;
           if (InheritanceUtil.isInheritorOrSelf(aClass, containingClass, true)) {
-            if (importStatementBase instanceof PsiImportStaticStatement) {
+            if (importStatement instanceof PsiImportStaticStatement) {
               if (member.hasModifierProperty(PsiModifier.STATIC)) {
-                return importStatementBase;
+                return importStatement;
               }
             }
-            else if (importStatementBase instanceof PsiImportStatement && member instanceof PsiClass) {
-              return importStatementBase;
+            else if (importStatement instanceof PsiImportStatement && member instanceof PsiClass) {
+              return importStatement;
             }
           }
         }

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ImportUtils.java

@@ -63,8 +63,7 @@ public final class ImportUtils {
     if (containingPackageName.equals(packageName) || importList.findSingleClassImportStatement(qualifiedName) != null) {
       return;
     }
-    if (importList.findOnDemandImportStatement(packageName) != null &&
-        !hasDefaultImportConflict(qualifiedName, javaFile) && !hasOnDemandImportConflict(qualifiedName, javaFile)) {
+    if (importList.findOnDemandImportStatement(packageName) != null && !hasOnDemandImportConflict(qualifiedName, javaFile)) {
       return;
     }
     if (hasExactImportConflict(qualifiedName, javaFile)) {
@@ -326,7 +325,7 @@ public final class ImportUtils {
         }
       }
     }
-    return hasJavaLangImportConflict(fqName, javaFile);
+    return hasJavaLangImportConflict(fqName, javaFile) || hasDefaultImportConflict(fqName, javaFile);
   }
 
   private static boolean hasDefaultImportConflict(String fqName, PsiJavaFile file) {

plugins/InspectionGadgets/testsrc/com/siyeh/ig/imports/JavaLangImportInspectionTest.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2000-2014 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.imports;
+
+import com.intellij.codeInspection.InspectionProfileEntry;
+import com.siyeh.ig.LightInspectionTestCase;
+import junit.framework.TestCase;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class JavaLangImportInspectionTest extends LightInspectionTestCase {
+
+  public void testSamePackageConflict() {
+    addEnvironmentClass("package a;" +
+                        "class String {}");
+    doTest("package a;" +
+           "import java.lang.String;" +
+           "class X {{" +
+           "  String s;" +
+           "}}");
+  }
+
+  public void testSimple() {
+    doTest("package a;" +
+           "/*Unnecessary import from package 'java.lang'*/import java.lang.String;/**/" +
+           "class X {{" +
+           "  String s;" +
+           "}}");
+  }
+
+  @Nullable
+  @Override
+  protected InspectionProfileEntry getInspection() {
+    return new JavaLangImportInspection();
+  }
+}

plugins/InspectionGadgets/testsrc/com/siyeh/ig/imports/UnusedImportInspectionTest.java

@@ -163,6 +163,26 @@ public class UnusedImportInspectionTest extends LightInspectionTestCase {
            "}");
   }
 
+  public void testOrderIsNotImportant() {
+    doTest("package a;" +
+           "import java.util.*;" +
+           "/*Unused import 'import java.util.List;'*/import java.util.List;/**/" +
+           "class X {{" +
+           "  List list = new ArrayList();" +
+           "}}");
+
+  }
+
+  public void testConflictInSamePackage() {
+    addEnvironmentClass("package a; public class List {}");
+    doTest("package a;" +
+           "import java.util.List;" +
+           "import java.util.*;" +
+           "class X {{" +
+           "  List list = new ArrayList();" +
+           "}}");
+  }
+
   @Override
   protected LocalInspectionTool getInspection() {
     return new UnusedImportInspection();