linter
linter copied to clipboard
proposal: `avoid_renaming_method_parameters ` should allow _, __, ___ etc. for unused parameters
avoid_renaming_method_parameters
should allow _, __, ___ etc. for unused parameters.
(this is more a bug on the existing rule than a proposal for a new rule)
I think this would be a good improvement, and would keep in line with the spirit of the lint rule.
I'm inclined to agree with this change but since it's a recommended lint, I'd like to get a gut check from a few other folks. (Mainly to test the waters for any objections.)
@munificent? @lrhn? @natebosch? @jakemac53 @goderbauer?
Thanks for bringing this up @wujek-srujek!
The lint seems to be about renaming parameters if you inherit DartDoc that might refer to the parameter by name. (Does it only trigger if the DartDoc does refer to the parameter, or are we abundantly cautious?)
Using _
or similar does not seem like it should be an exception to that problem. You still inherit DartDoc, its references are still not able to refer to the parameter at its new name.
So, what would the reasoning be to allow this? That a method using _
as name is very likely not publicly visible anyway?
(If so, probably fair!)
The questions about the scope of the lint are good ones. We should ensure that it's only flagging code that's really an issue for dart doc
. Including that it doesn't flag a method that has its own doc comment or parameters that aren't referenced in the inherited doc comment.
If those conditions are true, then it will only generate a diagnostic if the renamed parameter is mentioned in the inherited docs, in which case it's possible that the overriding method needs new documentation that minimally explains that the renamed parameter will be ignored. But there might well be a counter-example that I'm not thinking of.
I wasn't aware that DartDoc was the primary motivation for the lint. I lean towards allowing renaming unused parameters to _
, but I don't have very strong opinions.
I think the rules value transcends dart doc
(though the motivation does center it). That said, the current dart doc
implementation does introduce a wrinkle...
In its current incarnation, it looks like it will produce docs w/ the renamed params (e.g., _
) which is I think not desirable.
For example:
class A {
/// Do something.
void a(A a) { }
}
class A2 extends A {
@override
void a(A _) { }
}
produces the following doc for A2.a
/fyi @a14n
We could (and maybe should) consider updating dart doc
to inherit parameter names in case they're renamed with underscores. If that happened I'd be in favor of updating the lint as @wujek-srujek proposes but probably not otherwise.
/fyi @srawlins
The reference to DartDoc comes from the lint itself, as the lint's only justification.
If that's to be trusted,
class A {
/// Do something with [a].
void a(A a) { }
}
class A2 extends A {
@override
void a(A _) { }
}
would still be a problem for DartDoc, unless the [a]
link in the inherited documentation links to A
's a
's parameter.
I'd never change the parameter name in a method that's part of public API, and I'd have no problem doing so for a method that's not public. Detecting whether a method is intended to be public is a harder problem.