Skip to content

Commit a1df1c4

Browse files
authored
Always acknowledge restrictive annotations in JSpecify mode (#1144)
Previously, we neglected to acknowledge explicit `@Nullable` return types and `@NonNull` parameter types in `@NullUnmarked` code unless the `AcknowledgeRestrictiveAnnotations` setting was passed. But, the JSpecify spec requires acknowledging restrictive annotations in `@NullUnmarked` code. So now, in JSpecify mode, we always acknowledge restrictive annotations. Note that in JSpecify mode, this change will also impact handling of other "unannotated" code that is not explicitly annotated `@NullUnmarked`. But, as discussed in #978, we want to switch to acknowledging restrictive annotations by default anyway. It seems reasonable to go ahead and make this change in JSpecify mode, to ensure greater spec compliance.
1 parent 7fa7bf9 commit a1df1c4

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ public static Handler buildDefault(Config config) {
4646
ImmutableList.Builder<Handler> handlerListBuilder = ImmutableList.builder();
4747
MethodNameUtil methodNameUtil = new MethodNameUtil();
4848

49-
if (config.acknowledgeRestrictiveAnnotations()) {
49+
// Acknowledge restrictive annotations if in JSpecify mode or if the user has explicitly
50+
// requested it
51+
boolean acknowledgeRestrictive =
52+
config.acknowledgeRestrictiveAnnotations() || config.isJSpecifyMode();
53+
if (acknowledgeRestrictive) {
5054
// This runs before LibraryModelsHandler, so that library models can override third-party
5155
// bytecode annotations
5256
handlerListBuilder.add(new RestrictiveAnnotationHandler(config));

nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,68 @@ public void nullUnmarkedMethodLevel() {
866866
.doTest();
867867
}
868868

869+
@Test
870+
public void nullUnmarkedMethodWithNonNullParamJSpecifyMode() {
871+
makeTestHelperWithArgs(
872+
Arrays.asList(
873+
"-d",
874+
temporaryFolder.getRoot().getAbsolutePath(),
875+
"-XepOpt:NullAway:OnlyNullMarked=true",
876+
"-XepOpt:NullAway:JSpecifyMode=true"))
877+
.addSourceLines(
878+
"Foo.java",
879+
"package com.uber;",
880+
"import org.jspecify.annotations.NullUnmarked;",
881+
"import org.jspecify.annotations.NullMarked;",
882+
"import org.jspecify.annotations.NonNull;",
883+
"@NullMarked",
884+
"public class Foo {",
885+
" @NullUnmarked",
886+
" public static void callee(@NonNull Object o) {",
887+
" }",
888+
" @NullUnmarked",
889+
" public static void callee2(Object o) {",
890+
" }",
891+
" public static void caller() {",
892+
" // Error due to explicit @NonNull annotation",
893+
" // BUG: Diagnostic contains: passing @Nullable parameter",
894+
" callee(null);",
895+
" // this is fine",
896+
" callee2(null);",
897+
" }",
898+
"}")
899+
.doTest();
900+
}
901+
902+
@Test
903+
public void nullUnmarkedMethodWithNullableReturnJSpecifyMode() {
904+
makeTestHelperWithArgs(
905+
Arrays.asList(
906+
"-d",
907+
temporaryFolder.getRoot().getAbsolutePath(),
908+
"-XepOpt:NullAway:OnlyNullMarked=true",
909+
"-XepOpt:NullAway:JSpecifyMode=true"))
910+
.addSourceLines(
911+
"Foo.java",
912+
"package com.uber;",
913+
"import org.jspecify.annotations.NullUnmarked;",
914+
"import org.jspecify.annotations.NullMarked;",
915+
"import org.jspecify.annotations.Nullable;",
916+
"@NullMarked",
917+
"public class Foo {",
918+
" @NullUnmarked",
919+
" public static @Nullable String callee() {",
920+
" return null;",
921+
" }",
922+
" public static void caller() {",
923+
" // Error due to explicit @Nullable annotation",
924+
" // BUG: Diagnostic contains: dereferenced expression callee() is @Nullable",
925+
" callee().toString();",
926+
" }",
927+
"}")
928+
.doTest();
929+
}
930+
869931
@Test
870932
public void nullUnmarkedOuterMethodLevelWithLocalClass() {
871933
defaultCompilationHelper

0 commit comments

Comments
 (0)