DataFlowInspection: more precise check whether exception is expected inside test

This commit is contained in:
Tagir Valeev
2017-10-17 14:37:13 +07:00
parent 7f1eea9903
commit a4043c41f6
4 changed files with 70 additions and 28 deletions
@@ -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<PsiElement> 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");
@@ -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<? extends Throwable> expected();
}
""")
myFixture.configureFromExistingVirtualFile(myFixture.addFileToProject("test/Foo.java", """
class Foo {
@org.junit.Test(expected=RuntimeException.class)
void foo() {
assertTrue(false);
}
@@ -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 {
@@ -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;
}
}