Skip to content

Commit 386177e

Browse files
eric-milleseric-milles
committed
GROOVY-11614: SC: apply enum-case transformation after STC visitation
3_0_X backport
1 parent 41b7760 commit 386177e

4 files changed

Lines changed: 40 additions & 65 deletions

File tree

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

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,12 @@
2222
import groovy.lang.GroovyRuntimeException;
2323
import groovy.transform.CompilationUnitAware;
2424
import org.codehaus.groovy.GroovyBugError;
25-
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
2625
import org.codehaus.groovy.ast.ClassHelper;
2726
import org.codehaus.groovy.ast.ClassNode;
2827
import org.codehaus.groovy.ast.CompileUnit;
2928
import org.codehaus.groovy.ast.GroovyClassVisitor;
3029
import org.codehaus.groovy.ast.InnerClassNode;
3130
import org.codehaus.groovy.ast.ModuleNode;
32-
import org.codehaus.groovy.ast.expr.Expression;
33-
import org.codehaus.groovy.ast.expr.VariableExpression;
3431
import org.codehaus.groovy.classgen.AsmClassGenerator;
3532
import org.codehaus.groovy.classgen.ClassCompletionVerifier;
3633
import org.codehaus.groovy.classgen.EnumCompletionVisitor;
@@ -77,11 +74,8 @@
7774
import java.util.stream.Stream;
7875

7976
import static java.util.stream.Collectors.toList;
80-
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
81-
import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
8277
import static org.codehaus.groovy.runtime.StringGroovyMethods.isAtLeast;
8378
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.DYNAMIC_OUTER_NODE_CALLBACK;
84-
import static org.codehaus.groovy.transform.stc.StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE;
8579

