diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index bf670551e43c..99ad6247c391 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -21,7 +21,6 @@ import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.nullable.NullableStuffInspectionBase; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; -import com.intellij.openapi.roots.ProjectFileIndex; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.WriteExternalException; import com.intellij.openapi.util.text.StringUtil; @@ -32,10 +31,7 @@ import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; -import com.siyeh.ig.psiutils.ComparisonUtils; -import com.siyeh.ig.psiutils.ControlFlowUtils; -import com.siyeh.ig.psiutils.ExpressionUtils; -import com.siyeh.ig.psiutils.TypeUtils; +import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jdom.Element; import org.jetbrains.annotations.NonNls; @@ -397,11 +393,8 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool private static void reportAlwaysFailingCalls(ProblemsHolder holder, DataFlowInstructionVisitor visitor, HashSet reportedAnchors) { - if (ProjectFileIndex.SERVICE.getInstance(holder.getProject()).isInTestSourceContent(holder.getFile().getViewProvider().getVirtualFile())) { - return; - } - for (PsiCall call : visitor.getAlwaysFailingCalls()) { + if (TestUtils.isExceptionExpected(call)) continue; PsiMethod method = call.resolveMethod(); if (method != null && reportedAnchors.add(call)) { holder.registerProblem(getElementToHighlight(call), "The call to '#ref' always fails, according to its method contracts"); diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionHeavyTest.groovy b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionHeavyTest.groovy index de6082e4915e..b6f74f557b31 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionHeavyTest.groovy +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionHeavyTest.groovy @@ -80,8 +80,16 @@ class DataFlowInspectionHeavyTest extends JavaCodeInsightFixtureTestCase { void "test no always failing calls in tests"() { PsiTestUtil.addSourceRoot(myModule, myFixture.tempDirFixture.findOrCreateDir("test"), true) + myFixture.addFileToProject("test/org/junit/Test.java", """ +package org.junit; + +public @interface Test { + Class expected(); +} +""") myFixture.configureFromExistingVirtualFile(myFixture.addFileToProject("test/Foo.java", """ class Foo { + @org.junit.Test(expected=RuntimeException.class) void foo() { assertTrue(false); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/junit/TestMethodWithoutAssertionInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/junit/TestMethodWithoutAssertionInspectionBase.java index 3fe10acbd830..710d3734745a 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/junit/TestMethodWithoutAssertionInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/junit/TestMethodWithoutAssertionInspectionBase.java @@ -24,8 +24,8 @@ import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.psiutils.ControlFlowUtils; import com.siyeh.ig.psiutils.MethodMatcher; import com.siyeh.ig.psiutils.TestUtils; +import org.intellij.lang.annotations.Pattern; import org.jdom.Element; -import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; public class TestMethodWithoutAssertionInspectionBase extends BaseInspection { @@ -48,6 +48,7 @@ public class TestMethodWithoutAssertionInspectionBase extends BaseInspection { .finishDefault(); } + @Pattern(VALID_ID_PATTERN) @Override @NotNull public String getID() { @@ -91,7 +92,7 @@ public class TestMethodWithoutAssertionInspectionBase extends BaseInspection { if (!TestUtils.isJUnitTestMethod(method)) { return; } - if (hasExpectedExceptionAnnotation(method)) { + if (TestUtils.hasExpectedExceptionAnnotation(method)) { return; } if (containsAssertion(method)) { @@ -132,23 +133,6 @@ public class TestMethodWithoutAssertionInspectionBase extends BaseInspection { element.accept(visitor); return visitor.containsAssertion(); } - - private boolean hasExpectedExceptionAnnotation(PsiMethod method) { - final PsiModifierList modifierList = method.getModifierList(); - final PsiAnnotation testAnnotation = modifierList.findAnnotation("org.junit.Test"); - if (testAnnotation == null) { - return false; - } - final PsiAnnotationParameterList parameterList = testAnnotation.getParameterList(); - final PsiNameValuePair[] nameValuePairs = parameterList.getAttributes(); - for (PsiNameValuePair nameValuePair : nameValuePairs) { - @NonNls final String parameterName = nameValuePair.getName(); - if ("expected".equals(parameterName)) { - return true; - } - } - return false; - } } private class ContainsAssertionVisitor extends JavaRecursiveElementWalkingVisitor { diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TestUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TestUtils.java index d4816804014f..c4c587d90233 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TestUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TestUtils.java @@ -22,7 +22,10 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; import com.intellij.testIntegration.TestFramework; +import com.intellij.util.ObjectUtils; +import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.junit.JUnitCommonClassNames; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -32,6 +35,8 @@ import java.util.Set; public class TestUtils { public static final String RUN_WITH = "org.junit.runner.RunWith"; + private static final CallMatcher ASSERT_THROWS = + CallMatcher.staticCall(JUnitCommonClassNames.ORG_JUNIT_JUPITER_API_ASSERTIONS, "assertThrows"); private TestUtils() { } @@ -157,4 +162,56 @@ public class TestUtils { } return false; } + + /** + * Tries to determine whether exception is expected at given element (e.g. element is a part of method annotated with + * {@code @Test(expected = ...)} or part of lambda passed to {@code Assertions.assertThrows()}. + * + * Note that the test is not exhaustive: false positives and false negatives are possible. + * + * @param element to check + * @return true if it's likely that exception is expected at this point. + */ + public static boolean isExceptionExpected(PsiElement element) { + if (!isInTestSourceContent(element)) return false; + for(; element != null && !(element instanceof PsiFile); element = element.getParent()) { + if (element instanceof PsiMethod) { + return hasExpectedExceptionAnnotation((PsiMethod)element); + } + if (element instanceof PsiLambdaExpression) { + PsiExpressionList expressionList = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(element.getParent()), PsiExpressionList.class); + if (expressionList != null) { + PsiElement parent = expressionList.getParent(); + if (parent instanceof PsiMethodCallExpression && ASSERT_THROWS.test((PsiMethodCallExpression)parent)) return true; + } + } + if (element instanceof PsiTryStatement && ((PsiTryStatement)element).getCatchBlocks().length > 0) { + return true; + } + } + return false; + } + + public static boolean hasExpectedExceptionAnnotation(PsiMethod method) { + final PsiModifierList modifierList = method.getModifierList(); + return hasAnnotationWithParameter(modifierList, "org.junit.Test", "expected") || + hasAnnotationWithParameter(modifierList, "org.testng.annotations.Test", "expectedExceptions"); + } + + private static boolean hasAnnotationWithParameter(PsiModifierList modifierList, String annotationName, String expectedParameterName) { + final PsiAnnotation testAnnotation = modifierList.findAnnotation(annotationName); + if (testAnnotation == null) { + return false; + } + final PsiAnnotationParameterList parameterList = testAnnotation.getParameterList(); + final PsiNameValuePair[] nameValuePairs = parameterList.getAttributes(); + for (PsiNameValuePair nameValuePair : nameValuePairs) { + @NonNls final String parameterName = nameValuePair.getName(); + if (expectedParameterName.equals(parameterName)) { + return true; + } + } + return false; + } }