IDEA-81721 Groovy: 'Not all execution paths return a value' inspection reports error on good code

plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java

@@ -42,6 +42,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrM
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrAnonymousClassDefinition;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMember;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
 import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner;
 import org.jetbrains.plugins.groovy.lang.psi.controlFlow.*;
@@ -129,6 +130,10 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor {
   }
 
   private void handlePossibleReturn(GrStatement last) {
+    //last statement inside finally clause cannot be possible return statement
+    final GrFinallyClause finallyClause = PsiTreeUtil.getParentOfType(last, GrFinallyClause.class, false, GrClosableBlock.class, GrMember.class);
+    if (finallyClause != null) return;
+
     if (last instanceof GrExpression && PsiTreeUtil.isAncestor(myLastInScope, last, false)) {
       final MaybeReturnInstruction instruction = new MaybeReturnInstruction((GrExpression)last, myInstructionNumber++);
       checkPending(instruction);

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy

@@ -43,7 +43,6 @@ import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialConditio
 import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialIfInspection
 import org.jetbrains.plugins.groovy.codeInspection.control.GroovyUnnecessaryReturnInspection
 import org.jetbrains.plugins.groovy.codeInspection.metrics.GroovyOverlyLongMethodInspection
-import org.jetbrains.plugins.groovy.codeInspection.noReturnMethod.MissingReturnInspection
 import org.jetbrains.plugins.groovy.codeInspection.unassignedVariable.UnassignedVariableAccessInspection
 import org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.GroovyUnresolvedAccessInspection
 import org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.GroovyUntypedAccessInspection
@@ -502,10 +501,6 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase {
     doTest(new GroovyAssignabilityCheckInspection());
   }
   
-  public void testMissingReturnInClosure() {
-    doTest(new MissingReturnInspection());
-  }
-
   public void testImmutableConstructorFromJava() {
     myFixture.addFileToProject "a.groovy", '''@groovy.transform.Immutable class Foo { int a; String b }'''
     myFixture.configureByText 'a.java', '''

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.groovy

@@ -1,10 +1,11 @@
 package org.jetbrains.plugins.groovy.lang;
 
-import com.intellij.testFramework.LightProjectDescriptor;
-import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.plugins.groovy.codeInspection.noReturnMethod.MissingReturnInspection;
-import org.jetbrains.plugins.groovy.util.TestUtils;
+
+import com.intellij.testFramework.LightProjectDescriptor
+import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase
+import org.jetbrains.annotations.NotNull
+import org.jetbrains.plugins.groovy.codeInspection.noReturnMethod.MissingReturnInspection
+import org.jetbrains.plugins.groovy.util.TestUtils
 
 /**
  * @author peter
@@ -13,7 +14,7 @@ public class MissingReturnTest extends LightCodeInsightFixtureTestCase {
 
   @Override
   protected String getBasePath() {
-    return TestUtils.getTestDataPath() + "highlighting/missingReturn";
+    return "${TestUtils.testDataPath}highlighting/missingReturn";
   }
 
   @NotNull
@@ -38,8 +39,10 @@ public class MissingReturnTest extends LightCodeInsightFixtureTestCase {
   public void testExceptionWithFinally() throws Throwable {doTest();}
   public void testOnlyAssert() throws Throwable {doTest();}
   public void testImplicitReturnNull() throws Throwable {doTest();}
+  public void testMissingReturnInClosure() {doTest();}
+  public void testFinally() {doTest();}
 
-  private void doTest() throws Exception {
+  private void doTest() {
     myFixture.enableInspections(new MissingReturnInspection());
     myFixture.testHighlighting(true, false, false, getTestName(false) + ".groovy");
   }

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/controlFlow/ControlFlowTest.java

@@ -48,6 +48,7 @@ public class ControlFlowTest extends LightCodeInsightFixtureTestCase {
   public void testTry6() throws Throwable { doTest(); }
   public void testTry7() throws Throwable { doTest(); }
   public void testTry8() throws Throwable { doTest(); }
+  public void testTry9() throws Throwable { doTest(); }
   public void testWhile1() throws Throwable { doTest(); }
   public void testWhile2() throws Throwable { doTest(); }
   public void testWhileNonConstant() throws Throwable { doTest(); }

plugins/groovy/testdata/groovy/controlFlow/throwInCatch.test

@@ -16,10 +16,9 @@ try {
 7(9) THROW. element: THROW statement
 8(13) element: Finally clause
 9(8,10)  CALL 8
-10(16) AFTER CALL 9
+10(15) AFTER CALL 9
 11(8,12)  CALL 8
-12(16) AFTER CALL 11
+12(15) AFTER CALL 11
 13(14) READ c
-14(15) element: Reference expression MAYBE_RETURN
-15(10,12)  RETURN
-16() element: null
+14(10,12)  RETURN
+15() element: null

plugins/groovy/testdata/groovy/controlFlow/try2.test

@@ -16,10 +16,9 @@ try {
 7(11) element: Reference expression MAYBE_RETURN
 8(13) element: Finally clause
 9(8,10)  CALL 8
-10(16) AFTER CALL 9
+10(15) AFTER CALL 9
 11(8,12)  CALL 8
-12(16) AFTER CALL 11
+12(15) AFTER CALL 11
 13(14) READ c
-14(15) element: Reference expression MAYBE_RETURN
-15(10,12)  RETURN
-16() element: null
+14(10,12)  RETURN
+15() element: null

plugins/groovy/testdata/groovy/controlFlow/try3.test

@@ -15,10 +15,9 @@ try {
 5(9) element: Assignment expression MAYBE_RETURN
 6(11) element: Finally clause
 7(6,8)  CALL 6
-8(14) AFTER CALL 7
+8(13) AFTER CALL 7
 9(6,10)  CALL 6
-10(14) AFTER CALL 9
+10(13) AFTER CALL 9
 11(12) READ a
-12(13) element: Method call MAYBE_RETURN
-13(8,10)  RETURN
-14() element: null
+12(8,10)  RETURN
+13() element: null

plugins/groovy/testdata/groovy/controlFlow/try7.test

@@ -11,10 +11,9 @@ try {
 4(6) element: RETURN statement
 5(10) element: Finally clause
 6(5,7)  CALL 5
-7(13) AFTER CALL 6
+7(12) AFTER CALL 6
 8(5,9)  CALL 5
-9(13) AFTER CALL 8
+9(12) AFTER CALL 8
 10(11) READ bar
-11(12) element: Method call MAYBE_RETURN
-12(7,9)  RETURN
-13() element: null
+11(7,9)  RETURN
+12() element: null

plugins/groovy/testdata/groovy/controlFlow/try8.test

@@ -16,14 +16,13 @@ finally {
 6(14) element: Method call MAYBE_RETURN
 7(16) element: Finally clause
 8(7,9)  CALL 7
-9(19) AFTER CALL 8
+9(18) AFTER CALL 8
 10(7,11)  CALL 7
-11(19) AFTER CALL 10
+11(18) AFTER CALL 10
 12(7,13)  CALL 7
-13(19) AFTER CALL 12
+13(18) AFTER CALL 12
 14(7,15)  CALL 7
-15(19) AFTER CALL 14
+15(18) AFTER CALL 14
 16(17) READ b
-17(18) element: Method call MAYBE_RETURN
-18(9,11,13,15)  RETURN
-19() element: null
+17(9,11,13,15)  RETURN
+18() element: null

plugins/groovy/testdata/groovy/controlFlow/try9.test

@@ -0,0 +1,28 @@
+InputStream i = null
+
+try {
+  return 1;
+}
+finally {
+  try {
+    i.close()
+  }
+  catch (Exception ignored) {
+
+  }
+}
+-----
+0(1) element: null
+1(2) WRITE i
+2(3) element: Open block
+3(5) element: RETURN statement
+4(7) element: Finally clause
+5(4,6)  CALL 4
+6(13) AFTER CALL 5
+7(8) element: Open block
+8(9) READ i
+9(10,13) THROW. element: Method call
+10(11) element: Catch clause
+11(12) WRITE ignored
+12(6)  RETURN
+13() element: null

plugins/groovy/testdata/highlighting/missingReturn/Finally.groovy

@@ -0,0 +1,26 @@
+int missing() {
+  InputStream i = null
+
+  try {
+    return 1;
+  }
+  catch(Exception i) {
+
+  }
+<warning descr="Not all execution paths return a value">}</warning>
+
+int ok() {
+  InputStream i = null
+
+  try {
+    return 1;
+  }
+  finally {
+    try {
+      i.close()
+    }
+    catch (Exception ignored) {
+
+    }
+  }
+}