Correct handling of lambdas by 'Unnecessary Return' inspection

plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/UnnecessaryReturnInspection.java

@@ -26,10 +26,9 @@ import com.siyeh.ig.fixes.DeleteUnnecessaryStatementFix;
 import com.siyeh.ig.psiutils.ControlFlowUtils;
 import org.jetbrains.annotations.NotNull;
 
-import javax.swing.JComponent;
+import javax.swing.*;
 
 public class UnnecessaryReturnInspection extends BaseInspection {
-
   @SuppressWarnings("PublicField")
   public boolean ignoreInThenBranch = false;
 
@@ -61,15 +60,7 @@ public class UnnecessaryReturnInspection extends BaseInspection {
   @Override
   @NotNull
   public String buildErrorString(Object... infos) {
-    final boolean isConstructor = ((Boolean)infos[0]).booleanValue();
-    if (isConstructor) {
-      return InspectionGadgetsBundle.message(
-        "unnecessary.return.problem.descriptor");
-    }
-    else {
-      return InspectionGadgetsBundle.message(
-        "unnecessary.return.problem.descriptor1");
-    }
+    return InspectionGadgetsBundle.message("unnecessary.return.problem.descriptor");
   }
 
   @Override
@@ -82,63 +73,53 @@ public class UnnecessaryReturnInspection extends BaseInspection {
     return new UnnecessaryReturnVisitor();
   }
 
-  private class UnnecessaryReturnVisitor
-    extends BaseInspectionVisitor {
-
+  private class UnnecessaryReturnVisitor extends BaseInspectionVisitor {
     @Override
-    public void visitReturnStatement(
-      @NotNull PsiReturnStatement statement) {
+    public void visitReturnStatement(@NotNull PsiReturnStatement statement) {
       super.visitReturnStatement(statement);
       if (JspPsiUtil.isInJspFile(statement.getContainingFile())) {
         return;
       }
-      final PsiMethod method =
-        PsiTreeUtil.getParentOfType(statement, PsiMethod.class);
-      if (method == null) {
+      if (statement.getReturnValue() != null) {
         return;
       }
-      final Boolean isConstructor;
-      if (method.isConstructor()) {
-        isConstructor = Boolean.TRUE;
+      final PsiElement methodParent = PsiTreeUtil.getParentOfType(statement, PsiMethod.class, PsiLambdaExpression.class);
+      PsiCodeBlock codeBlock = null;
+      if (methodParent instanceof PsiMethod) {
+        codeBlock = ((PsiMethod)methodParent).getBody();
       }
-      else {
-        final PsiType returnType = method.getReturnType();
-        if (!PsiType.VOID.equals(returnType)) {
-          return;
+      else if (methodParent instanceof PsiLambdaExpression) {
+        final PsiElement lambdaBody = ((PsiLambdaExpression)methodParent).getBody();
+        if (lambdaBody instanceof PsiCodeBlock) {
+          codeBlock = (PsiCodeBlock)lambdaBody;
         }
-        isConstructor = Boolean.FALSE;
       }
-      final PsiCodeBlock body = method.getBody();
-      if (body == null) {
+      if (codeBlock == null) {
         return;
       }
-      if (!ControlFlowUtils.blockCompletesWithStatement(body,
-                                                        statement)) {
+      if (!ControlFlowUtils.blockCompletesWithStatement(codeBlock, statement)) {
         return;
       }
-      final PsiElement parent = statement.getParent();
-      if (ignoreInThenBranch && isInThenBranch(statement, parent)) {
+      if (ignoreInThenBranch && isInThenBranch(statement, statement.getParent())) {
         return;
       }
-      registerStatementError(statement, isConstructor);
+      registerStatementError(statement);
     }
 
-    private boolean isInThenBranch(PsiReturnStatement statement,
-                                   PsiElement parent) {
+    private boolean isInThenBranch(PsiReturnStatement statement, PsiElement parent) {
       if (!(parent instanceof PsiCodeBlock)) {
         return false;
       }
-      final PsiCodeBlock codeBlock = (PsiCodeBlock)parent;
-      final PsiElement grandParent = codeBlock.getParent();
+      final PsiElement grandParent = parent.getParent();
+      if (grandParent == null) {
+        return false;
+      }
       final PsiElement greatGrandParent = grandParent.getParent();
       if (!(greatGrandParent instanceof PsiIfStatement)) {
         return false;
       }
-      final PsiIfStatement ifStatement =
-        (PsiIfStatement)greatGrandParent;
-      final PsiStatement elseBranch = ifStatement.getElseBranch();
-      return elseBranch != null && !PsiTreeUtil.isAncestor(elseBranch,
-                                                           statement, true);
+      final PsiStatement elseBranch = ((PsiIfStatement)greatGrandParent).getElseBranch();
+      return elseBranch == null || !PsiTreeUtil.isAncestor(elseBranch, statement, true);
     }
   }
 }

plugins/InspectionGadgets/test/common/UnnecessaryReturns.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2000-2012 JetBrains s.r.o.
+ *
+ * Licensed 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.
+ */
+import java.util.concurrent.Callable;
+
+class C {
+  public C() {
+    <warning descr="'return' is unnecessary as the last statement in a 'void' method">return</warning>;
+  }
+
+  public void m1() {
+    <warning descr="'return' is unnecessary as the last statement in a 'void' method">return</warning>;
+  }
+
+  public boolean m2() {
+    return true;
+  }
+
+  public void m3(boolean f) {
+    if (!f) {
+      return;
+    }
+    System.out.println("m3()");
+    if (f) {
+      return;
+    }
+  }
+
+  public void m4(boolean f) {
+    if (f) {
+      System.out.println("m4()");
+      return;
+    }
+    else {
+      <warning descr="'return' is unnecessary as the last statement in a 'void' method">return</warning>;
+    }
+  }
+
+  public void m5() {
+    while (true) {
+      System.out.println("m5()");
+      return;
+    }
+  }
+
+  public void lambda() {
+    Runnable r = () -> { <warning descr="'return' is unnecessary as the last statement in a 'void' method">return</warning>; };
+    System.out.println(r);
+
+    Callable<Integer> c = () -> { return 42; };
+    System.out.println(c);
+  }
+}

plugins/InspectionGadgets/testsrc/com/siyeh/ig/CommonIGInspectionsTest.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright 2000-2012 JetBrains s.r.o.
+ *
+ * Licensed 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 com.siyeh.ig;
+
+import com.intellij.codeInsight.daemon.LightDaemonAnalyzerTestCase;
+import com.intellij.codeInspection.InspectionProfileEntry;
+import com.intellij.openapi.application.PluginPathManager;
+import com.siyeh.ig.controlflow.UnnecessaryReturnInspection;
+
+public class CommonIGInspectionsTest extends LightDaemonAnalyzerTestCase {
+  @Override
+  protected String getTestDataPath() {
+    return PluginPathManager.getPluginHomePath("InspectionGadgets") + "/test";
+  }
+
+  private void doTest(boolean checkWarnings, boolean checkInfos, InspectionProfileEntry... tools) throws Exception {
+    for (InspectionProfileEntry tool : tools) { enableInspectionTool(tool); }
+    doTest("/common/" + getTestName(false) + ".java", checkWarnings, checkInfos);
+  }
+
+  public void testUnnecessaryReturns() throws Exception {
+    final UnnecessaryReturnInspection inspection = new UnnecessaryReturnInspection();
+    inspection.ignoreInThenBranch = true;
+    doTest(true, false, inspection);
+  }
+}