GROOVY-11313

groovy · Feb 25, 2024 · 2b8625b · 2b8625b
1 parent d975843
commit 2b8625b
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 33 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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2023 the original author or authors.
+ * Copyright 2009-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1644,8 +1644,8 @@ public void testClosureReferencesSuperClass() {
         assertDeclaringType(contents, "insuper", "A");
     }
 
-    @Test
-    public void testGRECLIPSE1348() {
+    @Test // GRECLIPSE-1348
+    public void testClosureSpecialNameShadowing1() {
         //@formatter:off
         String contents =
             "class C {\n" +
@@ -1657,16 +1657,13 @@ public void testGRECLIPSE1348() {
         assertType(contents, "owner", "java.lang.String");
     }
 
-    @Test
-    public void testGRECLIPSE1348a() {
+    @Test // GRECLIPSE-1348 and GROOVY-11313
+    public void testClosureSpecialNameShadowing2() {
         //@formatter:off
         String contents =
-            "class C {\n" +
-            "  def m(String notOwner) {\n" +
-            "    return { return owner }\n" +
-            "  }\n" +
-            "}";
+            "List<String> list = ['a','b','c']\n" +
+            "list.each { owner -> { -> return owner } }\n";
         //@formatter:on
-        assertType(contents, "owner", "C");
+        assertType(contents, "owner", "java.lang.String");
     }
 }
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
@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CancellationException;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -947,41 +948,36 @@ public void visitClosureExpression(final ClosureExpression node) {
         VariableScope scope = new VariableScope(parent, node, false);
         scopes.add(scope);
         try {
-            // if enclosing closure, owner type is 'Closure', otherwise it's 'typeof(this)'
+            // GRECLIPSE-1348 and GROOVY-11313: don't override variable named "owner"
+            BiConsumer<String, ClassNode> impliedVariableDeclarator = (name, type) -> {
+                VariableScope.VariableInfo info = scope.lookupName(name);
+                if (info == null || info.type == null || info.declaringType.equals(VariableScope.CLOSURE_CLASS_NODE)) {
+                    scope.addVariable(name, type, VariableScope.CLOSURE_CLASS_NODE);
+                }
+                scope.addVariable("get" + org.apache.groovy.util.BeanUtils.capitalize(name), type, VariableScope.CLOSURE_CLASS_NODE);
+            };
+
+            // if enclosing closure, owner type is 'Closure', otherwise it is 'typeof(this)'
             if (parent.getEnclosingClosure() != null) {
                 ClassNode closureType = VariableScope.CLOSURE_CLASS_NODE.getPlainNodeReference();
                 closureType.putNodeMetaData("outer.scope", parent.getEnclosingClosureScope());
 
-                scope.addVariable("owner", closureType, VariableScope.CLOSURE_CLASS_NODE);
-                scope.addVariable("getOwner", closureType, VariableScope.CLOSURE_CLASS_NODE);
+                impliedVariableDeclarator.accept("owner", closureType);
             } else {
                 ClassNode ownerType = parent.getThis();
-                // GRECLIPSE-1348: if someone is silly enough to have a variable named "owner"; don't override it
-                VariableScope.VariableInfo info = scope.lookupName("owner");
-                if (info == null || info.type == null || info.scopeNode instanceof ClosureExpression) {
-                    scope.addVariable("owner", ownerType, VariableScope.CLOSURE_CLASS_NODE);
-                }
-                scope.addVariable("getOwner", ownerType, VariableScope.CLOSURE_CLASS_NODE);
-
-                // only set this if not already in a closure; type doesn't vary with nesting
-                scope.addVariable("thisObject", ownerType, VariableScope.CLOSURE_CLASS_NODE);
-                scope.addVariable("getThisObject", ownerType, VariableScope.CLOSURE_CLASS_NODE);
+                impliedVariableDeclarator.accept("owner", ownerType);
+                // only set this for first closure; its type doesn't vary
+                impliedVariableDeclarator.accept("thisObject", ownerType);
             }
 
-            // if enclosing method call, delegate type can be specified by the method, otherwise it's 'typeof(owner)'
+            // if enclosing method call, delegate type can be specified by the method, otherwise it is 'typeof(owner)'
             VariableScope.CallAndType cat = scope.getEnclosingMethodCallExpression();
             if (cat != null && cat.getDelegateType(node) != null) {
                 ClassNode delegateType = cat.getDelegateType(node);
-                scope.addVariable("delegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
-                scope.addVariable("getDelegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
+                impliedVariableDeclarator.accept("delegate", delegateType);
             } else {
                 ClassNode delegateType = scope.getOwner();
-                // GRECLIPSE-1348: if someone is silly enough to have a variable named "delegate"; don't override it
-                VariableScope.VariableInfo info = scope.lookupName("delegate");
-                if (info == null || info.type == null || info.scopeNode instanceof ClosureExpression) {
-                    scope.addVariable("delegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
-                }
-                scope.addVariable("getDelegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
+                impliedVariableDeclarator.accept("delegate", delegateType);
             }
 
             ClassNode[] inferredParamTypes = inferClosureParamTypes(node, scope);