ConvertSwitchToIfIntention: fixes after review IDEA-CR-37311:

1. Do not suggest intention if `default` case is not the last one and fallthrough is possible into or out of that case
2. Generate a code-block if side-effect should be extracted to variable
This commit is contained in:
Tagir Valeev
2018-09-27 10:05:29 +07:00
parent cf83ab3242
commit 625c1cc676
9 changed files with 118 additions and 18 deletions

View File

@@ -10,6 +10,7 @@ import com.intellij.psi.*;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.refactoring.util.RefactoringUtil;
@@ -45,8 +46,23 @@ public class ConvertSwitchToIfIntention implements IntentionAction {
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
final PsiCodeBlock body = mySwitchExpression.getBody();
return body != null && !body.isEmpty() && BreakConverter.from(mySwitchExpression) != null;
return isAvailable(mySwitchExpression);
}
public static boolean isAvailable(PsiSwitchStatement switchStatement) {
final PsiCodeBlock body = switchStatement.getBody();
return body != null && !body.isEmpty() && BreakConverter.from(switchStatement) != null && !mayFallThroughNonTerminalDefaultCase(body);
}
private static boolean mayFallThroughNonTerminalDefaultCase(PsiCodeBlock body) {
List<PsiSwitchLabelStatement> labels = PsiTreeUtil.getChildrenOfTypeAsList(body, PsiSwitchLabelStatement.class);
return StreamEx.of(labels).pairMap((prev, next) -> {
if (prev.isDefaultCase()) {
Set<PsiSwitchLabelStatement> targets = getFallThroughTargets(body);
return targets.contains(prev) || targets.contains(next);
}
return false;
}).has(true);
}
@Override
@@ -147,24 +163,25 @@ public class ConvertSwitchToIfIntention implements IntentionAction {
ifStatementText = ";";
}
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
if (hadSideEffects) {
final PsiStatement declarationStatement = factory.createStatementFromText(declarationString, switchStatement);
switchStatement.getParent().addBefore(declarationStatement, switchStatement);
}
final PsiStatement ifStatement = factory.createStatementFromText(ifStatementText, switchStatement);
if (unwrapDefault) {
StringBuilder sb = new StringBuilder();
dumpBody(defaultBranch, sb, commentTracker);
PsiBlockStatement defaultBody = (PsiBlockStatement)factory.createStatementFromText(sb.toString(), switchStatement);
PsiCodeBlock parent = ObjectUtils.tryCast(switchStatement.getParent(), PsiCodeBlock.class);
PsiCodeBlock parent = ObjectUtils.tryCast(switchStatement.getParent(), PsiCodeBlock.class);
if (unwrapDefault || hadSideEffects) {
if (parent == null) {
commentTracker.grabComments(switchStatement);
switchStatement = BlockUtils.expandSingleStatementToBlockStatement(switchStatement);
parent = (PsiCodeBlock)(switchStatement.getParent());
body = Objects.requireNonNull(switchStatement.getBody());
}
}
if (hadSideEffects) {
final PsiStatement declarationStatement = factory.createStatementFromText(declarationString, switchStatement);
parent.addBefore(declarationStatement, switchStatement);
}
final PsiStatement ifStatement = factory.createStatementFromText(ifStatementText, switchStatement);
if (unwrapDefault) {
PsiElement addedIf = parent.addBefore(ifStatement, switchStatement);
if (!BlockUtils.containsConflictingDeclarations(body, parent)) {
StringBuilder sb = new StringBuilder();
dumpBody(defaultBranch, sb, commentTracker);
PsiBlockStatement defaultBody = (PsiBlockStatement)factory.createStatementFromText(sb.toString(), switchStatement);
if (!BlockUtils.containsConflictingDeclarations(Objects.requireNonNull(switchStatement.getBody()), parent)) {
BlockUtils.inlineCodeBlock(switchStatement, defaultBody.getCodeBlock());
}
else {

View File

@@ -0,0 +1,15 @@
// "Replace 'switch' with 'if'" "true"
class X {
int m(int i) {
if (i > 0) {
int i1 = ++i;
if (i1 == 1) {
System.out.println(1);
System.out.println(2);
} else if (i1 == 2) {
System.out.println(2);
}
}
}
}

View File

@@ -0,0 +1,12 @@
// "Replace 'switch' with 'if'" "true"
class X {
void m(String s) {
if ("foo".equals(s)) {
System.out.println(1);
} else if ("bar".equals(s)) {
System.out.println(3);
} else {
System.out.println(2);
}
}
}

View File

@@ -0,0 +1,12 @@
// "Replace 'switch' with 'if'" "true"
class X {
int m(int i) {
if (i > 0)
sw<caret>itch (++i) {
case 1:
System.out.println(1);
case 2:
System.out.println(2);
}
}
}

View File

@@ -0,0 +1,15 @@
// "Replace 'switch' with 'if'" "true"
class X {
void m(String s) {
swi<caret>tch (s) {
case "foo":
System.out.println(1);
break;
default:
System.out.println(2);
break;
case "bar":
System.out.println(3);
}
}
}

View File

@@ -0,0 +1,15 @@
// "Replace 'switch' with 'if'" "false"
class X {
void m(String s) {
swi<caret>tch (s) {
case "foo":
System.out.println(1);
default:
System.out.println(2);
break;
case "bar":
System.out.println(3);
break;
}
}
}

View File

@@ -0,0 +1,15 @@
// "Replace 'switch' with 'if'" "false"
class X {
void m(String s) {
swi<caret>tch (s) {
case "foo":
System.out.println(1);
break;
default:
System.out.println(2);
case "bar":
System.out.println(3);
break;
}
}
}

View File

@@ -29,7 +29,6 @@ import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.psiutils.BreakConverter;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -97,7 +96,7 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
// Empty switch is reported by another inspection
return;
}
registerStatementError(statement, Integer.valueOf(branches), BreakConverter.from(statement) != null);
registerStatementError(statement, Integer.valueOf(branches), ConvertSwitchToIfIntention.isAvailable(statement));
}
}

View File

@@ -15,9 +15,9 @@
*/
package com.siyeh.ipp.switchtoif;
import com.intellij.codeInsight.daemon.impl.quickfix.ConvertSwitchToIfIntention;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.siyeh.ig.psiutils.BreakConverter;
import com.siyeh.ipp.base.PsiElementPredicate;
import com.siyeh.ipp.psiutils.ErrorUtil;
import org.jetbrains.annotations.NotNull;
@@ -53,7 +53,7 @@ class SwitchPredicate implements PsiElementPredicate {
if (ErrorUtil.containsError(switchStatement)) {
return false;
}
if (BreakConverter.from(switchStatement) == null) {
if (!ConvertSwitchToIfIntention.isAvailable(switchStatement)) {
return false;
}
final PsiStatement[] statements = body.getStatements();