Skip to content

Commit 8f74d9d

Browse files
eric-milleseric-milles
committed
GROOVY-11614: SC: apply enum-case transformation after STC visitation
4_0_X backport
1 parent 5798461 commit 8f74d9d

File tree

4 files changed

+40
-74
lines changed

4 files changed

+40
-74
lines changed

‎src/main/java/org/codehaus/groovy/control/CompilationUnit.java‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,12 @@
2323
import groovy.transform.CompilationUnitAware;
2424
import org.codehaus.groovy.GroovyBugError;
2525
import org.codehaus.groovy.ast.AnnotationNode;
26-
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
2726
import org.codehaus.groovy.ast.ClassHelper;
2827
import org.codehaus.groovy.ast.ClassNode;
2928
import org.codehaus.groovy.ast.CompileUnit;
3029
import org.codehaus.groovy.ast.GroovyClassVisitor;
3130
import org.codehaus.groovy.ast.InnerClassNode;
3231
import org.codehaus.groovy.ast.ModuleNode;
33-
import org.codehaus.groovy.ast.expr.ClosureExpression;
34-
import org.codehaus.groovy.ast.expr.Expression;
35-
import org.codehaus.groovy.ast.expr.MethodCallExpression;
36-
import org.codehaus.groovy.ast.expr.VariableExpression;
3732
import org.codehaus.groovy.classgen.AsmClassGenerator;
3833
import org.codehaus.groovy.classgen.ClassCompletionVerifier;
3934
import org.codehaus.groovy.classgen.EnumCompletionVisitor;
@@ -76,10 +71,7 @@
7671
import java.util.Set;
7772

7873
import static java.util.stream.Collectors.toList;
79-
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
80-
import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
8174
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.DYNAMIC_OUTER_NODE_CALLBACK;
82-
import static org.codehaus.groovy.transform.stc.StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE;
8375

