Java: Fixed order of 'catch' sections when collapsing identical 'catch' sections (IDEA-193703, IDEA-165900)

This commit is contained in:
Pavel Dolgov
2018-07-19 17:41:37 +03:00
parent 53dfc81e3b
commit be80ed2d7a
8 changed files with 406 additions and 147 deletions
@@ -19,22 +19,26 @@ import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.pom.java.JavaFeature;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.refactoring.extractMethod.InputVariables;
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.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.InspectionGadgetsFix;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
/**
@@ -88,106 +92,242 @@ public class TryWithIdenticalCatchesInspection extends BaseInspection {
@Override
public void visitTryStatement(PsiTryStatement statement) {
super.visitTryStatement(statement);
final PsiCatchSection[] catchSections = statement.getCatchSections();
if (catchSections.length < 2) {
return;
}
final PsiParameter[] parameters = statement.getCatchBlockParameters();
if (catchSections.length != parameters.length) {
return;
}
final boolean[] duplicates = new boolean[catchSections.length];
for (int i = 0; i < catchSections.length - 1; i++) {
final PsiCatchSection catchSection = catchSections[i];
final PsiCodeBlock catchBlock = catchSection.getCatchBlock();
if (catchBlock == null) {
continue;
}
final PsiParameter parameter = catchSection.getParameter();
if (parameter == null) {
continue;
}
final DuplicatesFinder finder = buildDuplicatesFinder(catchBlock, parameter);
for (int j = i + 1; j < catchSections.length; j++) {
if (duplicates[j]) {
continue;
}
final PsiCatchSection otherSection = catchSections[j];
final PsiCodeBlock otherCatchBlock = otherSection.getCatchBlock();
if (otherCatchBlock == null) {
continue;
}
final PsiParameter otherParameter = otherSection.getParameter();
if (otherParameter == null) {
continue;
}
final Match match = finder.isDuplicate(otherCatchBlock, true);
if (match == null) {
continue;
}
final DuplicatesFinder reverseFinder = buildDuplicatesFinder(otherCatchBlock, otherParameter);
final Match otherMatch = reverseFinder.isDuplicate(catchBlock, true);
if (otherMatch == null) {
continue;
}
final ReturnValue returnValue = match.getReturnValue();
final ReturnValue otherReturnValue = otherMatch.getReturnValue();
if (returnValue == null) {
if (otherReturnValue != null) {
continue;
}
}
else if (!returnValue.isEquivalent(otherReturnValue)) {
continue;
}
if (j > i ? !canCollapse(parameters, i, j) : !canCollapse(parameters, j, i)) {
continue;
}
final PsiJavaToken rParenth = otherSection.getRParenth();
if (rParenth != null) {
registerErrorAtOffset(otherSection, 0, rParenth.getStartOffsetInParent() + 1, catchSection.getParameter().getType(),
Integer.valueOf(i), Integer.valueOf(j));
}
duplicates[i] = true;
duplicates[j] = true;
final CatchSectionWrapper[] sections = CatchSectionWrapper.createWrappers(statement);
if (sections == null) return;
final CatchSectionIndices[] catchSectionIndices = getCatchSectionIndices(sections);
if (catchSectionIndices == null) return;
for (int index = 0; index < catchSectionIndices.length; index++) {
int collapseIntoIndex = catchSectionIndices[index].myCollapseIntoIndex;
if (collapseIntoIndex >= 0) {
registerProblem(sections, index, collapseIntoIndex);
}
}
}
private void registerProblem(@NotNull CatchSectionWrapper[] sections, int at, int collapseIntoIndex) {
final PsiCatchSection section = sections[at].myCatchSection;
final PsiJavaToken rParenth = section.getRParenth();
if (rParenth != null) {
registerErrorAtOffset(section, 0, rParenth.getStartOffsetInParent() + 1, sections[collapseIntoIndex].myParameter.getType());
}
}
}
@Nullable
static CatchSectionIndices[] getCatchSectionIndices(@NotNull CatchSectionWrapper[] sections) {
final CatchSectionIndices[] indices = new CatchSectionIndices[sections.length];
for (int index = 0; index < sections.length; index++) {
indices[index] = new CatchSectionIndices(index);
}
boolean duplicateFound = false;
for (int from = 0; from < sections.length - 1; from++) {
if (indices[from].myHasDuplicate) continue;
final CatchSectionWrapper section = sections[from];
if (section == null) continue;
for (int to = from + 1; to < sections.length; to++) {
if (indices[to].myHasDuplicate) continue;
final CatchSectionWrapper otherSection = sections[to];
if (otherSection == null || !section.isDuplicate(otherSection)) continue;
indices[from].addDuplicate(indices[to]);
duplicateFound = true;
}
}
if (!duplicateFound) return null;
final boolean[][] canSwap = new boolean[sections.length][sections.length];
for (int from = 0; from < sections.length; from++) {
for (int to = from + 1; to < sections.length; to++) {
canSwap[from][to] = canSwap[to][from] = sections[from] != null && sections[from].canSwapWith(sections[to]);
}
}
for (int index = 0; index < sections.length; index++) {
indices[index].computeInsertionRange(canSwap);
}
for (CatchSectionIndices idx : indices) {
final int[] duplicates = idx.myDuplicates;
if (duplicates == null) continue;
for (int from : duplicates) {
for (int to : duplicates) {
indices[to].tryCollapseInto(indices[from]);
}
}
}
return indices;
}
private static class CatchSectionIndices {
final int myIndex;
int myCanInsertBefore = -1;
int myCanInsertAfter = -1;
boolean myHasDuplicate;
int[] myDuplicates;
int myCollapseIntoIndex = -1;
CatchSectionIndices(int index) {
myIndex = index;
}
void addDuplicate(CatchSectionIndices duplicate) {
if (myDuplicates == null) {
myDuplicates = new int[]{myIndex, duplicate.myIndex};
myHasDuplicate = true;
}
else {
myDuplicates = ArrayUtil.append(myDuplicates, duplicate.myIndex);
}
duplicate.myHasDuplicate = true;
}
void computeInsertionRange(@NotNull boolean[][] canSwap) {
boolean[] canSwapWith = canSwap[myIndex];
for (int before = myIndex; ; before--) {
if (before - 1 < 0 || !canSwapWith[before - 1]) {
myCanInsertBefore = before;
break;
}
}
for (int after = myIndex; ; after++) {
if (after + 1 >= canSwapWith.length || !canSwapWith[after + 1]) {
myCanInsertAfter = after;
break;
}
}
}
public void tryCollapseInto(CatchSectionIndices collapseInto) {
if (myCollapseIntoIndex < 0 && myIndex > collapseInto.myIndex && myCanInsertBefore <= collapseInto.myCanInsertAfter + 1) {
myCollapseIntoIndex = collapseInto.myIndex;
}
}
}
private static class CatchSectionWrapper {
@NotNull final PsiCatchSection myCatchSection;
@NotNull final PsiCodeBlock myCodeBlock;
@NotNull final PsiParameter myParameter;
@NotNull final List<PsiClassType> myTypes;
@NotNull final DuplicatesFinder myFinder;
private CatchSectionWrapper(@NotNull PsiCatchSection catchSection,
@NotNull PsiCodeBlock codeBlock,
@NotNull PsiParameter parameter,
@NotNull List<PsiClassType> types,
@NotNull DuplicatesFinder finder) {
myCatchSection = catchSection;
myCodeBlock = codeBlock;
myParameter = parameter;
myTypes = types;
myFinder = finder;
}
boolean isDuplicate(@NotNull CatchSectionWrapper section) {
final Match match = findDuplicate(section);
if (match == null) {
return false;
}
final Match otherMatch = section.findDuplicate(this);
if (otherMatch == null) {
return false;
}
final ReturnValue returnValue = match.getReturnValue();
final ReturnValue otherReturnValue = otherMatch.getReturnValue();
if (returnValue == null) {
return otherReturnValue == null;
}
return returnValue.isEquivalent(otherReturnValue);
}
private Match findDuplicate(@NotNull CatchSectionWrapper section) {
return myFinder.isDuplicate(section.myCodeBlock, true);
}
boolean canSwapWith(@Nullable CatchSectionWrapper section) {
if (section == null) return false;
for (PsiClassType type : myTypes) {
for (PsiClassType otherType : section.myTypes) {
if (type.isAssignableFrom(otherType) || otherType.isAssignableFrom(type)) {
return false;
}
}
}
return true;
}
@Nullable
static CatchSectionWrapper[] createWrappers(@NotNull PsiTryStatement statement) {
final PsiCatchSection[] catchSections = statement.getCatchSections();
if (catchSections.length < 2) {
return null;
}
final PsiParameter[] parameters = statement.getCatchBlockParameters();
if (catchSections.length != parameters.length) {
return null;
}
final CatchSectionWrapper[] sections = new CatchSectionWrapper[catchSections.length];
for (int i = 0; i < sections.length; i++) {
sections[i] = createWrapper(catchSections[i]);
}
return sections;
}
@Nullable
private static CatchSectionWrapper createWrapper(@NotNull PsiCatchSection catchSection) {
final PsiParameter parameter = catchSection.getParameter();
final PsiCodeBlock codeBlock = catchSection.getCatchBlock();
if (parameter != null && codeBlock != null) {
final List<PsiClassType> types = getClassTypes(parameter.getType());
if (types != null) {
final DuplicatesFinder finder = buildDuplicatesFinder(codeBlock, parameter);
return new CatchSectionWrapper(catchSection, codeBlock, parameter, types, finder);
}
}
return null;
}
@Nullable
private static List<PsiClassType> getClassTypes(@Nullable PsiType type) {
if (type instanceof PsiClassType) {
return Collections.singletonList((PsiClassType)type);
}
if (type instanceof PsiDisjunctionType) {
final List<PsiType> disjunctions = ((PsiDisjunctionType)type).getDisjunctions();
if (!disjunctions.isEmpty()) {
final List<PsiClassType> classTypes = ContainerUtil.mapNotNull(disjunctions, t -> ObjectUtils.tryCast(t, PsiClassType.class));
if (classTypes.size() == disjunctions.size()) {
return classTypes;
}
}
}
return null;
}
@NotNull
private static DuplicatesFinder buildDuplicatesFinder(@NotNull PsiCodeBlock catchBlock, @NotNull PsiParameter parameter) {
final InputVariables inputVariables =
new InputVariables(Collections.singletonList(parameter), parameter.getProject(), new LocalSearchScope(catchBlock), false);
return new DuplicatesFinder(new PsiElement[]{catchBlock}, inputVariables, null, Collections.emptyList());
}
private static boolean canCollapse(PsiParameter[] parameters, int index1, int index2) {
if (index2 <= index1) throw new IllegalArgumentException();
final PsiType type = parameters[index2].getType();
for (int i = index1 + 1; i < index2; i++) {
final PsiType otherType = parameters[i].getType();
if (TypeConversionUtil.isAssignable(type, otherType)) {
return false;
}
}
return true;
}
}
@Override
protected InspectionGadgetsFix buildFix(Object... infos) {
return new CollapseCatchSectionsFix(((Integer)infos[1]).intValue(), ((Integer)infos[2]).intValue());
return new CollapseCatchSectionsFix();
}
private static class CollapseCatchSectionsFix extends InspectionGadgetsFix {
private final int myCollapseIntoIndex;
private final int mySectionIndex;
public CollapseCatchSectionsFix(int collapseIntoIndex, int sectionIndex) {
myCollapseIntoIndex = collapseIntoIndex;
mySectionIndex = sectionIndex;
}
@Override
@NotNull
public String getFamilyName() {
@@ -197,68 +337,60 @@ public class TryWithIdenticalCatchesInspection extends BaseInspection {
@Override
protected void doFix(Project project, ProblemDescriptor descriptor) {
// smart psi pointer lost correct catch section when multiple catch sections were collapsed in batch mode
// so use index of catch section to retrieve it instead.
final PsiTryStatement tryStatement = (PsiTryStatement)descriptor.getPsiElement().getParent();
final PsiCatchSection[] catchSections = tryStatement.getCatchSections();
if (myCollapseIntoIndex >= catchSections.length || mySectionIndex >= catchSections.length) {
return; // something has gone stale
}
final PsiCatchSection collapseInto = catchSections[myCollapseIntoIndex];
final PsiCatchSection section = catchSections[mySectionIndex];
final PsiParameter parameter1 = collapseInto.getParameter();
final PsiParameter parameter2 = section.getParameter();
if (parameter1 == null || parameter2 == null) {
return;
}
final PsiType type1 = parameter1.getType();
final PsiType type2 = parameter2.getType();
if (TypeConversionUtil.isAssignable(type1, type2)) {
section.delete();
return;
}
else if (TypeConversionUtil.isAssignable(type2, type1)) {
collapseInto.delete();
return;
}
final List<PsiType> types = new ArrayList<>();
collectDisjunctTypes(type1, types);
collectDisjunctTypes(type2, types);
final StringBuilder typeText = new StringBuilder();
for (PsiType type : types) {
if (typeText.length() > 0) {
typeText.append(" | ");
// so we need to re-calculate everything based on what exists at this point
final PsiCatchSection catchSection = (PsiCatchSection)descriptor.getPsiElement();
final PsiTryStatement tryStatement = (PsiTryStatement)catchSection.getParent();
final CatchSectionWrapper[] sections = CatchSectionWrapper.createWrappers(tryStatement);
if (sections == null) return;
int sectionIndex = getSectionIndex(sections, catchSection);
if (sectionIndex < 0) return;
CatchSectionWrapper duplicateSection = sections[sectionIndex];
if (duplicateSection == null) return;
final CatchSectionIndices[] duplicatesIndices = getCatchSectionIndices(sections);
if (duplicatesIndices == null) return;
final int collapseIntoIndex = duplicatesIndices[sectionIndex].myCollapseIntoIndex;
if (collapseIntoIndex < 0) return;
final CatchSectionWrapper collapseIntoSection = sections[collapseIntoIndex];
if (collapseIntoSection == null) return;
final PsiTypeElement collapseIntoTypeElement = collapseIntoSection.myParameter.getTypeElement();
if (collapseIntoTypeElement == null) return;
final List<PsiType> parameterTypes = new ArrayList<>(collapseIntoSection.myTypes);
parameterTypes.addAll(duplicateSection.myTypes);
final List<PsiType> filteredTypes = PsiDisjunctionType.flattenAndRemoveDuplicates(parameterTypes);
final PsiType disjunction = PsiDisjunctionType.createDisjunction(filteredTypes, tryStatement.getManager());
final PsiTypeElement newTypeElement = JavaPsiFacade.getElementFactory(project).createTypeElement(disjunction);
JavaCodeStyleManager.getInstance(project).shortenClassReferences(collapseIntoTypeElement.replace(newTypeElement));
int insertBeforeIndex = duplicatesIndices[sectionIndex].myCanInsertBefore;
if (collapseIntoIndex < insertBeforeIndex) {
// We can't leave the merged 'catch' section at collapseIntoIndex because it conflicts with other caught exceptions
final PsiCatchSection[] catchSections = tryStatement.getCatchSections();
if (insertBeforeIndex < catchSections.length && catchSections[insertBeforeIndex] != null) {
tryStatement.addBefore(collapseIntoSection.myCatchSection, catchSections[insertBeforeIndex]);
collapseIntoSection.myCatchSection.delete();
}
typeText.append(type.getCanonicalText());
}
final PsiTypeElement newTypeElement =
JavaPsiFacade.getElementFactory(project).createTypeElementFromText(typeText.toString(), tryStatement);
final PsiTypeElement typeElement = parameter1.getTypeElement();
if (typeElement == null) {
return;
}
typeElement.replace(newTypeElement);
section.delete();
duplicateSection.myCatchSection.delete();
}
private static void collectDisjunctTypes(PsiType type, List<PsiType> out) {
if (type instanceof PsiDisjunctionType) {
final PsiDisjunctionType disjunctionType = (PsiDisjunctionType)type;
final List<PsiType> disjunctions = disjunctionType.getDisjunctions();
for (PsiType disjunction : disjunctions) {
collectDisjunctTypes(disjunction, out);
}
return;
}
for (Iterator<PsiType> iterator = out.iterator(); iterator.hasNext(); ) {
final PsiType collectedType = iterator.next();
if (TypeConversionUtil.isAssignable(type, collectedType)) {
iterator.remove();
}
else if (TypeConversionUtil.isAssignable(collectedType, type)) {
return;
private static int getSectionIndex(@NotNull CatchSectionWrapper[] sections, @NotNull PsiElement catchSection) {
for (int i = 0; i < sections.length; i++) {
if (sections[i].myCatchSection == catchSection) {
return i;
}
}
out.add(type);
return -1;
}
}
}
@@ -0,0 +1,24 @@
class Main {
static class Ex1 extends Exception {}
static class Ex2 extends Exception {}
static class Ex3 extends Ex1 {}
public void test() {
try {
if(Math.random() > 0.5) {
throw new Ex1();
}
if(Math.random() > 0.5) {
throw new Ex2();
}
if(Math.random() > 0.5) {
throw new Ex3();
}
} catch(Ex3 ignored) {
} catch(RuntimeException | Ex2 | Ex1 ex) {
ex.printStackTrace();
} catch(Error e) {
return;
}
}
}
@@ -0,0 +1,33 @@
class Main {
static class Ex1 extends Exception {}
static class Ex2 extends Exception {}
static class Ex3 extends Ex1 {}
public void test() {
try {
if(Math.random() > 0.5) {
throw new Ex1();
}
if(Math.random() > 0.5) {
throw new Ex2();
}
if(Math.random() > 0.5) {
throw new Ex3();
}
}
catch(RuntimeException ex) {
ex.printStackTrace();
}
catch(Ex3 ignored) {
}
catch(Error e) {
return;
}
<warning descr="'catch' branch identical to 'RuntimeException' branch">catch(Ex2 ex)</warning> {
ex.printStackTrace();
}
<warning descr="'catch' branch identical to 'RuntimeException' branch">catch(Ex1 <caret>ex)</warning> {
ex.printStackTrace();
}
}
}
@@ -0,0 +1,17 @@
import java.io.IOException;
class C {
private static void throwStuff() throws IOException {}
public static void main(String[] args) {
try {
throwStuff();
} catch (NumberFormatException nfe) {
System.out.println("ERROR 2:" + nfe.getMessage());
System.exit(1);
} catch (IOException | IllegalArgumentException ioe) {
System.out.println("ERROR:" + ioe.getMessage());
System.exit(1);
}
}
}
@@ -0,0 +1,20 @@
import java.io.IOException;
class C {
private static void throwStuff() throws IOException {}
public static void main(String[] args) {
try {
throwStuff();
} catch (IOException ioe) {
System.out.println("ERROR:" + ioe.getMessage());
System.exit(1);
} catch (NumberFormatException nfe) {
System.out.println("ERROR 2:" + nfe.getMessage());
System.exit(1);
} <warning descr="'catch' branch identical to 'IOException' branch">catch (IllegalArgumentException i<caret>ae)</warning> {
System.out.println("ERROR:" + iae.getMessage());
System.exit(1);
}
}
}
@@ -14,8 +14,7 @@ public class NonDisjunctTypes {
if(Math.random() > 0.5) {
throw new Ex3();
}
}
catch(RuntimeException | Ex1 ex) {
} catch(RuntimeException | Ex1 ex) {
ex.printStackTrace();
}
}
@@ -107,7 +107,7 @@ class TryIdenticalCatches {
} catch (E4 e) {
} <warning descr="'catch' branch identical to 'E4' branch">catch (E2 e)</warning> {
} <warning descr="'catch' branch identical to 'E4' branch">catch (E3 e)</warning> {
} <warning descr="'catch' branch identical to 'E2' branch">catch (E1 e)</warning> {
} <warning descr="'catch' branch identical to 'E4' branch">catch (E1 e)</warning> {
}
}
@@ -17,8 +17,14 @@ package com.siyeh.ig.migration;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.openapi.application.PluginPathManager;
import com.intellij.psi.PsiCatchSection;
import com.intellij.psi.PsiTryStatement;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
import java.util.ArrayList;
import java.util.List;
/**
* @author yole
*/
@@ -35,12 +41,40 @@ public class TryWithIdenticalCatchesTest extends LightCodeInsightFixtureTestCase
highlightTest();
}
public void testIdenticalCatchUnrelatedExceptions() {
doTest();
}
public void testIdenticalCatchThreeOutOfFour() {
doTest(true);
}
public void doTest() {
doTest(false);
}
public void doTest(boolean processAll) {
highlightTest();
String name = getTestName(false);
IntentionAction intention = myFixture.findSingleIntention("Collapse 'catch' blocks");
assertNotNull(intention);
myFixture.launchAction(intention);
if (processAll) {
PsiTryStatement tryStatement = PsiTreeUtil.getParentOfType(myFixture.getElementAtCaret(), PsiTryStatement.class);
assertNotNull("tryStatement", tryStatement);
PsiCatchSection[] catchSections = tryStatement.getCatchSections();
List<IntentionAction> intentions = new ArrayList<>();
for (PsiCatchSection section : catchSections) {
getEditor().getCaretModel().moveToOffset(section.getTextOffset());
intentions.addAll(myFixture.filterAvailableIntentions("Collapse 'catch' blocks"));
}
assertFalse("intentions.isEmpty", intentions.isEmpty());
for (IntentionAction intention : intentions) {
myFixture.launchAction(intention);
}
}
else {
IntentionAction intention = myFixture.findSingleIntention("Collapse 'catch' blocks");
assertNotNull(intention);
myFixture.launchAction(intention);
}
myFixture.checkResultByFile("com/siyeh/igtest/errorhandling/try_identical_catches/" + name + ".after.java");
}