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:40 UTC
[groovy] branch GROOVY_3_0_X updated (e39ee93 -> 9b74ab3)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from e39ee93 GROOVY-9777: stubgen: add cast for Object parameter in special ctor call
new 94f8a2e GROOVY-9896, GROOVY-4727: add return for last case if no default present
new 9b74ab3 GROOVY-9880: retain the break statement for potential fall-through cases
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/codehaus/groovy/classgen/ReturnAdder.java | 49 +++++-----
src/test/groovy/SwitchTest.groovy | 105 +++++++++++++++++++--
src/test/groovy/bugs/Groovy3789Bug.groovy | 39 --------
3 files changed, 123 insertions(+), 70 deletions(-)
delete mode 100644 src/test/groovy/bugs/Groovy3789Bug.groovy
[groovy] 02/02: GROOVY-9880: retain the break statement for
potential fall-through cases
Posted by em...@apache.org.
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 9b74ab31b7dfb68e0a6cd99674b185d9fd4f7cea
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jan 14 13:26:03 2021 -0600
GROOVY-9880: retain the break statement for potential fall-through cases
---
src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java | 12 ++++++++++--
src/test/groovy/SwitchTest.groovy | 3 +--
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
index 109b266..24d2061 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ReturnAdder.java
@@ -41,6 +41,7 @@ import java.util.List;
import java.util.Objects;
import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
+import static org.codehaus.groovy.runtime.DefaultGroovyMethods.last;
/**
* Utility class to add return statements.
@@ -215,8 +216,15 @@ public class ReturnAdder {
int breakIndex = block.getStatements().size() - 1;
if (block.getStatements().get(breakIndex) instanceof BreakStatement) {
if (doAdd) {
- block.getStatements().remove(breakIndex);
- return addReturnsIfNeeded(block, scope);
+ Statement breakStatement = block.getStatements().remove(breakIndex);
+ if (breakIndex == 0) block.addStatement(EmptyStatement.INSTANCE);
+ addReturnsIfNeeded(block, scope);
+ // GROOVY-9880: some code structures will fall through
+ Statement lastStatement = last(block.getStatements());
+ if (!(lastStatement instanceof ReturnStatement
+ || lastStatement instanceof ThrowStatement)) {
+ block.addStatement(breakStatement);
+ }
} else {
addReturnsIfNeeded(new BlockStatement(block.getStatements().subList(0, breakIndex), null), scope);
}
diff --git a/src/test/groovy/SwitchTest.groovy b/src/test/groovy/SwitchTest.groovy
index 36c9cb0..7458432 100644
--- a/src/test/groovy/SwitchTest.groovy
+++ b/src/test/groovy/SwitchTest.groovy
@@ -19,7 +19,6 @@
package groovy
import groovy.test.GroovyTestCase
-import groovy.test.NotYetImplemented
class SwitchTest extends GroovyTestCase {
@@ -273,7 +272,7 @@ class SwitchTest extends GroovyTestCase {
'''
}
- @NotYetImplemented // GROOVY-9880
+ // GROOVY-9880
void testSwitchReturn4() {
assertScript '''
def test(sb) {
[groovy] 01/02: GROOVY-9896,
GROOVY-4727: add return for last case if no default present
Posted by em...@apache.org.
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
- }
-}