Fix for issue #390: visit statically-compiled enum constant members

groovy · Nov 16, 2017 · c4fa59e · c4fa59e
1 parent 6c3e2ad
commit c4fa59e
Show file tree

Hide file tree

Showing 7 changed files with 136 additions and 32 deletions.
diff --git a/....tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/Groovy21InferencingTests.java b/....tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/Groovy21InferencingTests.java
@@ -38,7 +38,7 @@ public void setUp() {
     }
 
     @Test
-    public void testDelegatesToValue1() {
+    public void testDelegatesToValue() {
         String contents =
             "class Other { }\n" +
                 "def meth(@DelegatesTo(Other) Closure c) { }\n" +
@@ -225,8 +225,41 @@ public void testDelegatesToTarget5() {
         assertUnknownConfidence(contents, offset, offset + 1, "B", false);
     }
 
+    @Test // https://github.com/groovy/groovy-eclipse/issues/389
+    public void testEnumOverrides() {
+        String contents =
+            "enum E {\n" +
+            "  ONE() {\n" +
+            "    void meth(Number param) { println param }\n" +
+            "  },\n" +
+            "  TWO() {\n" +
+            "    void meth(Number param) { null }\n" +
+            "  }\n" +
+            "  abstract void meth(Number param);\n" +
+            "}";
+        int offset = contents.indexOf("println param") + "println ".length();
+        assertType(contents, offset, offset + "param".length(), "java.lang.Number");
+    }
+
+    @Test // https://github.com/groovy/groovy-eclipse/issues/390
+    public void testEnumOverrides2() {
+        String contents =
+            "@groovy.transform.CompileStatic\n" +
+            "enum E {\n" +
+            "  ONE() {\n" +
+            "    void meth(Number param) { println param }\n" +
+            "  },\n" +
+            "  TWO() {\n" +
+            "    void meth(Number param) { null }\n" +
+            "  }\n" +
+            "  abstract void meth(Number param);\n" +
+            "}";
+        int offset = contents.indexOf("println param") + "println ".length();
+        assertType(contents, offset, offset + "param".length(), "java.lang.Number");
+    }
+
     @Test
-    public void testStaticCompile1() {
+    public void testStaticCompile() {
         Activator.getInstancePreferences().getBoolean(Activator.GROOVY_SCRIPT_FILTERS_ENABLED, Activator.DEFAULT_SCRIPT_FILTERS_ENABLED);
         Activator.getInstancePreferences().get(Activator.GROOVY_SCRIPT_FILTERS, Activator.DEFAULT_GROOVY_SCRIPT_FILTER);
         try {
@@ -261,13 +294,13 @@ public void testStaticCompile1() {
 
             String contents =
                 "import groovy.transform.TypeChecked\n" +
-                    "class Robot {\n" +
-                    "    void move(String dist) { println \"Moved $dist\" }\n" +
-                    "}\n" +
-                    "@TypeChecked(extensions = 'robot/RobotMove.groovy')\n" +
-                    "void operate() {\n" +
-                    "    robot.move \"left\"\n" +
-                    "}";
+                "class Robot {\n" +
+                "    void move(String dist) { println \"Moved $dist\" }\n" +
+                "}\n" +
+                "@TypeChecked(extensions = 'robot/RobotMove.groovy')\n" +
+                "void operate() {\n" +
+                "    robot.move \"left\"\n" +
+                "}";
 
             int start = contents.lastIndexOf("move");
             int end = start + "move".length();

diff --git a/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java b/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java
@@ -754,6 +754,28 @@ private void visitMethodInternal(MethodNode node, boolean isCtor) {
         }
     }
 
+    /**
+     * @param node anonymous inner class for enum constant
+     */
+    private void visitMethodOverrides(ClassNode node) {
+        scopes.add(new VariableScope(scopes.getLast(), node, false));
+        ASTNode  enclosingDeclaration0 = enclosingDeclarationNode;
+        IJavaElement enclosingElement0 = enclosingElement;
+        enclosingDeclarationNode = node;
+        try {
+            for (MethodNode method : node.getMethods()) {
+                if (method.getEnd() > 0) {
+                    //enclosingElement = ((SourceType) enclosingElement0).getMethod(method.getName(), getParameterTypeSignatures(method.getParameters()));
+                    visitMethodInternal(method, false);
+                }
+            }
+        } finally {
+            scopes.removeLast();
+            enclosingElement = enclosingElement0;
+            enclosingDeclarationNode = enclosingDeclaration0;
+        }
+    }
+
     //
 
     @Override
@@ -1493,6 +1515,11 @@ public void visitMethodCallExpression(MethodCallExpression node) {
         node.getArguments().visit(this);
         scope.forgetEnclosingMethodCall();
 
+        ClassNode type;
+        if (isEnumInit(node) && GroovyUtils.isAnonymous(type = ((Expression) node.getReceiver()).getType())) {
+            visitMethodOverrides(type);
+        }
+
         // if this method call is the primary of a larger expression, then pass the inferred type onwards
         if (node.isSpreadSafe()) {
             returnType = createParameterizedList(returnType);
@@ -1688,22 +1715,7 @@ public void visitStaticMethodCallExpression(StaticMethodCallExpression node) {
                 super.visitStaticMethodCallExpression(node);
                 // visit anonymous inner class members
                 if (GroovyUtils.isAnonymous(type)) {
-                    scopes.add(new VariableScope(scopes.getLast(), type, false));
-                    ASTNode  enclosingDeclaration0 = enclosingDeclarationNode;
-                    IJavaElement enclosingElement0 = enclosingElement;
-                    enclosingDeclarationNode = type;
-                    try {
-                        for (MethodNode method : type.getMethods()) {
-                            if (method.getEnd() > 0) {
-                                //enclosingElement = ((SourceType) enclosingElement0).getMethod(method.getName(), getParameterTypeSignatures(method.getParameters()));
-                                visitMethodInternal(method, false);
-                            }
-                        }
-                    } finally {
-                        scopes.removeLast();
-                        enclosingElement = enclosingElement0;
-                        enclosingDeclarationNode = enclosingDeclaration0;
-                    }
+                    visitMethodOverrides(type);
                 }
             }
         }
@@ -2756,6 +2768,10 @@ private static ClassNode[] inferClosureParamTypes(VariableScope scope, ClosureEx
         return inferredTypes;
     }
 
+    private static boolean isEnumInit(MethodCallExpression node) {
+        return (node.getType().isEnum() && node.getMethodAsString().equals("$INIT"));
+    }
+
     private static boolean isEnumInit(StaticMethodCallExpression node) {
         return (node.getOwnerType().isEnum() && node.getMethod().equals("$INIT"));
     }

diff --git a/...haus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides/in/A.groovy b/...haus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides/in/A.groovy
@@ -2,12 +2,12 @@ package p;
 
 enum A {
 
-	A() {
+	ONE() {
 		@Override
 		String getFoo() {
 		}
 	},
-	B() {
+	TWO() {
 		@Override
 		String getFoo() {
 			"bar"
@@ -16,4 +16,4 @@ enum A {
 
 	String getFoo() {
 	}
-}
+}
diff --git a/...aus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides/out/A.groovy b/...aus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides/out/A.groovy
@@ -2,12 +2,12 @@ package p;
 
 enum A {
 
-	A() {
+	ONE() {
 		@Override
 		String foo() {
 		}
 	},
-	B() {
+	TWO() {
 		@Override
 		String foo() {
 			"bar"
@@ -16,4 +16,4 @@ enum A {
 
 	String foo() {
 	}
-}
+}
diff --git a/...aus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides2/in/A.groovy b/...aus.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides2/in/A.groovy
@@ -0,0 +1,24 @@
+package p;
+
+import groovy.transform.CompileStatic;
+
+@CompileStatic
+enum A {
+
+  A() {
+    @Override
+    String getFoo() {
+      'bar'
+    }
+  },
+
+  B() {
+    @Override
+    String getFoo() {
+      'baz'
+    }
+  }
+
+  String getFoo() {
+  }
+}
diff --git a/...us.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides2/out/A.groovy b/...us.groovy.eclipse.refactoring.test/resources/RenameMethod/testEnumOverrides2/out/A.groovy
@@ -0,0 +1,24 @@
+package p;
+
+import groovy.transform.CompileStatic;
+
+@CompileStatic
+enum A {
+
+  A() {
+    @Override
+    String foo() {
+      'bar'
+    }
+  },
+
+  B() {
+    @Override
+    String foo() {
+      'baz'
+    }
+  }
+
+  String foo() {
+  }
+}
diff --git a/...ing.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/RenameMethodTests.groovy b/...ing.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/RenameMethodTests.groovy
@@ -129,13 +129,20 @@ final class RenameMethodTests extends RefactoringTestSuite {
         runTest('I', 'run', 'sam', [], true, false);
     }
 
-    @Test // GRECLIPSE-1538
+    @Test // https://github.com/groovy/groovy-eclipse/issues/389
     void testEnumOverrides() {
         assumeTrue(isAtLeastGroovy(21))
         // rename A.getFoo() to A.foo() and enum constant overrides should change
         runTest('A', 'getFoo', 'foo', [], true, false)
     }
 
+    @Test // https://github.com/groovy/groovy-eclipse/issues/390
+    void testEnumOverrides2() {
+        assumeTrue(isAtLeastGroovy(21))
+        // rename A.getFoo() to A.foo() and enum constant overrides should change
+        runTest('A', 'getFoo', 'foo', [], true, false)
+    }
+
     @Test
     void testInitializer1() {
         runTest('A', 'm', 'k', [], true, false)