Skip to content

Commit

Permalink
Support more test assertions in OptionalEmptinessHandler (#718)
Browse files Browse the repository at this point in the history 
supported with JUnit 4:
```
Assert.assertTrue(x.isPresent())
Assert.assertFalse(x.isEmpty())
```

supported with JUnit 5:
```
Assertions.assertTrue(x.isPresent())
Assertions.assertFalse(x.isEmpty())
```

supported with AssertJ:
```
assertThat(x.isPresent()).isTrue()
assertThat(x.isEmpty()).isFalse()

assertThat(x).isPresent()
assertThat(x).isNotEmpty()
```

supported with Truth:
```
assertThat(x.isPresent()).isTrue()
assertThat(x.isEmpty()).isFalse()
```
  • Loading branch information
@XN137
XN137 authored Jan 22, 2023
1 parent c4c7c4b commit d809795
Show file tree
Hide file tree
Showing 5 changed files with 413 additions and 30 deletions.
2 changes: 2 additions & 0 deletions jdk17-unit-tests/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ configurations {
dependencies {
testImplementation project(":nullaway")
testImplementation deps.test.junit4
testImplementation deps.test.junit5Jupiter
testImplementation deps.test.assertJ
testImplementation(deps.build.errorProneTestHelpers) {
exclude group: "junit", module: "junit"
}
Expand Down
99 changes: 99 additions & 0 deletions jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.google.errorprone.CompilationTestHelper;
import com.uber.nullaway.NullAway;
import java.util.Arrays;
import java.util.List;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -95,4 +96,102 @@ public void optionalIsEmptyPositive() {
"}")
.doTest();
}

@Test
public void optionalIsEmptyHandleAssertionLibraryTruthAssertThat() {
makeTestHelperWithArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:CheckOptionalEmptiness=true",
"-XepOpt:NullAway:HandleTestAssertionLibraries=true"))
.addSourceLines(
"Test.java",
"package com.uber;",
"import java.util.Optional;",
"import com.google.common.truth.Truth;",
"",
"public class Test {",
" void truthAssertThatIsEmptyIsFalse() {",
" Optional<Object> b = Optional.empty();",
" Truth.assertThat(b.isEmpty()).isTrue(); // no impact",
" // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b",
" b.get().toString();",
" Truth.assertThat(b.isEmpty()).isFalse();",
" b.get().toString();",
" }",
"}")
.doTest();
}

@Test
public void optionalIsEmptyHandleAssertionLibraryAssertJAssertThat() {
makeTestHelperWithArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:CheckOptionalEmptiness=true",
"-XepOpt:NullAway:HandleTestAssertionLibraries=true"))
.addSourceLines(
"Test.java",
"package com.uber;",
"import java.util.Optional;",
"import org.assertj.core.api.Assertions;",
"",
"public class Test {",
" void assertJAssertThatIsEmptyIsFalse() {",
" Optional<Object> b = Optional.empty();",
" Assertions.assertThat(b.isEmpty()).isTrue(); // no impact",
" // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b",
" b.get().toString();",
" Assertions.assertThat(b.isEmpty()).isFalse();",
" b.get().toString();",
" }",
"}")
.doTest();
}

@Test
public void optionalIsEmptyHandleAssertionLibraryJUnitAssertions() {
makeTestHelperWithArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:CheckOptionalEmptiness=true",
"-XepOpt:NullAway:HandleTestAssertionLibraries=true"))
.addSourceLines(
"Test.java",
"package com.uber;",
"import java.util.Optional;",
"import org.junit.Assert;",
"import org.junit.jupiter.api.Assertions;",
"",
"public class Test {",
" void junit4AssertFalseIsEmpty() {",
" Optional<Object> b = Optional.empty();",
" Assert.assertTrue(b.isEmpty()); // no impact",
" // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b",
" b.get().toString();",
" Assert.assertFalse(\"errormsg\", b.isEmpty());",
" b.get().toString();",
" }",
"",
" void junit5AssertFalseIsEmpty() {",
" Optional<Object> d = Optional.empty();",
" Assertions.assertTrue(d.isEmpty()); // no impact",
" // BUG: Diagnostic contains: Invoking get() on possibly empty Optional d",
" d.get().toString();",
" Assertions.assertFalse(d.isEmpty(), \"errormsg\");",
" d.get().toString();",
" }",
"}")
.doTest();
}

