diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java index 84871f45e6..759b8c5f0c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java @@ -49,6 +49,8 @@ public class AssertionHandler extends BaseNoOpHandler { private static final String JUNIT_ASSERT_CLASS = "org.junit.Assert"; private static final String MATCHERS_CLASS = "org.hamcrest.Matchers"; + private static final String CORE_MATCHERS_CLASS = "org.hamcrest.CoreMatchers"; + private static final String CORE_IS_NULL_CLASS = "org.hamcrest.core.IsNull"; private static final String IS_MATCHER = "is"; private static final String NOT_MATCHER = "not"; private static final String NOT_NULL_VALUE_MATCHER = "notNullValue"; @@ -68,6 +70,8 @@ public class AssertionHandler extends BaseNoOpHandler { // Names for hamcrest matchers. private Name matchersClass; + private Name coreMatchersClass; + private Name coreIsNullClass; private Name isMatcher; private Name notMatcher; private Name notNullValueMatcher; @@ -144,7 +148,8 @@ private boolean isMatcherIsNotNull(Node node) { // Matches with // * is(not(nullValue())) // * is(notNullValue()) - if (matchesMatcherMethod(node, isMatcher)) { + if (matchesMatcherMethod(node, isMatcher, matchersClass) + || matchesMatcherMethod(node, isMatcher, coreMatchersClass)) { // All overloads of `is` method have exactly one argument. return isMatcherNotNull(((MethodInvocationNode) node).getArgument(0)); } @@ -155,23 +160,28 @@ private boolean isMatcherNotNull(Node node) { // Matches with // * not(nullValue()) // * notNullValue() - if (matchesMatcherMethod(node, notMatcher)) { + if (matchesMatcherMethod(node, notMatcher, matchersClass) + || matchesMatcherMethod(node, notMatcher, coreMatchersClass)) { // All overloads of `not` method have exactly one argument. return isMatcherNull(((MethodInvocationNode) node).getArgument(0)); } - return matchesMatcherMethod(node, notNullValueMatcher); + return matchesMatcherMethod(node, notNullValueMatcher, matchersClass) + || matchesMatcherMethod(node, notNullValueMatcher, coreMatchersClass) + || matchesMatcherMethod(node, notNullValueMatcher, coreIsNullClass); } private boolean isMatcherNull(Node node) { // Matches with nullValue() - return matchesMatcherMethod(node, nullValueMatcher); + return matchesMatcherMethod(node, nullValueMatcher, matchersClass) + || matchesMatcherMethod(node, nullValueMatcher, coreMatchersClass) + || matchesMatcherMethod(node, nullValueMatcher, coreIsNullClass); } - private boolean matchesMatcherMethod(Node node, Name matcherName) { + private boolean matchesMatcherMethod(Node node, Name matcherName, Name matcherClass) { if (node instanceof MethodInvocationNode) { MethodInvocationNode methodInvocationNode = (MethodInvocationNode) node; Symbol.MethodSymbol callee = ASTHelpers.getSymbol(methodInvocationNode.getTree()); - return matchesMethod(callee, matcherName, matchersClass); + return matchesMethod(callee, matcherName, matcherClass); } return false; } @@ -196,6 +206,8 @@ private void initializeMethodNames(Name.Table table) { junitAssertClass = table.fromString(JUNIT_ASSERT_CLASS); matchersClass = table.fromString(MATCHERS_CLASS); + coreMatchersClass = table.fromString(CORE_MATCHERS_CLASS); + coreIsNullClass = table.fromString(CORE_IS_NULL_CLASS); isMatcher = table.fromString(IS_MATCHER); notMatcher = table.fromString(NOT_MATCHER); notNullValueMatcher = table.fromString(NOT_NULL_VALUE_MATCHER); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java index 28e69b93e0..6963fbf4a4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java @@ -1043,7 +1043,7 @@ public void doNotSupportTruthAssertThatWhenDisabled() { } @Test - public void supportHamcrestAssertThatIsNotNull_Object() { + public void supportHamcrestAssertThatMatchersIsNotNull() { compilationHelper .setArgs( Arrays.asList( @@ -1097,6 +1097,63 @@ public void doNotSupportHamcrestAssertThatWhenDisabled() { .doTest(); } + @Test + public void supportHamcrestAssertThatCoreMatchersIsNotNull() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:HandleTestAssertionLibraries=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.lang.Object;", + "import java.util.Objects;", + "import javax.annotation.Nullable;", + "import static org.hamcrest.MatcherAssert.assertThat;", + "import static org.hamcrest.CoreMatchers.*;", + "class Test {", + " private void foo(@Nullable Object a, @Nullable Object b) {", + " assertThat(a, is(notNullValue()));", + " a.toString();", + " assertThat(b, is(not(nullValue())));", + " b.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void supportHamcrestAssertThatCoreIsNotNull() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:HandleTestAssertionLibraries=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.lang.Object;", + "import java.util.Objects;", + "import javax.annotation.Nullable;", + "import static org.hamcrest.MatcherAssert.assertThat;", + "import static org.hamcrest.CoreMatchers.*;", + "import org.hamcrest.core.IsNull;", + "class Test {", + " private void foo(@Nullable Object a, @Nullable Object b) {", + " assertThat(a, is(IsNull.notNullValue()));", + " a.toString();", + " assertThat(b, is(not(IsNull.nullValue())));", + " b.toString();", + " }", + "}") + .doTest(); + } + @Test public void supportJunitAssertThatIsNotNull_Object() { compilationHelper