Skip to content

Commit 8f40500

Browse files
msridharmsridhar
authored
JSpecify: Improve error messages for type incompatibility at pseudo-assignments (#1279)
Here is the new test: ```java @test public void assignmentIncompatibilityViaExtendsErrorMessage() { makeHelper() .addSourceLines( "Test.java", "import org.jspecify.annotations.*;", "@NullMarked", "class Test {", " class A<T extends @nullable Object> {}", " class B<T extends @nullable Object> extends A<@nullable T> {}", " class C<T extends @nullable Object> extends A<@nonnull T> {}", " void test() {", " // BUG: Diagnostic contains: incompatible types: Test.B<Object> cannot be converted to Test.A<Object> (Test.B<Object> is a subtype of Test.A<@nullable Object>)", " A<Object> a = new B<Object>();", " // BUG: Diagnostic contains: incompatible types: Test.C<@nullable Object> cannot be converted to Test.A<@nullable Object> (Test.C<@nullable Object> is a subtype of Test.A<@nonnull Object>)", " A<@nullable Object> a2 = new C<@nullable Object>();", " }", "}") .doTest(); } ``` Before, the first error message would be something like: ``` Cannot assign from type Test.B<Object> to type Test.A<Object> due to mismatched nullability of type parameters. ``` This message is confusing, since it looks from the message like the type arguments have the same nullability. Here we change the message to: ``` incompatible types: Test.B<Object> cannot be converted to Test.A<Object> (Test.B<Object> is a subtype of Test.A<@nullable Object>) ``` The "incompatible types" and "cannot be converted to" verbiage is taken from `javac`, so hopefully it's familiar to developers. The final part of the message showing that `B<Object> <: A<@nullable Object>` should hopefully help developers figure out the root cause of the issue. (Interestingly, for an equivalent example in Swift, only `A<@nullable Object>` is shown in the error message, not `B<Object>`.) The final part of the message is only shown if the base types involved are not the same. We also make the error messages for parameter passing, returns, and assignments the same (matching `javac`), and update a whole bunch of tests to expect the new error messages. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Standardized diagnostics for generic nullability/type incompatibilities across assignments, parameter passing, and returns. * Messages now use “incompatible types: <rhs> cannot be converted to <lhs>” and may include helpful subtype hints where applicable. * No changes to public APIs. * Refactor * Centralized construction of error messages for consistency. * Tests * Updated expected diagnostics across generics and array tests to match the new format. * Added a test validating extends-based incompatibility messages. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 9272d8b commit 8f40500

5 files changed

Lines changed: 116 additions & 84 deletions

File tree

‎nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -253,32 +253,46 @@ private void reportInvalidInstantiationError(
253253
private void reportInvalidAssignmentInstantiationError(
254254
Tree tree, Type lhsType, Type rhsType, VisitorState state) {
255255
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
256+
String msg = errorMessageForIncompatibleTypesAtPseudoAssignment(lhsType, rhsType, state);
256257
ErrorMessage errorMessage =
257-
new ErrorMessage(
258-
ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE,
259-
String.format(
260-
"Cannot assign from type "
261-
+ prettyTypeForError(rhsType, state)
262-
+ " to type "
263-
+ prettyTypeForError(lhsType, state)
264-
+ " due to mismatched nullability of type parameters"));
258+
new ErrorMessage(ErrorMessage.MessageTypes.ASSIGN_GENERIC_NULLABLE, msg);
265259
state.reportMatch(
266260
errorBuilder.createErrorDescription(
267261
errorMessage, analysis.buildDescription(tree), state, null));
268262
}
269263

264+
private String errorMessageForIncompatibleTypesAtPseudoAssignment(
265+
Type lhsType, Type rhsType, VisitorState state) {
266+
String prettyRhsType = prettyTypeForError(rhsType, state);
267+
String result =
268+
String.format(
269+
"incompatible types: %s cannot be converted to %s",
270+
prettyRhsType, prettyTypeForError(lhsType, state));
271+
if (!ASTHelpers.isSameType(lhsType, rhsType, state)
272+
&& lhsType.getKind() == TypeKind.DECLARED
273+
&& rhsType.getKind() == TypeKind.DECLARED) {
274+
Symbol.TypeSymbol lhsSym = lhsType.asElement();
275+
if (lhsSym instanceof Symbol.ClassSymbol) {
276+
Type asSuper =
277+
TypeSubstitutionUtils.asSuper(
278+
state.getTypes(), rhsType, (Symbol.ClassSymbol) lhsSym, config);
279+
if (asSuper != null) {
280+
result +=
281+
String.format(
282+
" (%s is a subtype of %s)", prettyRhsType, prettyTypeForError(asSuper, state));
283+
}
284+
}
285+
}
286+
return result;
287+
}
288+
270289
private void reportInvalidReturnTypeError(
271290
Tree tree, Type methodType, Type returnType, VisitorState state) {
272291
ErrorBuilder errorBuilder = analysis.getErrorBuilder();
273292
ErrorMessage errorMessage =
274293
new ErrorMessage(
275294
ErrorMessage.MessageTypes.RETURN_NULLABLE_GENERIC,
276-
String.format(
277-
"Cannot return expression of type "
278-
+ prettyTypeForError(returnType, state)
279-
+ " from method with return type "
280-
+ prettyTypeForError(methodType, state)
281-
+ " due to mismatched nullability of type parameters"));
295+
errorMessageForIncompatibleTypesAtPseudoAssignment(methodType, returnType, state));
282296
state.reportMatch(
283297
errorBuilder.createErrorDescription(
284298
errorMessage, analysis.buildDescription(tree), state, null));
@@ -310,11 +324,8 @@ private void reportInvalidParametersNullabilityError(
310324
ErrorMessage errorMessage =
311325
new ErrorMessage(
312326
ErrorMessage.MessageTypes.PASS_NULLABLE_GENERIC,
313-
"Cannot pass parameter of type "
314-
+ prettyTypeForError(actualParameterType, state)
315-
+ ", as formal parameter has type "
316-
+ prettyTypeForError(formalParameterType, state)
317-
+ ", which has mismatched type parameter nullability");
327+
errorMessageForIncompatibleTypesAtPseudoAssignment(
328+
formalParameterType, actualParameterType, state));
318329
state.reportMatch(
319330
errorBuilder.createErrorDescription(
320331
errorMessage, analysis.buildDescription(paramExpression), state, null));

‎nullaway/src/test/java/com/uber/nullaway/jspecify/BytecodeGenericsTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void genericsChecksForAssignments() {
6464
"import com.uber.lib.generics.NullableTypeParam;",
6565
"class Test {",
6666
" static void testPositive(NullableTypeParam<@Nullable String> t1) {",
67-
" // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
67+
" // BUG: Diagnostic contains: incompatible types: NullableTypeParam<@Nullable String>",
6868
" NullableTypeParam<String> t2 = t1;",
6969
" }",
7070
" static void testNegative(NullableTypeParam<@Nullable String> t1) {",
@@ -86,9 +86,9 @@ public void genericsChecksForFieldAssignments() {
8686
"import com.uber.lib.generics.NullableTypeParam;",
8787
"class Test {",
8888
" static void testPositive(NullableTypeParam<String> t1) {",
89-
" // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<String>",
89+
" // BUG: Diagnostic contains: incompatible types: NullableTypeParam<String>",
9090
" NullableTypeParam.staticField = t1;",
91-
" // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
91+
" // BUG: Diagnostic contains: incompatible types: NullableTypeParam<@Nullable String>",
9292
" NullableTypeParam<String> t2 = NullableTypeParam.staticField;",
9393
" }",
9494
" static void testNegative(NullableTypeParam<@Nullable String> t1) {",
@@ -113,9 +113,9 @@ public void genericsChecksForParamPassingAndReturns() {
113113
"import com.uber.lib.generics.GenericTypeArgMethods;",
114114
"class Test {",
115115
" static void testPositive(NullableTypeParam<String> t1) {",
116-
" // BUG: Diagnostic contains: Cannot pass parameter of type NullableTypeParam<String>",
116+
" // BUG: Diagnostic contains: incompatible types: NullableTypeParam<String>",
117117
" GenericTypeArgMethods.nullableTypeParamArg(t1);",
118-
" // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
118+
" // BUG: Diagnostic contains: incompatible types: NullableTypeParam<@Nullable String>",
119119
" NullableTypeParam<String> t2 = GenericTypeArgMethods.nullableTypeParamReturn();",
120120
" }",
121121
" static void testNegative(NullableTypeParam<@Nullable String> t1) {",

‎nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public void genericInferenceOnAssignments() {
190190
" Foo<@Nullable Object> f5 = Foo.makeNonNull(null);",
191191
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
192192
" Foo<Object> f6 = Foo.makeNonNull(null);",
193-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
193+
" // BUG: Diagnostic contains: incompatible types:",
194194
" Foo<@Nullable Object> f7 = Foo.makeNonNull(new Object());",
195195
" Foo<Object> f8 = Foo.makeNonNull(new Object());",
196196
" }",
@@ -241,7 +241,7 @@ public void multipleParametersOneNested() {
241241
" static void test(Foo<@Nullable Object> f1, Foo<Object> f2) {",
242242
" // no error expected",
243243
" Foo<@Nullable Object> result = Foo.create(null, f1);",
244-
" // BUG: Diagnostic contains: Cannot assign from type Foo<Object> to type Foo<@Nullable Object>",
244+
" // BUG: Diagnostic contains: incompatible types: Foo<Object> cannot be converted to Foo<@Nullable Object>",
245245
" Foo<@Nullable Object> result2 = Foo.create(null, f2);",
246246
" }",
247247
" }",
@@ -324,9 +324,9 @@ public void genericsInferenceOnAssignmentsWithGenericClasses() {
324324
" }",
325325
" static void test(Foo<Void, Void> f) {",
326326
" Foo<Integer, ArrayList<String>> fooNonNull_1 = f.nonNullTest();",
327-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
327+
" // BUG: Diagnostic contains: incompatible types:",
328328
" Foo<Integer, ArrayList<@Nullable String>> fooNonNull_2 = f.nonNullTest();",
329-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
329+
" // BUG: Diagnostic contains: incompatible types:",
330330
" Foo<@Nullable Integer, ArrayList<String>> fooNonNull_3 = f.nonNullTest();",
331331
" Foo<Integer, ArrayList<String>> fooNull_1 = f.nullTest();",
332332
" Foo<Integer, ArrayList<@Nullable String>> fooNull_2 = f.nullTest();",
@@ -366,7 +366,7 @@ public void genericInferenceOnAssignmentsWithArrays() {
366366
" Foo<Foo<Object>[]> f3 = Foo.test1Null(null);",
367367
" Foo<Foo<@Nullable Object>[]> f4 = Foo.test1Null(null);",
368368
" Foo<Foo<Object>[]> f5 = Foo.test1Nonnull(new Object());",
369-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
369+
" // BUG: Diagnostic contains: incompatible types:",
370370
" Foo<Foo<@Nullable Object>[]> f6 = Foo.test1Nonnull(new Object());",
371371
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
372372
" Foo<Foo<Object>[]> f7 = Foo.test1Nonnull(null);",
@@ -378,7 +378,7 @@ public void genericInferenceOnAssignmentsWithArrays() {
378378
" Foo<Object>[] f11 = Foo.test2Null(null);",
379379
" Foo<@Nullable Object>[] f12 = Foo.test2Null(null);",
380380
" Foo<Object>[] f13 = Foo.test2Nonnull(new Object());",
381-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
381+
" // BUG: Diagnostic contains: incompatible types:",
382382
" Foo<@Nullable Object>[] f14 = Foo.test2Nonnull(new Object());",
383383
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
384384
" Foo<Object>[] f15 = Foo.test2Nonnull(null);",
@@ -400,7 +400,7 @@ public void inferNestedNonNullUpperBound() {
400400
" static class Bar<T extends @Nullable Object> {}",
401401
" abstract <U> Bar<U> make(Bar<U> other);",
402402
" void test(Bar<Bar<String>> other) {",
403-
" // BUG: Diagnostic contains: Cannot assign from type Bar<Bar<String>> to type Bar<Bar<@Nullable String>>",
403+
" // BUG: Diagnostic contains: incompatible types: Bar<Bar<String>> cannot be converted to Bar<Bar<@Nullable String>>",
404404
" Bar<Bar<@Nullable String>> unused = make(other);",
405405
" }",
406406
"}")
@@ -477,7 +477,7 @@ public void nullableAnnotOnMethodTypeVarUse() {
477477
" this.<String>foo(f);",
478478
" }",
479479
" void testPositive(Function<String, String> f) {",
480-
" // BUG: Diagnostic contains: Cannot pass parameter of type Function<String, String>, as formal parameter has type",
480+
" // BUG: Diagnostic contains: incompatible types: Function<String, String> cannot be converted to",
481481
" this.<String>foo(f);",
482482
" }",
483483
" void testPositive2(Function<@Nullable String, @Nullable String> f) {",
@@ -628,7 +628,7 @@ public void genericInferenceOnReturn() {
628628
" return Foo.makeNonNull(null);",
629629
" }",
630630
" static Foo<@Nullable Object> makeNonNullInvalid2() {",
631-
" // BUG: Diagnostic contains: due to mismatched nullability of type parameters",
631+
" // BUG: Diagnostic contains: incompatible types:",
632632
" return Foo.makeNonNull(new Object());",
633633
" }",
634634
" static Foo<Object> makeNonNullValid() {",
@@ -668,7 +668,7 @@ public void genericInferenceOnParameterPassing() {
668668
" handleFooNullable(Foo.makeNonNull(null));",
669669
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
670670
" handleFooNonNull(Foo.makeNonNull(null));",
671-
" // BUG: Diagnostic contains: Cannot pass parameter of type Foo<Object>",
671+
" // BUG: Diagnostic contains: incompatible types: Foo<Object>",
672672
" handleFooNullable(Foo.makeNonNull(new Object()));",
673673
" handleFooNonNull(Foo.makeNonNull(new Object()));",
674674
" }",
@@ -685,7 +685,7 @@ public void genericInferenceOnParameterPassing() {
685685
" handleFooNullableVarargs(Foo.makeNonNull(null));",
686686
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
687687
" handleFooNonNullVarargs(Foo.makeNonNull(null));",
688-
" // BUG: Diagnostic contains: Cannot pass parameter of type Foo<Object>",
688+
" // BUG: Diagnostic contains: incompatible types: Foo<Object>",
689689
" handleFooNullableVarargs(Foo.makeNonNull(new Object()));",
690690
" handleFooNonNullVarargs(Foo.makeNonNull(new Object()));",
691691
" }",
@@ -944,7 +944,7 @@ public void arrayCovariantSubtyping() {
944944
" String s2 = f(arr, \"hi\");",
945945
" // legal",
946946
" s2.hashCode();",
947-
" // BUG: Diagnostic contains: Cannot pass parameter of type @Nullable String []",
947+
" // BUG: Diagnostic contains: incompatible types: @Nullable String []",
948948
" stringField = f(arr2, \"hi\");",
949949
" }",
950950
"}")

0 commit comments

Comments
 (0)