protected CompilationTestHelper makeTestHelperWithArgs(List<String> args) {
return CompilationTestHelper.newInstance(NullAway.class, getClass()).setArgs(args);
}
}
72 changes: 68 additions & 4 deletions nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,25 @@ class MethodNameUtil {
private static final String IS_NOT_NULL_OWNER_TRUTH = "com.google.common.truth.Subject";
private static final String IS_NOT_NULL_OWNER_ASSERTJ = "org.assertj.core.api.AbstractAssert";
private static final String IS_TRUE_METHOD = "isTrue";
private static final String IS_TRUE_OWNER = "com.google.common.truth.BooleanSubject";
private static final String IS_FALSE_METHOD = "isFalse";
private static final String IS_TRUE_OWNER_TRUTH = "com.google.common.truth.BooleanSubject";
private static final String IS_TRUE_OWNER_ASSERTJ = "org.assertj.core.api.AbstractBooleanAssert";
private static final String BOOLEAN_VALUE_OF_METHOD = "valueOf";
private static final String BOOLEAN_VALUE_OF_OWNER = "java.lang.Boolean";
private static final String IS_PRESENT_METHOD = "isPresent";
private static final String IS_NOT_EMPTY_METHOD = "isNotEmpty";
private static final String IS_PRESENT_OWNER_ASSERTJ =
"org.assertj.core.api.AbstractOptionalAssert";
private static final String ASSERT_THAT_METHOD = "assertThat";
private static final String ASSERT_THAT_OWNER_TRUTH = "com.google.common.truth.Truth";
private static final String ASSERT_THAT_OWNER_ASSERTJ = "org.assertj.core.api.Assertions";

private static final String HAMCREST_ASSERT_CLASS = "org.hamcrest.MatcherAssert";
private static final String JUNIT_ASSERT_CLASS = "org.junit.Assert";
private static final String JUNIT5_ASSERTION_CLASS = "org.junit.jupiter.api.Assertions";

private static final String ASSERT_TRUE_METHOD = "assertTrue";
private static final String ASSERT_FALSE_METHOD = "assertFalse";

private static final String MATCHERS_CLASS = "org.hamcrest.Matchers";
private static final String CORE_MATCHERS_CLASS = "org.hamcrest.CoreMatchers";
Expand All @@ -67,7 +79,15 @@ class MethodNameUtil {
private Name isNotNullOwnerAssertJ;

private Name isTrue;
private Name isTrueOwner;
private Name isFalse;
private Name isTrueOwnerTruth;
private Name isTrueOwnerAssertJ;
private Name isPresent;
private Name isNotEmpty;
private Name isPresentOwnerAssertJ;

private Name isBooleanValueOfMethod;
private Name isBooleanValueOfOwner;

private Name assertThat;
private Name assertThatOwnerTruth;
Expand All @@ -76,6 +96,10 @@ class MethodNameUtil {
// Names for junit assertion libraries.
private Name hamcrestAssertClass;
private Name junitAssertClass;
private Name junit5AssertionClass;

private Name assertTrue;
private Name assertFalse;

// Names for hamcrest matchers.
private Name matchersClass;
Expand All @@ -93,14 +117,27 @@ void initializeMethodNames(Name.Table table) {
isNotNullOwnerAssertJ = table.fromString(IS_NOT_NULL_OWNER_ASSERTJ);

isTrue = table.fromString(IS_TRUE_METHOD);
isTrueOwner = table.fromString(IS_TRUE_OWNER);
isFalse = table.fromString(IS_FALSE_METHOD);
isTrueOwnerTruth = table.fromString(IS_TRUE_OWNER_TRUTH);
isTrueOwnerAssertJ = table.fromString(IS_TRUE_OWNER_ASSERTJ);

isBooleanValueOfMethod = table.fromString(BOOLEAN_VALUE_OF_METHOD);
isBooleanValueOfOwner = table.fromString(BOOLEAN_VALUE_OF_OWNER);

assertThat = table.fromString(ASSERT_THAT_METHOD);
assertThatOwnerTruth = table.fromString(ASSERT_THAT_OWNER_TRUTH);
assertThatOwnerAssertJ = table.fromString(ASSERT_THAT_OWNER_ASSERTJ);

isPresent = table.fromString(IS_PRESENT_METHOD);
isNotEmpty = table.fromString(IS_NOT_EMPTY_METHOD);
isPresentOwnerAssertJ = table.fromString(IS_PRESENT_OWNER_ASSERTJ);

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

assertTrue = table.fromString(ASSERT_TRUE_METHOD);
assertFalse = table.fromString(ASSERT_FALSE_METHOD);

matchersClass = table.fromString(MATCHERS_CLASS);
coreMatchersClass = table.fromString(CORE_MATCHERS_CLASS);
Expand All @@ -116,8 +153,35 @@ boolean isMethodIsNotNull(Symbol.MethodSymbol methodSymbol) {
|| matchesMethod(methodSymbol, isNotNull, isNotNullOwnerAssertJ);
}

boolean isMethodAssertTrue(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertTrue, junitAssertClass)
|| matchesMethod(methodSymbol, assertTrue, junit5AssertionClass);
}

boolean isMethodAssertFalse(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertFalse, junitAssertClass)
|| matchesMethod(methodSymbol, assertFalse, junit5AssertionClass);
}

boolean isMethodThatEnsuresOptionalPresent(Symbol.MethodSymbol methodSymbol) {
// same owner
return matchesMethod(methodSymbol, isPresent, isPresentOwnerAssertJ)
|| matchesMethod(methodSymbol, isNotEmpty, isPresentOwnerAssertJ);
}

boolean isMethodIsTrue(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, isTrue, isTrueOwner);
return matchesMethod(methodSymbol, isTrue, isTrueOwnerTruth)
|| matchesMethod(methodSymbol, isTrue, isTrueOwnerAssertJ);
}

boolean isMethodIsFalse(Symbol.MethodSymbol methodSymbol) {
// same owners as isTrue
return matchesMethod(methodSymbol, isFalse, isTrueOwnerTruth)
|| matchesMethod(methodSymbol, isFalse, isTrueOwnerAssertJ);
}

boolean isMethodBooleanValueOf(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, isBooleanValueOfMethod, isBooleanValueOfOwner);
}

boolean isMethodAssertThat(Symbol.MethodSymbol methodSymbol) {
Expand Down
Loading

0 comments on commit d809795

Please sign in to comment.