Skip to content

Commit b41a9e1

Browse files
eric-milleseric-milles
committed
GROOVY-6510
1 parent c22ea8a commit b41a9e1

File tree

4 files changed

+117
-18
lines changed

4 files changed

+117
-18
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CategoryTests.java

+33
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,39 @@ public void testCategory3() {
174174
"}\n");
175175
}
176176

177+
@Test
178+
public void testCategory6510() {
179+
//@formatter:off
180+
String[] sources = {
181+
"Main.groovy",
182+
"use(NumberCategory) {\n" +
183+
" print 1.something()\n" +
184+
"}\n",
185+
186+
"NumberCategory.groovy",
187+
"@Category(Number) class NumberCategory {\n" +
188+
" def something() {\n" +
189+
" def closure = { ->\n" +
190+
" getThing()\n" + // replaced by "$this.getThing()"
191+
" }\n" +
192+
" closure.resolveStrategy = Closure.DELEGATE_FIRST\n" +
193+
" closure.delegate = new NumberDelegate(this)\n" +
194+
" closure.call()\n" +
195+
" }\n" +
196+
"}\n",
197+
198+
"NumberDelegate.groovy",
199+
"class NumberDelegate {\n" +
200+
" private final Number n\n" +
201+
" NumberDelegate(Number n) { this.n = n }\n" +
202+
" String getThing() { 'works' + n.intValue() }\n" +
203+
"}\n",
204+
};
205+
//@formatter:on
206+
207+
runConformTest(sources, "works1");
208+
}
209+
177210
@Test
178211
public void testCategory8433() {
179212
//@formatter:off

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/CategoryASTTransformation.java

+28-6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.codehaus.groovy.ast.expr.ClosureExpression;
3333
import org.codehaus.groovy.ast.expr.DeclarationExpression;
3434
import org.codehaus.groovy.ast.expr.Expression;
35+
import org.codehaus.groovy.ast.expr.MethodCallExpression;
3536
import org.codehaus.groovy.ast.expr.PropertyExpression;
3637
import org.codehaus.groovy.ast.expr.TupleExpression;
3738
import org.codehaus.groovy.ast.expr.VariableExpression;
@@ -40,7 +41,6 @@
4041
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
4142
import org.codehaus.groovy.ast.stmt.ForStatement;
4243
import org.codehaus.groovy.classgen.VariableScopeVisitor;
43-
import org.codehaus.groovy.control.CompilePhase;
4444
import org.codehaus.groovy.control.SourceUnit;
4545
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
4646
import org.codehaus.groovy.syntax.SyntaxException;
@@ -51,6 +51,8 @@
5151
import java.util.List;
5252
import java.util.Set;
5353

54+
import static java.util.Collections.addAll;
55+
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
5456
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
5557
import static org.codehaus.groovy.ast.tools.ClosureUtils.hasImplicitParameter;
5658
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
@@ -65,8 +67,8 @@
6567
* <li>references to 'this' changed to the additional 'self' parameter</li>
6668
* </ul>
6769
*/
68-
@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
69-
public class CategoryASTTransformation implements ASTTransformation, Opcodes {
70+
@GroovyASTTransformation
71+
public class CategoryASTTransformation implements ASTTransformation {
7072
// should not use a static variable because of possible changes to node metadata
7173
// which would be visible to other compilation units
7274
private final VariableExpression thisExpression = createThisExpression();
@@ -144,9 +146,11 @@ public void visitBlockStatement(BlockStatement block) {
144146

145147
@Override
146148
public void visitClosureExpression(ClosureExpression ce) {
149+
/* GRECLIPSE edit
147150
addVariablesToStack(getParametersSafe(ce));
148151
super.visitClosureExpression(ce);
149152
varStack.removeLast();
153+
*/
150154
}
151155

152156
@Override
@@ -190,7 +194,7 @@ public void visitExpressionStatement(ExpressionStatement es) {
190194
public Expression transform(Expression exp) {
191195
if (exp instanceof VariableExpression) {
192196
VariableExpression ve = (VariableExpression) exp;
193-
/* GRECLIPSE edit -- "this" and "super" handling
197+
/* GRECLIPSE edit -- "this" and "super" handling
194198
if (ve.getName().equals("this"))
195199
return thisExpression;
196200
else {
@@ -216,24 +220,42 @@ public Expression transform(Expression exp) {
216220
thisExpression.setSourcePosition(ve);
217221
thisExpression.setType(targetClass);
218222
return thisExpression;
219-
} else if (!ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
223+
} else if (!inClosure && !ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
220224
PropertyExpression pe = new PropertyExpression(thisExpression, ve.getName());
221225
pe.setSourcePosition(ve);
222226
return pe;
223227
}
224-
// GRECLIPSE end
228+
} else if (exp instanceof MethodCallExpression) {
229+
MethodCallExpression mce = (MethodCallExpression) exp;
230+
if (inClosure && mce.isImplicitThis() && isThisExpression(mce.getObjectExpression())) {
231+
// GROOVY-6510: preserve implicit-this semantics
232+
mce.setArguments(transform(mce.getArguments()));
233+
mce.setMethod(transform(mce.getMethod()));
234+
return mce;
235+
}
236+
// GRECLIPSE end
225237
} else if (exp instanceof ClosureExpression) {
226238
ClosureExpression ce = (ClosureExpression) exp;
227239
ce.getVariableScope().putReferencedLocalVariable((Parameter) parameter.get());
228240
addVariablesToStack(
229241
hasImplicitParameter(ce)
230242
? params(param(ClassHelper.OBJECT_TYPE, "it"))
231243
: getParametersSafe(ce));
244+
// GRECLIPSE add
245+
addAll(varStack.getLast(), "owner", "delegate", "thisObject");
246+
boolean closure = inClosure; inClosure = true;
247+
// GRECLIPSE end
232248
ce.getCode().visit(this);
233249
varStack.removeLast();
250+
// GRECLIPSE add
251+
inClosure = closure;
252+
// GRECLIPSE end
234253
}
235254
return super.transform(exp);
236255
}
256+
// GRECLIPSE add
257+
private boolean inClosure;
258+
// GRECLIPSE end
237259
};
238260

239261
for (MethodNode method : parent.getMethods()) {

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/CategoryASTTransformation.java

+28-6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.codehaus.groovy.ast.expr.ClosureExpression;
3333
import org.codehaus.groovy.ast.expr.DeclarationExpression;
3434
import org.codehaus.groovy.ast.expr.Expression;
35+
import org.codehaus.groovy.ast.expr.MethodCallExpression;
3536
import org.codehaus.groovy.ast.expr.PropertyExpression;
3637
import org.codehaus.groovy.ast.expr.TupleExpression;
3738
import org.codehaus.groovy.ast.expr.VariableExpression;
@@ -40,7 +41,6 @@
4041
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
4142
import org.codehaus.groovy.ast.stmt.ForStatement;
4243
import org.codehaus.groovy.classgen.VariableScopeVisitor;
43-
import org.codehaus.groovy.control.CompilePhase;
4444
import org.codehaus.groovy.control.SourceUnit;
4545
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
4646
import org.codehaus.groovy.syntax.SyntaxException;
@@ -51,6 +51,8 @@
5151
import java.util.List;
5252
import java.util.Set;
5353

54+
import static java.util.Collections.addAll;
55+
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
5456
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
5557
import static org.codehaus.groovy.ast.tools.ClosureUtils.hasImplicitParameter;
5658
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
@@ -65,8 +67,8 @@
6567
* <li>references to 'this' changed to the additional 'self' parameter</li>
6668
* </ul>
6769
*/
68-
@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
69-
public class CategoryASTTransformation implements ASTTransformation, Opcodes {
70+
@GroovyASTTransformation
71+
public class CategoryASTTransformation implements ASTTransformation {
7072
// should not use a static variable because of possible changes to node metadata
7173
// which would be visible to other compilation units
7274
private final VariableExpression thisExpression = createThisExpression();
@@ -144,9 +146,11 @@ public void visitBlockStatement(BlockStatement block) {
144146

145147
@Override
146148
public void visitClosureExpression(ClosureExpression ce) {
149+
/* GRECLIPSE edit
147150
addVariablesToStack(getParametersSafe(ce));
148151
super.visitClosureExpression(ce);
149152
varStack.removeLast();
153+
*/
150154
}
151155

152156
@Override
@@ -190,7 +194,7 @@ public void visitExpressionStatement(ExpressionStatement es) {
190194
public Expression transform(Expression exp) {
191195
if (exp instanceof VariableExpression) {
192196
VariableExpression ve = (VariableExpression) exp;
193-
/* GRECLIPSE edit -- "this" and "super" handling
197+
/* GRECLIPSE edit -- "this" and "super" handling
194198
if (ve.getName().equals("this"))
195199
return thisExpression;
196200
else {
@@ -216,24 +220,42 @@ public Expression transform(Expression exp) {
216220
thisExpression.setSourcePosition(ve);
217221
thisExpression.setType(targetClass);
218222
return thisExpression;
219-
} else if (!ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
223+
} else if (!inClosure && !ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
220224
PropertyExpression pe = new PropertyExpression(thisExpression, ve.getName());
221225
pe.setSourcePosition(ve);
222226
return pe;
223227
}
224-
// GRECLIPSE end
228+
} else if (exp instanceof MethodCallExpression) {
229+
MethodCallExpression mce = (MethodCallExpression) exp;
230+
if (inClosure && mce.isImplicitThis() && isThisExpression(mce.getObjectExpression())) {
231+
// GROOVY-6510: preserve implicit-this semantics
232+
mce.setArguments(transform(mce.getArguments()));
233+
mce.setMethod(transform(mce.getMethod()));
234+
return mce;
235+
}
236+
// GRECLIPSE end
225237
} else if (exp instanceof ClosureExpression) {
226238
ClosureExpression ce = (ClosureExpression) exp;
227239
ce.getVariableScope().putReferencedLocalVariable((Parameter) parameter.get());
228240
addVariablesToStack(
229241
hasImplicitParameter(ce)
230242
? params(param(ClassHelper.OBJECT_TYPE, "it"))
231243
: getParametersSafe(ce));
244+
// GRECLIPSE add
245+
addAll(varStack.getLast(), "owner", "delegate", "thisObject");
246+
boolean closure = inClosure; inClosure = true;
247+
// GRECLIPSE end
232248
ce.getCode().visit(this);
233249
varStack.removeLast();
250+
// GRECLIPSE add
251+
inClosure = closure;
252+
// GRECLIPSE end
234253
}
235254
return super.transform(exp);
236255
}
256+
// GRECLIPSE add
257+
private boolean inClosure;
258+
// GRECLIPSE end
237259
};
238260

239261
for (MethodNode method : parent.getMethods()) {

‎base/org.codehaus.groovy40/src/org/codehaus/groovy/transform/CategoryASTTransformation.java

+28-6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.codehaus.groovy.ast.expr.ClosureExpression;
3333
import org.codehaus.groovy.ast.expr.DeclarationExpression;
3434
import org.codehaus.groovy.ast.expr.Expression;
35+
import org.codehaus.groovy.ast.expr.MethodCallExpression;
3536
import org.codehaus.groovy.ast.expr.PropertyExpression;
3637
import org.codehaus.groovy.ast.expr.TupleExpression;
3738
import org.codehaus.groovy.ast.expr.VariableExpression;
@@ -40,7 +41,6 @@
4041
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
4142
import org.codehaus.groovy.ast.stmt.ForStatement;
4243
import org.codehaus.groovy.classgen.VariableScopeVisitor;
43-
import org.codehaus.groovy.control.CompilePhase;
4444
import org.codehaus.groovy.control.SourceUnit;
4545
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
4646
import org.codehaus.groovy.syntax.SyntaxException;
@@ -51,6 +51,8 @@
5151
import java.util.List;
5252
import java.util.Set;
5353

54+
import static java.util.Collections.addAll;
55+
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
5456
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
5557
import static org.codehaus.groovy.ast.tools.ClosureUtils.hasImplicitParameter;
5658
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
@@ -65,8 +67,8 @@
6567
* <li>references to 'this' changed to the additional 'self' parameter</li>
6668
* </ul>
6769
*/
68-
@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
69-
public class CategoryASTTransformation implements ASTTransformation, Opcodes {
70+
@GroovyASTTransformation
71+
public class CategoryASTTransformation implements ASTTransformation {
7072
// should not use a static variable because of possible changes to node metadata
7173
// which would be visible to other compilation units
7274
private final VariableExpression thisExpression = createThisExpression();
@@ -146,9 +148,11 @@ public void visitBlockStatement(BlockStatement block) {
146148

147149
@Override
148150
public void visitClosureExpression(ClosureExpression ce) {
151+
/* GRECLIPSE edit
149152
addVariablesToStack(getParametersSafe(ce));
150153
super.visitClosureExpression(ce);
151154
varStack.removeLast();
155+
*/
152156
}
153157

154158
@Override
@@ -192,7 +196,7 @@ public void visitExpressionStatement(ExpressionStatement es) {
192196
public Expression transform(Expression exp) {
193197
if (exp instanceof VariableExpression) {
194198
VariableExpression ve = (VariableExpression) exp;
195-
/* GRECLIPSE edit -- "this" and "super" handling
199+
/* GRECLIPSE edit -- "this" and "super" handling
196200
if (ve.getName().equals("this"))
197201
return thisExpression;
198202
else {
@@ -218,24 +222,42 @@ public Expression transform(Expression exp) {
218222
thisExpression.setSourcePosition(ve);
219223
thisExpression.setType(targetClass);
220224
return thisExpression;
221-
} else if (!ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
225+
} else if (!inClosure && !ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
222226
PropertyExpression pe = new PropertyExpression(thisExpression, ve.getName());
223227
pe.setSourcePosition(ve);
224228
return pe;
225229
}
226-
// GRECLIPSE end
230+
} else if (exp instanceof MethodCallExpression) {
231+
MethodCallExpression mce = (MethodCallExpression) exp;
232+
if (inClosure && mce.isImplicitThis() && isThisExpression(mce.getObjectExpression())) {
233+
// GROOVY-6510: preserve implicit-this semantics
234+
mce.setArguments(transform(mce.getArguments()));
235+
mce.setMethod(transform(mce.getMethod()));
236+
return mce;
237+
}
238+
// GRECLIPSE end
227239
} else if (exp instanceof ClosureExpression) {
228240
ClosureExpression ce = (ClosureExpression) exp;
229241
ce.getVariableScope().putReferencedLocalVariable((Parameter) parameter.get());
230242
addVariablesToStack(
231243
hasImplicitParameter(ce)
232244
? params(param(ClassHelper.OBJECT_TYPE, "it"))
233245
: getParametersSafe(ce));
246+
// GRECLIPSE add
247+
addAll(varStack.getLast(), "owner", "delegate", "thisObject");
248+
boolean closure = inClosure; inClosure = true;
249+
// GRECLIPSE end
234250
ce.getCode().visit(this);
235251
varStack.removeLast();
252+
// GRECLIPSE add
253+
inClosure = closure;
254+
// GRECLIPSE end
236255
}
237256
return super.transform(exp);
238257
}
258+
// GRECLIPSE add
259+
private boolean inClosure;
260+
// GRECLIPSE end
239261
};
240262

241263
for (MethodNode method : parent.getMethods()) {

0 commit comments

Comments
 (0)