Java: Highlight identical branches in 'switch' statement, quick fix implemented (IDEA-181304)

This commit is contained in:
Pavel Dolgov
2018-11-09 12:44:27 +03:00
parent 8a308f5f90
commit d3a85ece73
35 changed files with 741 additions and 5 deletions
@@ -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<Branch> candidateBranches = null;
for (List<Branch> 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<PsiElement> 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<String> 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<String> existingComments) {
List<PsiElement> 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<String> 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<PsiElement> getBranchPrefix() {
List<PsiElement> 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) {
@@ -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);}
}
@@ -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){}
}
@@ -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;
}
}
@@ -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){}
}
@@ -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){}
}
@@ -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(){}
}
@@ -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 "";
}
}
@@ -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;
}
}
}
@@ -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){}
}
@@ -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 "";
}
}
@@ -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 "";
}
}
@@ -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){}
}
@@ -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){}
}
@@ -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){}
}
@@ -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 "";
}
}
@@ -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){}
}
@@ -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;
<caret>}
}
return 0;
}
int bar(String s){return s.charAt(0);}
}
@@ -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) {<caret>
bar("A");
} else {
bar("z");
}
bar("o");
break;
}
}
void bar(String s){}
}
@@ -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:
<caret>s += i;
continue;
default:
s += i;
}
s /= 2;
}
return s;
}
}
@@ -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:
<caret>bar("A");
break;
case 4:
bar("A");
case 5:
}
}
void bar(String s){}
}
@@ -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:
<caret>bar("A");
case 5:
}
}
void bar(String s){}
}
@@ -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
<caret>return "A"; // another comment
}
return "";
}
void foo(){}
}
@@ -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
<caret>return "A"; /*comment 3*/
}
return "";
}
}
@@ -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:
<caret>return t.ordinal();
case C:
return t.ordinal(); // comment 2
default:
return 0;
}
}
}
@@ -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:
<caret>bar("A");
}
}
void bar(String s){}
}
@@ -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:
<caret>return "A";
}
return "";
}
}
@@ -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
<caret>return "A";
}
return "";
}
}
@@ -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")<caret>;
break;
}
}
void bar(String s){}
}
@@ -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:
<caret>bar("A");
break;
}
}
void bar(String s){}
}
@@ -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");<caret>
break;
}
}
void bar(String s){}
}
@@ -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:
<caret>throw new IllegalArgumentException("A");
}
return "";
}
}
@@ -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:
<caret>bar("A");
break;
case 4:
bar("B");
break;
}
}
void bar(String s){}
}
@@ -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<LocalInspectionTool> = arrayOf(DuplicateBranchesInSwitchInspection())
override fun getBasePath() = "/inspection/duplicateBranchesInSwitchFix"
}
@@ -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
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}:''