Skip to content

Commit

Permalink
Fix for #1004: remove modifier synthetic from enum next() and previous()
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Dec 13, 2019
1 parent d7d53ec commit d37f2c2
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 62 deletions.
4 changes: 2 additions & 2 deletions base/org.codehaus.groovy24/src/org/codehaus/groovy/classgen/EnumVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token ge = Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, -1);
MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC/*GRECLIPSE | Opcodes.ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
Expand Down Expand Up @@ -209,7 +209,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token lt = Token.newSymbol(Types.COMPARE_LESS_THAN, -1, -1);
MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC/*GRECLIPSE | Opcodes.ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
Expand Down
4 changes: 2 additions & 2 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/EnumVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token ge = Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, -1);
MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC/*GRECLIPSE | Opcodes.ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
Expand Down Expand Up @@ -211,7 +211,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token lt = Token.newSymbol(Types.COMPARE_LESS_THAN, -1, -1);
MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC/*GRECLIPSE | Opcodes.ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
Expand Down
118 changes: 60 additions & 58 deletions base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/EnumVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,69 +56,71 @@
import org.codehaus.groovy.syntax.PreciseSyntaxException;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.Types;
import groovyjarjarasm.asm.Opcodes;

import java.util.ArrayList;
import java.util.List;

import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
import static groovyjarjarasm.asm.Opcodes.ACC_ABSTRACT;
import static groovyjarjarasm.asm.Opcodes.ACC_FINAL;
import static groovyjarjarasm.asm.Opcodes.ACC_PRIVATE;
import static groovyjarjarasm.asm.Opcodes.ACC_PUBLIC;
import static groovyjarjarasm.asm.Opcodes.ACC_STATIC;
import static groovyjarjarasm.asm.Opcodes.ACC_SYNTHETIC;

public class EnumVisitor extends ClassCodeVisitorSupport {
// some constants for modifiers
private static final int FS = Opcodes.ACC_FINAL | Opcodes.ACC_STATIC;
private static final int PS = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC;
private static final int PUBLIC_FS = Opcodes.ACC_PUBLIC | FS;
private static final int PRIVATE_FS = Opcodes.ACC_PRIVATE | FS;

private final SourceUnit sourceUnit;

public EnumVisitor(CompilationUnit cu, SourceUnit su) {
public EnumVisitor(final CompilationUnit cu, final SourceUnit su) {
sourceUnit = su;
}

public void visitClass(ClassNode node) {
if (!node.isEnum()) return;
completeEnum(node);
}

@Override
protected SourceUnit getSourceUnit() {
return sourceUnit;
}

private void completeEnum(ClassNode enumClass) {
boolean isAic = isAnonymousInnerClass(enumClass);
// create MIN_VALUE and MAX_VALUE fields
@Override
public void visitClass(final ClassNode node) {
if (!node.isEnum()) return;
completeEnum(node);
}

private void completeEnum(final ClassNode enumClass) {
FieldNode minValue = null, maxValue = null, values = null;

boolean isAic = isAnonymousInnerClass(enumClass);
if (!isAic) {
ClassNode enumRef = enumClass.getPlainNodeReference();

// create values field
values = new FieldNode("$VALUES", PRIVATE_FS | Opcodes.ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
// create $VALUES field
values = new FieldNode("$VALUES", ACC_FINAL | ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
values.setSynthetic(true);

addMethods(enumClass, values);
checkForAbstractMethods(enumClass);

// create MIN_VALUE and MAX_VALUE fields
minValue = new FieldNode("MIN_VALUE", PUBLIC_FS, enumRef, enumClass, null);
maxValue = new FieldNode("MAX_VALUE", PUBLIC_FS, enumRef, enumClass, null);
minValue = new FieldNode("MIN_VALUE", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef, enumClass, null);
maxValue = new FieldNode("MAX_VALUE", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef, enumClass, null);
}

addInit(enumClass, minValue, maxValue, values, isAic);
}

private static void checkForAbstractMethods(ClassNode enumClass) {
List<MethodNode> methods = enumClass.getMethods();
for (MethodNode m : methods) {
if (m.isAbstract()) {
// make the class abstract also see Effective Java p.152
enumClass.setModifiers(enumClass.getModifiers() | Opcodes.ACC_ABSTRACT);
private static void checkForAbstractMethods(final ClassNode enumClass) {
for (MethodNode method : enumClass.getMethods()) {
if (method.isAbstract()) {
// make the class abstract also; see Effective Java p.152
enumClass.setModifiers(enumClass.getModifiers() | ACC_ABSTRACT);
break;
}
}
}

private static void addMethods(ClassNode enumClass, FieldNode values) {
private static void addMethods(final ClassNode enumClass, final FieldNode values) {
List<MethodNode> methods = enumClass.getMethods();

boolean hasNext = false;
Expand All @@ -133,14 +135,14 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {

{
// create values() method
MethodNode valuesMethod = new MethodNode("values", PUBLIC_FS, enumRef.makeArray(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode valuesMethod = new MethodNode("values", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef.makeArray(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
valuesMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
MethodCallExpression cloneCall = new MethodCallExpression(new FieldExpression(values), "clone", MethodCallExpression.NO_ARGUMENTS);
cloneCall.setMethodTarget(values.getType().getMethod("clone", Parameter.EMPTY_ARRAY));
code.addStatement(new ReturnStatement(cloneCall));
valuesMethod.setCode(code);
enumClass.addMethod(valuesMethod);
addGeneratedMethod(enumClass, valuesMethod);
}

if (!hasNext) {
Expand All @@ -152,7 +154,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token ge = Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, -1);
MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
MethodNode nextMethod = new MethodNode("next", ACC_PUBLIC/*GRECLIPSE | ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
Expand Down Expand Up @@ -199,7 +201,7 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
)
);
nextMethod.setCode(code);
enumClass.addMethod(nextMethod);
addGeneratedMethod(enumClass, nextMethod);
}

if (!hasPrevious) {
Expand All @@ -211,8 +213,8 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
// }
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
Token lt = Token.newSymbol(Types.COMPARE_LESS_THAN, -1, -1);
MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
nextMethod.setSynthetic(true);
MethodNode prevMethod = new MethodNode("previous", ACC_PUBLIC/*GRECLIPSE | ACC_SYNTHETIC*/, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
prevMethod.setSynthetic(true);
BlockStatement code = new BlockStatement();
BlockStatement ifStatement = new BlockStatement();
ifStatement.addStatement(
Expand Down Expand Up @@ -263,14 +265,14 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
new MethodCallExpression(new FieldExpression(values), "getAt", new VariableExpression("ordinal"))
)
);
nextMethod.setCode(code);
enumClass.addMethod(nextMethod);
prevMethod.setCode(code);
addGeneratedMethod(enumClass, prevMethod);
}

