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/03/02 20:12:41 UTC
[groovy] 01/02: 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_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 94f8a2e028c192342b219db31efffed7b39bfd79
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
---
.../org/codehaus/groovy/classgen/ReturnAdder.java | 43 ++++-----
src/test/groovy/SwitchTest.groovy | 106 +++++++++++++++++++--
src/test/groovy/bugs/Groovy3789Bug.groovy | 39 --------
3 files changed, 117 insertions(+), 71 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
index b206c16..109b266 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
@@ -36,6 +36,7 @@ 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;
import java.util.Objects;
@@ -139,11 +140,16 @@ public class ReturnAdder {
if (statement instanceof SwitchStatement) {
SwitchStatement switchStatement = (SwitchStatement) statement;
- for (CaseStatement caseStatement : switchStatement.getCaseStatements()) {
- Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false);
+ 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);
}
- Statement defaultStatement = adjustSwitchCaseCode(switchStatement.getDefaultStatement(), scope, true);
+ defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true);
if (doAdd) switchStatement.setDefaultStatement(defaultStatement);
return switchStatement;
}
@@ -203,26 +209,19 @@ public class ReturnAdder {
return blockStatement;
}
- private Statement adjustSwitchCaseCode(final Statement statement, final VariableScope scope, final boolean defaultCase) {
- if (statement instanceof BlockStatement) {
- List<Statement> statements = ((BlockStatement) statement).getStatements();
- if (!statements.isEmpty()) {
- int lastIndex = statements.size() - 1;
- Statement last = statements.get(lastIndex);
- if (last instanceof BreakStatement) {
- if (doAdd) {
- statements.remove(lastIndex);
- return addReturnsIfNeeded(statement, scope);
- } else {
- BlockStatement newBlock = new BlockStatement();
- for (int i = 0; i < lastIndex; i += 1) {
- newBlock.addStatement(statements.get(i));
- }
- return addReturnsIfNeeded(newBlock, 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 89cc170..36c9cb0 100644
--- a/src/test/groovy/SwitchTest.groovy
+++ b/src/test/groovy/SwitchTest.groovy
@@ -19,6 +19,7 @@
package groovy
import groovy.test.GroovyTestCase
+import groovy.test.NotYetImplemented
class SwitchTest extends GroovyTestCase {
@@ -215,16 +216,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'
+ '''
+ }
+
+ @NotYetImplemented // 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
+}
diff --git a/src/test/groovy/bugs/Groovy3789Bug.groovy b/src/test/groovy/bugs/Groovy3789Bug.groovy
deleted file mode 100644
index 405e9de..0000000
--- a/src/test/groovy/bugs/Groovy3789Bug.groovy
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * 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.
- */
-package groovy.bugs
-
-import groovy.test.GroovyTestCase
-
-class Groovy3789Bug extends GroovyTestCase {
- void testAddReturnWhenLastStatementIsSwitch() {
- def ifClosure = { ->
- if ( 0 ) { 10 }
- else { 20 }
- }
- def switchClosure = { ->
- switch ( 0 ) {
- case 0 : 10 ; break
- default : 20 ; break
- }
- }
-
- assert ifClosure() == 20
- assert switchClosure() == 10
- }
-}