From d3a85ece738673fa8eda5a6edd9aef5306e7f01e Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Tue, 6 Nov 2018 17:59:00 +0300 Subject: [PATCH] Java: Highlight identical branches in 'switch' statement, quick fix implemented (IDEA-181304) --- .../DuplicateBranchesInSwitchInspection.java | 167 +++++++++++++++++- .../afterBreakAndReturnUnderIf.java | 22 +++ .../afterComplexBranches.java | 25 +++ .../afterContinue.java | 20 +++ .../afterFallThroughToBreak.java | 16 ++ .../afterFallThroughToBreak2.java | 17 ++ .../afterLeftoverComments.java | 14 ++ .../afterManyComments.java | 13 ++ .../afterMethodCallInReturn.java | 19 ++ .../afterNoLastBreak.java | 15 ++ .../afterReturn.java | 13 ++ .../afterSameCommentAfterLabel.java | 12 ++ .../afterSameCommentBeforeLabel.java | 13 ++ .../afterSimple.java | 15 ++ .../afterThreeDuplicates.java | 18 ++ .../afterThrow.java | 13 ++ .../afterTwoCaseLabels.java | 16 ++ .../beforeBreakAndReturnUnderIf.java | 27 +++ .../beforeComplexBranches.java | 32 ++++ .../beforeContinue.java | 22 +++ .../beforeFallThroughToBreak.java | 18 ++ .../beforeFallThroughToBreak2.java | 18 ++ .../beforeLeftoverComments.java | 15 ++ .../beforeManyComments.java | 16 ++ .../beforeMethodCallInReturn.java | 20 +++ .../beforeNoLastBreak.java | 16 ++ .../beforeReturn.java | 14 ++ .../beforeSameCommentAfterLabel.java | 14 ++ .../beforeSameCommentBeforeLabel.java | 16 ++ .../beforeSimple.java | 17 ++ .../beforeThreeDuplicates.java | 20 +++ .../beforeThrow.java | 14 ++ .../beforeTwoCaseLabels.java | 18 ++ .../DuplicateBranchesInSwitchFixTest.kt | 17 ++ .../src/messages/InspectionsBundle.properties | 4 +- 35 files changed, 741 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterBreakAndReturnUnderIf.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterComplexBranches.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterContinue.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak2.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterLeftoverComments.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterManyComments.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterMethodCallInReturn.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterNoLastBreak.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterReturn.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentAfterLabel.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentBeforeLabel.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSimple.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThreeDuplicates.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThrow.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterTwoCaseLabels.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeBreakAndReturnUnderIf.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeComplexBranches.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeContinue.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak2.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeLeftoverComments.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeManyComments.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeMethodCallInReturn.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeNoLastBreak.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeReturn.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentAfterLabel.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentBeforeLabel.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSimple.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThreeDuplicates.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThrow.java create mode 100644 java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeTwoCaseLabels.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateBranchesInSwitchFixTest.kt diff --git a/java/java-impl/src/com/intellij/codeInspection/DuplicateBranchesInSwitchInspection.java b/java/java-impl/src/com/intellij/codeInspection/DuplicateBranchesInSwitchInspection.java index 20857d8958c7..89cedf1fcf46 100644 --- a/java/java-impl/src/com/intellij/codeInspection/DuplicateBranchesInSwitchInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/DuplicateBranchesInSwitchInspection.java @@ -11,13 +11,18 @@ import com.intellij.refactoring.util.duplicates.DuplicatesFinder; import com.intellij.refactoring.util.duplicates.Match; import com.intellij.refactoring.util.duplicates.ReturnValue; import com.intellij.util.ArrayUtil; +import com.intellij.util.containers.ContainerUtil; +import com.siyeh.ig.psiutils.CommentTracker; import com.siyeh.ig.psiutils.ControlFlowUtils; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; import static com.siyeh.ig.migration.TryWithIdenticalCatchesInspection.collectCommentTexts; +import static com.siyeh.ig.migration.TryWithIdenticalCatchesInspection.getCommentText; /** * @author Pavel.Dolgov @@ -57,11 +62,11 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool { if (areDuplicates(branch, otherBranch)) { isDuplicate[otherIndex] = true; - registerProblem(otherBranch.myStatements); + registerProblem(otherBranch.myStatements, branch.getSwitchLabelText()); if (!isDuplicate[index]) { isDuplicate[index] = true; - registerProblem(branch.myStatements); + registerProblem(branch.myStatements, null); } } } @@ -69,11 +74,12 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool { } } - private void registerProblem(@NotNull PsiStatement[] statements) { + private void registerProblem(@NotNull PsiStatement[] statements, String switchLabelText) { ProblemDescriptor descriptor = InspectionManager.getInstance(myHolder.getProject()) .createProblemDescriptor(statements[0], statements[statements.length - 1], InspectionsBundle.message("inspection.duplicate.branches.in.switch.message"), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myHolder.isOnTheFly()); + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myHolder.isOnTheFly(), + switchLabelText != null ? new MergeBranchesFix(switchLabelText) : null); myHolder.registerProblem(descriptor); } } @@ -156,10 +162,117 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool { return statement == null || isBreakWithoutLabel(statement); } + @Contract("null -> false") private static boolean isBreakWithoutLabel(@Nullable PsiStatement statement) { return statement instanceof PsiBreakStatement && ((PsiBreakStatement)statement).getLabelIdentifier() == null; } + private static class MergeBranchesFix implements LocalQuickFix { + @NotNull private final String mySwitchLabelText; + + MergeBranchesFix(@NotNull String switchLabelText) { + mySwitchLabelText = switchLabelText; + } + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.duplicate.branches.in.switch.fix.family.name"); + } + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getName() { + return InspectionsBundle.message("inspection.duplicate.branches.in.switch.fix.name", mySwitchLabelText); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement startElement = descriptor.getStartElement(); + PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(startElement, PsiSwitchStatement.class); + if (switchStatement == null) return; + + Branch branchToDelete = null; + List candidateBranches = null; + for (List branches : collectSameLengthBranches(switchStatement)) { + branchToDelete = ContainerUtil.find(branches, branch -> branch.myStatements[0] == startElement); + if (branchToDelete != null) { + candidateBranches = branches; + break; + } + } + + if (branchToDelete == null) return; + + Branch branchToMergeWith = null; + for (Branch branch : candidateBranches) { + if (mySwitchLabelText.equals(branch.getSwitchLabelText()) && areDuplicates(branchToDelete, branch)) { + branchToMergeWith = branch; + break; + } + } + if (branchToMergeWith == null) return; + + List branchPrefixToMove = branchToDelete.getBranchPrefix(); + if (branchPrefixToMove.isEmpty()) return; + + PsiSwitchLabelStatement labelToMergeWith = + PsiTreeUtil.getPrevSiblingOfType(branchToMergeWith.myStatements[0], PsiSwitchLabelStatement.class); + if (labelToMergeWith == null) return; + + PsiElement oldNextElement = PsiTreeUtil.skipWhitespacesForward(labelToMergeWith); + + PsiElement firstElementToMove = branchPrefixToMove.get(0); + PsiElement lastElementToMove = branchPrefixToMove.get(branchPrefixToMove.size() - 1); + labelToMergeWith.getParent().addRangeAfter(firstElementToMove, lastElementToMove, labelToMergeWith); + firstElementToMove.getParent().deleteChildRange(firstElementToMove, lastElementToMove); + + Set commentsToMergeWith = ContainerUtil.set(branchToMergeWith.myCommentTexts); + deleteRedundantComments(labelToMergeWith.getNextSibling(), oldNextElement, commentsToMergeWith); + + CommentTracker tracker = new CommentTracker(); + PsiStatement[] statementsToDelete = branchToDelete.getStatementsToDelete(); + for (PsiStatement statement : statementsToDelete) { + PsiTreeUtil.processElements(statement, child -> { + if (isRedundantComment(commentsToMergeWith, child)) { + tracker.markUnchanged(child); + } + return true; + }); + } + for (int i = 0; i < statementsToDelete.length - 1; i++) { + tracker.delete(statementsToDelete[i]); + } + tracker.deleteAndRestoreComments(statementsToDelete[statementsToDelete.length - 1]); + } + + private static void deleteRedundantComments(@Nullable PsiElement startElement, + @Nullable PsiElement stopElement, + @NotNull Set existingComments) { + List redundantComments = new ArrayList<>(); + for (PsiElement element = startElement; element != null && element != stopElement; element = element.getNextSibling()) { + PsiTreeUtil.processElements(element, child -> { + if (isRedundantComment(existingComments, child)) { + redundantComments.add(child); + } + return true; + }); + } + redundantComments.forEach(PsiElement::delete); + } + + @Contract("_,null -> false") + private static boolean isRedundantComment(@NotNull Set existingComments, @Nullable PsiElement element) { + if (element instanceof PsiComment) { + String text = getCommentText((PsiComment)element); + return text.isEmpty() || existingComments.contains(text); + } + return false; + } + } + private static class Branch { private final PsiStatement[] myStatements; private final String[] myCommentTexts; @@ -197,6 +310,52 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool { return myStatements.length; } + @Nullable + String getSwitchLabelText() { + PsiSwitchLabelStatement switchLabel = null; + for (PsiStatement statement = PsiTreeUtil.getPrevSiblingOfType(myStatements[0], PsiStatement.class); + statement instanceof PsiSwitchLabelStatement; + statement = PsiTreeUtil.getPrevSiblingOfType(statement, PsiStatement.class)) { + switchLabel = (PsiSwitchLabelStatement)statement; + } + + if (switchLabel != null) { + if (switchLabel.isDefaultCase()) { + return PsiKeyword.DEFAULT; + } + PsiExpression value = switchLabel.getCaseValue(); + if (value != null) { + return PsiKeyword.CASE + ' ' + value.getText(); + } + } + return null; + } + + /** + * switch labels with comments and spaces + */ + @NotNull + List getBranchPrefix() { + List result = new ArrayList<>(); + for (PsiElement element = myStatements[0].getPrevSibling(); + element != null && (element instanceof PsiSwitchLabelStatement || !(element instanceof PsiStatement)); + element = element.getPrevSibling()) { + result.add(element); + } + Collections.reverse(result); + return result; + } + + PsiStatement[] getStatementsToDelete() { + PsiStatement nextStatement = PsiTreeUtil.getNextSiblingOfType(myStatements[myStatements.length - 1], PsiStatement.class); + if (isBreakWithoutLabel(nextStatement)) { + PsiStatement[] statements = Arrays.copyOf(myStatements, myStatements.length + 1); + statements[myStatements.length] = nextStatement; // it's the trailing 'break' + return statements; + } + return myStatements; + } + @NotNull private DuplicatesFinder getFinder() { if (myFinder == null) { diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterBreakAndReturnUnderIf.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterBreakAndReturnUnderIf.java new file mode 100644 index 000000000000..5da8e991bd52 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterBreakAndReturnUnderIf.java @@ -0,0 +1,22 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + int foo(int n, boolean b) { + switch (n) { + case 1: + case 3: + if(b) { + return bar("A"); + } else { + break; + } + case 2: + if(b) { + return bar("B"); + } else { + break; + } + } + return 0; + } + int bar(String s){return s.charAt(0);} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterComplexBranches.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterComplexBranches.java new file mode 100644 index 000000000000..fde660f56688 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterComplexBranches.java @@ -0,0 +1,25 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n, boolean b) { + switch (n) { + case 1: + case 3: + if(b) { + bar("A"); + } else { + bar("z"); + } + bar("o"); + break; + case 2: + if(b) { + bar("B"); + } else { + bar("z"); + } + bar("o"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterContinue.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterContinue.java new file mode 100644 index 000000000000..ea6d858535f4 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterContinue.java @@ -0,0 +1,20 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + int foo(int n) { + int s = 0; + for (int i = 0; i < n; i++) { + switch (i % 4) { + case 1: + case 3: + s += i; + continue; + case 2: + continue; + default: + s += i; + } + s /= 2; + } + return s; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak.java new file mode 100644 index 000000000000..8c0d0b3183a2 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak.java @@ -0,0 +1,16 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 3: + bar("A"); + case 2: + break; + case 4: + bar("A"); + case 5: + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak2.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak2.java new file mode 100644 index 000000000000..8b084e6be7d1 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterFallThroughToBreak2.java @@ -0,0 +1,17 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 4: + bar("A"); + case 2: + break; + case 3: + bar("A"); + break; + case 5: + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterLeftoverComments.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterLeftoverComments.java new file mode 100644 index 000000000000..2bbc2c93b23f --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterLeftoverComments.java @@ -0,0 +1,14 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + case 2: + foo(); // same comment + return "A"; + // another comment + } + return ""; + } + void foo(){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterManyComments.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterManyComments.java new file mode 100644 index 000000000000..56d8f85cea3c --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterManyComments.java @@ -0,0 +1,13 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + /* comment 1 */ + case 1: + case 2: + /* comment 2 */ + return "A"; // comment 3 + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterMethodCallInReturn.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterMethodCallInReturn.java new file mode 100644 index 000000000000..b5f5d1c23ea0 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterMethodCallInReturn.java @@ -0,0 +1,19 @@ +// "Merge with 'case A:'" "GENERIC_ERROR_OR_WARNING" +enum T { + A, B, C; + + int foo(T t) { + switch (t) { + case A: + + case B: + return t.ordinal(); // comment 1 + + case C: + return t.ordinal(); // comment 2 + + default: + return 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterNoLastBreak.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterNoLastBreak.java new file mode 100644 index 000000000000..acfdea991d36 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterNoLastBreak.java @@ -0,0 +1,15 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 3: + bar("A"); + break; + case 2: + bar("B"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterReturn.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterReturn.java new file mode 100644 index 000000000000..1410369ed422 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterReturn.java @@ -0,0 +1,13 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + case 3: + return "A"; + case 2: + return "B"; + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentAfterLabel.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentAfterLabel.java new file mode 100644 index 000000000000..a34edc876cbf --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentAfterLabel.java @@ -0,0 +1,12 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + case 2: + /* comment 1 */ + return "A"; + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentBeforeLabel.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentBeforeLabel.java new file mode 100644 index 000000000000..d1efe42d3568 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSameCommentBeforeLabel.java @@ -0,0 +1,13 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + // comment + case 1: + case 2: + bar("A"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSimple.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSimple.java new file mode 100644 index 000000000000..acfdea991d36 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterSimple.java @@ -0,0 +1,15 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 3: + bar("A"); + break; + case 2: + bar("B"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThreeDuplicates.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThreeDuplicates.java new file mode 100644 index 000000000000..7782028c39db --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThreeDuplicates.java @@ -0,0 +1,18 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + default: + bar("A"); + break; + case 2: + bar("B"); + break; + case 3: + bar("A"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThrow.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThrow.java new file mode 100644 index 000000000000..50ea996bb52e --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterThrow.java @@ -0,0 +1,13 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + case 3: + throw new IllegalArgumentException("A"); + case 2: + throw new IllegalStateException("A"); + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterTwoCaseLabels.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterTwoCaseLabels.java new file mode 100644 index 000000000000..ef3857766ffd --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/afterTwoCaseLabels.java @@ -0,0 +1,16 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 2: + case 3: + bar("A"); + break; + case 4: + bar("B"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeBreakAndReturnUnderIf.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeBreakAndReturnUnderIf.java new file mode 100644 index 000000000000..816a74373831 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeBreakAndReturnUnderIf.java @@ -0,0 +1,27 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + int foo(int n, boolean b) { + switch (n) { + case 1: + if(b) { + return bar("A"); + } else { + break; + } + case 2: + if(b) { + return bar("B"); + } else { + break; + } + case 3: + if(b) { + return bar("A"); + } else { + break; + } + } + return 0; + } + int bar(String s){return s.charAt(0);} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeComplexBranches.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeComplexBranches.java new file mode 100644 index 000000000000..b89524c38bef --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeComplexBranches.java @@ -0,0 +1,32 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n, boolean b) { + switch (n) { + case 1: + if(b) { + bar("A"); + } else { + bar("z"); + } + bar("o"); + break; + case 2: + if(b) { + bar("B"); + } else { + bar("z"); + } + bar("o"); + break; + case 3: + if(b) { + bar("A"); + } else { + bar("z"); + } + bar("o"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeContinue.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeContinue.java new file mode 100644 index 000000000000..a61416708468 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeContinue.java @@ -0,0 +1,22 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + int foo(int n) { + int s = 0; + for (int i = 0; i < n; i++) { + switch (i % 4) { + case 1: + s += i; + continue; + case 2: + continue; + case 3: + s += i; + continue; + default: + s += i; + } + s /= 2; + } + return s; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak.java new file mode 100644 index 000000000000..50a350cd0472 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak.java @@ -0,0 +1,18 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + bar("A"); + case 2: + break; + case 3: + bar("A"); + break; + case 4: + bar("A"); + case 5: + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak2.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak2.java new file mode 100644 index 000000000000..376aa430deb5 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeFallThroughToBreak2.java @@ -0,0 +1,18 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + bar("A"); + case 2: + break; + case 3: + bar("A"); + break; + case 4: + bar("A"); + case 5: + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeLeftoverComments.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeLeftoverComments.java new file mode 100644 index 000000000000..066ff31784ae --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeLeftoverComments.java @@ -0,0 +1,15 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + foo(); // same comment + return "A"; + case 2: + foo(); // same comment + return "A"; // another comment + } + return ""; + } + void foo(){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeManyComments.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeManyComments.java new file mode 100644 index 000000000000..bc85b65f586f --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeManyComments.java @@ -0,0 +1,16 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + /* comment 1 */ + case 1: + /* comment 2 */ + return "A"; // comment 3 + // comment 1 + case 2: + // comment 2 + return "A"; /*comment 3*/ + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeMethodCallInReturn.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeMethodCallInReturn.java new file mode 100644 index 000000000000..56ffab490ebd --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeMethodCallInReturn.java @@ -0,0 +1,20 @@ +// "Merge with 'case A:'" "GENERIC_ERROR_OR_WARNING" +enum T { + A, B, C; + + int foo(T t) { + switch (t) { + case A: + return t.ordinal(); // comment 1 + + case B: + return t.ordinal(); + + case C: + return t.ordinal(); // comment 2 + + default: + return 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeNoLastBreak.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeNoLastBreak.java new file mode 100644 index 000000000000..66696c51d912 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeNoLastBreak.java @@ -0,0 +1,16 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + bar("A"); + break; + case 2: + bar("B"); + break; + case 3: + bar("A"); + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeReturn.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeReturn.java new file mode 100644 index 000000000000..004133f9a283 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeReturn.java @@ -0,0 +1,14 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + return "A"; + case 2: + return "B"; + case 3: + return "A"; + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentAfterLabel.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentAfterLabel.java new file mode 100644 index 000000000000..ce9c139969fd --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentAfterLabel.java @@ -0,0 +1,14 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + /* comment 1 */ + return "A"; + case 2: + // comment 1 + return "A"; + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentBeforeLabel.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentBeforeLabel.java new file mode 100644 index 000000000000..c1839f34a5b5 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSameCommentBeforeLabel.java @@ -0,0 +1,16 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + // comment + case 1: + bar("A"); + break; + // comment + case 2: + bar("A"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSimple.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSimple.java new file mode 100644 index 000000000000..9b4deda0c55e --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeSimple.java @@ -0,0 +1,17 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + bar("A"); + break; + case 2: + bar("B"); + break; + case 3: + bar("A"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThreeDuplicates.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThreeDuplicates.java new file mode 100644 index 000000000000..a00ed706276b --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThreeDuplicates.java @@ -0,0 +1,20 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + bar("A"); + break; + case 2: + bar("B"); + break; + case 3: + bar("A"); + break; + default: + bar("A"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThrow.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThrow.java new file mode 100644 index 000000000000..b9aeb5cbe6fc --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeThrow.java @@ -0,0 +1,14 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + String foo(int n) { + switch (n) { + case 1: + throw new IllegalArgumentException("A"); + case 2: + throw new IllegalStateException("A"); + case 3: + throw new IllegalArgumentException("A"); + } + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeTwoCaseLabels.java b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeTwoCaseLabels.java new file mode 100644 index 000000000000..a344313f340a --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateBranchesInSwitchFix/beforeTwoCaseLabels.java @@ -0,0 +1,18 @@ +// "Merge with 'case 1:'" "GENERIC_ERROR_OR_WARNING" +class C { + void foo(int n) { + switch (n) { + case 1: + case 2: + bar("A"); + break; + case 3: + bar("A"); + break; + case 4: + bar("B"); + break; + } + } + void bar(String s){} +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateBranchesInSwitchFixTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateBranchesInSwitchFixTest.kt new file mode 100644 index 000000000000..9ad72b0d0f1e --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateBranchesInSwitchFixTest.kt @@ -0,0 +1,17 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection + +import com.intellij.JavaTestUtil +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase +import com.intellij.codeInspection.DuplicateBranchesInSwitchInspection +import com.intellij.codeInspection.LocalInspectionTool + +/** + * @author Pavel.Dolgov + */ +class DuplicateBranchesInSwitchFixTest : LightQuickFixParameterizedTestCase() { + + override fun configureLocalInspectionTools(): Array = arrayOf(DuplicateBranchesInSwitchInspection()) + + override fun getBasePath() = "/inspection/duplicateBranchesInSwitchFix" +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 549c049c3579..8b51ba868415 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -1024,4 +1024,6 @@ inspection.overflowing.loop.index.inspection.name=Loop executes zero or billions inspection.overflowing.loop.index.inspection.description=Loop executes zero or billions times inspection.duplicate.branches.in.switch.display.name=Duplicate branches in 'switch' statement -inspection.duplicate.branches.in.switch.message=Duplicate branch in 'switch' statement \ No newline at end of file +inspection.duplicate.branches.in.switch.message=Duplicate branch in 'switch' statement +inspection.duplicate.branches.in.switch.fix.family.name=Merge duplicate branches of 'switch' statement +inspection.duplicate.branches.in.switch.fix.name=Merge with ''{0}:'' \ No newline at end of file