Fix for #1184: check for inner of super, which is implicitly available

Author: eric-milles

ide-test/org.codehaus.groovy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTests.groovy

@@ -946,6 +946,37 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
         doContentsCompareTest(contents)
     }
 
+    @Test // https://github.com/groovy/groovy-eclipse/issues/1184
+    void testInnerClass11() {
+        createGroovyType 'p', 'A', '''\
+            |abstract class A {
+            |  protected static class B {
+            |  }
+            |}
+            |'''
+        String contents = '''\
+            |import p.A
+            |import p.A.B
+            |class C extends A {
+            |  def m(B b) {
+            |  }
+            |}
+            |'''
+        doContentsCompareTest(contents, contents - ~/\|import p.A.B\s+/)
+    }
+
+    @Test // https://github.com/groovy/groovy-eclipse/issues/1184
+    void testInnerClass12() {
+        String contents = '''\
+            |import java.util.Map.Entry
+            |class C implements Map {
+            |  def m(Entry e) {
+            |  }
+            |}
+            |'''
+        doContentsCompareTest(contents, contents - ~/\|import java.util.Map.Entry\s+/)
+    }
+
     @Test
     void testStaticImport1() {
         String contents = '''\

ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/refactoring/actions/OrganizeGroovyImports.java

@@ -360,6 +360,32 @@ private IType[] resolveMissingTypes(IProgressMonitor monitor) throws JavaModelEx
         }
     }
 
+    /**
+     * Determines if organize imports is unsafe due to syntax errors or other conditions.
+     */
+    private static boolean isUnclean(ModuleNodeInfo info, GroovyCompilationUnit unit) {
+        try {
+            if (info.module.encounteredUnrecoverableError() || !unit.isConsistent()) {
+                return true;
+            }
+            CategorizedProblem[] problems = info.result.getProblems();
+            if (problems != null && problems.length > 0) {
+                for (CategorizedProblem problem : problems) {
+                    if (problem.isError() && problem.getCategoryID() == CategorizedProblem.CAT_INTERNAL) {
+                        String message = problem.getMessage();
+                        if (message.contains("unexpected token")) {
+                            trace("Stopping due to error in compilation unit: %s", message);
+                            return true;
+                        }
+                    }
+                }
+            }
+        } catch (Exception e) {
+            return true;
+        }
+        return false;
+    }
+
     /**
      * GRECLIPSE-1390
      * Reorganizing imports (ie- sorting and grouping them) will remove annotations on import statements
@@ -376,16 +402,16 @@ private static boolean isSafeToReorganize(Iterable<ImportNode> allImports) {
         return true;
     }
 
-    private static boolean isAliased(ImportNode imp) {
-        String alias = imp.getAlias();
+    private static boolean isAliased(ImportNode node) {
+        String alias = node.getAlias();
         if (alias == null) {
             return false;
         }
-        String fieldName = imp.getFieldName();
+        String fieldName = node.getFieldName();
         if (fieldName != null) {
             return !fieldName.equals(alias);
         }
-        String className = imp.getClassName();
+        String className = node.getClassName();
         if (className != null) {
             // it is possible to import from the default package
             boolean aliasIsSameAsClassName = className.endsWith(alias) &&
@@ -395,30 +421,14 @@ private static boolean isAliased(ImportNode imp) {
         return false;
     }
 
-    /**
-     * Determines if organize imports is unsafe due to syntax errors or other conditions.
-     */
-    private static boolean isUnclean(ModuleNodeInfo info, GroovyCompilationUnit unit) {
-        try {
-            if (info.module.encounteredUnrecoverableError() || !unit.isConsistent()) {
-                return true;
-            }
-            CategorizedProblem[] problems = info.result.getProblems();
-            if (problems != null && problems.length > 0) {
-                for (CategorizedProblem problem : problems) {
-                    if (problem.isError() && problem.getCategoryID() == CategorizedProblem.CAT_INTERNAL) {
-                        String message = problem.getMessage();
-                        if (message.contains("unexpected token")) {
-                            trace("Stopping due to error in compilation unit: %s", message);
-                            return true;
-                        }
-                    }
-                }
-            }
-        } catch (Exception e) {
-            return true;
+    private static String getTypeName(ClassNode node) {
+        ClassNode type = GroovyUtils.getBaseType(node);
+        // unresolved name may have dots and/or dollars (e.g. 'a.b.C$D' or 'C$D' or even 'C.D')
+        if (!type.getName().matches(".*\\b" + type.getUnresolvedName().replace('$', '.'))) {
+            // synch up name and unresolved name (e.g. 'java.util.Map$Entry as Foo$Entry')
+            return type.getName() + " as " + type.getUnresolvedName().replace('.', '$');
         }
-        return false;
+        return type.getName();
     }
 
     private static void trace(String message, Object... arguments) {
@@ -691,10 +701,9 @@ private void handleTypeReference(ClassNode node, boolean isAnnotation) {
                     name = name.replaceAll(Pattern.quote(name.substring(i)) + "(?= |$)", "");
                 }
                 doNotRemoveImport(name.replace('$', '.'));
-                return;
-            }
-
-            if (!node.isResolved() && !current.getModule().getClasses().contains(node.redirect())) {
+            } else if (current.getModule().getClasses().contains(node)) {
+                // keep in importsSlatedForRemoval and leave out of missingTypes
+            } else if (!node.isResolved()) {
                 String[] parts = name.split("\\.");
                 if (Character.isUpperCase(name.charAt(0))) {
                     name = parts[0]; // 'Map.Entry' -> 'Map'
@@ -714,15 +723,17 @@ private void handleTypeReference(ClassNode node, boolean isAnnotation) {
                     while (i < chars.length && Character.isJavaIdentifierPart(chars[i])) {
                         i += 1;
                     }
+                    // for a 'Map.Entry' reference find '.Map' in 'java.util.Map' or '.Map$' in 'java.util.Map$Entry'
                     m = Pattern.compile("(?:\\A|\\$|\\.)" + String.valueOf(chars, 0, i) + "(?=\\$|$)").matcher(name);
                     if (m.find()) {
                         // 'java.util.Map$Entry' -> 'java.util.Map'
-                        String partialName = name.substring(0, m.end()).replace('$', '.');
-                        doNotRemoveImport(partialName);
+                        String partialName = name.substring(0, m.end());
+                        if (!isInnerOfSuper(partialName))
+                            doNotRemoveImport(partialName.replace('$', '.'));
                     }
                 } else {
-                    // We don't know exactly what the text is. We just know how
-                    // it resolves. This can be a problem if an inner class. We
+                    // We do not know exactly what the text is. We just know how
+                    // it resolves. This can be a problem for an inner class. We
                     // don't really know what is in the text nor what the import
                     // is, so just ensure that none are slated for removal.
                     String partialName = name.replace('$', '.');
@@ -740,14 +751,31 @@ private void handleTypeReference(ClassNode node, boolean isAnnotation) {
             }
         }
 
-        private String getTypeName(ClassNode node) {
-            ClassNode type = GroovyUtils.getBaseType(node);
-            // unresolved name may have dots and/or dollars (e.g. 'a.b.C$D' or 'C$D' or even 'C.D')
-            if (!type.getName().matches(".*\\b" + type.getUnresolvedName().replace('$', '.'))) {
-                // synch up name and unresolved name (e.g. 'java.util.Map$Entry as Foo$Entry')
-                return type.getName() + " as " + type.getUnresolvedName().replace('.', '$');
+        private boolean isInnerOfSuper(String name) {
+            if (name.lastIndexOf('$') > 0) {
+                for (ClassNode node = current.getSuperClass(); node != null && !node.equals(ClassHelper.OBJECT_TYPE); node = node.getSuperClass()) {
+                    for (Iterator<? extends ClassNode> it = node.redirect().getInnerClasses(); it.hasNext();) {
+                        if (it.next().getName().equals(name)) {
+                            return true;
+                        }
+                    }
+                }
+
+                java.util.Queue<ClassNode> todo = new java.util.ArrayDeque<>();
+                java.util.Collections.addAll(todo, current.getInterfaces());
+                Set<ClassNode> done = new LinkedHashSet<>();
+                ClassNode node;
+
+                while ((node = todo.poll()) != null) { if (!done.add(node)) continue;
+                    for (Iterator<? extends ClassNode> it = node.redirect().getInnerClasses(); it.hasNext();) {
+                        if (it.next().getName().equals(name)) {
+                            return true;
+                        }
+                    }
+                    java.util.Collections.addAll(todo, node.getInterfaces());
+                }
             }
-            return type.getName();
+            return false;
         }
 
         private boolean checkRetainImport(String name) {