{
// create valueOf
Parameter stringParameter = new Parameter(ClassHelper.STRING_TYPE, "name");
MethodNode valueOfMethod = new MethodNode("valueOf", PS, enumRef, new Parameter[]{stringParameter}, ClassNode.EMPTY_ARRAY, null);
MethodNode valueOfMethod = new MethodNode("valueOf", ACC_PUBLIC | ACC_STATIC, enumRef, new Parameter[]{stringParameter}, ClassNode.EMPTY_ARRAY, null);
ArgumentListExpression callArguments = new ArgumentListExpression();
callArguments.addExpression(new ClassExpression(enumClass));
callArguments.addExpression(new VariableExpression("name"));
Expand All @@ -283,27 +285,25 @@ private static void addMethods(ClassNode enumClass, FieldNode values) {
);
valueOfMethod.setCode(code);
valueOfMethod.setSynthetic(true);
enumClass.addMethod(valueOfMethod);
addGeneratedMethod(enumClass, valueOfMethod);
}
}

private void addInit(ClassNode enumClass, FieldNode minValue,
FieldNode maxValue, FieldNode values,
boolean isAic) {
private void addInit(final ClassNode enumClass, final FieldNode minValue, final FieldNode maxValue, final FieldNode values, final boolean isAic) {
// constructor helper
// This method is used instead of calling the constructor as
// calling the constructor may require a table with MetaClass
// selecting the constructor for each enum value. So instead we
// use this method to have a central point for constructor selection
// and only one table. The whole construction is needed because
// and only one table. The whole construction is needed because
// Reflection forbids access to the enum constructor.
// code:
// def $INIT(Object[] para) {
// return this(*para)
// }
ClassNode enumRef = enumClass.getPlainNodeReference();
Parameter[] parameter = new Parameter[]{new Parameter(ClassHelper.OBJECT_TYPE.makeArray(), "para")};
MethodNode initMethod = new MethodNode("$INIT", PUBLIC_FS | Opcodes.ACC_SYNTHETIC, enumRef, parameter, ClassNode.EMPTY_ARRAY, null);
MethodNode initMethod = new MethodNode("$INIT", ACC_FINAL | ACC_PUBLIC | ACC_STATIC | ACC_SYNTHETIC, enumRef, parameter, ClassNode.EMPTY_ARRAY, null);
initMethod.setSynthetic(true);
ConstructorCallExpression cce = new ConstructorCallExpression(
ClassNode.THIS,
Expand All @@ -314,19 +314,19 @@ private void addInit(ClassNode enumClass, FieldNode minValue,
BlockStatement code = new BlockStatement();
code.addStatement(new ReturnStatement(cce));
initMethod.setCode(code);
enumClass.addMethod(initMethod);
addGeneratedMethod(enumClass, initMethod);

// static init
List<FieldNode> fields = enumClass.getFields();
List<Expression> arrayInit = new ArrayList<Expression>();
List<Expression> arrayInit = new ArrayList<>();
int value = -1;
Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
List<Statement> block = new ArrayList<Statement>();
List<Statement> block = new ArrayList<>();
FieldNode tempMin = null;
FieldNode tempMax = null;
for (FieldNode field : fields) {
if ((field.getModifiers() & Opcodes.ACC_ENUM) == 0) continue;
value++;
if (!field.isEnum()) continue;
value += 1;
if (tempMin == null) tempMin = field;
tempMax = field;

Expand All @@ -335,13 +335,13 @@ private void addInit(ClassNode enumClass, FieldNode minValue,
args.addExpression(new ConstantExpression(field.getName()));
args.addExpression(new ConstantExpression(value));
if (field.getInitialExpression() == null) {
if ((enumClass.getModifiers() & Opcodes.ACC_ABSTRACT) != 0) {
if (enumClass.isAbstract()) {
addError(field, "The enum constant " + field.getName() + " must override abstract methods from " + enumBase.getName() + ".");
continue;
}
} else {
ListExpression oldArgs = (ListExpression) field.getInitialExpression();
List<MapEntryExpression> savedMapEntries = new ArrayList<MapEntryExpression>();
List<MapEntryExpression> savedMapEntries = new ArrayList<>();
for (Expression exp : oldArgs.getExpressions()) {
if (exp instanceof MapEntryExpression) {
savedMapEntries.add((MapEntryExpression) exp);
Expand All @@ -361,7 +361,7 @@ private void addInit(ClassNode enumClass, FieldNode minValue,
for (MethodNode methodNode : baseMethods) {
if (!methodNode.isAbstract()) continue;
MethodNode enumConstMethod = inner.getMethod(methodNode.getName(), methodNode.getParameters());
if (enumConstMethod == null || (enumConstMethod.getModifiers() & Opcodes.ACC_ABSTRACT) != 0) {
if (enumConstMethod == null || enumConstMethod.isAbstract()) {
addError(field, "Can't have an abstract method in enum constant " + field.getName() + ". Implement method '" + methodNode.getTypeDescriptor() + "'.");
}
}
Expand All @@ -372,7 +372,7 @@ private void addInit(ClassNode enumClass, FieldNode minValue,
* so that subclasses of enum generated for enum constants (aic) can provide
* their own $INIT method
*/
initMethod.setModifiers(initMethod.getModifiers() & ~Opcodes.ACC_FINAL);
initMethod.setModifiers(initMethod.getModifiers() & ~ACC_FINAL);
continue;
}
}
Expand Down Expand Up @@ -429,7 +429,7 @@ private void addInit(ClassNode enumClass, FieldNode minValue,
enumClass.addStaticInitializerStatements(block, true);
}

private void addError(AnnotatedNode exp, String msg) {
private void addError(final AnnotatedNode exp, final String msg) {
// GRECLIPSE add
int start = exp.getStart(), end = exp.getEnd();
if (exp instanceof FieldNode && ((FieldNode) exp).hasInitialExpression()) {
Expand All @@ -438,18 +438,20 @@ private void addError(AnnotatedNode exp, String msg) {
end = type.getStart() - 2; // cover arguments but not start of block
}
// GRECLIPSE end
sourceUnit.getErrorCollector().addErrorAndContinue(
getSourceUnit().getErrorCollector().addErrorAndContinue(
new SyntaxErrorMessage(
// GRECLIPSE edit
//new SyntaxException(msg + '\n', exp.getLineNumber(), exp.getColumnNumber(), exp.getLastLineNumber(), exp.getLastColumnNumber()), sourceUnit)
new PreciseSyntaxException(msg + '\n', exp.getLineNumber(), exp.getColumnNumber(), start, end), sourceUnit)
/* GRECLIPSE edit
new SyntaxException(msg + '\n', exp),
*/
new PreciseSyntaxException(msg + '\n', exp.getLineNumber(), exp.getColumnNumber(), start, end),
// GRECLIPSE end
getSourceUnit()
)
);
}

private static boolean isAnonymousInnerClass(ClassNode enumClass) {
private static boolean isAnonymousInnerClass(final ClassNode enumClass) {
if (!(enumClass instanceof EnumConstantClassNode)) return false;
InnerClassNode ic = (InnerClassNode) enumClass;
return ic.getVariableScope() == null;
return (((EnumConstantClassNode) enumClass).getVariableScope() == null);
}
}
31 changes: 31 additions & 0 deletions ...eassist.test/src/org/codehaus/groovy/eclipse/codeassist/tests/OtherCompletionTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,37 @@ final class OtherCompletionTests extends CompletionTestSuite {
checkReplacementString(proposals, 'xx', 1)
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1004
void testEnumCompletion1() {
String contents = 'enum E {F,G}\nE.F.n'
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'n'))
checkReplacementString(proposals, 'next()', 2)
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1004
void testEnumCompletion2() {
String contents = 'enum E {F,G}\nE.F.p'
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'p'))
checkReplacementString(proposals, 'previous()', 2)
}

@Test
void testEnumCompletion3() {
String contents = 'enum E {F,G}\nE.v'
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'v'))
checkReplacementString(proposals, 'values()', 1)
checkReplacementString(proposals, 'valueOf(name)', 1)
checkReplacementString(proposals, 'valueOf(enumType, name)', 1)
}

@Test
void testEnumCompletion4() {
String contents = 'enum E {F,G}\nE.M'
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'M'))
checkReplacementString(proposals, 'MIN_VALUE', 1)
checkReplacementString(proposals, 'MAX_VALUE', 1)
}

@Test
void testListCompletion1() {
String contents = '[].'
Expand Down

0 comments on commit d37f2c2

Please sign in to comment.