diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ConvertSwitchToIfIntention.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ConvertSwitchToIfIntention.java index 35c8ff64d479..e413070058ee 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ConvertSwitchToIfIntention.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ConvertSwitchToIfIntention.java @@ -11,6 +11,7 @@ import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.psi.util.JavaPsiPatternUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; @@ -21,6 +22,7 @@ import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.stream.Collectors; @@ -108,13 +110,13 @@ public class ConvertSwitchToIfIntention implements IntentionActionWithFixAllOpti BreakConverter converter = BreakConverter.from(switchStatement); if (converter == null) return; converter.process(); - final List allBranches = extractBranches(commentTracker, body, fallThroughTargets); + final List allBranches = extractBranches(body, fallThroughTargets); final String declarationString; final boolean hadSideEffects; final String expressionText; final Project project = switchStatement.getProject(); - int totalCases = allBranches.stream().mapToInt(br -> br.getCaseValues().size()).sum(); + int totalCases = allBranches.stream().mapToInt(br -> br.getCaseElements().size()).sum(); if (totalCases > 0) { commentTracker.markUnchanged(switchExpression); } @@ -200,8 +202,7 @@ public class ConvertSwitchToIfIntention implements IntentionActionWithFixAllOpti } @NotNull - private static List extractBranches(CommentTracker commentTracker, - PsiCodeBlock body, + private static List extractBranches(PsiCodeBlock body, Set fallThroughTargets) { final List openBranches = new ArrayList<>(); final Set declaredElements = new HashSet<>(); @@ -231,7 +232,7 @@ public class ConvertSwitchToIfIntention implements IntentionActionWithFixAllOpti if (label.isDefaultCase() && defaultAlwaysExecuted) { openBranches.retainAll(Collections.singleton(currentBranch)); } - currentBranch.addCaseValues(label, defaultAlwaysExecuted, commentTracker); + currentBranch.addCaseValues(label, defaultAlwaysExecuted); } else if (statement instanceof PsiSwitchLabeledRuleStatement) { openBranches.clear(); @@ -242,7 +243,7 @@ public class ConvertSwitchToIfIntention implements IntentionActionWithFixAllOpti if (ruleBody != null) { currentBranch.addStatement(ruleBody); } - currentBranch.addCaseValues(rule, defaultAlwaysExecuted, commentTracker); + currentBranch.addCaseValues(rule, defaultAlwaysExecuted); openBranches.add(currentBranch); allBranches.add(currentBranch); } @@ -287,37 +288,96 @@ public class ConvertSwitchToIfIntention implements IntentionActionWithFixAllOpti if (!firstBranch) { out.append("else "); } - dumpCaseValues(expressionText, branch.getCaseValues(), useEquals, out); + dumpCaseValues(expressionText, branch.getCaseElements(), useEquals, commentTracker, out); dumpBody(branch, out, commentTracker); } private static void dumpCaseValues(String expressionText, - List caseValues, + List caseElements, boolean useEquals, + CommentTracker commentTracker, @NonNls StringBuilder out) { out.append("if("); boolean firstCaseValue = true; - for (String caseValue : caseValues) { + for (PsiElement caseElement : caseElements) { if (!firstCaseValue) { out.append("||"); } firstCaseValue = false; - if (useEquals) { - out.append(caseValue).append(".equals(").append(expressionText).append(')'); + if (caseElement instanceof PsiExpression) { + PsiExpression caseExpression = (PsiExpression)caseElement; + String caseValue = getCaseValueText(caseExpression, commentTracker); + if (useEquals && !ExpressionUtils.isNullLiteral(caseExpression)) { + out.append(caseValue).append(".equals(").append(expressionText).append(')'); + } + else if (caseValue.equals("true")) { + out.append(expressionText); + } + else if (caseValue.equals("false")) { + out.append("!(").append(expressionText).append(")"); + } + else { + out.append(expressionText).append("==").append(caseValue); + } } - else if (caseValue.equals("true")) { - out.append(expressionText); - } - else if (caseValue.equals("false")) { - out.append("!(").append(expressionText).append(")"); - } - else { - out.append(expressionText).append("==").append(caseValue); + else if (caseElement instanceof PsiPattern) { + out.append(createIfCondition((PsiPattern)caseElement, expressionText, commentTracker)); } } out.append(')'); } + private static @Nullable String createIfCondition(PsiPattern pattern, String expressionText, CommentTracker commentTracker){ + PsiPattern normalizedPattern = JavaPsiPatternUtil.skipParenthesizedPatternDown(pattern); + if (normalizedPattern instanceof PsiTypeTestPattern) { + return createIfCondition((PsiTypeTestPattern)normalizedPattern, expressionText, commentTracker); + } + else if (normalizedPattern instanceof PsiGuardedPattern) { + PsiGuardedPattern guardedPattern = (PsiGuardedPattern)normalizedPattern; + PsiPattern primaryPattern = JavaPsiPatternUtil.skipParenthesizedPatternDown(guardedPattern.getPrimaryPattern()); + if (!(primaryPattern instanceof PsiTypeTestPattern)) return null; + String primaryCondition = createIfCondition((PsiTypeTestPattern)primaryPattern, expressionText, commentTracker); + String guardExpression = commentTracker.textWithComments(guardedPattern.getGuardingExpression()); + return primaryCondition + "&&" + guardExpression; + } + return null; + } + + @Nullable + private static String createIfCondition(PsiTypeTestPattern normalizedPattern, String expressionText, CommentTracker commentTracker) { + PsiTypeTestPattern typeTestPattern = normalizedPattern; + PsiTypeElement checkType = typeTestPattern.getCheckType(); + String typeText = commentTracker.textWithComments(checkType); + PsiPatternVariable variable = typeTestPattern.getPatternVariable(); + PsiElement context = PsiTreeUtil.getParentOfType(variable, PsiSwitchStatement.class); + boolean isUsedPatternVariable = context != null && VariableAccessUtils.variableIsUsed(variable, context); + PsiIdentifier identifier = variable.getNameIdentifier(); + String variableName = isUsedPatternVariable ? commentTracker.textWithComments(identifier) : commentTracker.commentsBefore(identifier); + return expressionText + " instanceof " + typeText + " " + variableName; + } + + private static String getCaseValueText(PsiExpression value, CommentTracker commentTracker) { + value = PsiUtil.skipParenthesizedExprDown(value); + if (value == null) { + return ""; + } + if (!(value instanceof PsiReferenceExpression)) { + return commentTracker.text(value); + } + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)value; + final PsiElement target = referenceExpression.resolve(); + + if (!(target instanceof PsiEnumConstant)) { + return commentTracker.text(value); + } + final PsiEnumConstant enumConstant = (PsiEnumConstant)target; + final PsiClass aClass = enumConstant.getContainingClass(); + if (aClass == null) { + return commentTracker.text(value); + } + return aClass.getQualifiedName() + '.' + commentTracker.text(referenceExpression); + } + private static void dumpBody(SwitchStatementBranch branch, @NonNls StringBuilder out, CommentTracker commentTracker) { final List bodyStatements = branch.getBodyElements(); out.append('{'); diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SwitchStatementBranch.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SwitchStatementBranch.java index 6aa712b5e50e..e68ad233bb1a 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SwitchStatementBranch.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SwitchStatementBranch.java @@ -16,8 +16,7 @@ package com.intellij.codeInsight.daemon.impl.quickfix; import com.intellij.psi.*; -import com.intellij.psi.util.PsiUtil; -import com.siyeh.ig.psiutils.CommentTracker; +import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.ControlFlowUtils; import java.util.*; @@ -25,15 +24,15 @@ import java.util.*; class SwitchStatementBranch { private final Set myPendingDeclarations = new HashSet<>(5); - private final List myCaseValues = new ArrayList<>(2); + private final List myCaseElements = new ArrayList<>(2); private final List myBodyElements = new ArrayList<>(5); private final List myPendingWhiteSpace = new ArrayList<>(2); private boolean myDefault; private boolean myHasStatements; private boolean myAlwaysExecuted; - public void addCaseValue(String labelString) { - myCaseValues.add(labelString); + public void addCaseValue(PsiElement caseElement) { + myCaseElements.add(caseElement); } public void addStatement(PsiStatement statement) { @@ -57,8 +56,8 @@ class SwitchStatementBranch { } } - public List getCaseValues() { - return Collections.unmodifiableList(myCaseValues); + public List getCaseElements() { + return Collections.unmodifiableList(myCaseElements); } public List getBodyElements() { @@ -93,8 +92,8 @@ class SwitchStatementBranch { return Collections.unmodifiableSet(myPendingDeclarations); } - void addCaseValues(PsiSwitchLabelStatementBase label, boolean defaultAlwaysExecuted, CommentTracker commentTracker) { - if (label.isDefaultCase()) { + void addCaseValues(PsiSwitchLabelStatementBase label, boolean defaultAlwaysExecuted) { + if (isDefaultLabel(label)) { setDefault(); setAlwaysExecuted(defaultAlwaysExecuted); } @@ -102,34 +101,16 @@ class SwitchStatementBranch { PsiCaseLabelElementList labelElementList = label.getCaseLabelElementList(); if (labelElementList != null) { for (PsiCaseLabelElement labelElement : labelElementList.getElements()) { - if (labelElement instanceof PsiExpression) { - final String valueText = getCaseValueText((PsiExpression)labelElement, commentTracker); - addCaseValue(valueText); - } + addCaseValue(labelElement); } } } } - private static String getCaseValueText(PsiExpression value, CommentTracker commentTracker) { - value = PsiUtil.skipParenthesizedExprDown(value); - if (value == null) { - return ""; - } - if (!(value instanceof PsiReferenceExpression)) { - return commentTracker.text(value); - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)value; - final PsiElement target = referenceExpression.resolve(); - - if (!(target instanceof PsiEnumConstant)) { - return commentTracker.text(value); - } - final PsiEnumConstant enumConstant = (PsiEnumConstant)target; - final PsiClass aClass = enumConstant.getContainingClass(); - if (aClass == null) { - return commentTracker.text(value); - } - return aClass.getQualifiedName() + '.' + commentTracker.text(referenceExpression); + private static boolean isDefaultLabel(PsiSwitchLabelStatementBase label) { + if (label.isDefaultCase()) return true; + PsiCaseLabelElementList labelElementList = label.getCaseLabelElementList(); + if (labelElementList == null) return false; + return ContainerUtil.exists(labelElementList.getElements(), element -> element instanceof PsiDefaultCaseLabelElement); } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterCaseDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterCaseDefault.java new file mode 100644 index 000000000000..e70860af304f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterCaseDefault.java @@ -0,0 +1,12 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(Object o) { + if (o instanceof String) { + System.out.println("one"); + } else { + System.out.println("default"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterPatternCheck.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterPatternCheck.java new file mode 100644 index 000000000000..cf56ff9f52f4 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterPatternCheck.java @@ -0,0 +1,18 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(Object o) { + if (o == null || o instanceof String) { + System.out.println("one"); + } else if (o instanceof Integer i && (i > 0)) { + System.out.println("two"); + } else if (o instanceof /*1*/ Float /*2*/ /*3*/f && f/*4*/ > 5 && f < 10) { + System.out.println("two"); + } else if (o instanceof Character c) { + System.out.println(c); + } else if (o instanceof Double) { + System.out.println(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterStringWithNullCheck.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterStringWithNullCheck.java new file mode 100644 index 000000000000..2171a932fa13 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/afterStringWithNullCheck.java @@ -0,0 +1,12 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(String s) { + if (s == null || "zero".equals(s)) { + System.out.println(0); + } else if ("one".equals(s)) { + System.out.println(1); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeCaseDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeCaseDefault.java new file mode 100644 index 000000000000..b8611d1fba5f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeCaseDefault.java @@ -0,0 +1,11 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(Object o) { + switch (o) { + case String s -> System.out.println("one"); + case null, default -> System.out.println("default"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforePatternCheck.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforePatternCheck.java new file mode 100644 index 000000000000..9545e2697f12 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforePatternCheck.java @@ -0,0 +1,15 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(Object o) { + switch (o) { + case null, String s -> System.out.println("one"); + case ((Integer i) && (i > 0)) -> System.out.println("two"); + case /*1*/Float/*2*/ /*3*/f && f/*4*/ > 5 && f < 10 -> System.out.println("two"); + case Character c -> System.out.println(c); + case Double c -> System.out.println(); + default -> {} + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeStringWithNullCheck.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeStringWithNullCheck.java new file mode 100644 index 000000000000..182a804566d6 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertSwitchToIf/beforeStringWithNullCheck.java @@ -0,0 +1,12 @@ +// "Replace 'switch' with 'if'" "true" +abstract class Test { + abstract Object getObject(); + + void foo(String s) { + switch (s) { + case null, "zero" -> System.out.println(0); + case "one" -> System.out.println(1); + default -> {} + } + } +} \ No newline at end of file