Skip to content

Commit b27f13d

Browse files
msridharmsridhar
authored
Better support for some contracts with boolean argument constraints (uber#1447)
1 parent 4c619e8 commit b27f13d

File tree

2 files changed

+156
-0
lines changed

2 files changed

+156
-0
lines changed

‎nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.errorprone.VisitorState;
3131
import com.google.errorprone.util.ASTHelpers;
3232
import com.sun.source.tree.ClassTree;
33+
import com.sun.source.tree.ExpressionTree;
3334
import com.sun.source.tree.MethodInvocationTree;
3435
import com.sun.tools.javac.code.Symbol;
3536
import com.uber.nullaway.Config;
@@ -209,6 +210,18 @@ public NullnessHint onDataflowVisitMethodInvocation(
209210
if (valueConstraint.equals("_")) {
210211
// do nothing
211212
} else if (valueConstraint.equals("false") || valueConstraint.equals("true")) {
213+
ExpressionTree argumentTree = tree.getArguments().get(i);
214+
Object constValue = ASTHelpers.constValue(argumentTree);
215+
if (constValue instanceof Boolean booleanValue) {
216+
boolean booleanConstraintValue = valueConstraint.equals("true");
217+
if (booleanValue == booleanConstraintValue) {
218+
// Antecedent is satisfied by a compile-time boolean constant
219+
continue;
220+
}
221+
// constant passed is opposite of antecedent
222+
supported = false;
223+
break;
224+
}
212225
// We handle boolean constraints in the case that the boolean argument is the result
213226
// of a null or not-null check. For example,
214227
// '@Contract("true -> true") boolean func(boolean v)'

‎nullaway/src/test/java/com/uber/nullaway/ContractsTests.java‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,149 @@ String test(Object required) {
732732
.doTest();
733733
}
734734

735+
@Test
736+
public void booleanToNotNullContract() {
737+
makeTestHelperWithArgs(
738+
withJSpecifyModeArgs(
739+
Arrays.asList(
740+
"-d",
741+
temporaryFolder.getRoot().getAbsolutePath(),
742+
"-XepOpt:NullAway:OnlyNullMarked=true")))
743+
.addSourceLines(
744+
"Test.java",
745+
"""
746+
import org.jspecify.annotations.NullMarked;
747+
import org.jspecify.annotations.Nullable;
748+
import org.jetbrains.annotations.Contract;
749+
@NullMarked
750+
class Test {
751+
@Contract("false -> !null")
752+
@Nullable String nonNullWhenPassedFalse(boolean returnNull) {
753+
if (returnNull) {
754+
return null;
755+
}
756+
return "foo";
757+
}
758+
@Contract("true -> !null")
759+
@Nullable String nonNullWhenPassedTrue(boolean dontReturnNull) {
760+
if (dontReturnNull) {
761+
return "foo";
762+
}
763+
return null;
764+
}
765+
void testNegative() {
766+
nonNullWhenPassedFalse(false).hashCode();
767+
nonNullWhenPassedTrue(true).hashCode();
768+
}
769+
void testPositive(boolean b) {
770+
// BUG: Diagnostic contains: dereferenced expression
771+
nonNullWhenPassedFalse(true).hashCode();
772+
// false positive expected here since we do not do boolean reasoning
773+
// BUG: Diagnostic contains: dereferenced expression
774+
nonNullWhenPassedFalse(b && !b).hashCode();
775+
// BUG: Diagnostic contains: dereferenced expression
776+
nonNullWhenPassedTrue(false).hashCode();
777+
// false positive expected here since we do not do boolean reasoning
778+
// BUG: Diagnostic contains: dereferenced expression
779+
nonNullWhenPassedTrue(b || !b).hashCode();
780+
}
781+
}
782+
""")
783+
.doTest();
784+
}
785+
786+
@Test
787+
public void multiArgBooleanToNotNullContract() {
788+
makeTestHelperWithArgs(
789+
withJSpecifyModeArgs(
790+
Arrays.asList(
791+
"-d",
792+
temporaryFolder.getRoot().getAbsolutePath(),
793+
"-XepOpt:NullAway:OnlyNullMarked=true")))
794+
.addSourceLines(
795+
"Test.java",
796+
"""
797+
import org.jspecify.annotations.NullMarked;
798+
import org.jspecify.annotations.Nullable;
799+
import org.jetbrains.annotations.Contract;
800+
@NullMarked
801+
class Test {
802+
@Contract("false, _ -> !null")
803+
@Nullable String nonNullWhenFirstFalse(boolean returnNull, @Nullable String value) {
804+
if (returnNull) {
805+
return null;
806+
}
807+
return "foo";
808+
}
809+
@Contract("false, true -> !null")
810+
@Nullable String nonNullWhenFalseTrue(boolean returnNull, boolean forceNonNull) {
811+
if (returnNull || !forceNonNull) {
812+
return null;
813+
}
814+
return "baz";
815+
}
816+
@Contract("_, true -> !null")
817+
@Nullable String nonNullWhenSecondTrue(@Nullable String value, boolean dontReturnNull) {
818+
if (dontReturnNull) {
819+
return "bar";
820+
}
821+
return null;
822+
}
823+
void testNegative() {
824+
nonNullWhenFirstFalse(false, null).hashCode();
825+
nonNullWhenFalseTrue(false, true).hashCode();
826+
nonNullWhenSecondTrue(null, true).hashCode();
827+
}
828+
void testPositive(boolean b) {
829+
// BUG: Diagnostic contains: dereferenced expression
830+
nonNullWhenFirstFalse(true, null).hashCode();
831+
// BUG: Diagnostic contains: dereferenced expression
832+
nonNullWhenFalseTrue(false, false).hashCode();
833+
// BUG: Diagnostic contains: dereferenced expression
834+
nonNullWhenSecondTrue(null, false).hashCode();
835+
}
836+
}
837+
""")
838+
.doTest();
839+
}
840+
841+
@Test
842+
public void multiArgBooleanAndNullToNotNullContract() {
843+
makeTestHelperWithArgs(
844+
withJSpecifyModeArgs(
845+
Arrays.asList(
846+
"-d",
847+
temporaryFolder.getRoot().getAbsolutePath(),
848+
"-XepOpt:NullAway:OnlyNullMarked=true")))
849+
.addSourceLines(
850+
"Test.java",
851+
"""
852+
import org.jspecify.annotations.NullMarked;
853+
import org.jspecify.annotations.Nullable;
854+
import org.jetbrains.annotations.Contract;
855+
@NullMarked
856+
class Test {
857+
@Contract("false, !null -> !null")
858+
@Nullable String nonNullWhenFalseAndNonNull(boolean returnNull, @Nullable Object obj) {
859+
if (returnNull || obj == null) {
860+
return null;
861+
}
862+
return "ok";
863+
}
864+
void testNegative() {
865+
nonNullWhenFalseAndNonNull(false, new Object()).hashCode();
866+
}
867+
void testPositive(boolean b, @Nullable Object obj) {
868+
// BUG: Diagnostic contains: dereferenced expression
869+
nonNullWhenFalseAndNonNull(true, new Object()).hashCode();
870+
// BUG: Diagnostic contains: dereferenced expression
871+
nonNullWhenFalseAndNonNull(false, null).hashCode();
872+
}
873+
}
874+
""")
875+
.doTest();
876+
}
877+
735878
@Test
736879
public void checkNotNullToNotNullContract() {
737880
makeTestHelperWithArgs(

0 commit comments

Comments
 (0)