From c3a56381e7947c0413ab7331ef8b02b0f23c8b48 Mon Sep 17 00:00:00 2001 From: Ilyas Selimov Date: Tue, 16 Nov 2021 17:54:51 +0700 Subject: [PATCH] java - unwrap switch label fix: support used pattern variables in the switch GitOrigin-RevId: 47387f009a92a5e17846da4fa131f754234d7cf6 --- .../dataFlow/DataFlowInspectionBase.java | 5 +- .../impl/quickfix/UnwrapSwitchLabelFix.java | 168 ++++++++++++++++-- .../com/intellij/codeInsight/BlockUtils.java | 26 ++- .../afterUnwrapPatternConflict1.java | 14 ++ .../afterUnwrapPatternConflict2.java | 12 ++ .../afterUnwrapPatternConflict3.java | 9 + .../afterUnwrapPatternConflict4.java | 11 ++ .../afterUnwrapPatternConflict5.java | 10 ++ .../afterUnwrapPatternConflict6.java | 9 + .../afterUnwrapPatternConflict7.java | 11 ++ .../afterUnwrapPatternNoConflict1.java | 18 ++ .../afterUnwrapPatternNoConflict2.java | 16 ++ .../afterUnwrapPatternNoConflict3.java | 9 + ...ava => afterUnwrapPatternNoConflict4.java} | 0 .../afterUnwrapPatternNoConflict5.java | 9 + .../beforeUnwrapPatternConflict1.java | 17 ++ .../beforeUnwrapPatternConflict2.java | 19 ++ .../beforeUnwrapPatternConflict3.java | 12 ++ .../beforeUnwrapPatternConflict4.java | 20 +++ .../beforeUnwrapPatternConflict5.java | 13 ++ .../beforeUnwrapPatternConflict6.java | 12 ++ .../beforeUnwrapPatternConflict7.java | 13 ++ .../beforeUnwrapPatternNoConflict1.java | 23 +++ .../beforeUnwrapPatternNoConflict2.java | 25 +++ .../beforeUnwrapPatternNoConflict3.java | 19 ++ ...va => beforeUnwrapPatternNoConflict4.java} | 0 .../beforeUnwrapPatternNoConflict5.java | 12 ++ ...StatementWithTooFewBranchesInspection.java | 19 +- 28 files changed, 488 insertions(+), 43 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict3.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict4.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict5.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict6.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict7.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict3.java rename java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/{afterUnwrapPattern.java => afterUnwrapPatternNoConflict4.java} (100%) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict5.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict3.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict4.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict5.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict6.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict7.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict3.java rename java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/{beforeUnwrapPattern.java => beforeUnwrapPatternNoConflict4.java} (100%) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict5.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index dca77c08f1ce..352ad1131605 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -278,7 +278,7 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec private void createDescription(ProblemsHolder holder, final DataFlowInstructionVisitor visitor, - PsiElement scope, + PsiElement scope, Instruction @NotNull [] instructions) { ProblemReporter reporter = new ProblemReporter(holder, scope); @@ -351,8 +351,7 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec continue; } coveredSwitches.add(switchBlock); - holder.registerProblem(label, JavaAnalysisBundle.message("dataflow.message.only.switch.label"), - createUnwrapSwitchLabelFix()); + holder.registerProblem(label, JavaAnalysisBundle.message("dataflow.message.only.switch.label"), createUnwrapSwitchLabelFix()); } PsiSwitchBlock switchBlock = PsiTreeUtil.getParentOfType(labelReachability.keySet().iterator().next(), PsiSwitchBlock.class); Set suspiciousElements = SwitchBlockHighlightingModel.findSuspiciousLabelElements(switchBlock); diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java index 3c0232b195ad..7dd72735100b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java @@ -8,14 +8,20 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.dataFlow.fix.DeleteSwitchLabelFix; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.impl.PsiImplUtil; +import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.psi.util.JavaPsiPatternUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; -import com.siyeh.ig.controlflow.SwitchStatementWithTooFewBranchesInspection.UnwrapSwitchStatementFix; +import com.intellij.util.containers.JBIterable; import com.siyeh.ig.psiutils.BreakConverter; import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.VariableNameGenerator; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.List; import java.util.Objects; @@ -49,46 +55,180 @@ public class UnwrapSwitchLabelFix implements LocalQuickFix { new CommentTracker().deleteAndRestoreComments(labelElement); } } - tryUnwrap(labelStatement, block); + tryUnwrap(labelStatement, label, block); } - private static void tryUnwrap(PsiSwitchLabelStatementBase labelStatement, PsiSwitchBlock block) { + private static void tryUnwrap(@NotNull PsiSwitchLabelStatementBase labelStatement, @NotNull PsiCaseLabelElement label, + @NotNull PsiSwitchBlock block) { if (block instanceof PsiSwitchStatement) { BreakConverter converter = BreakConverter.from(block); if (converter == null) return; converter.process(); - unwrapStatement(labelStatement, (PsiSwitchStatement)block); - } else { - UnwrapSwitchStatementFix.unwrapExpression((PsiSwitchExpression)block); + unwrapStatement(labelStatement, label, (PsiSwitchStatement)block); + } + else { + unwrapExpression((PsiSwitchExpression)block, label); } } - private static void unwrapStatement(PsiSwitchLabelStatementBase labelStatement, PsiSwitchStatement statement) { - PsiCodeBlock block = statement.getBody(); + private static void unwrapStatement(@NotNull PsiSwitchLabelStatementBase labelStatement, @NotNull PsiCaseLabelElement label, + @NotNull PsiSwitchStatement switchStatement) { + PsiCodeBlock block = switchStatement.getBody(); PsiStatement body = labelStatement instanceof PsiSwitchLabeledRuleStatement ? ((PsiSwitchLabeledRuleStatement)labelStatement).getBody() : null; + PsiVariable variable = null; if (body == null) { + if (block != null) { + variable = createVariable(label, switchStatement, block); + } new CommentTracker().deleteAndRestoreComments(labelStatement); } else if (body instanceof PsiBlockStatement) { + variable = createVariable(label, switchStatement, body); block = ((PsiBlockStatement)body).getCodeBlock(); } else { + variable = createVariable(label, switchStatement, body); new CommentTracker().replaceAndRestoreComments(labelStatement, body); } - PsiCodeBlock parent = ObjectUtils.tryCast(statement.getParent(), PsiCodeBlock.class); + PsiCodeBlock parent = ObjectUtils.tryCast(switchStatement.getParent(), PsiCodeBlock.class); CommentTracker ct = new CommentTracker(); if (parent != null && !BlockUtils.containsConflictingDeclarations(Objects.requireNonNull(block), parent)) { - ct.grabComments(statement); + ct.grabComments(switchStatement); ct.markUnchanged(block); - ct.insertCommentsBefore(statement); - BlockUtils.inlineCodeBlock(statement, block); + ct.insertCommentsBefore(switchStatement); + PsiElement firstElementAdded = BlockUtils.inlineCodeBlock(switchStatement, block); + if (variable != null && firstElementAdded != null) { + addVariable(variable, firstElementAdded, parent); + } } else if (block != null) { - ct.replaceAndRestoreComments(statement, ct.text(block)); + if (variable != null) { + PsiStatement firstStatement = ArrayUtil.getFirstElement(block.getStatements()); + if (firstStatement != null) { + block.addBefore(variable, firstStatement); + } + } + ct.replaceAndRestoreComments(switchStatement, ct.text(block)); } else { - ct.deleteAndRestoreComments(statement); + ct.deleteAndRestoreComments(switchStatement); } } + + /** + * Unwraps switch expression if it consists of single expression-branch; does nothing otherwise + * + * @param switchExpression expression to unwrap + */ + public static void unwrapExpression(@NotNull PsiSwitchExpression switchExpression) { + unwrapExpression(switchExpression, null); + } + + private static void unwrapExpression(@NotNull PsiSwitchExpression switchExpression, @Nullable PsiCaseLabelElement label) { + PsiCodeBlock body = switchExpression.getBody(); + if (body == null) return; + PsiStatement[] statements = body.getStatements(); + if (statements.length != 1 || !(statements[0] instanceof PsiSwitchLabeledRuleStatement)) return; + PsiSwitchLabeledRuleStatement rule = (PsiSwitchLabeledRuleStatement)statements[0]; + PsiStatement ruleBody = rule.getBody(); + if (!(ruleBody instanceof PsiExpressionStatement)) return; + if (label == null) { + new CommentTracker().replaceAndRestoreComments(switchExpression, ((PsiExpressionStatement)ruleBody).getExpression()); + return; + } + PsiVariable variable = createVariable(label, switchExpression, ruleBody); + if (variable == null) { + new CommentTracker().replaceAndRestoreComments(switchExpression, ((PsiExpressionStatement)ruleBody).getExpression()); + return; + } + PsiElement parent = switchExpression, gParent = switchExpression.getParent(); + while (!(gParent instanceof PsiCodeBlock || gParent instanceof PsiClass)) { + if (parent instanceof PsiFile) { + gParent = null; + parent = null; + break; + } + parent = gParent; + gParent = parent.getParent(); + } + new CommentTracker().replaceAndRestoreComments(switchExpression, ((PsiExpressionStatement)ruleBody).getExpression()); + if (gParent != null) { + addVariable(variable, parent, gParent); + } + } + + /** + * @param label a switch label element + * @param switchBlock a considered switch block + * @param body a body of either switch labeled rule if switchBlock consists of labeled rules, + * or a body of the entire switch statement + * @return either field or a local variable extracted from a pattern variable if it's possible and necessary. + * If a pattern variable type is not total for selector type, a type cast expression will be created then. + */ + @Nullable + private static PsiVariable createVariable(@NotNull PsiCaseLabelElement label, + @NotNull PsiSwitchBlock switchBlock, + @NotNull PsiElement body) { + if (!(label instanceof PsiPattern)) return null; + PsiExpression selector = switchBlock.getExpression(); + if (selector == null) return null; + PsiType selectorType = selector.getType(); + if (selectorType == null) return null; + PsiPatternVariable patternVar = JavaPsiPatternUtil.getPatternVariable(((PsiPattern)label)); + if (patternVar == null) return null; + JBIterable bodyTraverser = SyntaxTraverser.psiTraverser(body).filter(PsiReferenceExpression.class); + PsiSwitchLabelStatementBase labelStatement = PsiTreeUtil.getParentOfType(label, PsiSwitchLabelStatementBase.class); + if (labelStatement instanceof PsiSwitchLabelStatement) { + bodyTraverser = bodyTraverser.filter(expr -> PsiTreeUtil.getParentOfType(expr, PsiSwitchLabelStatementBase.class) != labelStatement); + } + if (bodyTraverser.find(expr -> expr.resolve() == patternVar) == null) return null; + + String declarationStatementText = Objects.requireNonNull(patternVar.getPattern()).getText() + "="; + if (!JavaPsiPatternUtil.isTotalForType(patternVar.getPattern(), selectorType)) { + declarationStatementText += "(" + patternVar.getTypeElement().getType().getPresentableText() + ")"; + } + declarationStatementText += selector.getText() + ";"; + if (switchBlock instanceof PsiSwitchExpression) { + PsiElement parent = PsiTreeUtil.getParentOfType(switchBlock, PsiClass.class, true, PsiCodeBlock.class); + if (parent != null) { + return JavaPsiFacade.getInstance(label.getProject()).getParserFacade().createFieldFromText(declarationStatementText, label); + } + } + return (PsiLocalVariable)((PsiDeclarationStatement)JavaPsiFacade.getInstance(label.getProject()).getParserFacade() + .createStatementFromText(declarationStatementText, label)).getDeclaredElements()[0]; + } + + /** + * If there is no naming conflict between variable and existing ones, the name of variable leaves as is. + * Otherwise, we need to select a new unique name for variable and rename its references as well. + * + * @param variable variable to add + * @param variableSibling is used as anchor for variable to be added + * @param variableParent parent for both variable and variableSibling. + * Mostly used to detect a conflict and to add variable as a child. + */ + private static void addVariable(@NotNull PsiVariable variable, @NotNull PsiElement variableSibling, @NotNull PsiElement variableParent) { + boolean hasConflictingDeclaration = hasConflictingDeclaration(variable, variableParent); + if (variable instanceof PsiLocalVariable) { + PsiStatement declaration = JavaPsiFacade.getInstance(variableSibling.getProject()).getParserFacade() + .createStatementFromText(variable.getText(), variableSibling); + variable = (PsiVariable)((PsiDeclarationStatement)variableParent.addBefore(declaration, variableSibling)).getDeclaredElements()[0]; + } + else { + variable = (PsiVariable)variableParent.addBefore(variable, variableSibling); + } + if (!hasConflictingDeclaration) return; + String newVarName = new VariableNameGenerator(variableParent, variableParent instanceof PsiClass ? VariableKind.FIELD : VariableKind.LOCAL_VARIABLE) + .byName(variable.getName()).generate(true); + for (PsiReference ref : ReferencesSearch.search(Objects.requireNonNull(variable), variable.getUseScope())) { + ref.handleElementRename(newVarName); + } + variable.setName(newVarName); + } + + private static boolean hasConflictingDeclaration(@NotNull PsiVariable var, @NotNull PsiElement context) { + return !new VariableNameGenerator(context, var instanceof PsiField ? VariableKind.FIELD : VariableKind.LOCAL_VARIABLE) + .byName(var.getName()).generate(true).equals(var.getName()); + } } diff --git a/java/java-psi-impl/src/com/intellij/codeInsight/BlockUtils.java b/java/java-psi-impl/src/com/intellij/codeInsight/BlockUtils.java index dd7706ce8b6c..68339a70c1b3 100644 --- a/java/java-psi-impl/src/com/intellij/codeInsight/BlockUtils.java +++ b/java/java-psi-impl/src/com/intellij/codeInsight/BlockUtils.java @@ -7,6 +7,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.PsiUtilCore; import com.intellij.util.SmartList; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -199,22 +200,29 @@ public final class BlockUtils { return false; } - public static void inlineCodeBlock(@NotNull PsiStatement orig, PsiCodeBlock codeBlock) { + /** + * Method inlines the statements inside codeBlock replacing statement. + * + * @param statement statement to delete + * @param codeBlock a block with statements to inline + * @return a first inlined statement, or null if codeBlock doesn't contain any statements. + */ + @Nullable + @Contract(pure = true) + public static PsiElement inlineCodeBlock(@NotNull PsiStatement statement, @NotNull PsiCodeBlock codeBlock) { PsiJavaToken lBrace = codeBlock.getLBrace(); PsiJavaToken rBrace = codeBlock.getRBrace(); - if (lBrace == null || rBrace == null) return; + if (lBrace == null || rBrace == null) return null; final PsiElement[] children = codeBlock.getChildren(); + PsiElement added = null; if (children.length > 2) { - final PsiElement added = - orig.getParent().addRangeBefore( - children[1], - children[children.length - 2], - orig); - final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(orig.getManager()); + added = statement.getParent().addRangeBefore(children[1], children[children.length - 2], statement); + final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(statement.getManager()); codeStyleManager.reformat(added); } - orig.delete(); + statement.delete(); + return added; } public static PsiBlockStatement createBlockStatement(Project project) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict1.java new file mode 100644 index 000000000000..df60ca0022b3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict1.java @@ -0,0 +1,14 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + { + Integer i = (Integer) n; + int j = 1; + System.out.println(i); + } + { + Object j = ""; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict2.java new file mode 100644 index 000000000000..73b1db3aebf7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict2.java @@ -0,0 +1,12 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + { + Integer i = (Integer) n; + int j = 1; + System.out.println(i); + } + Object j = ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict3.java new file mode 100644 index 000000000000..2e8c420ad285 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict3.java @@ -0,0 +1,9 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + Integer i1 = (Integer) n; + System.out.println(i1); + Object i = ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict4.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict4.java new file mode 100644 index 000000000000..3559c923be58 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict4.java @@ -0,0 +1,11 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + Integer i1 = (Integer) n; + System.out.println(i1); + { + Object i = ""; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict5.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict5.java new file mode 100644 index 000000000000..a7a1fb68e1d3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict5.java @@ -0,0 +1,10 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + Integer i1 = (Integer) n; + int result = i1 + 10; + int i = 5; + System.out.println(result + i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict6.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict6.java new file mode 100644 index 000000000000..0684712dfaac --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict6.java @@ -0,0 +1,9 @@ +// "Remove unreachable branches" "true" +class Test { + Number n = 1; + Integer i = (Integer) n; + int result = i + 10; + { + int i = 5; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict7.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict7.java new file mode 100644 index 000000000000..e26d3d20dfed --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternConflict7.java @@ -0,0 +1,11 @@ +// "Remove unreachable branches" "true" +class Test { + + int test(Number n) { + n = 1; + Object i1 = n; + System.out.println((int) i1 + 10); + int i = 5; + System.out.println(i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict1.java new file mode 100644 index 000000000000..45247209863b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict1.java @@ -0,0 +1,18 @@ +// "Remove unreachable branches" "true" +import java.util.concurrent.ThreadLocalRandom; + +class Test { + + void test(Number n) { + n = 1; + Integer i = (Integer) n; + int rand = ThreadLocalRandom.current().nextInt(); + if (rand > 10) { + i = 2; + } + else { + i = 3; + } + System.out.println(i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict2.java new file mode 100644 index 000000000000..f0e81f007d50 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict2.java @@ -0,0 +1,16 @@ +// "Remove unreachable branches" "true" +import java.util.concurrent.ThreadLocalRandom; +class Test { + void test(Number n) { + n = 1; + Integer i = (Integer) n; + int rand = ThreadLocalRandom.current().nextInt(); + if (rand > 10) { + i = 2; + } + else { + i = 3; + } + System.out.println(i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict3.java new file mode 100644 index 000000000000..382c2bb43402 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict3.java @@ -0,0 +1,9 @@ +// "Remove unreachable branches" "true" + +class Test { + final String s = "abc"; + + void test() { + System.out.println(1); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPattern.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict4.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPattern.java rename to java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict4.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict5.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict5.java new file mode 100644 index 000000000000..0ffb186cbb34 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrapPatternNoConflict5.java @@ -0,0 +1,9 @@ +// "Remove unreachable branches" "true" +class Test { + + int test(Number n) { + n = 1; + Integer i = (Integer) n; + return 1 + i; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict1.java new file mode 100644 index 000000000000..0ac3c9a11169 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict1.java @@ -0,0 +1,17 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1 -> { + int j = 1; + System.out.println(i); + } + case Long s -> System.out.println(s); + default -> System.out.println(); + } + { + Object j = ""; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict2.java new file mode 100644 index 000000000000..1c27d29e639f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict2.java @@ -0,0 +1,19 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1: + int j = 1; + System.out.println(i); + break; + case Long s: + System.out.println(s); + break; + default: + System.out.println(); + break; + } + Object j = ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict3.java new file mode 100644 index 000000000000..7449b8a00355 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict3.java @@ -0,0 +1,12 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1 -> System.out.println(i); + case Long s -> System.out.println(s); + default -> System.out.println(); + } + Object i = ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict4.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict4.java new file mode 100644 index 000000000000..f0d5da859b4d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict4.java @@ -0,0 +1,20 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1: + System.out.println(i); + break; + case Long s: + System.out.println(s); + break; + default: + System.out.println(); + break; + } + { + Object i = ""; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict5.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict5.java new file mode 100644 index 000000000000..aeb5fc08016f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict5.java @@ -0,0 +1,13 @@ +// "Remove unreachable branches" "true" +class Test { + void test(Number n) { + n = 1; + int result = switch (n) { + case Integer i && i == 1 -> i; + case Long l -> l.intValue(); + case default -> 1; + } + 10; + int i = 5; + System.out.println(result + i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict6.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict6.java new file mode 100644 index 000000000000..0c0bba88447e --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict6.java @@ -0,0 +1,12 @@ +// "Remove unreachable branches" "true" +class Test { + Number n = 1; + int result = switch (n) { + case Integer i && i == 1 -> i; + case Long l -> l.intValue(); + case default -> 1; + } + 10; + { + int i = 5; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict7.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict7.java new file mode 100644 index 000000000000..1039f89648de --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternConflict7.java @@ -0,0 +1,13 @@ +// "Remove unreachable branches" "true" +class Test { + + int test(Number n) { + n = 1; + System.out.println(switch (n) { + case Long l -> l.intValue(); + case Object i -> (int)i; + } + 10); + int i = 5; + System.out.println(i); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict1.java new file mode 100644 index 000000000000..239c5938217f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict1.java @@ -0,0 +1,23 @@ +// "Remove unreachable branches" "true" +import java.util.concurrent.ThreadLocalRandom; + +class Test { + + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1 -> { + int rand = ThreadLocalRandom.current().nextInt(); + if (rand > 10) { + i = 2; + } + else { + i = 3; + } + System.out.println(i); + } + case Long s -> System.out.println(s); + default -> System.out.println(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict2.java new file mode 100644 index 000000000000..7fb12789acb7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict2.java @@ -0,0 +1,25 @@ +// "Remove unreachable branches" "true" +import java.util.concurrent.ThreadLocalRandom; +class Test { + void test(Number n) { + n = 1; + switch (n) { + case Integer i && i == 1: + int rand = ThreadLocalRandom.current().nextInt(); + if (rand > 10) { + i = 2; + } + else { + i = 3; + } + System.out.println(i); + break; + case Long s: + System.out.println(s); + break; + default: + System.out.println(); + break; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict3.java new file mode 100644 index 000000000000..79050fc76507 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict3.java @@ -0,0 +1,19 @@ +// "Remove unreachable branches" "true" + +class Test { + final String s = "abc"; + + void test() { + switch (s) { + case String ss && ss.length() <= 3: + System.out.println(1); + break; + case "fsd": + System.out.println(2); + break; + case default: + System.out.println(3); + break; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPattern.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict4.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPattern.java rename to java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict4.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict5.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict5.java new file mode 100644 index 000000000000..2d0b5123bff9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrapPatternNoConflict5.java @@ -0,0 +1,12 @@ +// "Remove unreachable branches" "true" +class Test { + + int test(Number n) { + n = 1; + return 1 + switch (n) { + case Integer i && i == 1 -> i; + case Long l -> l.intValue(); + case default -> 1; + }; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java index ad98fdfa67f0..b1124d52db52 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java @@ -16,6 +16,7 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInsight.daemon.impl.quickfix.ConvertSwitchToIfIntention; +import com.intellij.codeInsight.daemon.impl.quickfix.UnwrapSwitchLabelFix; import com.intellij.codeInspection.CommonQuickFixBundle; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ui.SingleIntegerFieldOptionsPanel; @@ -26,7 +27,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.CommentTracker; import com.siyeh.ig.psiutils.SwitchUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -146,23 +146,8 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection if (block instanceof PsiSwitchStatement) { ConvertSwitchToIfIntention.doProcessIntention((PsiSwitchStatement)block); } else if (block instanceof PsiSwitchExpression) { - unwrapExpression((PsiSwitchExpression)block); + UnwrapSwitchLabelFix.unwrapExpression((PsiSwitchExpression)block); } } - - /** - * Unwraps switch expression if it consists of single expression-branch; does nothing otherwise - * @param switchExpression expression to unwrap - */ - public static void unwrapExpression(PsiSwitchExpression switchExpression) { - PsiCodeBlock body = switchExpression.getBody(); - if (body == null) return; - PsiStatement[] statements = body.getStatements(); - if (statements.length != 1 || !(statements[0] instanceof PsiSwitchLabeledRuleStatement)) return; - PsiSwitchLabeledRuleStatement rule = (PsiSwitchLabeledRuleStatement)statements[0]; - PsiStatement ruleBody = rule.getBody(); - if (!(ruleBody instanceof PsiExpressionStatement)) return; - new CommentTracker().replaceAndRestoreComments(switchExpression, ((PsiExpressionStatement)ruleBody).getExpression()); - } } }