You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/08/26 15:40:58 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-9896,
GROOVY-4727: add return for last case if no default present
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new c666e5b GROOVY-9896, GROOVY-4727: add return for last case if no default present
c666e5b is described below
commit c666e5b1f7340baffa447b6faa3176d5962641fa
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jan 14 12:58:08 2021 -0600
GROOVY-9896, GROOVY-4727: add return for last case if no default present
Conflicts:
src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
src/test/groovy/SwitchTest.groovy
---
.../org/codehaus/groovy/classgen/ReturnAdder.java | 58 ++++++------
src/test/groovy/SwitchTest.groovy | 105 +++++++++++++++++++--
2 files changed, 124 insertions(+), 39 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
index 22f3059..4efe9fc 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
@@ -38,11 +38,13 @@ import org.codehaus.groovy.ast.stmt.ThrowStatement;
import org.codehaus.groovy.ast.stmt.TryCatchStatement;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
/**
* Utility class to add return statements.
- * Extracted from Verifier as it can be useful for some AST transformations
+ * <p>
+ * Extracted from Verifier as it can be useful for some AST transformations.
*/
public class ReturnAdder {
@@ -60,11 +62,11 @@ public class ReturnAdder {
private final ReturnStatementListener listener;
public ReturnAdder() {
- doAdd = true;
- listener = DEFAULT_LISTENER;
+ this.listener = DEFAULT_LISTENER;
+ this.doAdd = true;
}
- public ReturnAdder(ReturnStatementListener listener) {
+ public ReturnAdder(final ReturnStatementListener listener) {
this.listener = listener;
this.doAdd = false;
}
@@ -152,14 +154,19 @@ public class ReturnAdder {
}
if (statement instanceof SwitchStatement) {
- SwitchStatement swi = (SwitchStatement) statement;
- for (CaseStatement caseStatement : swi.getCaseStatements()) {
- final Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false);
+ SwitchStatement switchStatement = (SwitchStatement) statement;
+ Statement defaultStatement = switchStatement.getDefaultStatement();
+ List<CaseStatement> caseStatements = switchStatement.getCaseStatements();
+ for (Iterator<CaseStatement> it = caseStatements.iterator(); it.hasNext(); ) {
+ CaseStatement caseStatement = it.next();
+ Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope,
+ // GROOVY-9896: return if no default and last case lacks break
+ defaultStatement == EmptyStatement.INSTANCE && !it.hasNext());
if (doAdd) caseStatement.setCode(code);
}
- final Statement defaultStatement = adjustSwitchCaseCode(swi.getDefaultStatement(), scope, true);
- if (doAdd) swi.setDefaultStatement(defaultStatement);
- return swi;
+ defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true);
+ if (doAdd) switchStatement.setDefaultStatement(defaultStatement);
+ return switchStatement;
}
if (statement instanceof TryCatchStatement) {
@@ -231,26 +238,19 @@ public class ReturnAdder {
}
}
- private Statement adjustSwitchCaseCode(Statement statement, VariableScope scope, boolean defaultCase) {
- if(statement instanceof BlockStatement) {
- final List list = ((BlockStatement)statement).getStatements();
- if (!list.isEmpty()) {
- int idx = list.size() - 1;
- Statement last = (Statement) list.get(idx);
- if(last instanceof BreakStatement) {
- if (doAdd) {
- list.remove(idx);
- return addReturnsIfNeeded(statement, scope);
- } else {
- BlockStatement newStmt = new BlockStatement();
- for (int i=0;i<idx; i++) {
- newStmt.addStatement((Statement) list.get(i));
- }
- return addReturnsIfNeeded(newStmt, scope);
- }
- } else if(defaultCase) {
- return addReturnsIfNeeded(statement, scope);
+ private Statement adjustSwitchCaseCode(final Statement statement, final VariableScope scope, final boolean lastCase) {
+ if (!statement.isEmpty() && statement instanceof BlockStatement) {
+ BlockStatement block = (BlockStatement) statement;
+ int breakIndex = block.getStatements().size() - 1;
+ if (block.getStatements().get(breakIndex) instanceof BreakStatement) {
+ if (doAdd) {
+ block.getStatements().remove(breakIndex);
+ return addReturnsIfNeeded(block, scope);
+ } else {
+ addReturnsIfNeeded(new BlockStatement(block.getStatements().subList(0, breakIndex), null), scope);
}
+ } else if (lastCase) {
+ return addReturnsIfNeeded(statement, scope);
}
}
return statement;
diff --git a/src/test/groovy/SwitchTest.groovy b/src/test/groovy/SwitchTest.groovy
index 2aba958..077afa4 100644
--- a/src/test/groovy/SwitchTest.groovy
+++ b/src/test/groovy/SwitchTest.groovy
@@ -213,16 +213,101 @@ class SwitchTest extends GroovyTestCase {
}
assertEquals 1, i
}
-
- void testSwitchReturnFromDefaultCase() {
- assert m3('a') == 'letter A' && m3('b') == 'letter B' && m3('z') == 'Unknown letter'
+
+ void testSwitchReturn1() {
+ assertScript '''
+ def test(x) {
+ switch (x) {
+ case 'a': 'letter A'; break
+ case 'b': 'letter B'; break
+ default : 'Unknown letter'
+ }
+ }
+ assert test('a') == 'letter A'
+ assert test('b') == 'letter B'
+ assert test('z') == 'Unknown letter'
+ '''
+ }
+
+ // GROOVY-3789
+ void testSwitchReturn2() {
+ assertScript '''
+ def test = { ->
+ if ( 0 ) { 10 }
+ else { 20 }
+ }
+ assert test() == 20
+ '''
+ assertScript '''
+ def test = { ->
+ switch ( 0 ) {
+ case 0 : 10 ; break
+ default : 20 ; break
+ }
+ }
+ assert test() == 10
+ '''
+ }
+
+ // GROOVY-4727
+ void testSwitchReturn3() {
+ assertScript '''
+ def test(x,y) {
+ switch (x) {
+ case 'x1':
+ switch (y) {
+ case 'y1':
+ 'r1'
+ break
+ case 'y2':
+ 'r2'
+ break
+ }
+ // no break
+ }
+ }
+ assert test('x1','y1') == 'r1'
+ '''
+ }
+
+ // GROOVY-9880
+ void testSwitchReturn4() {
+ assertScript '''
+ def test(sb) {
+ switch ('value') {
+ case 'value':
+ sb.append('foo')
+ if (false) sb.append('X')
+ // implicit "else ;"
+ break
+ default:
+ sb.append('bar')
+ }
+ }
+ def sb = new StringBuilder()
+ test(sb); assert sb.toString() == 'foo'
+ '''
}
- def m3(s) {
- switch(s) {
- case 'a': 'letter A'; break
- case 'b': 'letter B'; break
- default: 'Unknown letter'
- }
+ // GROOVY-9896
+ void testSwitchReturn5() {
+ assertScript '''
+ def test(x) {
+ switch(x) {
+ case 1:
+ 'a'
+ break
+ case 2:
+ 'b'
+ break
+ case 3:
+ 'c'
+ }
+ }
+ assert test(1) == 'a'
+ assert test(2) == 'b'
+ assert test(3) == 'c'
+ assert test(4) == null
+ '''
}
-}
\ No newline at end of file
+}