Skip to content

Commit

Permalink
Handle assertThat in JUnit and Hamcrest. (#310)
Browse files Browse the repository at this point in the history 
* Adding support for assertThat in Junit and Hamcrest.

* Splitting negative tests and adding static imports for assertion libraries and matchers to make the code more readable.
  • Loading branch information
@navahgar
navahgar committed Apr 29, 2019
1 parent f8d255c commit c3979b4
Show file tree
Hide file tree
Showing 2 changed files with 209 additions and 8 deletions.
88 changes: 88 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java
View file
Expand Up @@ -31,6 +31,7 @@
import com.sun.tools.javac.util.Name;
import com.uber.nullaway.dataflow.AccessPath;
import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
import java.util.List;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.dataflow.cfg.node.Node;

Expand All @@ -44,6 +45,15 @@ public class AssertionHandler extends BaseNoOpHandler {
private static final String ASSERT_THAT_METHOD = "assertThat";
private static final String ASSERT_THAT_OWNER = "com.google.common.truth.Truth";

private static final String HAMCREST_ASSERT_CLASS = "org.hamcrest.MatcherAssert";
private static final String JUNIT_ASSERT_CLASS = "org.junit.Assert";

private static final String MATCHERS_CLASS = "org.hamcrest.Matchers";
private static final String IS_MATCHER = "is";
private static final String NOT_MATCHER = "not";
private static final String NOT_NULL_VALUE_MATCHER = "notNullValue";
private static final String NULL_VALUE_MATCHER = "nullValue";

// Names of the methods (and their owners) used to identify assertions in this handler. Name used
// here refers to com.sun.tools.javac.util.Name. Comparing methods using Names is faster than
// comparing using strings.
Expand All @@ -52,6 +62,17 @@ public class AssertionHandler extends BaseNoOpHandler {
private Name assertThat;
private Name assertThatOwner;

// Names for junit assertion libraries.
private Name hamcrestAssertClass;
private Name junitAssertClass;

// Names for hamcrest matchers.
private Name matchersClass;
private Name isMatcher;
private Name notMatcher;
private Name notNullValueMatcher;
private Name nullValueMatcher;

@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Expand Down Expand Up @@ -86,6 +107,20 @@ public NullnessHint onDataflowVisitMethodInvocation(
}
}
}

// Look for statements of the form:
// * assertThat(A, is(not(nullValue())))
// * assertThat(A, is(notNullValue()))
if (isMethodHamcrestAssertThat(callee) || isMethodJunitAssertThat(callee)) {
List<Node> args = node.getArguments();
if (args.size() == 2 && isMatcherIsNotNull(args.get(1))) {
AccessPath ap = AccessPath.getAccessPathForNodeNoMapGet(args.get(0));
if (ap != null) {
bothUpdates.set(ap, NONNULL);
}
}
}

return NullnessHint.UNKNOWN;
}

Expand All @@ -97,6 +132,50 @@ private boolean isMethodAssertThat(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertThat, assertThatOwner);
}

private boolean isMethodHamcrestAssertThat(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertThat, hamcrestAssertClass);
}

private boolean isMethodJunitAssertThat(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertThat, junitAssertClass);
}

private boolean isMatcherIsNotNull(Node node) {
// Matches with
// * is(not(nullValue()))
// * is(notNullValue())
if (matchesMatcherMethod(node, isMatcher)) {
// All overloads of `is` method have exactly one argument.
return isMatcherNotNull(((MethodInvocationNode) node).getArgument(0));
}
return false;
}

private boolean isMatcherNotNull(Node node) {
// Matches with
// * not(nullValue())
// * notNullValue()
if (matchesMatcherMethod(node, notMatcher)) {
// All overloads of `not` method have exactly one argument.
return isMatcherNull(((MethodInvocationNode) node).getArgument(0));
}
return matchesMatcherMethod(node, notNullValueMatcher);
}

private boolean isMatcherNull(Node node) {
// Matches with nullValue()
return matchesMatcherMethod(node, nullValueMatcher);
}

private boolean matchesMatcherMethod(Node node, Name matcherName) {
if (node instanceof MethodInvocationNode) {
MethodInvocationNode methodInvocationNode = (MethodInvocationNode) node;
Symbol.MethodSymbol callee = ASTHelpers.getSymbol(methodInvocationNode.getTree());
return matchesMethod(callee, matcherName, matchersClass);
}
return false;
}

