Fix for #1582: closure property precedence

groovy · May 26, 2024 · d01cb5e · d01cb5e
1 parent ac13cc1
commit d01cb5e
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 31 deletions.
diff --git a/...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ClosureInferencingTests.java b/...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ClosureInferencingTests.java
@@ -1496,6 +1496,29 @@ public void testClosureReturnType4() {
         assertType(contents, "length", "java.lang.Integer");
     }
 
+    @Test
+    public void testSwitchClosureCase1() {
+        String contents =
+            "switch (123) {\n" +
+            "  case {i -> i > 10}:\n" +
+            "  break\n" +
+            "}\n";
+
+        assertType(contents, "i", "java.lang.Integer");
+    }
+
+    @Test
+    public void testSwitchClosureCase2() {
+        String contents =
+            "switch (123) {\n" +
+            "  case {i,j -> i > 10}:\n" +
+            "  break\n" +
+            "}\n";
+
+        assertType(contents, "i", "java.lang.Integer");
+        assertType(contents, "j", "java.lang.Object");
+    }
+
     @Test
     public void testClosureParamsAnnotation1() {
         //@formatter:off

diff --git a/...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/GenericInferencingTests.java b/...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/GenericInferencingTests.java
@@ -554,7 +554,7 @@ public void testMap21() {
         assertType(contents, offset, offset + "getMetaClass".length(), "groovy.lang.MetaClass");
 
             offset = contents.lastIndexOf("metaClass");
-        assertType(contents, offset, offset + "metaClass".length(), "java.lang.String");
+        assertType(contents, offset, offset + "metaClass".length(), "groovy.lang.MetaClass");
 
             offset = contents.indexOf("isEmpty");
         assertType(contents, offset, offset + "isEmpty".length(), "java.lang.Boolean");

diff --git a/...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java b/...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java
@@ -915,6 +915,30 @@ public void testNotMapProperty2() {
         assertType(contents, "name", isAtLeastGroovy(50) ? "java.lang.Number" : "java.lang.String");
     }
 
+    @Test // GROOVY-11367
+    public void testNotMapProperty3() {
+        String contents =
+            "Map map= [:]\n" +
+            "map.with {  \n" +
+            "  delegate  \n" +
+            "  directive \n" +
+            "  metaClass \n" +
+            "  thisObject;\n" +
+            "  { -> owner}\n" +
+            "}\n";
+        // DELEGATE_FIRST: Closure really comes first
+        assertType(contents, "delegate", "java.util.Map");
+        assertDeclaringType(contents, "delegate", "groovy.lang.Closure<V extends java.lang.Object>");
+        assertType(contents, "directive", "java.lang.Integer");
+        assertDeclaringType(contents, "directive", "groovy.lang.Closure");
+        assertType(contents, "metaClass", "groovy.lang.MetaClass");
+        assertDeclaringType(contents, "metaClass", "groovy.lang.GroovyObjectSupport");
+        assertType(contents, "thisObject", DEFAULT_UNIT_NAME);
+        assertDeclaringType(contents, "thisObject", "groovy.lang.Closure<V extends java.lang.Object>");
+        assertType(contents, "owner", "groovy.lang.Closure"); // not DEFAULT_UNIT_NAME
+        assertDeclaringType(contents, "owner", "groovy.lang.Closure<V extends java.lang.Object>");
+    }
+
     @Test
     public void testBoolean1() {
         assertType("!x", "java.lang.Boolean");
@@ -4694,29 +4718,6 @@ public void testThisInInnerClass() {
         assertType(contents, "source", "java.lang.Number");
     }
 
-    @Test
-    public void testSwitchClosureCase2() {
-        String contents =
-            "switch (123) {\n" +
-            "  case {i -> i > 10}:\n" +
-            "  break\n" +
-            "}\n";
-
-        assertType(contents, "i", "java.lang.Integer");
-    }
-
-    @Test
-    public void testSwitchClosureCase3() {
-        String contents =
-            "switch (123) {\n" +
-            "  case {i,j -> i > 10}:\n" +
-            "  break\n" +
-            "}\n";
-
-        assertType(contents, "i", "java.lang.Integer");
-        assertType(contents, "j", "java.lang.Object");
-    }
-
     @Test // GRECLIPSE-1798
     public void testFieldAndPropertyWithSameName() {
         createJavaUnit("Wrapper",

diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
@@ -604,7 +604,12 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var
         ASTNode candidate = null;
         List<ClassNode> callArgs = scope.getMethodCallArgumentTypes();
 
-        if (resolveStrategy == Closure.DELEGATE_FIRST || resolveStrategy == Closure.DELEGATE_ONLY) {
+        if (scope.getEnclosingClosure() != null) {
+            candidate = (!isAssignTarget && callArgs != null) // findDeclaration without the extra stuff
+                ? findMethodDeclaration(var.getName(), VariableScope.CLOSURE_CLASS_NODE, callArgs, false)
+                : findPropertyAccessorMethod(var.getName(), VariableScope.CLOSURE_CLASS_NODE, isAssignTarget, false, callArgs).orElse(null);
+        }
+        if (candidate == null && (resolveStrategy == Closure.DELEGATE_FIRST || resolveStrategy == Closure.DELEGATE_ONLY)) {
             // TODO: If strategy is DELEGATE_ONLY and delegate is enclosing closure, do outer search.
             candidate = findDeclaration(var.getName(), scope.getDelegate(), isAssignTarget, false, 0, callArgs);
             if (candidate != null) var.putNodeMetaData("_from_type_", scope.getDelegate());
@@ -635,9 +640,6 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var
                 }
             }
         }
-        if (candidate == null && resolveStrategy <= Closure.TO_SELF && (resolveStrategy > 0 || scope.getEnclosingClosure() != null)) {
-            candidate = findDeclaration(var.getName(), VariableScope.CLOSURE_CLASS_NODE, isAssignTarget, false, 0, callArgs);
-        }
 
         return candidate;
     }
@@ -752,7 +754,8 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
             }
         }
 
-        if (!declaringType.equals(VariableScope.CLASS_CLASS_NODE) && !declaringType.equals(VariableScope.OBJECT_CLASS_NODE) && !declaringType.equals(ClassHelper.SCRIPT_TYPE)) {
+        if (!declaringType.equals(VariableScope.CLASS_CLASS_NODE) && !declaringType.equals(VariableScope.OBJECT_CLASS_NODE) &&
+                    !declaringType.equals(VariableScope.CLOSURE_CLASS_NODE) && !declaringType.equals(ClassHelper.SCRIPT_TYPE)) {
             Optional<MethodNode> mopMethod = findMetaObjectMethods(declaringType, isLhsExpression, isStaticExpression, methodCallArgumentTypes).filter(mm -> {
                 if (isSynthetic(mm)) return false;
                 Parameter[] p = mm.getParameters();
@@ -1224,8 +1227,7 @@ protected static boolean isNotThisOrOuterClass(final ClassNode thisType, final C
      * implicit in some sense).
      */
     protected static boolean isSynthetic(final MethodNode method) {
-        return method.isSynthetic() || method.getDeclaringClass().equals(VariableScope.CLOSURE_CLASS_NODE) ||
-            GroovyUtils.getAnnotations(method, "groovy.transform.Generated").anyMatch(annotationNode -> true);
+        return method.isSynthetic() || GroovyUtils.getAnnotations(method, "groovy.transform.Generated").anyMatch(annotationNode -> true);
     }
 
     protected static boolean isTraitBridge(final MethodNode method) {