-
Notifications
You must be signed in to change notification settings - Fork 170
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
New rule: Use matching super parameter names #4263
Conversation
1742665
to
d300b59
Compare
I'd like to get a little more conversation on the proposal (#4259) before landing. |
Sounds good. Anything specific? |
As per https://github.com/dart-lang/linter/blob/main/doc/lint-lifecycle.md we generally try and get a bunch of input before "accepting" a proposed lint. I'll go ahead and cc some folks who are reliably opinionated. |
|
||
@override | ||
void visitConstructorDeclaration(ConstructorDeclaration node) { | ||
var positionalSuperParameters = node.parameters.parameters |
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.
I'd be inclined to do this less idiomatically in preference of just one iteration and without all the temporary objects. (I did a series of changes that dumbed down code like this and made it measurably faster.)
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.
I mean, parameter lists are short but I guess I think every bit counts. 🤔
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.
FWIW, here's an extreme motivating example: #1838
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.
Done
Implements the proposal at #4259