Skip to content

Commit

Permalink
GROOVY-8991: Difference in behaviour with closure and lambda (#49)
Browse files Browse the repository at this point in the history
* GROOVY-8991: Difference in behaviour with closure and lambda

* Minor refactoring
  • Loading branch information
daniellansun authored Feb 19, 2019
1 parent f3079c1 commit 6fa4561
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 17 deletions.
16 changes: 11 additions & 5 deletions src/main/antlr/GroovyParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ options { baseContext = standardLambdaExpression; }
: lambdaParameters nls ARROW nls lambdaBody
;

// JAVA STANDARD LAMBDA EXPRESSION
standardLambdaExpression
: standardLambdaParameters nls ARROW nls lambdaBody
;
Expand All @@ -503,12 +504,17 @@ lambdaBody
| statementExpression
;


// CLOSURE
closure
: LBRACE nls (formalParameterList? nls ARROW nls)? blockStatementsOpt RBRACE
;

// GROOVY-8991: Difference in behaviour with closure and lambda
closureOrLambdaExpression
: closure
| lambdaExpression
;

blockStatementsOpt
: blockStatements?
;
Expand Down Expand Up @@ -933,7 +939,8 @@ commandArgument
* (Compare to a C lvalue, or LeftHandSide in the JLS section 15.26.)
* General expressions are built up from path expressions, using operators like '+' and '='.
*
* t 0: primary, 1: namePart, 2: arguments, 3: closure, 4: indexPropertyArgs, 5: namedPropertyArgs, 6: non-static inner class creator
* t 0: primary, 1: namePart, 2: arguments, 3: closureOrLambdaExpression, 4: indexPropertyArgs, 5: namedPropertyArgs,
* 6: non-static inner class creator
*/
pathExpression returns [int t]
: primary (pathElement { $t = $pathElement.t; })*
Expand Down Expand Up @@ -963,7 +970,7 @@ pathElement returns [int t]
{ $t = 2; }

// Can always append a block, as foo{bar}
| nls closure
| nls closureOrLambdaExpression
{ $t = 3; }

// Element selection is always an option, too.
Expand Down Expand Up @@ -1033,8 +1040,7 @@ primary
| THIS #thisPrmrAlt
| SUPER #superPrmrAlt
| parExpression #parenPrmrAlt
| closure #closurePrmrAlt
| lambdaExpression #lambdaPrmrAlt
| closureOrLambdaExpression #closureOrLambdaExpressionPrmrAlt
| list #listPrmrAlt
| map #mapPrmrAlt
| builtInType #builtInTypePrmrAlt
Expand Down
36 changes: 24 additions & 12 deletions src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClassicalForControlContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClassifiedModifiersContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClosureContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClosurePrmrAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClosureOrLambdaExpressionContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ClosureOrLambdaExpressionPrmrAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.CommandArgumentContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.CommandExprAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.CommandExpressionContext;
Expand Down Expand Up @@ -250,7 +251,6 @@
import static org.apache.groovy.parser.antlr4.GroovyLangParser.LT;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.LabeledStmtAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.LambdaBodyContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.LambdaPrmrAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ListContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.ListPrmrAltContext;
import static org.apache.groovy.parser.antlr4.GroovyLangParser.LiteralPrmrAltContext;
Expand Down Expand Up @@ -2454,8 +2454,8 @@ public Expression visitPathElement(PathElementContext ctx) {

// e.g. 1(), 1.1(), ((int) 1 / 2)(1, 2), {a, b -> a + b }(1, 2), m()()
return configureAST(createCallMethodCallExpression(baseExpr, argumentsExpr), ctx);
} else if (asBoolean(ctx.closure())) {
ClosureExpression closureExpression = this.visitClosure(ctx.closure());
} else if (asBoolean(ctx.closureOrLambdaExpression())) {
ClosureExpression closureExpression = this.visitClosureOrLambdaExpression(ctx.closureOrLambdaExpression());

if (baseExpr instanceof MethodCallExpression) {
MethodCallExpression methodCallExpression = (MethodCallExpression) baseExpr;
Expand Down Expand Up @@ -3160,13 +3160,8 @@ public Expression visitParenPrmrAlt(ParenPrmrAltContext ctx) {
}

@Override
public ClosureExpression visitClosurePrmrAlt(ClosurePrmrAltContext ctx) {
return configureAST(this.visitClosure(ctx.closure()), ctx);
}

@Override
public ClosureExpression visitLambdaPrmrAlt(LambdaPrmrAltContext ctx) {
return configureAST(this.visitStandardLambdaExpression(ctx.standardLambdaExpression()), ctx);
public ClosureExpression visitClosureOrLambdaExpressionPrmrAlt(ClosureOrLambdaExpressionPrmrAltContext ctx) {
return configureAST(this.visitClosureOrLambdaExpression(ctx.closureOrLambdaExpression()), ctx);
}

@Override
Expand Down Expand Up @@ -3589,7 +3584,7 @@ public GStringExpression visitGstring(GstringContext ctx) {
.map(e -> {
Expression expression = this.visitGstringValue(e);

if (expression instanceof ClosureExpression && !asBoolean(e.closure().ARROW())) {
if (expression instanceof ClosureExpression && !hasArrow(e)) {
List<Statement> statementList = ((BlockStatement) ((ClosureExpression) expression).getCode()).getStatements();

if (statementList.stream().noneMatch(DefaultGroovyMethods::asBoolean)) {
Expand Down Expand Up @@ -3623,6 +3618,10 @@ public GStringExpression visitGstring(GstringContext ctx) {
return configureAST(new GStringExpression(verbatimText.toString(), stringLiteralList, values), ctx);
}

private boolean hasArrow(GstringValueContext e) {
return asBoolean(e.closure().ARROW());
}

private String parseGStringEnd(GstringContext ctx, String beginQuotation) {
StringBuilder text = new StringBuilder(ctx.GStringEnd().getText());
text.insert(0, beginQuotation);
Expand Down Expand Up @@ -4083,6 +4082,19 @@ public TupleExpression visitVariableNames(VariableNamesContext ctx) {
ctx);
}

@Override
public ClosureExpression visitClosureOrLambdaExpression(ClosureOrLambdaExpressionContext ctx) {
// GROOVY-8991: Difference in behaviour with closure and lambda
if (asBoolean(ctx.closure())) {
return configureAST(this.visitClosure(ctx.closure()), ctx);
} else if (asBoolean(ctx.standardLambdaExpression())) {
return configureAST(this.visitStandardLambdaExpression(ctx.standardLambdaExpression()), ctx);
}

// should never reach here
throw createParsingFailedException("The node is not expected here" + ctx.getText(), ctx);
}

@Override
public BlockStatement visitBlockStatementsOpt(BlockStatementsOptContext ctx) {
if (asBoolean(ctx.blockStatements())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,5 +432,6 @@ class GroovyParserTest extends GroovyTestCase {
doRunAndTest('bugs/BUG-GROOVY-8171.groovy');
doTest('bugs/BUG-GROOVY-8641.groovy');
doTest('bugs/BUG-GROOVY-8913.groovy');
doRunAndTestAntlr4('bugs/BUG-GROOVY-8991.groovy');
}
}
68 changes: 68 additions & 0 deletions src/test/resources/bugs/BUG-GROOVY-8991.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

class BarHolder {
def foo2(lambda) {
lambda()
}
def bar(lambda) {
lambda() + 1
}
def bar2(lambda) {
lambda(6) + 1
}
}

def foo(lambda) {
lambda()
}

def result =
foo () -> {
new BarHolder()
}
.bar () -> {
2
}
assert 3 == result

def result2 =
foo () -> {
new BarHolder()
}
.bar2 (e) -> {
2 + e
}
assert 9 == result2

def result3 =
foo () -> {
new BarHolder()
}
.foo2 () -> {
new BarHolder()
}
.bar () -> {
2
}
assert 3 == result3

def foo5(c) {c()}
def c2 = foo5 { { Integer x -> 1} }
assert 1 == c2()

0 comments on commit 6fa4561

Please sign in to comment.