-
-This inspection reports any Groovy methods with both value-returning and non-value-returning return statements.
- While theoretically valid, such inconsistency is almost certainly due to coding error.
-
Powered by InspectorGroovy
-
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java
index 5937c022109b..13a92bf7135b 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java
@@ -37,7 +37,6 @@ import org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.Groov
import org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.GroovyUntypedAccessInspection;
import org.jetbrains.plugins.groovy.codeInspection.unusedDef.UnusedDefInspection;
import org.jetbrains.plugins.groovy.codeInspection.validity.GroovyDuplicateSwitchBranchInspection;
-import org.jetbrains.plugins.groovy.codeInspection.validity.GroovyMethodWithInconsistentReturnsInspection;
import org.jetbrains.plugins.groovy.codeInspection.validity.GroovyUnreachableStatementInspection;
/**
@@ -129,7 +128,6 @@ public class GroovyInspectionProvider implements InspectionToolProvider, Applica
GroovyOctalIntegerInspection.class,
GroovyDuplicateSwitchBranchInspection.class,
- GroovyMethodWithInconsistentReturnsInspection.class,
GroovyNonShortCircuitBooleanInspection.class,
GroovyInfiniteLoopStatementInspection.class,
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java
index 0c9c070dc4bb..4acea75b2082 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java
@@ -24,9 +24,9 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.GroovyBundle;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
+import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils;
import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes;
import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentLabel;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrNamedArgument;
@@ -36,6 +36,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrAssign
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
+import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction;
import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
@@ -72,11 +73,12 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection {
return new MyVisitor();
}
- private class MyVisitor extends BaseInspectionVisitor {
+ private static class MyVisitor extends BaseInspectionVisitor {
private void checkAssignability(@NotNull PsiType expectedType, @NotNull GrExpression expression, GroovyPsiElement element) {
if (PsiUtil.isRawClassMemberAccess(expression)) return; //GRVY-2197
final PsiType rType = expression.getType();
- if (rType != null && !TypesUtil.isAssignable(expectedType, rType, element.getManager(), element.getResolveScope())) {
+ if (rType == null || rType == PsiType.VOID) return;
+ if (!TypesUtil.isAssignable(expectedType, rType, element.getManager(), element.getResolveScope())) {
registerError(element, rType.getPresentableText(), expectedType.getPresentableText());
}
}
@@ -85,21 +87,21 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection {
@Override
public void visitOpenBlock(GrOpenBlock block) {
super.visitOpenBlock(block);
-
final PsiElement element = block.getParent();
if (!(element instanceof GrMethod)) return;
GrMethod method = (GrMethod)element;
final PsiType expectedType = method.getReturnType();
- if (PsiType.VOID.equals(expectedType)) return;
+ if (expectedType == null || PsiType.VOID.equals(expectedType)) return;
- final GrStatement[] statements = block.getStatements();
- if (statements.length == 0) return;
- final GrStatement last = statements[statements.length - 1];
- if (!(last instanceof GrExpression)) return;
- final GrExpression expression = (GrExpression)last;
-
- if (expectedType == null) return;
- checkAssignability(expectedType, expression, last);
+ ControlFlowUtils.visitAllExitPoints(block, new ControlFlowUtils.ExitPointVisitor() {
+ public boolean visit(Instruction instruction) {
+ final PsiElement psiElement = instruction.getElement();
+ if (psiElement instanceof GrExpression) {
+ checkAssignability(expectedType, (GrExpression)psiElement, (GrExpression)psiElement);
+ }
+ return true;
+ }
+ });
}
@Override
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 be272479ed7a..9600ce6c0328 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
@@ -16,6 +16,7 @@
package org.jetbrains.plugins.groovy.codeInspection.noReturnMethod;
import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiElementVisitor;
@@ -25,6 +26,7 @@ import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle;
import org.jetbrains.plugins.groovy.codeInspection.GroovySuppressableInspectionTool;
+import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils;
import org.jetbrains.plugins.groovy.lang.psi.GroovyElementVisitor;
import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementVisitor;
@@ -32,9 +34,9 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrCodeBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrOpenBlock;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrAssertStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrAssertStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction;
import org.jetbrains.plugins.groovy.lang.psi.controlFlow.impl.MaybeReturnInstruction;
@@ -82,7 +84,7 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool {
}
private static void check(GrCodeBlock block, ProblemsHolder holder, boolean mustReturnValue) {
- if ((mustReturnValue || hasReturnStatements(block)) && !alwaysReturns(block.getControlFlow())) {
+ if ((mustReturnValue || hasReturnStatements(block)) && !alwaysReturns(block)) {
addNoReturnMessage(block, holder);
}
}
@@ -130,32 +132,25 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool {
return true;
}
- public static boolean alwaysReturns(Instruction[] flow) {
- boolean[] visited = new boolean[flow.length];
- return alwaysReturnsInner(flow[flow.length - 1], flow[0], visited);
- }
-
- private static boolean alwaysReturnsInner(Instruction last, Instruction first, boolean[] visited) {
- if (first == last) return false;
- if (last instanceof MaybeReturnInstruction) {
- return ((MaybeReturnInstruction)last).mayReturnValue();
- }
-
- final PsiElement element = last.getElement();
- if (element instanceof GrReturnStatement || element instanceof GrThrowStatement || element instanceof GrAssertStatement) {
- return true;
- }
-
- if (last.getElement() != null) {
- return false;
- }
-
- visited[last.num()] = true;
- for (Instruction pred : last.allPred()) {
- if (!visited[pred.num()]) {
- if (!alwaysReturnsInner(pred, first, visited)) return false;
+ public static boolean alwaysReturns(GrCodeBlock block) {
+ final Ref always = new Ref(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 true;
+ });
+ return always.get();
}
}
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java
index a3f0059bfbe4..86aa6c0503e4 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java
@@ -15,6 +15,7 @@
*/
package org.jetbrains.plugins.groovy.codeInspection.utils;
+import com.intellij.psi.PsiElement;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -31,6 +32,8 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnState
import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.clauses.GrCaseSection;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
+import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction;
+import org.jetbrains.plugins.groovy.lang.psi.controlFlow.impl.MaybeReturnInstruction;
@SuppressWarnings({"OverlyComplexClass"})
public class ControlFlowUtils {
@@ -538,4 +541,34 @@ public class ControlFlowUtils {
return throwStatement != null;
}
+
+ public interface ExitPointVisitor {
+ boolean visit(Instruction instruction);
+ }
+
+ public static void visitAllExitPoints(@Nullable GrCodeBlock block, ExitPointVisitor visitor) {
+ if (block == null) return;
+ final Instruction[] flow = block.getControlFlow();
+ boolean[] visited = new boolean[flow.length];
+ visitAllExitPointsInner(flow[flow.length - 1], flow[0], visited, visitor);
+ }
+
+ private static boolean visitAllExitPointsInner(Instruction last, Instruction first, boolean[] visited, ExitPointVisitor visitor) {
+ if (first == last) return true;
+ if (last instanceof MaybeReturnInstruction) {
+ return visitor.visit(last);
+ }
+
+ final PsiElement element = last.getElement();
+ if (element != null) {
+ return visitor.visit(last);
+ }
+ visited[last.num()] = true;
+ for (Instruction pred : last.allPred()) {
+ if (!visited[pred.num()]) {
+ if (!visitAllExitPointsInner(pred, first, visited, visitor)) return false;
+ }
+ }
+ return true;
+ }
}
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/validity/GroovyMethodWithInconsistentReturnsInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/validity/GroovyMethodWithInconsistentReturnsInspection.java
deleted file mode 100644
index 9f6d4babf645..000000000000
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/validity/GroovyMethodWithInconsistentReturnsInspection.java
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- * Copyright 2007-2008 Dave Griffith
- *
- * 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 org.jetbrains.plugins.groovy.codeInspection.validity;
-
-import com.intellij.psi.PsiElement;
-import com.intellij.psi.util.PsiTreeUtil;
-import org.jetbrains.annotations.Nls;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
-import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
-import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
-import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils;
-import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrBlockStatement;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
-
-public class GroovyMethodWithInconsistentReturnsInspection extends BaseInspection {
-
- @Nls
- @NotNull
- public String getGroupDisplayName() {
- return VALIDITY_ISSUES;
- }
-
- @NotNull
- public String getDisplayName() {
- return "Method with inconsistent returns";
- }
-
-
- public boolean isEnabledByDefault() {
- return true;
- }
-
- @Nullable
- protected String buildErrorString(Object... args) {
- return "Method '#ref' has inconsistent return points #loc";
- }
-
- public BaseInspectionVisitor buildVisitor() {
- return new Visitor();
- }
-
- private static class Visitor extends BaseInspectionVisitor {
- public void visitMethod(GrMethod method) {
- super.visitMethod(method);
- if (!methodHasReturnValues(method)) {
- return;
- }
- if (!methodHasValuelessReturns(method)) {
- return;
- }
- registerMethodError(method);
- }
- }
-
- private static boolean methodHasReturnValues(GrMethod method) {
- final ReturnValuesVisitor visitor = new ReturnValuesVisitor(method);
- method.accept(visitor);
- return visitor.hasReturnValues();
- }
-
- private static boolean methodHasValuelessReturns(GrMethod method) {
- final PsiElement lastChild = method.getLastChild();
- if (lastChild instanceof GrBlockStatement) {
- if (ControlFlowUtils.statementMayCompleteNormally((GrStatement) lastChild)) {
- return true;
- }
- }
- final ValuelessReturnVisitor visitor = new ValuelessReturnVisitor(method);
- method.accept(visitor);
- return visitor.hasValuelessReturns();
- }
-
- private static class ReturnValuesVisitor extends GroovyRecursiveElementVisitor {
- private final GrMethod method;
- private boolean hasReturnValues = false;
-
- ReturnValuesVisitor(GrMethod method) {
- this.method = method;
- }
-
- public void visitReturnStatement(GrReturnStatement statement) {
- super.visitReturnStatement(statement);
- if (statement.getReturnValue() != null) {
- final GrMethod containingMethod =
- PsiTreeUtil.getParentOfType(statement, GrMethod.class);
- if (method.equals(containingMethod)) {
- hasReturnValues = true;
- }
- }
- }
-
- public boolean hasReturnValues() {
- return hasReturnValues;
- }
- }
-
- private static class ValuelessReturnVisitor extends GroovyRecursiveElementVisitor {
- private final GrMethod method;
- private boolean hasValuelessReturns = false;
-
- ValuelessReturnVisitor(GrMethod method) {
- this.method = method;
- }
-
- public void visitReturnStatement(GrReturnStatement statement) {
- super.visitReturnStatement(statement);
- if (statement.getReturnValue() == null) {
- final GrMethod containingMethod =
- PsiTreeUtil.getParentOfType(statement, GrMethod.class);
- if (method.equals(containingMethod)) {
- hasValuelessReturns = true;
- }
- }
- }
-
- public void visitGrMethod(GrMethod method) {
- // do nothing, so that it doesn't drill into nested methods
- }
-
- public boolean hasValuelessReturns() {
- return hasValuelessReturns;
- }
- }
-}
\ No newline at end of file
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/TypesUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/TypesUtil.java
index a21546214a7c..b280f86159cf 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/TypesUtil.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/TypesUtil.java
@@ -194,7 +194,7 @@ public class TypesUtil {
public static boolean isAssignable(PsiType lType, PsiType rType, PsiManager manager, GlobalSearchScope scope) {
//all numeric types are assignable
if (isNumericType(lType)) {
- return isNumericType(rType) || rType.equalsToText(JAVA_LANG_STRING) || rType.equals(PsiType.NULL);
+ return isNumericType(rType) || rType.equals(PsiType.NULL);
}
if (rType instanceof GrTupleType) {
final GrTupleType tuple = (GrTupleType)rType;
@@ -264,7 +264,7 @@ public class TypesUtil {
}
public static PsiType boxPrimitiveType(PsiType result, PsiManager manager, GlobalSearchScope resolveScope) {
- if (result instanceof PsiPrimitiveType) {
+ if (result instanceof PsiPrimitiveType && result != PsiType.VOID) {
PsiPrimitiveType primitive = (PsiPrimitiveType)result;
String boxedTypeName = primitive.getBoxedTypeName();
if (boxedTypeName != null) {