8680
/**
8781
* The CompilationUnit collects all compilation data as it is generated by the compiler system.
@@ -310,32 +304,6 @@ private void addPhaseOperations() {
310304
classNode.removeNodeMetaData(DYNAMIC_OUTER_NODE_CALLBACK);
311305
}
312306
}, Phases.INSTRUCTION_SELECTION);
313-
314-
addPhaseOperation((final SourceUnit source, final GeneratorContext context, final ClassNode classNode) -> {
315-
// TODO: Can this be moved into org.codehaus.groovy.transform.sc.transformers.VariableExpressionTransformer?
316-
GroovyClassVisitor visitor = new ClassCodeExpressionTransformer() {
317-
@Override
318-
protected SourceUnit getSourceUnit() {
319-
return source;
320-
}
321-
322-
@Override
323-
public Expression transform(final Expression expression) {
324-
if (expression instanceof VariableExpression) {
325-
// check for "switch(enumType) { case CONST: ... }"
326-
ClassNode enumType = expression.getNodeMetaData(SWITCH_CONDITION_EXPRESSION_TYPE);
327-
if (enumType != null) {
328-
// replace "CONST" variable expression with "EnumType.CONST" property expression
329-
Expression propertyExpression = propX(classX(enumType), expression.getText());
330-
setSourcePosition(propertyExpression, expression);
331-
return propertyExpression;
332-
}
333-
}
334-
return expression;
335-
}
336-
};
337-
visitor.visitClass(classNode);
338-
}, Phases.INSTRUCTION_SELECTION);
339307
}
340308

341309
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
@@ -41,6 +41,9 @@ public class VariableExpressionTransformer {
4141

4242
public Expression transformVariableExpression(final VariableExpression ve) {
4343
Expression xe = tryTransformImplicitReceiver(ve);
44+
if (xe == null) {
45+
xe = tryTransformEnumConstantAccess(ve);
46+
}
4447
if (xe == null) {
4548
xe = tryTransformPrivateFieldAccess(ve);
4649
}
@@ -81,6 +84,19 @@ private static Expression tryTransformImplicitReceiver(final VariableExpression
8184
return pe;
8285
}
8386

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

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

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

26-
import java.lang.reflect.Modifier;
27-
28-
import static org.codehaus.groovy.transform.stc.StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE;
29-
3026
/**
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.
27+
* A type checking extension that will take care of handling errors which are
28+
* specific to enums. In particular, it will handle the enum constants within
29+
* a switch's case statements.
3330
*
3431
* @since 3.0.0
3532
*/
3633
public class EnumTypeCheckingExtension extends TypeCheckingExtension {
37-
public EnumTypeCheckingExtension(StaticTypeCheckingVisitor staticTypeCheckingVisitor) {
34+
35+
public EnumTypeCheckingExtension(final StaticTypeCheckingVisitor staticTypeCheckingVisitor) {
3836
super(staticTypeCheckingVisitor);
3937
}
4038

4139
@Override
42-
public boolean handleUnresolvedVariableExpression(VariableExpression vexp) {
40+
public boolean handleUnresolvedVariableExpression(final VariableExpression vexp) {
4341
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())) {
57-
vexp.putNodeMetaData(SWITCH_CONDITION_EXPRESSION_TYPE, type);
42+
if (switchStatement != null) {
43+
ClassNode type = switchStatement.getExpression().getNodeMetaData(StaticTypesMarker.TYPE);
44+
if (type != null && type.isEnum()) {
45+
FieldNode fieldNode = type.redirect().getField(vexp.getName());
46+
if (fieldNode != null && fieldNode.isEnum()) {
47+
vexp.putNodeMetaData(StaticTypesMarker.SWITCH_CONDITION_EXPRESSION_TYPE, type);
5848
return true;
5949
}
6050
}
6151
}
62-
6352
return false;
6453
}
6554
}

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package groovy.bugs
2020

2121
import groovy.transform.CompileStatic
22+
import org.codehaus.groovy.control.CompilerConfiguration
23+
import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
2224
import org.junit.Test
2325

2426
import static groovy.test.GroovyAssert.assertScript
@@ -29,11 +31,13 @@ final class Groovy8444 {
2931

3032
@Test
3133
void testAccessingEnumConstantInSwitchCase() {
32-
assertScript '''\
34+
def shell = new GroovyShell(new CompilerConfiguration().tap {
35+
addCompilationCustomizers(new ASTTransformationCustomizer(CompileStatic))
36+
})
37+
shell.evaluate '''\
3338
enum SomeEnum {
3439
A, B
3540
}
36-
@groovy.transform.CompileStatic
3741
def meth(SomeEnum e) {
3842
switch (e) {
3943
case A: return 1
@@ -90,7 +94,7 @@ final class Groovy8444 {
9094
def err = shouldFail '''\
9195
enum SomeEnum {
9296
A, B
93-
97+
9498
static final String C = 'C'
9599
}
96100
@groovy.transform.CompileStatic
@@ -111,7 +115,7 @@ final class Groovy8444 {
111115
def err = shouldFail '''\
112116
enum SomeEnum {
113117
A, B
114-
118+
115119
SomeEnum C = A
116120
}
117121
@groovy.transform.CompileStatic
@@ -132,7 +136,7 @@ final class Groovy8444 {
132136
def err = shouldFail '''\
133137
enum SomeEnum {
134138
A, B
135-
139+
136140
static SomeEnum C = A
137141
}
138142
@groovy.transform.CompileStatic
@@ -153,7 +157,7 @@ final class Groovy8444 {
153157
def err = shouldFail '''\
154158
enum SomeEnum {
155159
A, B
156-
160+
157161
static final SomeEnum C = A
158162
}
159163
@groovy.transform.CompileStatic
@@ -178,7 +182,7 @@ final class Groovy8444 {
178182
@groovy.transform.CompileStatic
179183
def meth(SomeEnum e) {
180184
switch (e) {
181-
case A:
185+
case A:
182186
switch(e) {
183187
case A: return 1.1
184188
case B: return 1.2
@@ -207,7 +211,7 @@ final class Groovy8444 {
207211
@groovy.transform.CompileStatic
208212
def meth(SomeEnum e, OtherEnum e2) {
209213
switch (e) {
210-
case A:
214+
case A:
211215
switch(e2) {
212216
case C: return 1.1
213217
case D: return 1.2
@@ -225,6 +229,4 @@ final class Groovy8444 {
225229
assert 2.2 == meth(SomeEnum.B, OtherEnum.D)
226230
'''
227231
}
228-
229-
230232
}

0 commit comments

Comments
 (0)