MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

improve method return statements inspection

Browse Source
This commit is contained in:
Maxim Medvedev
2009-11-13 20:24:22 +03:00
parent 157611291d
commit ecf82235b9
7 changed files with 73 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/groovy/resources/inspectionDescriptions/GroovyMethodWithInconsistentReturns.html
-7
View File
@@ -1,7 +0,0 @@
<html>
<body><table> <tr> <td valign="top" height="150">
<font face="verdana" size="-1">
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.
</font></td> </tr> <tr> <td height="20"> <font face="verdana" size="-2">Powered by InspectorGroovy </font> </td> </tr> </table> </body>
</html>
plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java
-2
View File
@@ -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,
plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java
+15 -13
View File
@@ -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
plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java
+23 -28
View File
@@ -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<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 true;
});
return always.get();
}
}
plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java
+33
View File
@@ -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;
}
}
plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/validity/GroovyMethodWithInconsistentReturnsInspection.java
-141
View File
@@ -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;
}
}
}
plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/TypesUtil.java
+2 -2
View File
@@ -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) {