-
Notifications
You must be signed in to change notification settings - Fork 299
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.
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
JSpecify: Handle @Nullable assignments to @Nonnull arrays #929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! I have a couple of comments but this looks great
String message = "assigning @Nullable expression to @NonNull field."; | ||
return buildDescription(tree).setMessage(message).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a more appropriate message here, like "Writing @Nullable expression into array with @NonNull contents"
. Also you should use the ErrorBuilder
APIs and add a new ErrorMessage
message type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #929 +/- ##
============================================
- Coverage 87.07% 87.07% -0.01%
- Complexity 1991 1994 +3
============================================
Files 77 77
Lines 6430 6444 +14
Branches 1246 1249 +3
============================================
+ Hits 5599 5611 +12
Misses 422 422
- Partials 409 411 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! Just a couple more things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the contribution!
Handles cases in JSpecify mode where a
@Nullable
element is assigned to an unannotated array. Added relevant unit tests.Current Behavior
Both are valid assignments.
New Behavior
Assignment to
foo
generates an error since array elements are@Nonnull
.