MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

Java: Highlight identical branches in 'switch' statement, optimize duplicates matching (IDEA-181304)

Browse Source
This commit is contained in:
Pavel Dolgov
2018-11-09 12:44:27 +03:00
parent 4d7679a0b5
commit 8a308f5f90
4 changed files with 71 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInspection/DuplicateBranchesInSwitchInspection.java
+55 -65
View File
@@ -1,12 +1,8 @@
// 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.codeInspection;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.AnalysisCanceledException;
import com.intellij.psi.controlFlow.ControlFlow;
import com.intellij.psi.controlFlow.ControlFlowUtil;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
@@ -15,13 +11,11 @@ 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.siyeh.ig.psiutils.ControlFlowUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.*;
import static com.siyeh.ig.migration.TryWithIdenticalCatchesInspection.collectCommentTexts;
@@ -45,21 +39,29 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
public void visitSwitchStatement(PsiSwitchStatement switchStatement) {
super.visitSwitchStatement(switchStatement);
List<Branch> branches = collectBranches(switchStatement);
for (List<Branch> branches : collectSameLengthBranches(switchStatement)) {
registerProblems(branches);
}
}
void registerProblems(List<Branch> branches) {
int size = branches.size();
if (size > 1) {
boolean[] isDuplicate = new boolean[size];
for (int i = 0; i < size - 1; i++) {
if (isDuplicate[i]) continue;
for (int index = 0; index < size - 1; index++) {
if (isDuplicate[index]) continue;
for (int j = i + 1; j < size; j++) {
if (areDuplicates(branches, i, j)) {
isDuplicate[j] = true;
registerProblem(branches.get(j).myStatements);
for (int otherIndex = index + 1; otherIndex < size; otherIndex++) {
Branch branch = branches.get(index);
Branch otherBranch = branches.get(otherIndex);
if (!isDuplicate[i]) {
isDuplicate[i] = true;
registerProblem(branches.get(i).myStatements);
if (areDuplicates(branch, otherBranch)) {
isDuplicate[otherIndex] = true;
registerProblem(otherBranch.myStatements);
if (!isDuplicate[index]) {
isDuplicate[index] = true;
registerProblem(branch.myStatements);
}
}
}
@@ -77,21 +79,21 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
}
@NotNull
static List<Branch> collectBranches(@NotNull PsiSwitchStatement switchStatement) {
static Collection<List<Branch>> collectSameLengthBranches(@NotNull PsiSwitchStatement switchStatement) {
PsiCodeBlock body = switchStatement.getBody();
if (body == null) return Collections.emptyList();
List<Branch> branches = new ArrayList<>();
List<PsiStatement> statementList = null;
Comments comments = new Comments();
Branch previousBranch = null;
Map<Integer, List<Branch>> branchesByLength = new HashMap<>();
for (PsiElement child = body.getFirstChild(); child != null; child = child.getNextSibling()) {
if (child instanceof PsiSwitchLabelStatement) {
PsiSwitchLabelStatement switchLabel = (PsiSwitchLabelStatement)child;
if (statementList != null) {
branches.add(new Branch(statementList, hasImplicitBreak(switchLabel), comments.fetchTexts()));
statementList = null;
}
previousBranch = addBranchToMap(branchesByLength, statementList, hasImplicitBreak(switchLabel), comments, previousBranch);
statementList = null;
comments.addFrom(switchLabel);
}
else if (child instanceof PsiStatement) {
@@ -107,17 +109,30 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
}
}
if (statementList != null) {
branches.add(new Branch(statementList, true, comments.fetchTexts()));
}
return branches;
addBranchToMap(branchesByLength, statementList, true, comments, previousBranch);
return branchesByLength.values();
}
static boolean areDuplicates(List<Branch> branches, int index, int otherIndex) {
Branch branch = branches.get(index);
Branch otherBranch = branches.get(otherIndex);
if (branch.canFallThrough() || otherBranch.canFallThrough() ||
branch.isSimpleExit() != otherBranch.isSimpleExit()) {
@Nullable
private static Branch addBranchToMap(@NotNull Map<Integer, List<Branch>> branchesByLength,
@Nullable List<PsiStatement> statementList,
boolean hasImplicitBreak,
@NotNull Comments comments,
@Nullable Branch previousBranch) {
if (statementList == null || statementList.isEmpty()) {
return previousBranch;
}
Branch branch = new Branch(statementList, hasImplicitBreak, comments.fetchTexts());
if (previousBranch == null || !previousBranch.canFallThrough()) {
List<Branch> branches = branchesByLength.computeIfAbsent(branch.length(), unused -> new ArrayList<>());
branches.add(branch);
}
return branch;
}
static boolean areDuplicates(Branch branch, Branch otherBranch) {
if (branch.isSimpleExit() != otherBranch.isSimpleExit() ||
branch.canFallThrough() || otherBranch.canFallThrough()) {
return false;
}
@@ -149,20 +164,14 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
private final PsiStatement[] myStatements;
private final String[] myCommentTexts;
private final boolean myIsSimpleExit;
private final boolean myCanFallThrough;
private DuplicatesFinder myFinder;
private Boolean myCanFallThrough;
Branch(@NotNull List<PsiStatement> statementList, boolean hasImplicitBreak, String[] commentTexts) {
Branch(@NotNull List<PsiStatement> statementList, boolean hasImplicitBreak, @NotNull String[] commentTexts) {
int lastIndex = statementList.size() - 1;
PsiStatement lastStatement = statementList.get(lastIndex);
if (hasImplicitBreak ||
lastStatement instanceof PsiBreakStatement ||
lastStatement instanceof PsiReturnStatement ||
lastStatement instanceof PsiContinueStatement ||
lastStatement instanceof PsiThrowStatement) {
myCanFallThrough = false; // in more complex cases it will be computed lazily
}
myCanFallThrough = !hasImplicitBreak && ControlFlowUtils.statementMayCompleteNormally(lastStatement);
myIsSimpleExit = lastIndex == 0 && isSimpleExit(lastStatement);
if (lastIndex > 0 && isBreakWithoutLabel(lastStatement)) {
statementList = statementList.subList(0, lastIndex); // trailing 'break' is already taken into account in myCanFallThrough
@@ -177,9 +186,6 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
}
boolean canFallThrough() {
if (myCanFallThrough == null) {
myCanFallThrough = calculateCanFallThrough(myStatements);
}
return myCanFallThrough;
}
@@ -187,6 +193,10 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
return myIsSimpleExit;
}
int length() {
return myStatements.length;
}
@NotNull
private DuplicatesFinder getFinder() {
if (myFinder == null) {
@@ -202,26 +212,6 @@ public class DuplicateBranchesInSwitchInspection extends LocalInspectionTool {
return new DuplicatesFinder(statements, noVariables, null, Collections.emptyList());
}
private static boolean calculateCanFallThrough(@NotNull PsiStatement[] statements) {
PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(statements[0], PsiSwitchStatement.class);
if (switchStatement != null) {
PsiElement switchBody = switchStatement.getBody();
if (switchBody != null) {
try {
ControlFlow flow = HighlightControlFlowUtil.getControlFlowNoConstantEvaluate(switchBody);
int branchStart = flow.getStartOffset(statements[0]);
int branchEnd = flow.getEndOffset(statements[statements.length - 1]);
if (branchStart >= 0 && branchEnd >= 0) {
return ControlFlowUtil.isInstructionReachable(flow, branchEnd, branchStart);
}
}
catch (AnalysisCanceledException ignore) {
}
}
}
return true;
}
private static boolean isSimpleExit(@Nullable PsiStatement statement) {
if (statement instanceof PsiBreakStatement ||
statement instanceof PsiContinueStatement ||
java/java-impl/src/inspectionDescriptions/DuplicateBranchesInSwitch.html
+1
View File
@@ -2,5 +2,6 @@
<body>
Reports <code>switch</code> statements containing the same code in different branches.
<!-- tooltip end -->
<p><small>New in 2019.1</small></p>
</body>
</html>
java/java-tests/testData/inspection/duplicateBranchesInSwitch/FallThroughBefore.java
+14
View File
@@ -0,0 +1,14 @@
class C {
void foo(String s) {
switch (s) {
case "A":
bar(1);
break;
case "B":
bar(2);
case "C":
bar(1);
}
}
void bar(int n) {}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateBranchesInSwitchTest.kt
+1
View File
@@ -23,6 +23,7 @@ class DuplicateBranchesInSwitchTest : LightCodeInsightFixtureTestCase() {
fun testThrow() = doTest()
fun testContinue() = doTest()
fun testFallThrough() = doTest()
fun testFallThroughBefore() = doTest()
fun testAllFallThrough() = doTest()
fun testNoLastBreak() = doTest()
fun testFallThroughToBreak() = doTest()