From 475354f2b60d7fc726d706078d14b22b47188413 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Tue, 3 May 2011 21:08:39 +0200 Subject: [PATCH] [ann] Improved catch analysis --- .../intellij/codeInsight/ExceptionUtil.java | 6 +- .../daemon/impl/analysis/HighlightUtil.java | 94 ++++++- .../impl/analysis/HighlightVisitorImpl.java | 12 +- .../ImprovedCatchAnalysis.java | 241 ++++++++++++++++++ .../daemon/LightAdvHighlightingJdk7Test.java | 19 ++ .../src/messages/JavaErrorMessages.properties | 1 + 6 files changed, 356 insertions(+), 17 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ImprovedCatchAnalysis.java diff --git a/java/java-impl/src/com/intellij/codeInsight/ExceptionUtil.java b/java/java-impl/src/com/intellij/codeInsight/ExceptionUtil.java index d6c0d5faac58..c22feeadfd8b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/ExceptionUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/ExceptionUtil.java @@ -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 collectUnhandledExceptions(@NotNull PsiElement element, PsiElement topElement, @Nullable Set foundExceptions) { + private static Set collectUnhandledExceptions(@NotNull PsiElement element, + PsiElement topElement, + @Nullable Set foundExceptions) { Collection unhandledExceptions = null; if (element instanceof PsiCallExpression) { PsiCallExpression expression = (PsiCallExpression)element; diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index 81e93c065c6e..496dc667a19b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java @@ -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 unhandled, final String source) { - final String exceptions = StringUtil.join(unhandled, new Function() { - @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 unhandled) { + return StringUtil.join(unhandled, new Function() { + @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 checkExceptionThrownInTry(final PsiParameter parameter) { - final PsiElement declarationScope = parameter.getDeclarationScope(); - if (!(declarationScope instanceof PsiCatchSection)) return null; - + @NotNull + static Set collectUnhandledExceptions(final PsiTryStatement statement) { final Set 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 checkExceptionThrownInTry(final PsiParameter parameter, final Set 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 checkWithImprovedCatchAnalysis(final PsiParameter parameter, Collection 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 result = Lists.newArrayList(); + + final List 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 caught = ContainerUtil.findAll(thrownTypes, new Condition() { + @Override public boolean value(PsiClassType type) { return catchType.isAssignableFrom(type); } + }); + if (caught.isEmpty()) continue; + final Collection 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 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)) { diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index bcbeb7e75e4f..365c796548f0 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -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 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)); + } } } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ImprovedCatchAnalysis.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ImprovedCatchAnalysis.java new file mode 100644 index 000000000000..eb8d8e73d187 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ImprovedCatchAnalysis.java @@ -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) { } + catch (IOException e) { } + } + + 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) { } + catch (Exception e) { } + } + + void m16() { + try { g(); } + catch (RuntimeException e) { } + catch (Exception e) { } + } + + void m17() { + try { h(); } + catch (RuntimeException e) { } + catch (Exception e) { } + } + + void m18() { + try { f(); } + catch (RuntimeException e) { } + catch (E e) { } + catch (Exception e) { } + } + + void m19() { + try { g(); } + catch (RuntimeException e) { } + catch (E e) { } + catch (Exception e) { } + } + + void m20() { + try { h(); } + catch (RuntimeException e) { } + catch (E e) { } + catch (Exception e) { } + } + + void m21() { + try { f(); } + catch (RuntimeException e) { } + catch (Exception e) { } + } + + void m22() { + try { g(); } + catch (RuntimeException e) { } + catch (Exception e) { } + } + + void m23() { + try { h(); } + catch (RuntimeException e) { } + catch (Exception e) { } + } + + void m24() { + try { f(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m25() { + try { g(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m26() { + try { h(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m27() { + try { f(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (E e) { } + catch (Throwable t) { } + } + + void m28() { + try { g(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (E e) { } + catch (Throwable t) { } + } + + void m29() { + try { h(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (E e) { } + catch (Throwable t) { } + } + + void m30() { + try { f(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m31() { + try { g(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m32() { + try { h(); } + catch (RuntimeException e) { } + catch (Error e) { } + catch (Throwable t) { } + } + + void m33() { + try { g(); } + catch (E e) { } + } + + void m34() { + try { h(); } + catch (E e) { } + } + + void m35() { + try { f(); } + catch (E e) { } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingJdk7Test.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingJdk7Test.java index 807c4c91fd65..69b931cd13c1 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingJdk7Test.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingJdk7Test.java @@ -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); } diff --git a/resources-en/src/messages/JavaErrorMessages.properties b/resources-en/src/messages/JavaErrorMessages.properties index caac6812f031..465e1e1b75e1 100644 --- a/resources-en/src/messages/JavaErrorMessages.properties +++ b/resources-en/src/messages/JavaErrorMessages.properties @@ -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