Skip to content

Commit 2b8625b

Browse files
eric-milleseric-milles
committed
GROOVY-11313
1 parent d975843 commit 2b8625b

File tree

2 files changed

+26
-33
lines changed

2 files changed

+26
-33
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ClosureInferencingTests.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2009-2023 the original author or authors.
2+
* Copyright 2009-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -1644,8 +1644,8 @@ public void testClosureReferencesSuperClass() {
16441644
assertDeclaringType(contents, "insuper", "A");
16451645
}
16461646

1647-
@Test
1648-
public void testGRECLIPSE1348() {
1647+
@Test // GRECLIPSE-1348
1648+
public void testClosureSpecialNameShadowing1() {
16491649
//@formatter:off
16501650
String contents =
16511651
"class C {\n" +
@@ -1657,16 +1657,13 @@ public void testGRECLIPSE1348() {
16571657
assertType(contents, "owner", "java.lang.String");
16581658
}
16591659

1660-
@Test
1661-
public void testGRECLIPSE1348a() {
1660+
@Test // GRECLIPSE-1348 and GROOVY-11313
1661+
public void testClosureSpecialNameShadowing2() {
16621662
//@formatter:off
16631663
String contents =
1664-
"class C {\n" +
1665-
" def m(String notOwner) {\n" +
1666-
" return { return owner }\n" +
1667-
" }\n" +
1668-
"}";
1664+
"List<String> list = ['a','b','c']\n" +
1665+
"list.each { owner -> { -> return owner } }\n";
16691666
//@formatter:on
1670-
assertType(contents, "owner", "C");
1667+
assertType(contents, "owner", "java.lang.String");
16711668
}
16721669
}

‎base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.Map;
2828
import java.util.Optional;
2929
import java.util.concurrent.CancellationException;
30+
import java.util.function.BiConsumer;
3031
import java.util.function.Consumer;
3132
import java.util.stream.Collectors;
3233
import java.util.stream.Stream;
@@ -947,41 +948,36 @@ public void visitClosureExpression(final ClosureExpression node) {
947948
VariableScope scope = new VariableScope(parent, node, false);
948949
scopes.add(scope);
949950
try {
950-
// if enclosing closure, owner type is 'Closure', otherwise it's 'typeof(this)'
951+
// GRECLIPSE-1348 and GROOVY-11313: don't override variable named "owner"
952+
BiConsumer<String, ClassNode> impliedVariableDeclarator = (name, type) -> {
953+
VariableScope.VariableInfo info = scope.lookupName(name);
954+
if (info == null || info.type == null || info.declaringType.equals(VariableScope.CLOSURE_CLASS_NODE)) {
955+
scope.addVariable(name, type, VariableScope.CLOSURE_CLASS_NODE);
956+
}
957+
scope.addVariable("get" + org.apache.groovy.util.BeanUtils.capitalize(name), type, VariableScope.CLOSURE_CLASS_NODE);
958+
};
959+
960+
// if enclosing closure, owner type is 'Closure', otherwise it is 'typeof(this)'
951961
if (parent.getEnclosingClosure() != null) {
952962
ClassNode closureType = VariableScope.CLOSURE_CLASS_NODE.getPlainNodeReference();
953963
closureType.putNodeMetaData("outer.scope", parent.getEnclosingClosureScope());
954964

955-
scope.addVariable("owner", closureType, VariableScope.CLOSURE_CLASS_NODE);
956-
scope.addVariable("getOwner", closureType, VariableScope.CLOSURE_CLASS_NODE);
965+
impliedVariableDeclarator.accept("owner", closureType);
957966
} else {
958967
ClassNode ownerType = parent.getThis();
959-
// GRECLIPSE-1348: if someone is silly enough to have a variable named "owner"; don't override it
960-
VariableScope.VariableInfo info = scope.lookupName("owner");
961-
if (info == null || info.type == null || info.scopeNode instanceof ClosureExpression) {
962-
scope.addVariable("owner", ownerType, VariableScope.CLOSURE_CLASS_NODE);
963-
}
964-
scope.addVariable("getOwner", ownerType, VariableScope.CLOSURE_CLASS_NODE);
965-
966-
// only set this if not already in a closure; type doesn't vary with nesting
967-
scope.addVariable("thisObject", ownerType, VariableScope.CLOSURE_CLASS_NODE);
968-
scope.addVariable("getThisObject", ownerType, VariableScope.CLOSURE_CLASS_NODE);
968+
impliedVariableDeclarator.accept("owner", ownerType);
969+
// only set this for first closure; its type doesn't vary
970+
impliedVariableDeclarator.accept("thisObject", ownerType);
969971
}
970972

971-
// if enclosing method call, delegate type can be specified by the method, otherwise it's 'typeof(owner)'
973+
// if enclosing method call, delegate type can be specified by the method, otherwise it is 'typeof(owner)'
972974
VariableScope.CallAndType cat = scope.getEnclosingMethodCallExpression();
973975
if (cat != null && cat.getDelegateType(node) != null) {
974976
ClassNode delegateType = cat.getDelegateType(node);
975-
scope.addVariable("delegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
976-
scope.addVariable("getDelegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
977+
impliedVariableDeclarator.accept("delegate", delegateType);
977978
} else {
978979
ClassNode delegateType = scope.getOwner();
979-
// GRECLIPSE-1348: if someone is silly enough to have a variable named "delegate"; don't override it
980-
VariableScope.VariableInfo info = scope.lookupName("delegate");
981-
if (info == null || info.type == null || info.scopeNode instanceof ClosureExpression) {
982-
scope.addVariable("delegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
983-
}
984-
scope.addVariable("getDelegate", delegateType, VariableScope.CLOSURE_CLASS_NODE);
980+
impliedVariableDeclarator.accept("delegate", delegateType);
985981
}
986982

987983
ClassNode[] inferredParamTypes = inferClosureParamTypes(node, scope);

0 commit comments

Comments
 (0)