8476
/**
8577
* The CompilationUnit collects all compilation data as it is generated by the compiler system.
@@ -348,39 +340,6 @@ private void addPhaseOperations() {
348340
classNode.removeNodeMetaData(DYNAMIC_OUTER_NODE_CALLBACK);
349341
}
350342
}, Phases.INSTRUCTION_SELECTION);
351-
352-
addPhaseOperation((final SourceUnit source, final GeneratorContext context, final ClassNode classNode) -> {
353-
// TODO: Can this be moved into org.codehaus.groovy.transform.sc.transformers.VariableExpressionTransformer?
354-
GroovyClassVisitor visitor = new ClassCodeExpressionTransformer() {
355-
@Override
356-
protected SourceUnit getSourceUnit() {
357-
return source;
358-
}
359-
360-
@Override
361-
public Expression transform(final Expression expression) {
362-
if (expression instanceof VariableExpression) {
363-
// check for "switch(enumType) { case CONST: ... }"
364-
ClassNode enumType = expression.getNodeMetaData(SWITCH_CONDITION_EXPRESSION_TYPE);
365-
if (enumType != null) {
366-
// replace "CONST" variable expression with "EnumType.CONST" property expression
367-
Expression propertyExpression = propX(classX(enumType), expression.getText());
368-
setSourcePosition(propertyExpression, expression);
369-
return propertyExpression;
370-
}
371-
} else if (expression instanceof MethodCallExpression) {
372-
// we wrap SwitchExpressions into a method call on a ClosureExpression
373-
MethodCallExpression mce = (MethodCallExpression) expression;
374-
if (mce.getObjectExpression() instanceof ClosureExpression) {
375-
expression.visit(this);
376-
return expression;
377-
}
378-
}
379-
return expression;
380-
}
381-
};
382-
visitor.visitClass(classNode);
383-
}, Phases.INSTRUCTION_SELECTION);
384343
}
385344

386345
private void applyCompilationCustomizers() {

‎src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ class VariableExpressionTransformer {
3737

3838
Expression transformVariableExpression(final VariableExpression ve) {
3939
Expression xe = tryTransformImplicitReceiver(ve);
40+
if (xe == null) {
41+
xe = tryTransformEnumConstantAccess(ve);
42+
}
4043
if (xe == null) {
4144
xe = tryTransformPrivateFieldAccess(ve);
4245
}
@@ -77,6 +80,19 @@ private static Expression tryTransformImplicitReceiver(final VariableExpression
7780
return pe;
7881
}
7982

83+
private static Expression tryTransformEnumConstantAccess(final VariableExpression ve) {
84+
ClassNode enumType = ve.getNodeMetaData(StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE);
85+
if (enumType == null) {
86+
return null;
87+
}
88+
89+
// GROOVY-8444, GROOVY-11614: replace "CONST" expression with an "EnumType.CONST" expression
90+
PropertyExpression pe = propX(classX(enumType), ve.getText());
91+
pe.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, enumType);
92+
pe.getProperty().setSourcePosition(ve);
93+
return pe;
94+
}
95+
8096
private static Expression tryTransformPrivateFieldAccess(final VariableExpression ve) {
8197
FieldNode field = ve.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
8298
if (field == null) {

‎src/main/java/org/codehaus/groovy/transform/stc/EnumTypeCheckingExtension.java‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,43 +23,34 @@
2323
import org.codehaus.groovy.ast.expr.VariableExpression;
2424
import org.codehaus.groovy.ast.stmt.SwitchStatement;
2525

26-
import java.lang.reflect.Modifier;
27-
2826
import static org.codehaus.groovy.transform.stc.StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE;
2927

3028
/**
31-
* A type checking extension that will take care of handling errors which are specific to enums. In particular, it will
32-
* handle the enum constants within switch-case statement.
29+
* A type checking extension that will take care of handling errors which are
30+
* specific to enums. In particular, it will handle the enum constants within
31+
* a switch's case statements.
3332
*
3433
* @since 3.0.0
3534
*/
3635
public class EnumTypeCheckingExtension extends TypeCheckingExtension {
37-
public EnumTypeCheckingExtension(StaticTypeCheckingVisitor staticTypeCheckingVisitor) {
36+
37+
public EnumTypeCheckingExtension(final StaticTypeCheckingVisitor staticTypeCheckingVisitor) {
3838
super(staticTypeCheckingVisitor);
3939
}
4040

4141
@Override
42-
public boolean handleUnresolvedVariableExpression(VariableExpression vexp) {
42+
public boolean handleUnresolvedVariableExpression(final VariableExpression vexp) {
4343
SwitchStatement switchStatement = this.typeCheckingVisitor.typeCheckingContext.getEnclosingSwitchStatement();
44-
45-
if (null == switchStatement) return false;
46-
47-
ClassNode type = switchStatement.getExpression().getNodeMetaData(StaticTypesMarker.TYPE);
48-
49-
if (null == type) return false;
50-
51-
if (type.isEnum()) {
52-
FieldNode fieldNode = type.redirect().getField(vexp.getName());
53-
if (null != fieldNode) {
54-
int modifiers = fieldNode.getModifiers();
55-
if (Modifier.isPublic(modifiers) && Modifier.isStatic(modifiers) && Modifier.isFinal(modifiers)
56-
&& type.equals(fieldNode.getType())) {
44+
if (switchStatement != null) {
45+
ClassNode type = switchStatement.getExpression().getNodeMetaData(StaticTypesMarker.TYPE);
46+
if (type != null && type.isEnum()) {
47+
FieldNode fieldNode = type.redirect().getField(vexp.getName());
48+
if (fieldNode != null && fieldNode.isEnum()) {
5749
vexp.putNodeMetaData(SWITCH_CONDITION_EXPRESSION_TYPE, type);
5850
return true;
5951
}
6052
}
6153
}
62-
6354
return false;
6455
}
6556
}

‎src/test/groovy/bugs/Groovy8444.groovy‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,11 @@
1818
*/
1919
package bugs
2020

21-
import groovy.transform.CompileStatic
2221
import org.junit.Test
2322

2423
import static groovy.test.GroovyAssert.assertScript
2524
import static groovy.test.GroovyAssert.shouldFail
2625

27-
@CompileStatic
2826
final class Groovy8444 {
2927

3028
@Test
@@ -226,14 +224,16 @@ final class Groovy8444 {
226224
'''
227225
}
228226

229-
@Test
230227
// GROOVY-11614
228+
@Test
231229
void testAccessingEnumConstantInSwitchExprCase() {
232-
assertScript '''\
230+
def shell = GroovyShell.withConfig {
231+
ast(groovy.transform.CompileStatic)
232+
}
233+
assertScript shell, '''\
233234
enum SomeEnum {
234235
A, B
235236
}
236-
@groovy.transform.CompileStatic
237237
def meth(SomeEnum e) {
238238
switch (e) {
239239
case A -> 1
@@ -245,8 +245,8 @@ final class Groovy8444 {
245245
'''
246246
}
247247

248-
@Test
249248
// GROOVY-11614
249+
@Test
250250
void testAccessingEnumConstantInSwitchExprCase2() {
251251
assertScript '''\
252252
enum SomeEnum {
@@ -268,8 +268,8 @@ final class Groovy8444 {
268268
'''
269269
}
270270

271-
@Test
272271
// GROOVY-11614
272+
@Test
273273
void testAccessingEnumConstantInSwitchExprCase3() {
274274
assertScript '''\
275275
enum SomeEnum {
@@ -287,8 +287,8 @@ final class Groovy8444 {
287287
'''
288288
}
289289

290-
@Test
291290
// GROOVY-11614
291+
@Test
292292
void testAccessingNonEnumConstantInSwitchExprCase() {
293293
def err = shouldFail '''\
294294
enum SomeEnum {
@@ -309,8 +309,8 @@ final class Groovy8444 {
309309
assert err.message.contains('@ line 9, column 26.')
310310
}
311311

312-
@Test
313312
// GROOVY-11614
313+
@Test
314314
void testAccessingNonEnumConstantInSwitchExprCase2() {
315315
def err = shouldFail '''\
316316
enum SomeEnum {
@@ -331,8 +331,8 @@ final class Groovy8444 {
331331
assert err.message.contains('@ line 9, column 26.')
332332
}
333333

334-
@Test
335334
// GROOVY-11614
335+
@Test
336336
void testAccessingNonEnumConstantInSwitchExprCase3() {
337337
def err = shouldFail '''\
338338
enum SomeEnum {
@@ -353,8 +353,8 @@ final class Groovy8444 {
353353
assert err.message.contains('@ line 9, column 26.')
354354
}
355355

356-
@Test
357356
// GROOVY-11614
357+
@Test
358358
void testAccessingNonEnumConstantInSwitchExprCase4() {
359359
def err = shouldFail '''\
360360
enum SomeEnum {
@@ -375,8 +375,8 @@ final class Groovy8444 {
375375
assert err.message.contains('@ line 9, column 26.')
376376
}
377377

378-
@Test
379378
// GROOVY-11614
379+
@Test
380380
void testAccessingEnumConstantInNestedSwitchExprCase() {
381381
assertScript '''\
382382
enum SomeEnum {
@@ -402,8 +402,8 @@ final class Groovy8444 {
402402
'''
403403
}
404404

405-
@Test
406405
// GROOVY-11614
406+
@Test
407407
void testAccessingEnumConstantInNestedSwitchExprCase2() {
408408
assertScript '''\
409409
enum SomeEnum {

0 commit comments

Comments
 (0)