You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2021/07/17 07:02:57 UTC
[groovy] branch master updated: GROOVY-9272: Tweak switch expression
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 5372011 GROOVY-9272: Tweak switch expression
5372011 is described below
commit 5372011510522bf5fb7f5e5277686e00a590f0f8
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Jul 17 14:34:15 2021 +0800
GROOVY-9272: Tweak switch expression
---
.../apache/groovy/parser/antlr4/AstBuilder.java | 142 +++++++++++++--------
.../core/SwitchExpression_25x.groovy | 31 +++++
.../core/SwitchExpression_26x.groovy | 33 +++++
.../fail/SwitchExpression_09x.groovy | 24 ++++
.../groovy/parser/antlr4/GroovyParserTest.groovy | 2 +
.../groovy/parser/antlr4/SyntaxErrorTest.groovy | 1 +
6 files changed, 178 insertions(+), 55 deletions(-)
diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index eea1e49..2f53780 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -641,7 +641,13 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
@Override
public Statement visitLoopStmtAlt(final LoopStmtAltContext ctx) {
visitingLoopStatementCount += 1;
- Statement result = configureAST((Statement) this.visit(ctx.loopStatement()), ctx);
+ switchExpressionRuleContextStack.push(ctx);
+ Statement result;
+ try {
+ result = configureAST((Statement) this.visit(ctx.loopStatement()), ctx);
+ } finally {
+ switchExpressionRuleContextStack.pop();
+ }
visitingLoopStatementCount -= 1;
return result;
@@ -906,43 +912,49 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
@Override
public SwitchStatement visitSwitchStatement(final SwitchStatementContext ctx) {
visitingSwitchStatementCount += 1;
+ switchExpressionRuleContextStack.push(ctx);
- List<Statement> statementList =
- ctx.switchBlockStatementGroup().stream()
- .map(this::visitSwitchBlockStatementGroup)
- .reduce(new LinkedList<>(), (r, e) -> {
- r.addAll(e);
- return r;
- });
+ SwitchStatement result;
+ try {
+ List<Statement> statementList =
+ ctx.switchBlockStatementGroup().stream()
+ .map(this::visitSwitchBlockStatementGroup)
+ .reduce(new LinkedList<>(), (r, e) -> {
+ r.addAll(e);
+ return r;
+ });
- List<CaseStatement> caseStatementList = new LinkedList<>();
- List<Statement> defaultStatementList = new LinkedList<>();
+ List<CaseStatement> caseStatementList = new LinkedList<>();
+ List<Statement> defaultStatementList = new LinkedList<>();
+
+ statementList.forEach(e -> {
+ if (e instanceof CaseStatement) {
+ caseStatementList.add((CaseStatement) e);
+ } else if (isTrue(e, IS_SWITCH_DEFAULT)) {
+ defaultStatementList.add(e);
+ }
+ });
- statementList.forEach(e -> {
- if (e instanceof CaseStatement) {
- caseStatementList.add((CaseStatement) e);
- } else if (isTrue(e, IS_SWITCH_DEFAULT)) {
- defaultStatementList.add(e);
+ int defaultStatementListSize = defaultStatementList.size();
+ if (defaultStatementListSize > 1) {
+ throw createParsingFailedException("a switch must only have one default branch", defaultStatementList.get(0));
}
- });
- int defaultStatementListSize = defaultStatementList.size();
- if (defaultStatementListSize > 1) {
- throw createParsingFailedException("a switch must only have one default branch", defaultStatementList.get(0));
- }
+ if (defaultStatementListSize > 0 && last(statementList) instanceof CaseStatement) {
+ throw createParsingFailedException("a default branch must only appear as the last branch of a switch", defaultStatementList.get(0));
+ }
- if (defaultStatementListSize > 0 && last(statementList) instanceof CaseStatement) {
- throw createParsingFailedException("a default branch must only appear as the last branch of a switch", defaultStatementList.get(0));
+ result = configureAST(
+ new SwitchStatement(
+ this.visitExpressionInPar(ctx.expressionInPar()),
+ caseStatementList,
+ defaultStatementListSize == 0 ? EmptyStatement.INSTANCE : defaultStatementList.get(0)
+ ),
+ ctx);
+ } finally {
+ switchExpressionRuleContextStack.pop();
}
- SwitchStatement result = configureAST(
- new SwitchStatement(
- this.visitExpressionInPar(ctx.expressionInPar()),
- caseStatementList,
- defaultStatementListSize == 0 ? EmptyStatement.INSTANCE : defaultStatementList.get(0)
- ),
- ctx);
-
visitingSwitchStatementCount -= 1;
return result;
@@ -1128,9 +1140,21 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
switchExpressionRuleContextStack.push(ctx);
try {
validateSwitchExpressionLabels(ctx);
- List<Statement> statementList =
+ List<Tuple3<List<Statement>, Boolean, Boolean>> statementInfoList =
ctx.switchBlockStatementExpressionGroup().stream()
.map(e -> this.visitSwitchBlockStatementExpressionGroup(e))
+ .collect(Collectors.toList());
+
+ Boolean isArrow = statementInfoList.get(0).getV2();
+ if (!isArrow && statementInfoList.stream().noneMatch(e -> {
+ Boolean hasYieldOrThrowStatement = e.getV3();
+ return hasYieldOrThrowStatement;
+ })) {
+ throw createParsingFailedException("`yield` or `throw` is expected", ctx);
+ }
+
+ List<Statement> statementList =
+ statementInfoList.stream().map(e -> e.getV1())
.reduce(new LinkedList<>(), (r, e) -> {
r.addAll(e);
return r;
@@ -1179,18 +1203,21 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
@Override
@SuppressWarnings("unchecked")
- public List<Statement> visitSwitchBlockStatementExpressionGroup(SwitchBlockStatementExpressionGroupContext ctx) {
+ public Tuple3<List<Statement>, Boolean, Boolean> visitSwitchBlockStatementExpressionGroup(SwitchBlockStatementExpressionGroupContext ctx) {
int labelCnt = ctx.switchExpressionLabel().size();
List<Token> firstLabelHolder = new ArrayList<>(1);
final int[] arrowCntHolder = new int[1];
- return (List<Statement>) ctx.switchExpressionLabel().stream()
+ boolean[] isArrowHolder = new boolean[1];
+ boolean[] hasResultStmtHolder = new boolean[1];
+ List<Statement> result = (List<Statement>) ctx.switchExpressionLabel().stream()
.map(e -> (Object) this.visitSwitchExpressionLabel(e))
.reduce(new ArrayList<Statement>(4), (r, e) -> {
List<Statement> statementList = (List<Statement>) r;
Tuple3<Token, List<Expression>, Integer> tuple = (Tuple3<Token, List<Expression>, Integer>) e;
boolean isArrow = ARROW == tuple.getV3();
+ isArrowHolder[0] = isArrow;
if (isArrow) {
if (++arrowCntHolder[0] > 1 && !firstLabelHolder.isEmpty()) {
throw createParsingFailedException("`case ... ->` does not support falling through cases", firstLabelHolder.get(0));
@@ -1206,8 +1233,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
throw createParsingFailedException("`yield` is expected", ctx.blockStatements());
}
- if (statementsCnt > 1) {
- throw new GroovyBugError("statements count should be 1, but the count is actually: " + statementsCnt);
+ if (isArrow && statementsCnt > 1) {
+ throw createParsingFailedException("Expect only 1 statement, but " + statementsCnt + " statements found", ctx.blockStatements());
}
if (!isArrow) {
@@ -1229,9 +1256,11 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
hasThrowHolder[0] = true;
}
});
- if (!(hasYieldHolder[0] || hasThrowHolder[0])) {
- throw createParsingFailedException("`yield` or `throw` is expected", ctx.blockStatements());
+
+ if (hasYieldHolder[0] || hasThrowHolder[0]) {
+ hasResultStmtHolder[0] = true;
}
+
}
Statement exprOrBlockStatement = statements.get(0);
@@ -1244,24 +1273,25 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
}
if (!(exprOrBlockStatement instanceof ReturnStatement || exprOrBlockStatement instanceof ThrowStatement)) {
- MethodCallExpression callClosure = callX(
- configureAST(
- closureX(null, exprOrBlockStatement),
- exprOrBlockStatement
- ), "call");
- callClosure.setImplicitThis(false);
-
- codeBlock = configureAST(
- createBlockStatement(configureAST(
- returnS(
- exprOrBlockStatement instanceof ExpressionStatement
- ? ((ExpressionStatement) exprOrBlockStatement).getExpression()
- : callClosure
- ),
- exprOrBlockStatement
- )),
- exprOrBlockStatement
- );
+ if (isArrow) {
+ MethodCallExpression callClosure = callX(
+ configureAST(
+ closureX(null, exprOrBlockStatement),
+ exprOrBlockStatement
+ ), "call");
+ callClosure.setImplicitThis(false);
+ Expression resultExpr = exprOrBlockStatement instanceof ExpressionStatement
+ ? ((ExpressionStatement) exprOrBlockStatement).getExpression()
+ : callClosure;
+
+ codeBlock = configureAST(
+ createBlockStatement(configureAST(
+ returnS(resultExpr),
+ exprOrBlockStatement
+ )),
+ exprOrBlockStatement
+ );
+ }
}
switch (tuple.getV1().getType()) {
@@ -1301,6 +1331,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
return statementList;
});
+
+ return tuple(result, isArrowHolder[0], hasResultStmtHolder[0]);
}
private void validateSwitchExpressionLabels(SwitchExpressionContext ctx) {
diff --git a/src/test-resources/core/SwitchExpression_25x.groovy b/src/test-resources/core/SwitchExpression_25x.groovy
new file mode 100644
index 0000000..c3d5679
--- /dev/null
+++ b/src/test-resources/core/SwitchExpression_25x.groovy
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+
+def s = 'Bar'
+int result = switch (s) {
+ case "Foo":
+ yield 1;
+ case "Bar":
+ System.out.println("Bar!!");
+ case "Baz":
+ yield 2;
+ default:
+ yield 0;
+};
+assert 2 == result
diff --git a/src/test-resources/core/SwitchExpression_26x.groovy b/src/test-resources/core/SwitchExpression_26x.groovy
new file mode 100644
index 0000000..b4ad391
--- /dev/null
+++ b/src/test-resources/core/SwitchExpression_26x.groovy
@@ -0,0 +1,33 @@
+/*
+ * 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.
+ */
+
+def s = 100
+def result = switch (s) {
+ case 100:
+ int x = 0
+ int y = 0
+ for (int i = 100; i < 110; i++) {
+ if (i < 103) continue
+ x++
+ if (i >= 107) break
+ y++
+ }
+ yield "$x, $y"
+};
+assert "5, 4" == result
diff --git a/src/test-resources/fail/SwitchExpression_09x.groovy b/src/test-resources/fail/SwitchExpression_09x.groovy
new file mode 100644
index 0000000..c8caac2
--- /dev/null
+++ b/src/test-resources/fail/SwitchExpression_09x.groovy
@@ -0,0 +1,24 @@
+/*
+ * 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.
+ */
+
+def a = 6
+def r = switch(a) {
+ case 6 -> def x = 'a'; yield x
+ default -> throw new RuntimeException('z')
+}
diff --git a/src/test/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy b/src/test/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
index 506d6c0..10c2af0 100644
--- a/src/test/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
+++ b/src/test/org/apache/groovy/parser/antlr4/GroovyParserTest.groovy
@@ -448,6 +448,8 @@ final class GroovyParserTest extends GroovyTestCase {
doRunAndTestAntlr4('core/SwitchExpression_22x.groovy')
doRunAndTestAntlr4('core/SwitchExpression_23x.groovy')
doRunAndTestAntlr4('core/SwitchExpression_24x.groovy')
+ doRunAndTestAntlr4('core/SwitchExpression_25x.groovy')
+ doRunAndTestAntlr4('core/SwitchExpression_26x.groovy')
}
void "test groovy core - BUG"() {
diff --git a/src/test/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/src/test/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
index e5ecbae..e089e02 100644
--- a/src/test/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
+++ b/src/test/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
@@ -429,6 +429,7 @@ final class SyntaxErrorTest extends GroovyTestCase {
TestUtils.doRunAndShouldFail('fail/SwitchExpression_06x.groovy')
TestUtils.doRunAndShouldFail('fail/SwitchExpression_07x.groovy')
TestUtils.doRunAndShouldFail('fail/SwitchExpression_08x.groovy')
+ TestUtils.doRunAndShouldFail('fail/SwitchExpression_09x.groovy')
}
@NotYetImplemented