GROOVY-11263: Analyze dead code

src/main/java/org/codehaus/groovy/classgen/DeadCodeAnalyzer.java

@@ -0,0 +1,68 @@
+/*
+ *  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.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
+            ) {
+                foundCnt++;
+                if (1 == foundCnt) continue;
+            }
+
+            if (foundCnt > 0) {
+                addError("Unreachable statement found", statement);
+            }
+        }
+    }
+}

src/main/java/org/codehaus/groovy/classgen/Verifier.java

@@ -64,6 +64,7 @@
 import org.codehaus.groovy.classgen.asm.MopWriter;
 import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.ClassNodeSkip;
 import org.codehaus.groovy.classgen.asm.WriterController;
+import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.reflection.ClassInfo;
 import org.codehaus.groovy.syntax.RuntimeParserException;
 import org.codehaus.groovy.syntax.Token;
@@ -167,6 +168,15 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
     private ClassNode classNode;
     private MethodNode methodNode;
 
+    private SourceUnit source;
+
+    public Verifier() {
+    }
+
+    public Verifier(SourceUnit source) {
+        this.source = source;
+    }
+
     public ClassNode getClassNode() {
         return classNode;
     }
@@ -267,6 +277,7 @@ public void visitClass(final ClassNode node) {
         addCovariantMethods(node);
         detectNonSealedClasses(node);
         checkFinalVariables(node);
+        checkDeadCode(node);
     }
 
     private static final String[] INVALID_COMPONENTS = {"clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"};
@@ -302,6 +313,12 @@ private void checkFinalVariables(final ClassNode node) {
         visitor.visitClass(node);
     }
 
+    private void checkDeadCode(final ClassNode node) {
+        if (null == source) return;
+        GroovyClassVisitor visitor = new DeadCodeAnalyzer(source);
+        visitor.visitClass(node);
+    }
+
     protected FinalVariableAnalyzer.VariableNotFinalCallback getFinalVariablesCallback() {
         return new FinalVariableAnalyzer.VariableNotFinalCallback() {
             @Override

src/main/java/org/codehaus/groovy/control/CompilationUnit.java

@@ -750,7 +750,7 @@ 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 = new Verifier(source);
             try {
                 visitor.visitClass(classNode);
             } catch (RuntimeParserException rpe) {

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