-
Notifications
You must be signed in to change notification settings - Fork 284
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
Fix handling of references to methods of array types and type variables #926
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #926 +/- ##
=========================================
Coverage 87.09% 87.09%
- Complexity 1991 1992 +1
=========================================
Files 77 77
Lines 6430 6431 +1
Branches 1245 1246 +1
=========================================
+ Hits 5600 5601 +1
Misses 422 422
Partials 408 408 ☔ View full report in Codecov by Sentry. |
@@ -2326,7 +2326,7 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) { | |||
return true; | |||
case NEW_CLASS: | |||
case NEW_ARRAY: | |||
// for string concatenation, auto-boxing |
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.
This was an outdated comment so I deleted it
|| ElementUtils.isTypeElement(baseExpressionSymbol) | ||
|| baseExpressionSymbol.getKind() == ElementKind.TYPE_PARAMETER) { |
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.
This seems to suggest to me that we were handling cases where the expression is an array type (presumably isTypeElement
check), but not the case for type parameters (the second negative test added in this PR). Right?
Makes sense to me either way, but just want to understand this better :)
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.
Not quite. The isTypeElement
method only catches when the element is a class, enum, interface, or annotation type; array types are not handled. That case is handled by the line this PR adds to mayBeNullExpr
.
For type variables, the expression itself is just an IdentifierTree
. We know it can't be null because it represents a type parameter; that is caught by the check on this line.
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.
Makes sense to me! :)
After #920 we check the nullability of the qualifier expression of a method reference. We weren't correctly handling the cases where that expression was an array type or a type variable