Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore incompatibly annotated var args from Kotlin code. #721

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Ignore incompatibly annotated var args from Kotlin code. #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff9314b
Ignore incompatibly annotated var args from Kotlin code.
lazaroclapp Jan 25, 2023
021f5a4
Formatting
lazaroclapp Jan 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

public class RestrictiveAnnotationHandler extends BaseNoOpHandler {

private static final String JETBRAINS_NOT_NULL = "org.jetbrains.annotations.NotNull";

private final Config config;

RestrictiveAnnotationHandler(Config config) {
Expand Down Expand Up @@ -101,6 +103,20 @@ public Nullness[] onOverrideMethodInvocationParametersNullability(
}
for (int i = 0; i < methodSymbol.getParameters().size(); ++i) {
if (Nullness.paramHasNonNullAnnotation(methodSymbol, i, config)) {
if (methodSymbol.isVarArgs() && i == methodSymbol.getParameters().size() - 1) {
// Special handling: ignore org.jetbrains.annotations.NotNull on varargs parameters
// to handle kotlinc generated jars (see #720)
// We explicitly ignore type-use annotations here, looking for @NotNull used as a
// declaration annotation, which is why this logic is simpler than e.g.
// NullabilityUtil.getAllAnnotationsForParameter.
boolean jetBrainsNotNullAnnotated =
methodSymbol.getParameters().get(i).getAnnotationMirrors().stream()
.map(a -> a.getAnnotationType().toString())
.anyMatch(annotName -> annotName.equals(JETBRAINS_NOT_NULL));
if (jetBrainsNotNullAnnotated) {
continue;
}
}
argumentPositionNullness[i] = Nullness.NONNULL;
} else if (Nullness.paramHasNullableAnnotation(methodSymbol, i, config)) {
argumentPositionNullness[i] = Nullness.NULLABLE;
Expand Down
82 changes: 81 additions & 1 deletion nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,31 @@ public void testNullableVarargs() {
"public class Utilities {",
" public static String takesNullableVarargs(Object o, @Nullable Object... others) {",
" String s = o.toString() + \" \";",
" // BUG: Diagnostic contains: enhanced-for expression others is @Nullable",
" // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", // Shouldn't be an error!
" for (Object other : others) {",
" s += (other == null) ? \"(null) \" : other.toString() + \" \";",
" }",
" // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", // Shouldn't be an error!
" for (Object other : others) {",
" s += other.toString();", // SHOULD be an error!
" }",
" return s;",
" }",
"}")
.addSourceLines(
"Test.java",
"package com.uber;",
"import javax.annotation.Nullable;",
"public class Test {",
" public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {",
" Utilities.takesNullableVarargs(o1, o2, o3, o4);",
" Utilities.takesNullableVarargs(o1);", // Empty var args passed
" Utilities.takesNullableVarargs(o1, o4);",
" Utilities.takesNullableVarargs(o1, (java.lang.Object) null);",
// SHOULD be an error!
" Utilities.takesNullableVarargs(o1, (java.lang.Object[]) null);",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out there is a way to pass null varargs array from Java 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting. Didn't think of this one

" }",
"}")
.doTest();
}

Expand Down Expand Up @@ -97,4 +115,66 @@ public void testNonNullVarargsFromHandler() {
"}")
.doTest();
}

// This is required for compatibility with kotlinc generated jars
// See https://github.com/uber/NullAway/issues/720
@Test
public void testSkipJetbrainsNotNullOnVarArgsFromThirdPartyJars() {
makeTestHelperWithArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated",
"-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
.addSourceLines(
"ThirdParty.java",
"package com.uber.nullaway.lib.unannotated;",
"import org.jetbrains.annotations.NotNull;",
"public class ThirdParty {",
" public static String takesNullableVarargs(@NotNull Object o, @NotNull Object... others) {",
" String s = o.toString() + \" \";",
" for (Object other : others) {",
" s += (other == null) ? \"(null) \" : other.toString() + \" \";",
" }",
" return s;",
" }",
"}")
.addSourceLines(
"FirstParty.java",
"package com.uber;",
"import org.jetbrains.annotations.NotNull;",
"public class FirstParty {",
" public static String takesNonNullVarargs(@NotNull Object o, @NotNull Object... others) {",
" String s = o.toString() + \" \";",
" for (Object other : others) {",
" s += other.toString() + \" \";",
" }",
" return s;",
" }",
"}")
.addSourceLines(
"Test.java",
"package com.uber;",
"import javax.annotation.Nullable;",
"import com.uber.nullaway.lib.unannotated.ThirdParty;",
"public class Test {",
" public void testNullableVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {",
" ThirdParty.takesNullableVarargs(o1, o2, o3, o4);",
" ThirdParty.takesNullableVarargs(o1);", // Empty var args passed
" ThirdParty.takesNullableVarargs(o1, o4);",
" ThirdParty.takesNullableVarargs(o1, (Object) null);",
" ThirdParty.takesNullableVarargs(o1, (Object[]) null);", // SHOULD be an error!
" // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull",
" ThirdParty.takesNullableVarargs(o4);", // First arg is not varargs.
" }",
" public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {",
" FirstParty.takesNonNullVarargs(o1, o2, o3);",
" FirstParty.takesNonNullVarargs(o1);", // Empty var args passed
" // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull",
" FirstParty.takesNonNullVarargs(o1, o4);",
" }",
"}")
.doTest();
}
}