diff --git a/src/main/java/org/codehaus/groovy/classgen/DeadCodeAnalyzer.java b/src/main/java/org/codehaus/groovy/classgen/DeadCodeAnalyzer.java new file mode 100644 index 00000000000..c856b618614 --- /dev/null +++ b/src/main/java/org/codehaus/groovy/classgen/DeadCodeAnalyzer.java @@ -0,0 +1,69 @@ +/* + * 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 org.codehaus.groovy.classgen; + +import org.codehaus.groovy.ast.ClassCodeVisitorSupport; +import org.codehaus.groovy.ast.stmt.BlockStatement; +import org.codehaus.groovy.ast.stmt.BreakStatement; +import org.codehaus.groovy.ast.stmt.ContinueStatement; +import org.codehaus.groovy.ast.stmt.ReturnStatement; +import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.ast.stmt.ThrowStatement; +import org.codehaus.groovy.control.SourceUnit; + +/** + * Analyze AST for dead code + * + * @since 5.0.0 + */ +public class DeadCodeAnalyzer extends ClassCodeVisitorSupport { + + private final SourceUnit sourceUnit; + + public DeadCodeAnalyzer(final SourceUnit sourceUnit) { + this.sourceUnit = sourceUnit; + } + + @Override + protected SourceUnit getSourceUnit() { + return sourceUnit; + } + @Override + public void visitBlockStatement(BlockStatement statement) { + analyzeDeadCode(statement); + super.visitBlockStatement(statement); + } + + private void analyzeDeadCode(BlockStatement block) { + int foundCnt = 0; + for (Statement statement : block.getStatements()) { + if (statement instanceof ReturnStatement + || statement instanceof BreakStatement + || statement instanceof ContinueStatement + || statement instanceof ThrowStatement) { + foundCnt++; + if (1 == foundCnt) continue; + } + + if (foundCnt > 0) { + addError("Unreachable statement found", statement); + } + } + } +} diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java index 071f6e3a0cb..598a741be5b 100644 --- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java +++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java @@ -34,6 +34,7 @@ import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.ClassCompletionVerifier; +import org.codehaus.groovy.classgen.DeadCodeAnalyzer; import org.codehaus.groovy.classgen.EnumCompletionVisitor; import org.codehaus.groovy.classgen.EnumVisitor; import org.codehaus.groovy.classgen.ExtendedVerifier; @@ -750,13 +751,19 @@ protected boolean dequeued() throws CompilationFailedException { public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException { new OptimizerVisitor(CompilationUnit.this).visitClass(classNode, source); // GROOVY-4272: repositioned from static import visitor - GroovyClassVisitor visitor = new Verifier(); + GroovyClassVisitor visitor; try { + visitor = new Verifier(); visitor.visitClass(classNode); } catch (RuntimeParserException rpe) { getErrorCollector().addError(new SyntaxException(rpe.getMessage(), rpe.getNode()), source); } + if (Boolean.TRUE.equals(configuration.getOptimizationOptions().get(CompilerConfiguration.ANALYZE_DEAD_CODE))) { + visitor = new DeadCodeAnalyzer(source); + visitor.visitClass(classNode); + } + visitor = new LabelVerifier(source); visitor.visitClass(classNode); diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java index 6d9522acd54..8b662fff30d 100644 --- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java +++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java @@ -66,6 +66,9 @@ public class CompilerConfiguration { /** Joint Compilation Option for enabling generating stubs in memory. */ public static final String MEM_STUB = "memStub"; + /** Optimization Option for enabling dead code analysis */ + public static final String ANALYZE_DEAD_CODE = "analyzeDeadCode"; + /** This ("1.4") is the value for targetBytecode to compile for a JDK 1.4. */ @Deprecated public static final String JDK4 = "1.4"; /** This ("1.5") is the value for targetBytecode to compile for a JDK 1.5. */ @@ -469,6 +472,8 @@ public CompilerConfiguration() { handleOptimizationOption(GROOVYDOC, getSystemPropertySafe("groovy.attach.groovydoc")); handleOptimizationOption(RUNTIME_GROOVYDOC, getSystemPropertySafe("groovy.attach.runtime.groovydoc")); handleOptimizationOption(PARALLEL_PARSE, getSystemPropertySafe("groovy.parallel.parse", "true")); + handleOptimizationOption(ANALYZE_DEAD_CODE, getSystemPropertySafe("groovy.analyze.deadcode", "true")); + if (getBooleanSafe("groovy.mem.stub")) { jointCompilationOptions = new HashMap<>(2); diff --git a/src/test/gls/statements/ReturnTest.groovy b/src/test/gls/statements/ReturnTest.groovy index 66effd7b765..5495a3d4908 100644 --- a/src/test/gls/statements/ReturnTest.groovy +++ b/src/test/gls/statements/ReturnTest.groovy @@ -26,18 +26,22 @@ class ReturnTest extends CompilableTestSupport { shouldNotCompile """ class A { {return} - } + } """ } void testStaticInitializer() { - assertScript """ + def err = shouldFail """\ class A { static foo=2 - static { return; foo=1 } + static { + return; + foo=1 + } } assert A.foo==2 - """ + """ + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 5, column 17\.\s+foo=1\s+(.+)/ } void testReturnAdditionInFinally() { @@ -59,4 +63,4 @@ class ReturnTest extends CompilableTestSupport { assert finalCountDown().counter == 9 """ } -} \ No newline at end of file +} diff --git a/src/test/groovy/BreakContinueLabelTest.groovy b/src/test/groovy/BreakContinueLabelTest.groovy index a993f203826..e76fa006eaa 100644 --- a/src/test/groovy/BreakContinueLabelTest.groovy +++ b/src/test/groovy/BreakContinueLabelTest.groovy @@ -31,70 +31,89 @@ class BreakContinueLabelTest extends CompilableTestSupport { assert true } void testBreakLabelInSimpleForLoop() { - label_1: for (i in [1]) { - break label_1 - assert false - } + def err = shouldFail '''\ + label_1: for (i in [1]) { + break label_1 + assert false + } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+assert false\s+(.+)/ } void testBreakLabelInNestedForLoop() { - label: for (i in [1]) { - for (j in [1]){ - break label - assert false, 'did not break inner loop' + def err = shouldFail '''\ + label: for (i in [1]) { + for (j in [1]){ + break label + assert false, 'did not break inner loop' + } + assert false, 'did not break outer loop' } - assert false, 'did not break outer loop' - } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 21\.\s+assert false, 'did not break inner loop'\s+(.+)/ } void testUnlabelledBreakInNestedForLoop() { - def reached = false - for (i in [1]) { - for (j in [1]){ - break - assert false, 'did not break inner loop' + def err = shouldFail '''\ + def reached = false + for (i in [1]) { + for (j in [1]){ + break + assert false, 'did not break inner loop' + } + reached = true } - reached = true - } - assert reached, 'must not break outer loop' + assert reached, 'must not break outer loop' + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 5, column 21\.\s+assert false, 'did not break inner loop'\s+(.+)/ } void testBreakLabelInSimpleWhileLoop() { - label_1: while (true) { - break label_1 - assert false - } + def err = shouldFail '''\ + label_1: while (true) { + break label_1 + assert false + } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+assert false\s+(.+)/ } void testBreakLabelInNestedWhileLoop() { - def count = 0 - label: while (count < 1) { - count++ - while (true){ - break label - assert false, 'did not break inner loop' + def err = shouldFail '''\ + def count = 0 + label: while (count < 1) { + count++ + while (true){ + break label + assert false, 'did not break inner loop' + } + assert false, 'did not break outer loop' } - assert false, 'did not break outer loop' - } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 6, column 21\.\s+assert false, 'did not break inner loop'\s+(.+)/ } void testBreakLabelInNestedMixedForAndWhileLoop() { - def count = 0 - label_1: while (count < 1) { - count++ - for (i in [1]){ - break label_1 - assert false, 'did not break inner loop' + def err = shouldFail '''\ + def count = 0 + label_1: while (count < 1) { + count++ + for (i in [1]){ + break label_1 + assert false, 'did not break inner loop' + } + assert false, 'did not break outer loop' } - assert false, 'did not break outer loop' - } - label_2: for (i in [1]) { - while (true){ - break label_2 - assert false, 'did not break inner loop' + label_2: for (i in [1]) { + while (true){ + break label_2 + assert false, 'did not break inner loop' + } + assert false, 'did not break outer loop' } - assert false, 'did not break outer loop' - } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 6, column 21\.\s+assert false, 'did not break inner loop'\s+(.+)/ + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 13, column 21\.\s+assert false, 'did not break inner loop'\s+(.+)/ } void testUnlabelledContinueInNestedForLoop() { @@ -123,13 +142,16 @@ class BreakContinueLabelTest extends CompilableTestSupport { } void testBreakToLastLabelSucceeds() { - one: - two: - three: - for (i in 1..2) { - break three - fail() - } + def err = shouldFail '''\ + one: + two: + three: + for (i in 1..2) { + break three + assert false + } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 6, column 17\.\s+assert false\s+(.+)/ } void testMultipleLabelSupport() { @@ -152,15 +174,19 @@ class BreakContinueLabelTest extends CompilableTestSupport { // this is in accordance with Java; Spock Framework relies on this void testLabelCanOccurMultipleTimesInSameScope() { - one: - for (i in 1..2) { - break one - fail() - } - one: - for (i in 1..2) { - break one - fail() - } + def err = shouldFail '''\ + one: + for (i in 1..2) { + break one + assert false + } + one: + for (i in 1..2) { + break one + assert false + } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 17\.\s+assert false\s+(.+)/ + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 9, column 17\.\s+assert false\s+(.+)/ } -} \ No newline at end of file +} diff --git a/src/test/groovy/ThrowTest.groovy b/src/test/groovy/ThrowTest.groovy index e0d6ae0b7a0..093c9375a3a 100644 --- a/src/test/groovy/ThrowTest.groovy +++ b/src/test/groovy/ThrowTest.groovy @@ -22,12 +22,15 @@ import groovy.test.GroovyTestCase class ThrowTest extends GroovyTestCase { void testThrow() { - try { - throw new Exception("abcd") - fail("Should have thrown an exception by now") - } - catch (Exception e) { - assert e.message == "abcd" - } + def err = shouldFail '''\ + try { + throw new Exception("abcd") + assert false: "Should have thrown an exception by now" + } + catch (Exception e) { + assert e.message == "abcd" + } + ''' + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+assert false: "Should have thrown an exception by now"\s+(.+)/ } } diff --git a/src/test/groovy/bugs/Groovy11263.groovy b/src/test/groovy/bugs/Groovy11263.groovy new file mode 100644 index 00000000000..0eb10a6b7ab --- /dev/null +++ b/src/test/groovy/bugs/Groovy11263.groovy @@ -0,0 +1,402 @@ +/* + * 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 bugs + +import org.junit.Test + +import static groovy.test.GroovyAssert.shouldFail + +final class Groovy11263 { + + @Test + void testUnreachableStatementAfterReturn1() { + def err = shouldFail '''\ + def m() { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn2() { + def err = shouldFail '''\ + class X { + X() { + return + def a = 1 + } + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 21\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn3() { + def err = shouldFail '''\ + { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn4() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn5() { + def err = shouldFail '''\ + while (true) { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn6() { + def err = shouldFail '''\ + do { + return + def a = 1 + } while (true) + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn7() { + def err = shouldFail '''\ + if (true) { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn8() { + def err = shouldFail '''\ + if (true) { + } else { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn9() { + def err = shouldFail '''\ + try { + return + def a = 1 + } catch (e) { + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn10() { + def err = shouldFail '''\ + try { + } catch (e) { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn11() { + def err = shouldFail '''\ + try { + } finally { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn12() { + def err = shouldFail '''\ + switch(1) { + case 1: + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 21\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn13() { + def err = shouldFail '''\ + switch(1) { + default: + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 4, column 21\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn14() { + def err = shouldFail '''\ + [1, 2, 3].each { + return + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn15() { + def err = shouldFail '''\ + [1, 2, 3].each(e -> { + return + def a = 1 + }) + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn16() { + def err = shouldFail '''\ + return + def a = 1 + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 2, column 13\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn17() { + def err = shouldFail '''\ + return + return + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 2, column 13\.\s+return\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn19() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + return + break + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+break\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterReturn20() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + return + continue + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+continue\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterBreak1() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + break + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterBreak2() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + break + break + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+break\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterBreak3() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + break + continue + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+continue\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterBreak4() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + break + return + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+return\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterContinue1() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + continue + def a = 1 + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+def a = 1\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterContinue2() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + continue + continue + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+continue\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterContinue3() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + continue + break + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+break\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterContinue4() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + continue + return + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+return\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterThrow1() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + throw new RuntimeException("just for test") + throw new RuntimeException("just for test.") + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+throw new RuntimeException\("just for test\."\)\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterThrow2() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + throw new RuntimeException("just for test") + return + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+return\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterThrow3() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + throw new RuntimeException("just for test") + break + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+break\s+(.+)/ + } + + @Test + void testUnreachableStatementAfterThrow4() { + def err = shouldFail '''\ + for (var i in [1, 2, 3]) { + throw new RuntimeException("just for test") + continue + } + ''' + + assert err ==~ /(?s)(.+)\s+Unreachable statement found\s+@ line 3, column 17\.\s+continue\s+(.+)/ + } +} diff --git a/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/SimplePostconditionTests.groovy b/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/SimplePostconditionTests.groovy index 0da74a6a691..85fc678bcbc 100644 --- a/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/SimplePostconditionTests.groovy +++ b/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/SimplePostconditionTests.groovy @@ -120,7 +120,7 @@ class Account { if (true) { try { throw new Exception ('test') - return 1 + // return 1 // GROOVY-11263 } finally { return 3 } @@ -148,7 +148,7 @@ class Account { if (true) { try { throw new Exception ('test') - return 1 + // return 1 // GROOVY-11263 } finally { return 3 }