From 1dbdb1377c088732bf1870668e4e63339a4ec231 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Thu, 17 Feb 2011 19:51:15 +0100 Subject: [PATCH] first stab at an inspection to detect identical 'catch' blocks in a 'try' statement (quickfix coming soon) --- .../util/duplicates/DuplicatesFinder.java | 21 ++-- .../refactoring/util/duplicates/Match.java | 12 ++- .../siyeh/InspectionGadgetsBundle.properties | 4 +- .../src/com/siyeh/ig/BaseInspection.java | 7 ++ .../com/siyeh/ig/BaseInspectionVisitor.java | 9 +- .../com/siyeh/ig/InspectionGadgetsPlugin.java | 1 + .../TryWithIdenticalCatchesInspection.java | 95 +++++++++++++++++++ .../TryWithIdenticalCatches.html | 7 ++ .../TryIdenticalCatches.java | 35 +++++++ .../TryWithIdenticalCatchesTest.java | 35 +++++++ 10 files changed, 213 insertions(+), 13 deletions(-) create mode 100644 plugins/InspectionGadgets/src/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesInspection.java create mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/TryWithIdenticalCatches.html create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/errorhandling/try_identical_catches/TryIdenticalCatches.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesTest.java diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java index 65f2783627ac..fb1f1494d5ed 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java @@ -34,6 +34,7 @@ import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.IntArrayList; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; @@ -49,12 +50,12 @@ public class DuplicatesFinder { private final List myOutputParameters; private final List myPatternAsList; private boolean myMultipleExitPoints = false; - private final ReturnValue myReturnValue; + @Nullable private final ReturnValue myReturnValue; public DuplicatesFinder(PsiElement[] pattern, InputVariables parameters, - ReturnValue returnValue, - List outputParameters + @Nullable ReturnValue returnValue, + @NotNull List outputParameters ) { myReturnValue = returnValue; LOG.assertTrue(pattern.length > 0); @@ -109,6 +110,14 @@ public class DuplicatesFinder { return result; } + @Nullable + public Match isDuplicate(PsiElement element, boolean ignoreParameterTypes) { + annotatePattern(); + Match match = isDuplicateFragment(element, ignoreParameterTypes); + deannotatePattern(); + return match; + } + private void annotatePattern() { for (final PsiElement patternComponent : myPattern) { patternComponent.accept(new JavaRecursiveElementWalkingVisitor() { @@ -146,7 +155,7 @@ public class DuplicatesFinder { private void findPatternOccurrences(List array, PsiElement scope) { PsiElement[] children = scope.getChildren(); for (PsiElement child : children) { - final Match match = isDuplicateFragment(child); + final Match match = isDuplicateFragment(child, false); if (match != null) { array.add(match); continue; @@ -157,7 +166,7 @@ public class DuplicatesFinder { @Nullable - private Match isDuplicateFragment(PsiElement candidate) { + private Match isDuplicateFragment(PsiElement candidate, boolean ignoreParameterTypes) { if (PsiTreeUtil.isAncestor(myPattern[0], candidate, false)) return null; PsiElement sibling = candidate; ArrayList candidates = new ArrayList(); @@ -188,7 +197,7 @@ public class DuplicatesFinder { } } - final Match match = new Match(candidates.get(0), candidates.get(candidates.size() - 1)); + final Match match = new Match(candidates.get(0), candidates.get(candidates.size() - 1), ignoreParameterTypes); for (int i = 0; i < myPattern.length; i++) { if (!matchPattern(myPattern[i], candidates.get(i), candidates, match)) return null; } diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/Match.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/Match.java index c2383b7cec7b..7686ec98a965 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/Match.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/Match.java @@ -48,16 +48,18 @@ public final class Match { private final PsiElement myMatchStart; private final PsiElement myMatchEnd; private final Map> myParameterValues = new HashMap>(); - private final Map> myParameterOccurences = new HashMap>(); + private final Map> myParameterOccurrences = new HashMap>(); private final Map myDeclarationCorrespondence = new HashMap(); private ReturnValue myReturnValue = null; private Ref myInstanceExpression = null; private final Map myChangedParams = new HashMap(); + private final boolean myIgnoreParameterTypes; - Match(PsiElement start, PsiElement end) { + Match(PsiElement start, PsiElement end, boolean ignoreParameterTypes) { LOG.assertTrue(start.getParent() == end.getParent()); myMatchStart = start; myMatchEnd = end; + myIgnoreParameterTypes = ignoreParameterTypes; } @@ -135,14 +137,14 @@ public final class Match { myChangedParams.put(psiVariable, new PsiEllipsisType(parameterType)); } } else { - if (!parameterType.isAssignableFrom(type)) return false; //todo + if (!myIgnoreParameterTypes && !parameterType.isAssignableFrom(type)) return false; //todo } } final List values = new ArrayList(); values.add(value); myParameterValues.put(psiVariable, values); final ArrayList elements = new ArrayList(); - myParameterOccurences.put(psiVariable, elements); + myParameterOccurrences.put(psiVariable, elements); return true; } else { @@ -157,7 +159,7 @@ public final class Match { currentValue.add(value); } } - myParameterOccurences.get(psiVariable).add(value); + myParameterOccurrences.get(psiVariable).add(value); return true; } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties index 0a9522cf8c9c..86d8cc95e568 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1843,4 +1843,6 @@ package.dot.html.delete.command=package.html deletion package.dot.html.may.be.package.info.convert.quickfix=Convert to package-info.java package.dot.html.convert.command=package.html to package-info.java conversion choose.super.class.to.ignore=Choose class -ignore.anonymous.inner.classes=Ignore anonymous inner classes \ No newline at end of file +ignore.anonymous.inner.classes=Ignore anonymous inner classes +try.with.identical.catches.display.name=Identical 'catch' branches in 'try' statement +try.with.identical.catches.problem.descriptor=Identical 'catch' branches in 'try' statement #loc diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspection.java index 0607fd78b62c..5deb22cc2a72 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspection.java @@ -24,7 +24,9 @@ import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ProjectManager; +import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.ui.DocumentAdapter; import com.siyeh.ig.ui.FormattedTextFieldMacFix; @@ -283,4 +285,9 @@ public abstract class BaseInspection extends BaseJavaLocalInspectionTool { inspectionGadgetsPlugin = null; } } + + @Nullable + public TextRange getProblemTextRange(PsiElement element) { + return null; + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspectionVisitor.java b/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspectionVisitor.java index c365f9e72c54..e93b09e95c3a 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspectionVisitor.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/BaseInspectionVisitor.java @@ -16,6 +16,7 @@ package com.siyeh.ig; import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -167,7 +168,13 @@ public abstract class BaseInspectionVisitor extends JavaElementVisitor{ fix.setOnTheFly(onTheFly); } final String description = inspection.buildErrorString(infos); - holder.registerProblem(location, description, fixes); + TextRange range = inspection.getProblemTextRange(location); + if (range != null) { + holder.registerProblem(location, range, description, fixes); + } + else { + holder.registerProblem(location, description, fixes); + } } @NotNull diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java index f16ffba98be5..dea51ebec133 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java @@ -833,6 +833,7 @@ public class InspectionGadgetsPlugin implements ApplicationComponent, m_inspectionClasses.add(ThrowFromFinallyBlockInspection.class); m_inspectionClasses.add(TooBroadCatchInspection.class); m_inspectionClasses.add(TooBroadThrowsInspection.class); + m_inspectionClasses.add(TryWithIdenticalCatchesInspection.class); m_inspectionClasses.add(UncheckedExceptionClassInspection.class); m_inspectionClasses.add(UnusedCatchParameterInspection.class); } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesInspection.java new file mode 100644 index 000000000000..0bbceef35b6c --- /dev/null +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesInspection.java @@ -0,0 +1,95 @@ +/* + * Copyright 2000-2011 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.errorhandling; + +import com.intellij.openapi.util.TextRange; +import com.intellij.psi.*; +import com.intellij.psi.search.LocalSearchScope; +import com.intellij.psi.util.PsiUtil; +import com.intellij.refactoring.extractMethod.InputVariables; +import com.intellij.refactoring.util.duplicates.DuplicatesFinder; +import com.intellij.refactoring.util.duplicates.Match; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import java.util.Collections; + +/** + * @author yole + */ +public class TryWithIdenticalCatchesInspection extends BaseInspection { + @NotNull + @Override + protected String buildErrorString(Object... infos) { + return InspectionGadgetsBundle.message("try.with.identical.catches.problem.descriptor"); + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new TryWithIdenticalCatchesVisitor(); + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return InspectionGadgetsBundle.message("try.with.identical.catches.display.name"); + } + + @Override + public TextRange getProblemTextRange(PsiElement element) { + if (element instanceof PsiCatchSection) { + PsiJavaToken rParenth = ((PsiCatchSection)element).getRParenth(); + if (rParenth != null) { + return new TextRange(0, rParenth.getTextOffset() + 1 - element.getTextOffset()); + } + } + return null; + } + + private static class TryWithIdenticalCatchesVisitor extends BaseInspectionVisitor { + @Override + public void visitTryStatement(PsiTryStatement statement) { + super.visitTryStatement(statement); + if (!PsiUtil.isLanguageLevel7OrHigher(statement)) { + return; + } + PsiCatchSection[] catchSections = statement.getCatchSections(); + boolean[] duplicates = new boolean[catchSections.length]; + for (int i = 0; i < catchSections.length; i++) { + if (duplicates[i]) continue; + InputVariables inputVariables = new InputVariables(Collections.singletonList(catchSections[i].getParameter()), + statement.getProject(), + new LocalSearchScope(catchSections[i].getCatchBlock()), + false); + DuplicatesFinder finder = new DuplicatesFinder(new PsiElement[] { catchSections [i].getCatchBlock() }, + inputVariables, null, Collections.emptyList()); + for (int j = 0; j < catchSections.length; j++) { + if (i == j || duplicates[j]) continue; + Match match = finder.isDuplicate(catchSections[j].getCatchBlock(), true); + if (match != null) { + registerError(catchSections[j]); + duplicates[i] = true; + duplicates[j] = true; + } + } + } + } + } +} diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/TryWithIdenticalCatches.html b/plugins/InspectionGadgets/src/inspectionDescriptions/TryWithIdenticalCatches.html new file mode 100644 index 000000000000..887abc484396 --- /dev/null +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/TryWithIdenticalCatches.html @@ -0,0 +1,7 @@ + + +This inspection reports identical 'catch' sections in 'try' blocks under JDK 7. A quickfix is provided to collapse the sections into +a multi-catch section. +Powered by InspectionGadgets + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/errorhandling/try_identical_catches/TryIdenticalCatches.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/errorhandling/try_identical_catches/TryIdenticalCatches.java new file mode 100644 index 000000000000..bb3049b3e80e --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/errorhandling/try_identical_catches/TryIdenticalCatches.java @@ -0,0 +1,35 @@ +package com.siyeh.igtest.errorhandling.try_identical_catches; + +class TryIdenticalCatches { + public void notIdentical() { + try { + + } + catch(NumberFormatException e) { + log(e); + } + catch(RuntimeException e) { + throw e; + } + } + + public void identical(boolean value) { + try { + if (value) { + throw new ClassNotFoundException(); + } + else { + throw new NumberFormatException(); + } + } + catch(ClassNotFoundException cnfe) { + log(cnfe); + } + catch(NumberFormatException nfe) { + log(nfe); + } + } + + private void log(Exception e) { + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesTest.java new file mode 100644 index 000000000000..69771c0cf90d --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/errorhandling/TryWithIdenticalCatchesTest.java @@ -0,0 +1,35 @@ +/* + * Copyright 2000-2011 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.errorhandling; + +import com.intellij.openapi.application.PluginPathManager; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; + +/** + * @author yole + */ +public class TryWithIdenticalCatchesTest extends LightCodeInsightFixtureTestCase { + public void test() { + myFixture.enableInspections(TryWithIdenticalCatchesInspection.class); + myFixture.configureByFile("com/siyeh/igtest/errorhandling/try_identical_catches/TryIdenticalCatches.java"); + myFixture.checkHighlighting(true, false, false); + } + + @Override + protected String getBasePath() { + return PluginPathManager.getPluginHomePathRelative("InspectionGadgets") + "/test"; + } +}