[ann] Improved catch analysis

2026-09-27 10:03:11 +07:00 · 2011-05-03 21:10:11 +02:00
parent 23087cdf9c
commit 475354f2b6
6 changed files with 356 additions and 17 deletions
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * 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.
@@ -197,7 +197,9 @@ public class ExceptionUtil {
  }

  @Nullable
-  private static Set<PsiClassType> collectUnhandledExceptions(@NotNull PsiElement element, PsiElement topElement, @Nullable Set<PsiClassType> foundExceptions) {
+  private static Set<PsiClassType> collectUnhandledExceptions(@NotNull PsiElement element,
+                                                              PsiElement topElement,
+                                                              @Nullable Set<PsiClassType> foundExceptions) {
    Collection<PsiClassType> unhandledExceptions = null;
    if (element instanceof PsiCallExpression) {
      PsiCallExpression expression = (PsiCallExpression)element;
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * 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.
@@ -34,6 +34,7 @@ import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.extensions.Extensions;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Comparing;
+import com.intellij.openapi.util.Condition;
 import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.util.TextRange;
 import com.intellij.openapi.util.text.StringUtil;
@@ -48,6 +49,7 @@ import com.intellij.psi.util.*;
 import com.intellij.util.ArrayUtil;
 import com.intellij.util.Function;
 import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.containers.ContainerUtil;
 import com.intellij.xml.util.XmlStringUtil;
 import gnu.trove.THashMap;
 import gnu.trove.THashSet;
@@ -554,13 +556,17 @@ public class HighlightUtil {
  }

  private static String getUnhandledExceptionsDescriptor(final Collection<PsiClassType> unhandled, final String source) {
-    final String exceptions = StringUtil.join(unhandled, new Function<PsiClassType, String>() {
-      @Override public String fun(PsiClassType type) { return formatType(type); }
-    }, ", ");
+    final String exceptions = formatTypes(unhandled);
    return source != null ? JavaErrorMessages.message("unhandled.close.exceptions", exceptions, unhandled.size(), source)
                          : JavaErrorMessages.message("unhandled.exceptions", exceptions, unhandled.size());
  }

+  private static String formatTypes(Collection<PsiClassType> unhandled) {
+    return StringUtil.join(unhandled, new Function<PsiClassType, String>() {
+      @Override public String fun(PsiClassType type) { return formatType(type); }
+    }, ", ");
+  }
+
  @Nullable
  static HighlightInfo checkVariableAlreadyDefined(PsiVariable variable) {
    if (variable instanceof ExternallyDefinedPsiElement) return null;
@@ -888,23 +894,28 @@ public class HighlightUtil {
  }


-  @Nullable
-  static Collection<HighlightInfo> checkExceptionThrownInTry(final PsiParameter parameter) {
-    final PsiElement declarationScope = parameter.getDeclarationScope();
-    if (!(declarationScope instanceof PsiCatchSection)) return null;
-
+  @NotNull
+  static Set<PsiClassType> collectUnhandledExceptions(final PsiTryStatement statement) {
    final Set<PsiClassType> thrownTypes = Sets.newHashSet();

-    final PsiTryStatement statement = ((PsiCatchSection)declarationScope).getTryStatement();
    final PsiCodeBlock tryBlock = statement.getTryBlock();
-    assert tryBlock != null : statement;
-    thrownTypes.addAll(ExceptionUtil.collectUnhandledExceptions(tryBlock, tryBlock));
+    if (tryBlock != null) {
+      thrownTypes.addAll(ExceptionUtil.collectUnhandledExceptions(tryBlock, tryBlock));
+    }

    final PsiResourceList resources = statement.getResourceList();
    if (resources != null) {
      thrownTypes.addAll(ExceptionUtil.collectUnhandledExceptions(resources, resources));
    }

+    return thrownTypes;
+  }
+
+  @Nullable
+  static Collection<HighlightInfo> checkExceptionThrownInTry(final PsiParameter parameter, final Set<PsiClassType> thrownTypes) {
+    final PsiElement declarationScope = parameter.getDeclarationScope();
+    if (!(declarationScope instanceof PsiCatchSection)) return null;
+
    final PsiType caughtType = parameter.getType();
    if (caughtType instanceof PsiClassType) {
      return checkSimpleCatchParameter(parameter, thrownTypes, (PsiClassType)caughtType);
@@ -929,7 +940,7 @@ public class HighlightUtil {
    final String description = JavaErrorMessages.message("exception.never.thrown.try", formatType(caughtType));
    final HighlightInfo errorResult = HighlightInfo.createHighlightInfo(HighlightInfoType.ERROR, parameter, description);
    QuickFixAction.registerQuickFixAction(errorResult, new DeleteCatchFix(parameter));
-    return Lists.newArrayList(errorResult);
+    return Collections.singleton(errorResult);
  }

  @Nullable
@@ -962,6 +973,63 @@ public class HighlightUtil {
  }


+  @Nullable
+  static Collection<HighlightInfo> checkWithImprovedCatchAnalysis(final PsiParameter parameter, Collection<PsiClassType> thrownTypes) {
+    final PsiElement scope = parameter.getDeclarationScope();
+    if (!(scope instanceof PsiCatchSection)) return null;
+
+    final PsiCatchSection catchSection = (PsiCatchSection)scope;
+    final PsiCatchSection[] allCatchSections = catchSection.getTryStatement().getCatchSections();
+    final int idx = ArrayUtil.find(allCatchSections, catchSection);
+    if (idx <= 0) return null;
+
+    thrownTypes = Sets.newHashSet(thrownTypes);
+    thrownTypes.add(PsiType.getJavaLangError(parameter.getManager(), parameter.getResolveScope()));
+    thrownTypes.add(PsiType.getJavaLangRuntimeException(parameter.getManager(), parameter.getResolveScope()));
+    final Collection<HighlightInfo> result = Lists.newArrayList();
+
+    final List<PsiTypeElement> parameterTypeElements = PsiUtil.getParameterTypeElements(parameter);
+    final boolean isMultiCatch = parameterTypeElements.size() > 1;
+    for (PsiTypeElement catchTypeElement : parameterTypeElements) {
+      // collect exceptions which are caught by this type
+      final PsiType catchType = catchTypeElement.getType();
+      Collection<PsiClassType> caught = ContainerUtil.findAll(thrownTypes, new Condition<PsiClassType>() {
+        @Override public boolean value(PsiClassType type) { return catchType.isAssignableFrom(type); }
+      });
+      if (caught.isEmpty()) continue;
+      final Collection<PsiClassType> caughtCopy = Sets.newHashSet(caught);
+
+      // exclude all which are caught by previous catch sections
+      for (int i = 0; i < idx; i++) {
+        final PsiParameter prevCatchParameter = allCatchSections[i].getParameter();
+        if (prevCatchParameter == null) continue;
+        for (PsiTypeElement prevCatchTypeElement : PsiUtil.getParameterTypeElements(prevCatchParameter)) {
+          final PsiType prevCatchType = prevCatchTypeElement.getType();
+          for (Iterator<PsiClassType> iterator = caught.iterator(); iterator.hasNext(); ) {
+            if (prevCatchType.isAssignableFrom(iterator.next())) iterator.remove();
+          }
+          if (caught.isEmpty()) break;
+        }
+      }
+
+      // check & warn
+      if (caught.isEmpty()) {
+        final String message = JavaErrorMessages.message("exception.already.caught.warn", formatTypes(caughtCopy), caughtCopy.size());
+        final HighlightInfo highlightInfo = HighlightInfo.createHighlightInfo(HighlightInfoType.WARNING, catchSection, message);
+        if (isMultiCatch) {
+          QuickFixAction.registerQuickFixAction(highlightInfo, new DeleteMultiCatchFix(catchTypeElement));
+        }
+        else {
+          QuickFixAction.registerQuickFixAction(highlightInfo, new DeleteCatchFix(parameter));
+        }
+        result.add(highlightInfo);
+      }
+    }
+
+    return result;
+  }
+
+
  @Nullable
  static HighlightInfo checkNotAStatement(PsiStatement statement) {
    if (!PsiUtil.isStatement(statement) && !PsiUtilBase.hasErrorElementChild(statement)) {
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * 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.
@@ -50,6 +50,7 @@ import org.jetbrains.annotations.NotNull;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;

 public class HighlightVisitorImpl extends JavaElementVisitor implements HighlightVisitor, DumbAware {
  private static final Logger LOG = Logger.getInstance("#com.intellij.codeInsight.daemon.impl.analysis.HighlightVisitorImpl");
@@ -910,9 +911,16 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
  public void visitTryStatement(PsiTryStatement statement) {
    super.visitTryStatement(statement);
    if (!myHolder.hasErrorResults()) {
+      final Set<PsiClassType> thrownTypes = HighlightUtil.collectUnhandledExceptions(statement);
+      final boolean improvedCheck = PsiUtil.isLanguageLevel7OrHigher(statement);
      for (PsiParameter parameter : statement.getCatchBlockParameters()) {
        boolean added = myHolder.addAll(HighlightUtil.checkExceptionAlreadyCaught(parameter));
-        if (!added) myHolder.addAll(HighlightUtil.checkExceptionThrownInTry(parameter));
+        if (!added) {
+          added = myHolder.addAll(HighlightUtil.checkExceptionThrownInTry(parameter, thrownTypes));
+        }
+        if (!added && improvedCheck) {
+          myHolder.addAll(HighlightUtil.checkWithImprovedCatchAnalysis(parameter, thrownTypes));
+        }
      }
    }
  }
@@ -0,0 +1,241 @@
+/*
+ * 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.
+ */
+import java.io.*;
+
+class C {
+  static class E extends Exception { }
+  static class RE extends RuntimeException { }
+
+  void f() { }
+  void g() throws E { }
+  void h() throws RE { }
+
+  void m0() {
+    try { throw new FileNotFoundException(); }
+    catch (FileNotFoundException e) { }
+    <warning descr="Unreachable section: exception 'java.io.FileNotFoundException' has already been caught">catch (IOException e) { }</warning>
+  }
+
+  void m1() {
+    try { throw new IOException(); }
+    catch (FileNotFoundException e) { }
+    catch (IOException e) { }
+  }
+
+  void m2() {
+    try { f(); }
+    catch (Exception e) { }
+  }
+
+  void m3() {
+    try { g(); }
+    catch (Exception e) { }
+  }
+
+  void m4() {
+    try { h(); }
+    catch (Exception e) { }
+  }
+
+  void m5() {
+    try { f(); }
+    catch (Throwable t) { }
+  }
+
+  void m6() {
+    try { g(); }
+    catch (Throwable t) { }
+  }
+
+  void m7() {
+    try { h(); }
+    catch (Throwable t) { }
+  }
+
+  void m9() {
+    try { f(); }
+    catch (Error e) { }
+    catch (Throwable t) { }
+  }
+
+  void m10() {
+    try { g(); }
+    catch (Error e) { }
+    catch (Throwable t) { }
+  }
+
+  void m11() {
+    try { h(); }
+    catch (Error e) { }
+    catch (Throwable t) { }
+  }
+
+  void m12() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    catch (Throwable t) { }
+  }
+
+  void m13() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Throwable t) { }
+  }
+
+  void m14() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    catch (Throwable t) { }
+  }
+
+  void m15() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m16() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Exception e) { }
+  }
+
+  void m17() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m18() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m19() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (E e) { }
+    <warning descr="Unreachable section: exceptions 'C.E, java.lang.RuntimeException' have already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m20() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m21() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m22() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Exception e) { }
+  }
+
+  void m23() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    <warning descr="Unreachable section: exception 'java.lang.RuntimeException' has already been caught">catch (Exception e) { }</warning>
+  }
+
+  void m24() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m25() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    catch (Throwable t) { }
+  }
+
+  void m26() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m27() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m28() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    catch (E e) { }
+    <warning descr="Unreachable section: exceptions 'C.E, java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m29() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m30() {
+    try { f(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m31() {
+    try { g(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    catch (Throwable t) { }
+  }
+
+  void m32() {
+    try { h(); }
+    catch (RuntimeException e) { }
+    catch (Error e) { }
+    <warning descr="Unreachable section: exceptions 'java.lang.RuntimeException, java.lang.Error' have already been caught">catch (Throwable t) { }</warning>
+  }
+
+  void m33() {
+    try { g(); }
+    catch (E e) { }
+  }
+
+  void m34() {
+    try { h(); }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+  }
+
+  void m35() {
+    try { f(); }
+    catch (<error descr="Exception 'C.E' is never thrown in the corresponding try block">E e</error>) { }
+  }
+}
@@ -1,3 +1,18 @@
+/*
+ * 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.intellij.codeInsight.daemon;

 import com.intellij.ExtensionPoints;
@@ -238,6 +253,10 @@ public class LightAdvHighlightingJdk7Test extends LightDaemonAnalyzerTestCase {
    doTest(false, false);
  }

+  public void testImprovedCatchAnalysis() throws Exception {
+    doTest(true, false);
+  }
+
  public void testJavacQuirks() throws Exception {
    doTest(true, false);
  }
@@ -195,6 +195,7 @@ not.loop.label=Not a loop label: ''{0}''
 incompatible.modifiers=Illegal combination of modifiers: ''{0}'' and ''{1}''
 modifier.not.allowed=Modifier ''{0}'' not allowed here
 exception.never.thrown.try=Exception ''{0}'' is never thrown in the corresponding try block
+exception.already.caught.warn=Unreachable section: {1, choice, 0#exception|2#exceptions} ''{0}'' {1, choice, 0#has|2#have} already been caught
 not.a.statement=Not a statement
 incompatible.types=Incompatible types. Found: ''{1}'', required: ''{0}''
 valid.switch.selector.types=byte, char, short or int