GROOVY-11263: Analyze dead code

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);
+            }
+        }
+    }
+}

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);
 

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 (<code>"1.4"</code>) is the value for targetBytecode to compile for a JDK 1.4. */
     @Deprecated public static final String JDK4 = "1.4";
     /** This (<code>"1.5"</code>) 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);

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
       """
   }
-}
+}

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+(.+)/
     }
-}
+}

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+(.+)/
     }
 }