Generics checks for parameter passing #746

NikitaAware · 2023-03-10T21:48:14Z

For method calls, the nullability annotations of the type parameters for formal parameter types and actual parameter types should be exactly the same.
This pull request adds code to compare nullability annotations of actual and formal parameters of generic type and reports an error if the annotations don't match. example:

  static class A<T extends @Nullable Object> { }
  static A<String> sampleMethod(A<A<String>> a1, A<String> a2) {
     return a2;
  }
  static void methodCall(A<A<@Nullable String>> a1, A<String> a2) {

 // **_here the code will report an error as the annotations of formal and actual parameters don't match_**

       A<String> a = sampleMethod(a1, a2);

  }

msridhar · 2023-03-13T20:18:01Z

nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java

@@ -44,10 +44,6 @@ public void constructorTypeParamInstantiation() {
            "    NonNullTypeParam<String> t2 = new NonNullTypeParam<@Nullable String>();",
            "    // BUG: Diagnostic contains: Generic type parameter",
            "    testBadNonNull(new NonNullTypeParam<@Nullable String>());",
-            "    testBadNonNull(",


Why is this deleted?

Same question from me

msridhar · 2023-03-13T20:19:18Z

@lazaroclapp I think this is ready for you to take a look

lazaroclapp

Some minor questions:

lazaroclapp · 2023-03-15T19:08:43Z

nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java

@@ -44,10 +44,6 @@ public void constructorTypeParamInstantiation() {
            "    NonNullTypeParam<String> t2 = new NonNullTypeParam<@Nullable String>();",
            "    // BUG: Diagnostic contains: Generic type parameter",
            "    testBadNonNull(new NonNullTypeParam<@Nullable String>());",
-            "    testBadNonNull(",


Same question from me

lazaroclapp · 2023-03-15T19:09:59Z

nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java

+            "  }",
+            "  static void testPositive(A<A<@Nullable String>> a1, A<String> a2) {",
+            "    // BUG: Diagnostic contains: Cannot pass parameter of type",
+            "    A<String> a = sampleMethod(a1, a2);",


How about the other way around? This is invariant, right? So we should also test that we can't pass A<String> to something expecting A<@Nullable String>, correct?

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

lazaroclapp

Very minor comment nit, otherwise looks good to go!

lazaroclapp · 2023-03-17T16:30:01Z

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

-    for (int i = 0; i < formalParams.size(); i++) {
+    int n = formalParams.size();
+    if (isVarArgs) {
+      n = n - 1; // don't check for the last argument if


If what? I mean, it's fairly clear, but lets complete the comment. Maybe it should be a comment above like:

// If the last argument is var args, don't check it now, it will be checked against // all remaining actual arguments in the next loop.

lazaroclapp

LGTM! (@msridhar?)

msridhar

LGTM too! Landing

This reverts commit a1d1eed.

msridhar and others added 30 commits October 3, 2022 13:29

tweak error message

90d471d

more tests

98f7fa6

code cleanup.remove print statements

53ddad7

Add temporary check to consider unannoted base type

7daa52b

minor code changes

cd01ff9

add generics check for new class - check if the class is generic

202d5db

add generics check for new class - check for the type arguments

3998949

generics check for new class

fc8b99f

generics check for extended class

6425909

generics check for new class

bae84dc

generics check for new class - minor changes for error message

4aba42c

generics check for interface implementation

bca4f24

minor identation changes

99e8bf9

nullability checks for build failures

a0caac8

Add Test: generics check for subclasses

5b9d918

restructure generics tests

4cb51d9

restructure generics tests

2e4a2ef

Add tests

b1668e9

restructure code

9c8b3ae

restructure code

97e088b

restructure code

036febf

Merge branch 'master' into add-generics-checks

f065bd9

add jspecify mode flag

00ad702

check jspecify flag

31a0e4d

add test case for nested type parameter instantiation

fe66b4a

nested type

89ca225

nested generics type check for new class

8ba736b

Add test - generics check for function return type

fa627be

Add - generics check for function return type

5745cf7

code cleaning - generics check

1518629

msridhar added 2 commits March 10, 2023 13:18

Merge branch 'master' into generics-checks-for-parameter-passing

3fb9c6f

cleanup and docs

ecc5d51

msridhar reviewed Mar 13, 2023

View reviewed changes

msridhar requested a review from lazaroclapp March 13, 2023 20:18

msridhar added the jspecify Related to support for jspecify standard (see jspecify.dev) label Mar 13, 2023

lazaroclapp reviewed Mar 15, 2023

View reviewed changes

NikitaAware added 2 commits March 17, 2023 01:54

suggested changes

5ab576e

test case

6222585

NikitaAware requested review from lazaroclapp and msridhar and removed request for lazaroclapp March 17, 2023 09:01

suggested changes

e6442a2

NikitaAware requested review from lazaroclapp and msridhar and removed request for msridhar and lazaroclapp March 17, 2023 09:16

lazaroclapp reviewed Mar 17, 2023

View reviewed changes

suggested change

fed2004

NikitaAware requested a review from lazaroclapp March 18, 2023 06:10

lazaroclapp approved these changes Mar 20, 2023

View reviewed changes

msridhar approved these changes Mar 20, 2023

View reviewed changes

msridhar merged commit a1d1eed into uber:master Mar 20, 2023

msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 18, 2023

Revert "Generics checks for parameter passing (uber#746)"

9b7ea55

This reverts commit a1d1eed.

msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 18, 2023

Revert "Generics checks for parameter passing (uber#746)"

5a665ea

This reverts commit a1d1eed.

msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023

Revert "Generics checks for parameter passing (uber#746)"

f6b57b1

This reverts commit a1d1eed.

msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023

Revert "Generics checks for parameter passing (uber#746)"

d4aaa43

This reverts commit a1d1eed.

msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023

Revert "Generics checks for parameter passing (uber#746)"

a3d863f

This reverts commit a1d1eed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generics checks for parameter passing #746

Generics checks for parameter passing #746

NikitaAware commented Mar 10, 2023 •

edited by msridhar

msridhar Mar 13, 2023

lazaroclapp Mar 15, 2023

msridhar commented Mar 13, 2023

lazaroclapp left a comment

lazaroclapp Mar 15, 2023

lazaroclapp Mar 15, 2023

lazaroclapp left a comment

lazaroclapp Mar 17, 2023

lazaroclapp left a comment

msridhar left a comment

Generics checks for parameter passing #746

Generics checks for parameter passing #746

Conversation

NikitaAware commented Mar 10, 2023 • edited by msridhar

msridhar Mar 13, 2023

Choose a reason for hiding this comment

lazaroclapp Mar 15, 2023

Choose a reason for hiding this comment

msridhar commented Mar 13, 2023

lazaroclapp left a comment

Choose a reason for hiding this comment

lazaroclapp Mar 15, 2023

Choose a reason for hiding this comment

lazaroclapp Mar 15, 2023

Choose a reason for hiding this comment

lazaroclapp left a comment

Choose a reason for hiding this comment

lazaroclapp Mar 17, 2023

Choose a reason for hiding this comment

lazaroclapp left a comment

Choose a reason for hiding this comment

msridhar left a comment

Choose a reason for hiding this comment

NikitaAware commented Mar 10, 2023 •

edited by msridhar