Skip to content

Commit cfe0353

Browse files
eric-milleseric-milles
committed
GROOVY-11492, GROOVY-11845: curly brace array for attribute default
1 parent 5597fb6 commit cfe0353

File tree

4 files changed

+98
-72
lines changed

4 files changed

+98
-72
lines changed

‎src/antlr/GroovyParser.g4‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ elementValue
543543

544544
elementValueArrayInitializer
545545
: LBRACK (elementValue (COMMA elementValue)* COMMA?)? RBRACK
546+
| LBRACE (elementValue COMMA)+ elementValue? RBRACE // avoid ambiguity with closure
546547
;
547548

548549
// STATEMENTS / BLOCKS

‎src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,8 @@
159159
import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
160160
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
161161
import static org.codehaus.groovy.ast.tools.GeneralUtils.closureX;
162-
import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
163162
import static org.codehaus.groovy.ast.tools.GeneralUtils.declX;
164163
import static org.codehaus.groovy.ast.tools.GeneralUtils.listX;
165-
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
166164
import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
167165
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
168166
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@@ -1820,19 +1818,24 @@ private MethodNode createConstructorOrMethodNodeForClass(final MethodDeclaration
18201818
}
18211819

18221820
private MethodNode createMethodNodeForClass(final MethodDeclarationContext ctx, final ModifierManager modifierManager, final String methodName, final ClassNode returnType, final Parameter[] parameters, final ClassNode[] exceptions, Statement code, final ClassNode classNode, int modifiers) {
1823-
if (asBoolean(ctx.elementValue())) { // the code of annotation method
1824-
code = configureAST(
1825-
new ExpressionStatement(
1826-
this.visitElementValue(ctx.elementValue())),
1827-
ctx.elementValue());
1821+
ElementValueContext elementValue = ctx.elementValue();
1822+
if (asBoolean(elementValue)) {
1823+
assert this.isAnnotationDeclaration(classNode);
1824+
Expression defaultValue = this.visitElementValue(elementValue);
1825+
if (returnType.isArray() && defaultValue instanceof ClosureExpression closure // GROOVY-11492
1826+
&& ClosureUtils.hasImplicitParameter(closure) && closure.getCode() instanceof BlockStatement block) {
1827+
defaultValue = listX(block.getStatements().stream().map(s -> ((ExpressionStatement) s).getExpression()).toList());
1828+
}
1829+
code = configureAST(stmt(defaultValue), elementValue);
1830+
}
18281831

1832+
if (classNode.isInterface() && !modifierManager.containsAny(STATIC) && !(isTrue(classNode, IS_INTERFACE_WITH_DEFAULT_METHODS) && modifierManager.containsAny(DEFAULT, PRIVATE))) {
1833+
modifiers |= Opcodes.ACC_ABSTRACT;
18291834
}
18301835

1831-
modifiers |= !modifierManager.containsAny(STATIC) && classNode.isInterface() && !(isTrue(classNode, IS_INTERFACE_WITH_DEFAULT_METHODS) && modifierManager.containsAny(DEFAULT, PRIVATE)) ? Opcodes.ACC_ABSTRACT : 0;
18321836
MethodNode methodNode = new MethodNode(methodName, modifiers, returnType, parameters, exceptions, code);
1837+
methodNode.setAnnotationDefault(asBoolean(elementValue));
18331838
classNode.addMethod(methodNode);
1834-
1835-
methodNode.setAnnotationDefault(asBoolean(ctx.elementValue()));
18361839
return methodNode;
18371840
}
18381841

@@ -2246,34 +2249,19 @@ public Expression visitCommandExpression(final CommandExpressionContext ctx) {
22462249
* ,it may be a command expression
22472250
*/
22482251
private void validateInvalidMethodDefinition(final Expression baseExpr, final Expression arguments) {
2249-
if (baseExpr instanceof VariableExpression) {
2250-
if (isBuiltInType(baseExpr) || Character.isUpperCase(baseExpr.getText().codePointAt(0))) {
2251-
if (arguments instanceof ArgumentListExpression) {
2252-
List<Expression> expressionList = ((ArgumentListExpression) arguments).getExpressions();
2253-
if (1 == expressionList.size()) {
2254-
final Expression expression = expressionList.get(0);
2255-
if (expression instanceof MethodCallExpression mce) {
2256-
final Expression methodCallArguments = mce.getArguments();
2257-
2258-
// check the method call tails with a closure
2259-
if (methodCallArguments instanceof ArgumentListExpression) {
2260-
List<Expression> methodCallArgumentExpressionList = ((ArgumentListExpression) methodCallArguments).getExpressions();
2261-
final int argumentCnt = methodCallArgumentExpressionList.size();
2262-
if (argumentCnt > 0) {
2263-
final Expression lastArgumentExpression = methodCallArgumentExpressionList.get(argumentCnt - 1);
2264-
if (lastArgumentExpression instanceof ClosureExpression) {
2265-
if (ClosureUtils.hasImplicitParameter(((ClosureExpression) lastArgumentExpression))) {
2266-
throw createParsingFailedException(
2267-
"Method definition not expected here",
2268-
tuple(baseExpr.getLineNumber(), baseExpr.getColumnNumber()),
2269-
tuple(expression.getLastLineNumber(), expression.getLastColumnNumber())
2270-
);
2271-
}
2272-
}
2273-
}
2274-
}
2275-
}
2276-
}
2252+
if (baseExpr instanceof VariableExpression && arguments instanceof ArgumentListExpression argsList
2253+
&& (isBuiltInType(baseExpr) || Character.isUpperCase(baseExpr.getText().codePointAt(0)))) {
2254+
List<Expression> exprList = argsList.getExpressions();
2255+
if (exprList.size() == 1
2256+
&& exprList.get(0) instanceof MethodCallExpression callExpr
2257+
&& callExpr.getArguments() instanceof ArgumentListExpression callArgs) {
2258+
exprList = callArgs.getExpressions(); // check the method call tails with a closure
2259+
if (!exprList.isEmpty() && last(exprList) instanceof ClosureExpression closure && ClosureUtils.hasImplicitParameter(closure)) {
2260+
throw createParsingFailedException(
2261+
"Method definition not expected here",
2262+
tuple(baseExpr.getLineNumber(), baseExpr.getColumnNumber()),
2263+
tuple(callExpr.getLastLineNumber(), callExpr.getLastColumnNumber())
2264+
);
22772265
}
22782266
}
22792267
}
@@ -4226,7 +4214,7 @@ public Expression visitElementValue(final ElementValueContext ctx) {
42264214

42274215
@Override
42284216
public ListExpression visitElementValueArrayInitializer(final ElementValueArrayInitializerContext ctx) {
4229-
return configureAST(new ListExpression(ctx.elementValue().stream().map(this::visitElementValue).collect(Collectors.toList())), ctx);
4217+
return configureAST(listX(ctx.elementValue().stream().map(this::visitElementValue).toList()), ctx);
42304218
}
42314219

42324220
@Override

‎src/test-resources/core/AnnotationDeclaration_01.groovy‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import org.codehaus.groovy.transform.GroovyASTTransformationClass
20-
19+
import org.codehaus.groovy.transform.*
2120
import java.lang.annotation.Documented
2221
import java.lang.annotation.ElementType
2322
import java.lang.annotation.Retention
@@ -58,4 +57,11 @@ import java.lang.annotation.Target
5857

5958
@interface C {
6059
String name() default ''
61-
}
60+
}
61+
62+
//GROOVY-11492
63+
@interface D {
64+
String[] zero() default { }
65+
String[] one () default { "foo" }
66+
String[] two () default { "foo", "bar" }
67+
}

‎src/test/groovy/gls/annotations/AnnotationTest.groovy‎

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
package gls.annotations
2020

2121
import groovy.test.NotYetImplemented
22-
import org.junit.Test
22+
import org.junit.jupiter.api.Test
23+
import org.junit.jupiter.params.ParameterizedTest
24+
import org.junit.jupiter.params.provider.ValueSource
2325

2426
import static groovy.test.GroovyAssert.assertScript
2527
import static groovy.test.GroovyAssert.shouldFail
@@ -73,7 +75,7 @@ final class AnnotationTest {
7375
*/
7476
@Test
7577
void testOmittingBracketsForSingleValueArrayParameter() {
76-
shell.parse '''
78+
assertScript shell, '''
7779
import gls.annotations.*
7880
7981
class Book {}
@@ -97,7 +99,7 @@ final class AnnotationTest {
9799
// If this should be changed, then further discussion is needed.
98100
shouldFail shell, '''
99101
@interface X {
100-
int x() default "1" // must be integer
102+
int x() default '1' // must be integer
101103
}
102104
'''
103105

@@ -138,46 +140,75 @@ final class AnnotationTest {
138140
assertScript shell, '''
139141
@Retention(RetentionPolicy.RUNTIME)
140142
@Target(ElementType.TYPE)
141-
@interface Temp {
142-
String[] bar() default '1' // coerced to list as per Java but must be correct type
143+
@interface A {
144+
String[] values() default '1' // coerced to list as per Java but must be correct type
143145
}
144146
145-
@Temp
146-
class Bar {}
147-
148-
assert Bar.getAnnotation(Temp).bar() == ['1']
147+
@A class B { }
148+
String[] array = ['1']
149+
assert B.getAnnotation(A).values() == array
149150
'''
150151

151-
shouldFail shell, '''
152-
@interface X {
153-
String[] x() default ["1",2] // list must contain elements of correct type
152+
def err = shouldFail shell, '''
153+
@interface A {
154+
String[] values() default ['1',2] // list must contain elements of correct type
154155
}
155156
'''
157+
assert err =~ /Attribute 'values' should have type 'java.lang.String'; but found type 'int' in @A/
158+
}
156159

157-
shell.parse '''
158-
@interface X {
159-
String[] x() default ["a","b"]
160+
// GROOVY-11492
161+
@ParameterizedTest
162+
@ValueSource(strings=['','"a"','"a","b"','"a","b",'])
163+
void testArrayDefault2(String values) {
164+
assertScript shell, """
165+
@Retention(RetentionPolicy.RUNTIME)
166+
@Target(ElementType.TYPE)
167+
@interface A {
168+
String[] values() default [$values]
160169
}
161-
'''
170+
171+
@A class B { }
172+
String[] array = [$values]
173+
assert B.getAnnotation(A).values() == array
174+
"""
175+
176+
assertScript shell, """
177+
@Retention(RetentionPolicy.RUNTIME)
178+
@Target(ElementType.TYPE)
179+
@interface A {
180+
String[] values() default {$values}
181+
}
182+
183+
@A class B { }
184+
String[] array = [$values]
185+
assert B.getAnnotation(A).values() == array
186+
"""
162187
}
163188

164189
@Test
165190
void testClassDefault() {
166-
shouldFail shell, '''
167-
@interface X {
168-
Class x() default "1" // must be list
191+
shell.parse '''
192+
@interface A {
193+
Class c() default Number
169194
}
170195
'''
171196

172197
shell.parse '''
173-
@interface X {
174-
Class x() default Number.class // class with .class
198+
@interface A {
199+
Class c() default Number.class
175200
}
176201
'''
177202

178203
shell.parse '''
179-
@interface X {
180-
Class x() default Number
204+
@interface A {
205+
Class c() default { -> } // closure
206+
}
207+
'''
208+
209+
shouldFail shell, '''
210+
@interface A {
211+
Class c() default '1' // must be class
181212
}
182213
'''
183214
}
@@ -186,7 +217,7 @@ final class AnnotationTest {
186217
void testEnumDefault() {
187218
shouldFail shell, '''
188219
@interface X {
189-
ElementType x() default "1" // must be Enum
220+
ElementType x() default '1' // must be Enum
190221
}
191222
'''
192223

@@ -207,7 +238,7 @@ final class AnnotationTest {
207238
void testAnnotationDefault() {
208239
shouldFail shell, '''
209240
@interface X {
210-
Target x() default "1" // must be Annotation
241+
Target x() default '1' // must be Annotation
211242
}
212243
'''
213244

@@ -250,7 +281,7 @@ final class AnnotationTest {
250281
void testParameter() {
251282
shouldFail shell, '''
252283
@interface X {
253-
String x(int x) default "" // annotation members can't have parameters
284+
String x(int x) default '' // annotation members can't have parameters
254285
}
255286
'''
256287
}
@@ -300,22 +331,22 @@ final class AnnotationTest {
300331
String stringValue()
301332
int intValue()
302333
int defaultInt() default 1
303-
String defaultString() default ""
334+
String defaultString() default ''
304335
Class defaultClass() default Integer.class
305336
ElementType defaultEnum() default ElementType.TYPE
306337
Target defaultAnnotation() default @Target([ElementType.TYPE])
307338
}
308339
309-
@MyAnnotation(stringValue = "for class", intValue = 100)
340+
@MyAnnotation(stringValue = 'for class', intValue = 100)
310341
class Foo {}
311342
312343
Annotation[] annotations = Foo.class.annotations
313344
assert annotations.size() == 1
314345
MyAnnotation my = annotations[0]
315-
assert my.stringValue() == "for class"
346+
assert my.stringValue() == 'for class'
316347
assert my.intValue() == 100
317348
assert my.defaultInt() == 1
318-
assert my.defaultString() == ""
349+
assert my.defaultString() == ''
319350
assert my.defaultClass() == Integer
320351
321352
assert my.defaultEnum() == ElementType.TYPE

0 commit comments

Comments
 (0)