From 19f39c5db5ee3e32743fa22314d5cf6220fbddea Mon Sep 17 00:00:00 2001
From: Maxim Medvedev <maxim.medvedev@jetbrains.com>
Date: Sat, 19 Dec 2009 16:39:48 +0300
Subject: [PATCH] IDEADEV-42000: Groovy missing return statement inspection

---
 .../noReturnMethod/MissingReturnInspection.java    | 70 ++++++++--------------
 .../groovy/lang/GroovyHighlightingTest.java        |  1 +
 .../MissingReturnImplicitReturns.groovy            | 11 ++++
 3 files changed, 36 insertions(+), 46 deletions(-)
 create mode 100644 plugins/groovy/testdata/highlighting/MissingReturnImplicitReturns.groovy

diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java
index 9600ce6c03..0b4ec77640 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java
@@ -84,7 +84,30 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool {
   }
 
   private static void check(GrCodeBlock block, ProblemsHolder holder, boolean mustReturnValue) {
-    if ((mustReturnValue || hasReturnStatements(block)) && !alwaysReturns(block)) {
+    final Ref<Boolean> always = new Ref<Boolean>(true);
+    final Ref<Boolean> sometimes = new Ref<Boolean>(false);
+    ControlFlowUtils.visitAllExitPoints(block, new ControlFlowUtils.ExitPointVisitor() {
+      public boolean visit(Instruction instruction) {
+        if (instruction instanceof MaybeReturnInstruction) {
+          if (((MaybeReturnInstruction)instruction).mayReturnValue()) {
+            sometimes.set(true);
+          }
+          else {
+            always.set(false);
+          }
+          return true;
+        }
+        final PsiElement element = instruction.getElement();
+        if (element instanceof GrReturnStatement || element instanceof GrThrowStatement || element instanceof GrAssertStatement) {
+          sometimes.set(true);
+        }
+        else {
+          always.set(false);
+        }
+        return true;
+      }
+    });
+    if ((mustReturnValue && !sometimes.get()) || (sometimes.get() && !always.get())) {
       addNoReturnMessage(block, holder);
     }
   }
@@ -99,29 +122,6 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool {
     holder.registerProblem(lastChild, GroovyInspectionBundle.message("no.return.message"));
   }
 
-  private static boolean hasReturnStatements(GrCodeBlock block) {
-    class Visitor extends GroovyRecursiveElementVisitor {
-      private boolean myFound = false;
-
-      public boolean isFound() {
-        return myFound;
-      }
-
-      public void visitReturnStatement(GrReturnStatement returnStatement) {
-        if (returnStatement.getReturnValue() != null) myFound = true;
-      }
-
-      public void visitElement(GroovyPsiElement element) {
-        if (!myFound) {
-          super.visitElement(element);
-        }
-      }
-    }
-    Visitor visitor = new Visitor();
-    block.accept(visitor);
-    return visitor.isFound();
-  }
-
   @NonNls
   @NotNull
   public String getShortName() {
@@ -131,26 +131,4 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool {
   public boolean isEnabledByDefault() {
     return true;
   }
-
-  public static boolean alwaysReturns(GrCodeBlock block) {
-    final Ref<Boolean> always = new Ref<Boolean>(true);
-    ControlFlowUtils.visitAllExitPoints(block, new ControlFlowUtils.ExitPointVisitor() {
-      public boolean visit(Instruction instruction) {
-        if (instruction instanceof MaybeReturnInstruction) {
-          if (!((MaybeReturnInstruction)instruction).mayReturnValue()) {
-            always.set(false);
-            return false;
-          }
-          return true;
-        }
-        final PsiElement element = instruction.getElement();
-        if (!(element instanceof GrReturnStatement || element instanceof GrThrowStatement || element instanceof GrAssertStatement)) {
-          always.set(false);
-          return false;
-        }
-        return true;
-      }
-    });
-    return always.get();
-  }
 }
diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java
index d9ae1ed39e..f114047df0 100644
--- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java
+++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java
@@ -148,6 +148,7 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase {
   public void testMissingReturnThrowException() throws Throwable { doTest(new MissingReturnInspection()); }
   public void testMissingReturnTryCatch() throws Throwable { doTest(new MissingReturnInspection()); }
   public void testMissingReturnLastNull() throws Throwable { doTest(new MissingReturnInspection()); }
+  public void testMissingReturnImplicitReturns() throws Throwable {doTest(new MissingReturnInspection());}
 
   public void testUnresolvedMethodCallWithTwoDeclarations() throws Throwable{
     doTest();
diff --git a/plugins/groovy/testdata/highlighting/MissingReturnImplicitReturns.groovy b/plugins/groovy/testdata/highlighting/MissingReturnImplicitReturns.groovy
new file mode 100644
index 0000000000..00211f2056
--- /dev/null
+++ b/plugins/groovy/testdata/highlighting/MissingReturnImplicitReturns.groovy
@@ -0,0 +1,11 @@
+def foo() {
+  if (i==5) {
+    print 5;
+  } else {
+    if (j==5) {
+      Math.min(3, 4)
+    } else {
+      print 3;
+    }
+  }
+<warning descr="Not all execution paths return a value">}</warning>
\ No newline at end of file
-- 
2.11.4.GIT