rework avoid_types_on_closure_parameters to allow not-redundant types #3568
Conversation
pq
added
needs internal testing
set-internal-available
|
Fantastic. We'll need to do an internal dry-run regardless , but if I understand the change correctly, this just addresses false-positives? |
Yes it should relax the original version. So it shouldn't add new diagnostics. |
| } else if (staticParameterElement != null) { | ||
| expectedType = staticParameterElement.type; | ||
| } else if (parent is NamedExpression) { | ||
| expectedType = parent.element?.type; |
There was a problem hiding this comment.
@bwilkerson I was surprised to have to add this line (to catch the Future.then(..., onError: ...) case). I thought staticParameterElement wouldn't have been null here. Is it the normal behaviour or a bug?
There was a problem hiding this comment.
It does seem inconsistent that an expression used as an argument wouldn't have a staticParameterElement. I'm not sure whether there's a reason for the current behavior, but I'd like to explore changing it. We should probably open an issue in the sdk repo and loop Konstantin into the conversation.
There was a problem hiding this comment.
Looks like this bug was already reported (by me :D) in dart-lang/sdk#45964
Add? If these are false positives, I'd expect there to be fewer... Sorry if I'm being thick! |
|
@pq that's what I tried to say : fewer lints |
Did you have a chance to run it internally? |
Sorry, no. It may be a bit --- I'm still trying to land the last linter release in the SDK (https://dart-review.googlesource.com/c/sdk/+/253501) and have a backlog of other tasks. Will definitely queue this up for our next release though. Thanks for your patience! |
|
(And also for the nudge!) |
Description
rework avoid_types_on_closure_parameters to allow not-redundant types
Fixes #3330
Fixes #2131
Fixes #1099