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

TrivialIfInspection: support cases when implicit return is after enclosing 'if'

Browse Source
This commit is contained in:
Tagir Valeev
2017-09-22 15:36:37 +07:00
parent 8eb5297282
commit 839d461ced
10 changed files with 96 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInspection/streamMigration/BaseStreamApiMigration.java
-13
View File
@@ -17,7 +17,6 @@ package com.intellij.codeInspection.streamMigration;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ControlFlowUtils.InitializerUsageStatus;
@@ -124,16 +123,4 @@ abstract class BaseStreamApiMigration {
statement.delete();
}
}
static boolean isReachable(PsiReturnStatement target) {
ControlFlow flow;
try {
flow = ControlFlowFactory.getInstance(target.getProject())
.getControlFlow(target.getParent(), LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance());
}
catch (AnalysisCanceledException e) {
return true;
}
return ControlFlowUtil.isInstructionReachable(flow, flow.getStartOffset(target), 0);
}
}
java/java-impl/src/com/intellij/codeInspection/streamMigration/FindFirstMigration.java
+2 -2
View File
@@ -42,7 +42,7 @@ class FindFirstMigration extends BaseStreamApiMigration {
PsiReturnStatement returnStatement = (PsiReturnStatement)statement;
PsiExpression value = returnStatement.getReturnValue();
if (value == null) return null;
PsiReturnStatement nextReturnStatement = StreamApiMigrationInspection.getNextReturnStatement(loopStatement);
PsiReturnStatement nextReturnStatement = ControlFlowUtils.getNextReturnStatement(loopStatement);
if (nextReturnStatement == null) return null;
PsiExpression orElseExpression = nextReturnStatement.getReturnValue();
if (!ExpressionUtils.isSimpleExpression(orElseExpression)) return null;
@@ -50,7 +50,7 @@ class FindFirstMigration extends BaseStreamApiMigration {
restoreComments(loopStatement, body);
boolean sibling = nextReturnStatement.getParent() == loopStatement.getParent();
PsiElement replacement = loopStatement.replace(elementFactory.createStatementFromText("return " + stream + ";", loopStatement));
if(sibling || !isReachable(nextReturnStatement)) {
if(sibling || !ControlFlowUtils.isReachable(nextReturnStatement)) {
nextReturnStatement.delete();
}
return replacement;
java/java-impl/src/com/intellij/codeInspection/streamMigration/MatchMigration.java
+2 -2
View File
@@ -44,7 +44,7 @@ class MatchMigration extends BaseStreamApiMigration {
PsiExpression value = returnStatement.getReturnValue();
if (ExpressionUtils.isLiteral(value, Boolean.TRUE) || ExpressionUtils.isLiteral(value, Boolean.FALSE)) {
boolean foundResult = (boolean)((PsiLiteralExpression)value).getValue();
PsiReturnStatement nextReturnStatement = StreamApiMigrationInspection.getNextReturnStatement(sourceStatement);
PsiReturnStatement nextReturnStatement = ControlFlowUtils.getNextReturnStatement(sourceStatement);
if (nextReturnStatement != null) {
PsiExpression returnValue = nextReturnStatement.getReturnValue();
if(returnValue == null) return null;
@@ -59,7 +59,7 @@ class MatchMigration extends BaseStreamApiMigration {
return returnValue.replace(elementFactory.createExpressionFromText(streamText, nextReturnStatement));
}
PsiElement result = sourceStatement.replace(elementFactory.createStatementFromText("return " + streamText + ";", sourceStatement));
if(!isReachable(nextReturnStatement)) {
if(!ControlFlowUtils.isReachable(nextReturnStatement)) {
nextReturnStatement.delete();
}
return result;
java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java
+1 -17
View File
@@ -111,22 +111,6 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
return new StreamApiMigrationVisitor(holder, isOnTheFly);
}
@Nullable
static PsiReturnStatement getNextReturnStatement(PsiStatement statement) {
PsiElement nextStatement = PsiTreeUtil.skipWhitespacesAndCommentsForward(statement);
if (nextStatement instanceof PsiReturnStatement) return (PsiReturnStatement)nextStatement;
PsiElement parent = statement.getParent();
if (parent instanceof PsiCodeBlock) {
PsiStatement[] statements = ((PsiCodeBlock)parent).getStatements();
if (statements.length == 0 || statements[statements.length - 1] != statement) return null;
parent = parent.getParent();
if (!(parent instanceof PsiBlockStatement)) return null;
parent = parent.getParent();
}
if (parent instanceof PsiIfStatement) return getNextReturnStatement((PsiStatement)parent);
return null;
}
@Contract("null, null -> true; null, !null -> false")
private static boolean sameReference(PsiExpression expr1, PsiExpression expr2) {
if (expr1 == null && expr2 == null) return true;
@@ -570,7 +554,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
boolean shouldWarn = replaceTrivialForEach || tb.hasOperations();
PsiReturnStatement returnStatement = (PsiReturnStatement)tb.getSingleStatement();
PsiExpression value = returnStatement.getReturnValue();
PsiReturnStatement nextReturnStatement = getNextReturnStatement(statement);
PsiReturnStatement nextReturnStatement = ControlFlowUtils.getNextReturnStatement(statement);
if (nextReturnStatement != null &&
(ExpressionUtils.isLiteral(value, Boolean.TRUE) || ExpressionUtils.isLiteral(value, Boolean.FALSE))) {
boolean foundResult = (boolean)((PsiLiteralExpression)value).getValue();
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/TrivialIfInspection.java
+12 -10
View File
@@ -32,6 +32,7 @@ import com.siyeh.ig.psiutils.BoolUtils;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.EquivalenceChecker;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import org.intellij.lang.annotations.Pattern;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
@@ -39,6 +40,7 @@ import java.util.Collection;
public class TrivialIfInspection extends BaseInspection implements CleanupLocalInspectionTool {
@Pattern(VALID_ID_PATTERN)
@Override
@NotNull
public String getID() {
@@ -144,11 +146,13 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI
return;
}
final String conditionText = condition.getText();
final PsiElement nextStatement = PsiTreeUtil.skipWhitespacesForward(statement);
PsiReturnStatement nextStatement = ControlFlowUtils.getNextReturnStatement(statement);
@NonNls final String newStatement = "return " + conditionText + ';';
PsiReplacementUtil.replaceStatement(statement, newStatement);
assert nextStatement != null;
nextStatement.delete();
if (!ControlFlowUtils.isReachable(nextStatement)) {
nextStatement.delete();
}
}
private static void replaceSimplifiableReturn(PsiIfStatement statement) {
@@ -243,17 +247,19 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI
return;
}
final String conditionText = BoolUtils.getNegatedExpressionText(condition);
final PsiElement nextStatement = PsiTreeUtil.skipWhitespacesForward(statement);
final PsiReturnStatement nextStatement = ControlFlowUtils.getNextReturnStatement(statement);
if (nextStatement == null) {
return;
}
final PsiElement nextSibling = statement.getNextSibling();
if (nextSibling != nextStatement) {
if (nextSibling != nextStatement && nextStatement.getParent() == statement.getParent()) {
statement.getParent().deleteChildRange(nextSibling, nextStatement.getPrevSibling());
}
@NonNls final String newStatement = "return " + conditionText + ';';
PsiReplacementUtil.replaceStatement(statement, newStatement);
nextStatement.delete();
if (!ControlFlowUtils.isReachable(nextStatement)) {
nextStatement.delete();
}
}
private static void replaceSimplifiableReturnNegated(PsiIfStatement statement) {
@@ -334,11 +340,7 @@ public class TrivialIfInspection extends BaseInspection implements CleanupLocalI
}
PsiStatement thenBranch = ifStatement.getThenBranch();
thenBranch = ControlFlowUtils.stripBraces(thenBranch);
final PsiElement nextStatement = PsiTreeUtil.skipWhitespacesForward(ifStatement);
if (!(nextStatement instanceof PsiStatement)) {
return false;
}
final PsiStatement elseBranch = (PsiStatement)nextStatement;
PsiReturnStatement elseBranch = ControlFlowUtils.getNextReturnStatement(ifStatement);
return isReturn(thenBranch, thenReturn) && isReturn(elseBranch, elseReturn);
}
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java
+46
View File
@@ -937,6 +937,52 @@ public class ControlFlowUtils {
return false;
}
/**
* Finds the return statement which will be always executed after the supplied statement. It supports constructs like this:
* <pre>{@code
* if(condition) {
* statement(); // this statement is supplied as a parameter
* }
* return true; // this return statement will be returned
* }</pre>
*
* @param statement statement to find the return after
* @return the found return statement or null.
*/
@Nullable
public static PsiReturnStatement getNextReturnStatement(PsiStatement statement) {
PsiElement nextStatement = PsiTreeUtil.skipWhitespacesAndCommentsForward(statement);
if (nextStatement instanceof PsiReturnStatement) return (PsiReturnStatement)nextStatement;
PsiElement parent = statement.getParent();
if (parent instanceof PsiCodeBlock) {
PsiStatement[] statements = ((PsiCodeBlock)parent).getStatements();
if (statements.length == 0 || statements[statements.length - 1] != statement) return null;
parent = parent.getParent();
if (!(parent instanceof PsiBlockStatement)) return null;
parent = parent.getParent();
}
if (parent instanceof PsiIfStatement) return getNextReturnStatement((PsiStatement)parent);
return null;
}
/**
* @param statement statement to test
* @return true if statement is reachable or code is incomplete and reachability cannot be defined
*/
public static boolean isReachable(@NotNull PsiStatement statement) {
ControlFlow flow;
PsiCodeBlock block = PsiTreeUtil.getParentOfType(statement, PsiCodeBlock.class);
if (block == null) return true;
try {
flow = ControlFlowFactory.getInstance(statement.getProject())
.getControlFlow(block, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance());
}
catch (AnalysisCanceledException e) {
return true;
}
return ControlFlowUtil.isInstructionReachable(flow, flow.getStartOffset(statement), 0);
}
public enum InitializerUsageStatus {
// Variable is declared just before the wanted place
DECLARED_JUST_BEFORE,
plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/trivialIf/Nested.after.java
+9
View File
@@ -0,0 +1,9 @@
class Nested {
public boolean test(String s) {
if(s != null) {
int i = Integer.parseInt(s);
return i > 0;
}
return false;
}
}
plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/trivialIf/Nested.java
+11
View File
@@ -0,0 +1,11 @@
class Nested {
public boolean test(String s) {
if(s != null) {
int i = Integer.parseInt(s);
i<caret>f(i > 0) {
return true;
}
}
return false;
}
}
plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/TrivialIfInspectionTest.java
+12
View File
@@ -33,6 +33,18 @@ public class TrivialIfInspectionTest extends LightInspectionTestCase {
"}");
}
public void testParenthesesReturnNestedIf() {
doMemberTest("\n" +
" boolean b(int[] array) {\n" +
" if (array != null) {\n" +
" int len = array.length;\n" +
" /*'if' statement can be simplified*/if/**/(len == 10) return true;\n" +
" }\n" +
" return false;\n" +
" }\n" +
"");
}
public void testParenthesesAssignment() {
doMemberTest("void b(int[] array) {" +
" boolean result;" +
plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/TrivialIfFixTest.java
+1
View File
@@ -35,4 +35,5 @@ public class TrivialIfFixTest extends IGQuickFixesTestCase {
public void testAssert1() { doTest(); }
public void testAssert2() { doTest(); }
public void testParentheses() { doTest(); }
public void testNested() { doTest(); }
}