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

Fix printing of array types in JSpecify errors #1145

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Fix printing of array types in JSpecify errors #1145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,13 @@ public String visitCapturedType(Type.CapturedType t, Void s) {

@Override
public String visitArrayType(Type.ArrayType t, Void unused) {
// TODO properly print cases like int @Nullable[]
return t.elemtype.accept(this, null) + "[]";
StringBuilder sb = new StringBuilder();
sb.append(t.elemtype.accept(this, null));
for (Attribute.TypeCompound compound : t.getAnnotationMirrors()) {
sb.append(" @");
sb.append(compound.type.accept(this, null));
}
return sb.append(" []").toString();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ public void genericPrimitiveArrayTypeAssignment() {
"class Test {",
" static class A<T extends @Nullable Object> { }",
" static void testPositive() {",
" // BUG: Diagnostic contains: Cannot assign from type A<int[]>",
" // BUG: Diagnostic contains: Cannot assign from type A<int []>",
" A<int @Nullable[]> x = new A<int[]>();",
" }",
" static void testNegative() {",
Expand Down
20 changes: 10 additions & 10 deletions nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyArrayTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public void arraySubtyping() {
" @Nullable Integer[] x2 = nullableIntArr;",
" // legal (covariant array subtypes)",
" x2 = nonnullIntArr;",
" // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]",
" // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer [] to type Integer []",
" x1 = nullableIntArr;",
" }",
"}")
Expand All @@ -272,7 +272,7 @@ public void arraySubtypingWithNewExpression() {
" @Nullable Integer[] x2 = new Integer[0];",
" // legal",
" x2 = new @Nullable Integer[0];",
" // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]",
" // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer [] to type Integer []",
" x1 = new @Nullable Integer[0];",
" }",
"}")
Expand All @@ -290,7 +290,7 @@ public void arraysAndGenerics() {
"class Test {",
" void foo(List<@Nullable Integer[]> l) {}",
" void testPositive(List<Integer[]> p) {",
" // BUG: Diagnostic contains: Cannot pass parameter of type List<Integer[]>",
" // BUG: Diagnostic contains: Cannot pass parameter of type List<Integer []>",
" foo(p);",
" }",
" void testNegative(List<@Nullable Integer[]> p) {",
Expand All @@ -312,7 +312,7 @@ public void genericArraysReturnedAndPassed() {
" static class Bar<T> {",
" Foo<T>[] getFoosPositive() {",
" @Nullable Foo<T>[] result = new Foo[0];",
" // BUG: Diagnostic contains: Cannot return expression of type @Nullable Foo<T>[] from method",
" // BUG: Diagnostic contains: Cannot return expression of type @Nullable Foo<T> [] from method",
" return result;",
" }",
" Foo<T>[] getFoosNegative() {",
Expand All @@ -321,7 +321,7 @@ public void genericArraysReturnedAndPassed() {
" }",
" void takeFoos(Foo<T>[] foos) {}",
" void callTakeFoosPositive(@Nullable Foo<T>[] p) {",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T>[]",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T> []",
" takeFoos(p);",
" }",
" void callTakeFoosNegative(Foo<T>[] p) {",
Expand All @@ -331,9 +331,9 @@ public void genericArraysReturnedAndPassed() {
" void callTakeFoosVarargsPositive(@Nullable Foo<T>[] p, Foo<T>[] p2) {",
" // Under the hood, a @Nullable Foo<T>[][] is passed, which is not a subtype",
" // of the formal parameter type Foo<T>[][]",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T>[]",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T> []",
" takeFoosVarargs(p);",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T>[]",
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable Foo<T> []",
" takeFoosVarargs(p2, p);",
" }",
" void callTakeFoosVarargsNegative(Foo<T>[] p) {",
Expand Down Expand Up @@ -367,7 +367,7 @@ public void overridesReturnType() {
" @Override",
" Integer[] foo() { return new Integer[0]; }",
" @Override",
" // BUG: Diagnostic contains: Method returns @Nullable Integer[], but overridden method returns Integer[]",
" // BUG: Diagnostic contains: Method returns @Nullable Integer [], but overridden method returns Integer []",
" @Nullable Integer[] bar() { return new @Nullable Integer[0]; }",
" }",
"}")
Expand All @@ -389,7 +389,7 @@ public void overridesParameterType() {
" }",
" class Sub extends Super {",
" @Override",
" // BUG: Diagnostic contains: Parameter has type Integer[], but overridden method has parameter type @Nullable Integer[]",
" // BUG: Diagnostic contains: Parameter has type Integer [], but overridden method has parameter type @Nullable Integer []",
" void foo(Integer[] p) { }",
" @Override",
" void bar(@Nullable Integer[] p) { }",
Expand All @@ -407,7 +407,7 @@ public void ternaryOperator() {
"import org.jspecify.annotations.Nullable;",
"class Test {",
" static Integer[] testPositive(Integer[] p, boolean t) {",
" // BUG: Diagnostic contains: Conditional expression must have type Integer[]",
" // BUG: Diagnostic contains: Conditional expression must have type Integer []",
" Integer[] t1 = t ? new Integer[0] : new @Nullable Integer[0];",
" // BUG: Diagnostic contains: Conditional expression must have type",
" return t ? new @Nullable Integer[0] : new @Nullable Integer[0];",
Expand Down
7 changes: 3 additions & 4 deletions nullaway/src/test/java/com/uber/nullaway/jspecify/JSpecifyVarargsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,14 +535,13 @@ public void varargsOverride() {
" }",
" static class NullableVarargsContentsImpl2 implements NullableVarargsContents {",
" @Override",
" // BUG: Diagnostic contains: Parameter has type Object[], but overridden method",
" // BUG: Diagnostic contains: Parameter has type Object [], but overridden method",
" public void varargs(Object... params) {",
" }",
" }",
" static class NullableVarargsContentsImpl3 implements NullableVarargsContents {",
" @Override",
// TODO open an issue to improve the error message in a follow up
" // BUG: Diagnostic contains: Parameter has type Object[]",
" // BUG: Diagnostic contains: Parameter has type Object @Nullable []",
" public void varargs(Object @Nullable... params) {",
" }",
" }",
Expand Down Expand Up @@ -596,7 +595,7 @@ public void varargsOverride() {
" }",
" static class NullableVarargsBothImpl3 implements NullableVarargsBoth {",
" @Override",
" // BUG: Diagnostic contains: Parameter has type Object[]",
" // BUG: Diagnostic contains: Parameter has type Object @Nullable []",
Copy link
Collaborator

@akshayutture akshayutture Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Does the diagnostic say "Object @nullable []" now instead of "Object []" because the default behavior is that un-annotated parameters are nullable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, we just weren't printing the type properly at all before. If we have a varargs parameter Object @Nullable... params, the type of the params array is Object @Nullable []. We're just fixing a serious bug in printing types in error messages.

" public void varargs(Object @Nullable... params) {",
" }",
" }",
Expand Down