private boolean matchesMethod(
Symbol.MethodSymbol methodSymbol, Name toMatchMethodName, Name toMatchOwnerName) {
return methodSymbol.name.equals(toMatchMethodName)
Expand All @@ -112,5 +191,14 @@ private void initializeMethodNames(Name.Table table) {
isNotNullOwner = table.fromString(IS_NOT_NULL_OWNER);
assertThat = table.fromString(ASSERT_THAT_METHOD);
assertThatOwner = table.fromString(ASSERT_THAT_OWNER);

hamcrestAssertClass = table.fromString(HAMCREST_ASSERT_CLASS);
junitAssertClass = table.fromString(JUNIT_ASSERT_CLASS);

matchersClass = table.fromString(MATCHERS_CLASS);
isMatcher = table.fromString(IS_MATCHER);
notMatcher = table.fromString(NOT_MATCHER);
notNullValueMatcher = table.fromString(NOT_NULL_VALUE_MATCHER);
nullValueMatcher = table.fromString(NULL_VALUE_MATCHER);
}
}
129 changes: 121 additions & 8 deletions nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java
View file
Expand Up @@ -968,7 +968,7 @@ public void supportObjectsIsNull() {
}

@Test
public void supportAssertThatIsNotNull_Object() {
public void supportTruthAssertThatIsNotNull_Object() {
compilationHelper
.setArgs(
Arrays.asList(
Expand All @@ -982,17 +982,18 @@ public void supportAssertThatIsNotNull_Object() {
"import java.lang.Object;",
"import java.util.Objects;",
"import javax.annotation.Nullable;",
"import static com.google.common.truth.Truth.assertThat;",
"class Test {",
" private void foo(@Nullable Object o) {",
" com.google.common.truth.Truth.assertThat(o).isNotNull();",
" assertThat(o).isNotNull();",
" o.toString();",
" }",
"}")
.doTest();
}

@Test
public void supportAssertThatIsNotNull_String() {
public void supportTruthAssertThatIsNotNull_String() {
compilationHelper
.setArgs(
Arrays.asList(
Expand All @@ -1005,17 +1006,18 @@ public void supportAssertThatIsNotNull_String() {
"package com.uber;",
"import java.util.Objects;",
"import javax.annotation.Nullable;",
"import static com.google.common.truth.Truth.assertThat;",
"class Test {",
" private void foo(@Nullable String s) {",
" com.google.common.truth.Truth.assertThat(s).isNotNull();",
" assertThat(s).isNotNull();",
" s.toString();",
" }",
"}")
.doTest();
}

@Test
public void doNotSupportAssertThatWhenDisabled() {
public void doNotSupportTruthAssertThatWhenDisabled() {
compilationHelper
.setArgs(
Arrays.asList(
Expand All @@ -1029,11 +1031,122 @@ public void doNotSupportAssertThatWhenDisabled() {
"import java.lang.Object;",
"import java.util.Objects;",
"import javax.annotation.Nullable;",
"import static com.google.common.truth.Truth.assertThat;",
"class Test {",
" private void foo(@Nullable Object o) {",
" com.google.common.truth.Truth.assertThat(o).isNotNull();",
" private void foo(@Nullable Object a) {",
" assertThat(a).isNotNull();",
" // BUG: Diagnostic contains: dereferenced expression",
" o.toString();",
" a.toString();",
" }",
"}")
.doTest();
}

@Test
public void supportHamcrestAssertThatIsNotNull_Object() {
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.Matchers.*;",
"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 doNotSupportHamcrestAssertThatWhenDisabled() {
compilationHelper
.setArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:HandleTestAssertionLibraries=false"))
.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.Matchers.*;",
"class Test {",
" private void foo(@Nullable Object a) {",
" assertThat(a, is(notNullValue()));",
" // BUG: Diagnostic contains: dereferenced expression",
" a.toString();",
" }",
"}")
.doTest();
}

@Test
public void supportJunitAssertThatIsNotNull_Object() {
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.junit.Assert.assertThat;",
"import static org.hamcrest.Matchers.*;",
"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 doNotSupportJunitAssertThatWhenDisabled() {
compilationHelper
.setArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:HandleTestAssertionLibraries=false"))
.addSourceLines(
"Test.java",
"package com.uber;",
"import java.lang.Object;",
"import java.util.Objects;",
"import javax.annotation.Nullable;",
"import static org.junit.Assert.assertThat;",
"import static org.hamcrest.Matchers.*;",
"class Test {",
" private void foo(@Nullable Object a) {",
" assertThat(a, is(notNullValue()));",
" // BUG: Diagnostic contains: dereferenced expression",
" a.toString();",
" }",
"}")
.doTest();
Expand Down

0 comments on commit c3979b4

Please sign in to comment.