From 6d9cf39e0e1d9e4d25d174155130376fcf1a2126 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 16 Jul 2014 13:54:13 +0200 Subject: [PATCH] dfa: understand assertThat notnull (IDEA-125977, IDEA-65004) --- .../dataFlow/ControlFlowAnalyzer.java | 48 +------ .../dataFlow/HardcodedContracts.java | 121 ++++++++++++++++++ .../dataFlow/fixture/AssertThat.java | 18 +++ .../DataFlowInspectionTest.java | 15 +++ .../com/intellij/spellchecker/jetbrains.dic | 1 + 5 files changed, 161 insertions(+), 42 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/AssertThat.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 65e7fd7c859a..defb28a90dce 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -30,14 +30,12 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.Stack; import com.siyeh.ig.numeric.UnnecessaryExplicitNumericCastInspection; import org.jetbrains.annotations.Contract; -import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.regex.Pattern; -import static com.intellij.codeInspection.dataFlow.MethodContract.ValueConstraint; import static com.intellij.psi.CommonClassNames.*; public class ControlFlowAnalyzer extends JavaElementVisitor { @@ -1396,7 +1394,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } addConditionalRuntimeThrow(); - List contracts = method instanceof PsiMethod ? getMethodContracts((PsiMethod)method) : Collections.emptyList(); + List contracts = method instanceof PsiMethod ? getMethodCallContracts((PsiMethod)method, expression) : Collections.emptyList(); addInstruction(new MethodCallInstruction(expression, createChainedVariableValue(expression), contracts)); if (!contracts.isEmpty()) { // if a contract resulted in 'fail', handle it @@ -1431,6 +1429,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } + private static List getMethodCallContracts(@NotNull final PsiMethod method, @NotNull PsiMethodCallExpression call) { + List contracts = HardcodedContracts.getHardcodedContracts(method, call); + return !contracts.isEmpty() ? contracts : getMethodContracts(method); + } + static List getMethodContracts(@NotNull final PsiMethod method) { final PsiAnnotation contractAnno = findContractAnnotation(method); final int paramCount = method.getParameterList().getParametersCount(); @@ -1458,45 +1461,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { }); } - @NonNls String methodName = method.getName(); - - PsiClass owner = method.getContainingClass(); - if (owner != null) { - final String className = owner.getQualifiedName(); - if ("java.lang.System".equals(className)) { - if ("exit".equals(methodName)) { - return Collections.singletonList(new MethodContract(MethodContract.createConstraintArray(paramCount), ValueConstraint.THROW_EXCEPTION)); - } - } - else if ("junit.framework.Assert".equals(className) || "org.junit.Assert".equals(className) || - "junit.framework.TestCase".equals(className) || "org.testng.Assert".equals(className) || "org.testng.AssertJUnit".equals(className)) { - boolean testng = className.startsWith("org.testng."); - if ("fail".equals(methodName)) { - return Collections.singletonList(new MethodContract(MethodContract.createConstraintArray(paramCount), ValueConstraint.THROW_EXCEPTION)); - } - - int checkedParam = testng ? 0 : paramCount - 1; - ValueConstraint[] constraints = MethodContract.createConstraintArray(paramCount); - if ("assertTrue".equals(methodName)) { - constraints[checkedParam] = ValueConstraint.FALSE_VALUE; - return Collections.singletonList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); - } - if ("assertFalse".equals(methodName)) { - constraints[checkedParam] = ValueConstraint.TRUE_VALUE; - return Collections.singletonList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); - } - if ("assertNull".equals(methodName)) { - constraints[checkedParam] = ValueConstraint.NOT_NULL_VALUE; - return Collections.singletonList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); - } - if ("assertNotNull".equals(methodName)) { - constraints[checkedParam] = ValueConstraint.NULL_VALUE; - return Collections.singletonList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); - } - return Collections.emptyList(); - } - } - return Collections.emptyList(); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java new file mode 100644 index 000000000000..f475bfece5de --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java @@ -0,0 +1,121 @@ +/* + * Copyright 2000-2014 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.codeInspection.dataFlow; + +import com.intellij.psi.*; +import org.jetbrains.annotations.NotNull; + +import java.util.Collections; +import java.util.List; + +import static com.intellij.codeInspection.dataFlow.MethodContract.ValueConstraint.*; +import static com.intellij.codeInspection.dataFlow.MethodContract.createConstraintArray; + +/** + * @author peter + */ +class HardcodedContracts { + static List getHardcodedContracts(@NotNull PsiMethod method, @NotNull PsiMethodCallExpression call) { + PsiClass owner = method.getContainingClass(); + if (owner == null) return Collections.emptyList(); + + final int paramCount = method.getParameterList().getParametersCount(); + String className = owner.getQualifiedName(); + String methodName = method.getName(); + + if ("java.lang.System".equals(className)) { + if ("exit".equals(methodName)) { + return Collections.singletonList(new MethodContract(createConstraintArray(paramCount), THROW_EXCEPTION)); + } + } + else if ("junit.framework.Assert".equals(className) || "org.junit.Assert".equals(className) || + "junit.framework.TestCase".equals(className) || "org.testng.Assert".equals(className) || "org.testng.AssertJUnit".equals(className)) { + return handleTestFrameworks(paramCount, className, methodName, call); + } + return Collections.emptyList(); + } + + private static boolean isNotNullMatcher(PsiExpression expr) { + if (expr instanceof PsiMethodCallExpression) { + String calledName = ((PsiMethodCallExpression)expr).getMethodExpression().getReferenceName(); + if ("notNullValue".equals(calledName)) { + return true; + } + if ("not".equals(calledName)) { + PsiExpression[] notArgs = ((PsiMethodCallExpression)expr).getArgumentList().getExpressions(); + if (notArgs.length == 1 && + notArgs[0] instanceof PsiMethodCallExpression && + "equalTo".equals(((PsiMethodCallExpression)notArgs[0]).getMethodExpression().getReferenceName())) { + PsiExpression[] equalArgs = ((PsiMethodCallExpression)notArgs[0]).getArgumentList().getExpressions(); + if (equalArgs.length == 1 && equalArgs[0] instanceof PsiLiteralExpression && equalArgs[0].textMatches(PsiKeyword.NULL)) { + return true; + } + } + } + } + return false; + } + + private static List handleTestFrameworks(int paramCount, String className, String methodName, + @NotNull PsiMethodCallExpression call) { + if ("assertThat".equals(methodName)) { + PsiExpression[] args = call.getArgumentList().getExpressions(); + if (args.length == paramCount) { + for (int i = 1; i < args.length; i++) { + if (isNotNullMatcher(args[i])) { + MethodContract.ValueConstraint[] constraints = createConstraintArray(args.length); + constraints[i - 1] = NULL_VALUE; + return Collections.singletonList(new MethodContract(constraints, THROW_EXCEPTION)); + } + } + } + return Collections.emptyList(); + } + + if (!"junit.framework.Assert".equals(className) && + !"junit.framework.TestCase".equals(className) && + !"org.junit.Assert".equals(className) && + !"org.testng.Assert".equals(className) && + !"org.testng.AssertJUnit".equals(className)) { + return Collections.emptyList(); + } + + boolean testng = className.startsWith("org.testng."); + if ("fail".equals(methodName)) { + return Collections.singletonList(new MethodContract(createConstraintArray(paramCount), THROW_EXCEPTION)); + } + + int checkedParam = testng ? 0 : paramCount - 1; + MethodContract.ValueConstraint[] constraints = createConstraintArray(paramCount); + if ("assertTrue".equals(methodName)) { + constraints[checkedParam] = FALSE_VALUE; + return Collections.singletonList(new MethodContract(constraints, THROW_EXCEPTION)); + } + if ("assertFalse".equals(methodName)) { + constraints[checkedParam] = TRUE_VALUE; + return Collections.singletonList(new MethodContract(constraints, THROW_EXCEPTION)); + } + if ("assertNull".equals(methodName)) { + constraints[checkedParam] = NOT_NULL_VALUE; + return Collections.singletonList(new MethodContract(constraints, THROW_EXCEPTION)); + } + if ("assertNotNull".equals(methodName)) { + constraints[checkedParam] = NULL_VALUE; + return Collections.singletonList(new MethodContract(constraints, THROW_EXCEPTION)); + } + return Collections.emptyList(); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AssertThat.java b/java/java-tests/testData/inspection/dataFlow/fixture/AssertThat.java new file mode 100644 index 000000000000..b3d614cd8cb3 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AssertThat.java @@ -0,0 +1,18 @@ +import org.hamcrest.CoreMatchers; +import org.jetbrains.annotations.Nullable; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.assertThat; + +class Contracts { + + private void checkNotNullValue(@Nullable Object o) { + assertThat(o, CoreMatchers.notNullValue()); + System.out.println(o.hashCode()); + } + + private void checkNotEqualToNull(@Nullable String test) { + assertThat("String is null", test, not(equalTo(null))); + int length = test.length(); + } + +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 0c8f67275ae2..b59f2f650782 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -282,6 +282,21 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { myFixture.launchAction(myFixture.findSingleIntention("Remove redundant assignment")); myFixture.checkResultByFile(getTestName(false) + "_after.java"); } + + public void testAssertThat() { + myFixture.addClass("package org.hamcrest; public class CoreMatchers { " + + "public static Matcher notNullValue() {}\n" + + "public static Matcher not(Matcher matcher) {}\n" + + "public static Matcher equalTo(T operand) {}\n" + + "}"); + myFixture.addClass("package org.hamcrest; public interface Matcher {}"); + myFixture.addClass("package org.junit; public class Assert { " + + "public static void assertThat(T actual, org.hamcrest.Matcher matcher) {}\n" + + "public static void assertThat(String msg, T actual, org.hamcrest.Matcher matcher) {}\n" + + "}"); + myFixture.enableInspections(new DataFlowInspection()); + myFixture.testHighlighting(true, false, true, getTestName(false) + ".java"); + } public void _testNullCheckBeforeInstanceof() { doTest(); } // http://youtrack.jetbrains.com/issue/IDEA-113220 } diff --git a/spellchecker/src/com/intellij/spellchecker/jetbrains.dic b/spellchecker/src/com/intellij/spellchecker/jetbrains.dic index 83ebf270290b..52b856089b10 100644 --- a/spellchecker/src/com/intellij/spellchecker/jetbrains.dic +++ b/spellchecker/src/com/intellij/spellchecker/jetbrains.dic @@ -168,6 +168,7 @@ globals google gzip gruntfile +hamcrest hardcoded hardlink hardlinks