-
Notifications
You must be signed in to change notification settings - Fork 300
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
Remove need to use JSpecify's @Nullable annotation #1142
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1142 +/- ##
============================================
- Coverage 88.24% 88.22% -0.02%
+ Complexity 2263 2260 -3
============================================
Files 85 85
Lines 7314 7312 -2
Branches 1458 1461 +3
============================================
- Hits 6454 6451 -3
Misses 432 432
- Partials 428 429 +1 ☔ View full report in Codecov by Sentry. |
@@ -103,7 +93,7 @@ public static void checkInstantiationForParameterizedTypedTree( | |||
return; | |||
} | |||
boolean[] typeParamsWithNullableUpperBound = | |||
getTypeParamsWithNullableUpperBound(baseType, config, state, handler); |
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.
Why do we no longer need 'state' (VisitorState) here (and in other places in this diff)?
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.
It's because hasNullableAnnotation
(https://github.com/uber/NullAway/pull/1142/files#diff-82d66c6ba15b87acc24509d7a3440bbe7606944197b929733d315fbe0a025435L1159-L1171) used to rely on the state
object in order to check for the presence of the JSpecify @Nullable
annotation. Now we don't do that, so in various places state
is no longer needed (but config
is).
Fixes #1139. There are real scenarios where projects may not want to ship with a JSpecify dependence; see #1139 (comment). So, we remove any cases where we were specifically checking for or using JSpecify's
@Nullable
annotation.Most of the code changes are due to the fact that now, we check if an annotation is a
@Nullable
annotation usingNullness.hasNullableAnnotation
, which requires aConfig
object as a parameter. So we need to thread aConfig
object as a parameter through a bunch of methods.