[Wildcard Variables][lint] proposal: `use_wildcard_rather_than_underscores`

kallentu commented 4 months ago

`use_wildcard_rather_than_underscores`

Description

Use _ instead of underscores such as __ (etc.).

Details

DO use _ instead of multiple underscores such as __ (etc.) when declaring a local wildcard.

Kind

This lint enforces the usage of the new wildcard feature where _ is non-binding, over __ and etc which are binding.

Bad Examples

var __ = 1;
final ___ = 1;

void foo(int __) {}

for (var ___ in list) {}

Good Examples

var _ = 1;
final _ = 1;

void foo(int _) {}

for (var _ in list) {}

Discussion

Related issue and discussion:

This should complement the UNUSED_VARIABLE warning and the no_wildcard_variable_uses lint once they're updated to take the wildcard feature into account.

We should also have a quickfix that can easily change these bad instances to _.

Discussion checklist

[X] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
[X] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
[X] If there's any prior art (e.g., in other linters), please add references here.
[X] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
[ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

kallentu commented 4 months ago

cc. @pq

bwilkerson commented 4 months ago

I'm confused about when UNUSED_VARIABLE will be reported and when use_wildcard_rather_than_underscores will be reported. Could you clarify?

kallentu commented 4 months ago

I'm confused about when UNUSED_VARIABLE will be reported and when use_wildcard_rather_than_underscores will be reported. Could you clarify?

use_wildcard_rather_than_underscores will be reported for __ and etc. And I suppose after https://github.com/dart-lang/sdk/issues/55719, they will both report on the same things.

Is this confusing for users?

bwilkerson commented 4 months ago

I'm not sure I'd say that it's confusing, but it definitely isn't helpful. There should really be a single diagnostic for a single problem.

It isn't clear to me that we need two lints that do the same thing. Is there a motivation here that I'm missing?

kallentu commented 4 months ago

I think it depends on what type of fix the user is looking for and what they intend on doing with __.

In the case where a user actually wants a non-binding _ and have been using __ etc, the use_wildcard_rather_than_underscores is what they need to make that fix and to use _.

In the case where a user using __ as a normal binding variable... well, they shouldn't really be doing that either (for readability reasons), but __ works as any other variable and should probably report UNUSED_VARIABLE?

bwilkerson commented 4 months ago

If a multi-underscore identifier is actually being used then we won't report either diagnostic.

But in the case where the variable is not being used, and we report an unused variable, we could offer to convert it to a wildcard (when the experiment is enabled). And if there were a different fix for the problem we could offer both without needing a second diagnostic.

I still don't see the value to the user of having two lints for the same problem.

If there's enough support for it in the community we could add a lint to flag multi-underscore identifiers that are being used with a recommendation that they be renamed, but I suspect that might not be a common enough problem to justify a lint.

kallentu commented 4 months ago

if there were a different fix for the problem we could offer both without needing a second diagnostic.

Fair enough. I wasn't sure if we could have more than one fix on an error.

If we can do that, let's just add an additional fix for UNUSED_VARIABLE (and its variations) rather than add this extra lint.

Does that sound reasonable?

kallentu commented 4 months ago

Okay decided.

Add an additional fix for UNUSED_VARIABLE (and its variations). In the case where the variable is not being used, and we report an unused variable, we offer to convert it to a wildcard.

Closing issue.

dart-lang / linter