Skip to content
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

[Wildcard Variables][spec] Treat all multiple underscores as wildcards? #3793

Closed
Tracked by #55661
kallentu opened this issue May 9, 2024 · 6 comments
Closed
Tracked by #55661
Labels
specification wildcard-variables The wildcard-variables feature

Comments

@kallentu
Copy link
Member

kallentu commented May 9, 2024

@lrhn mentioned that perhaps we should treat all of the multiple underscores as non-binding declarations as well.

This came up during an analyzer meeting while we were looking at how #unused-variable-warnings is already implemented in the analyzer, but with no unused variable warnings on _ and __, ___, etc.

We would change this with the wildcard variables feature to have no unused variable warnings on only _, unless we would potentially see us having __, ___, etc be non-binding in the future as well, which then maybe it would be useful to keep the behaviour as is?

We should finalize whether we want to do that in this spec, or whether we want to do that at all in the future.

cc. @dart-lang/language-team @bwilkerson @stereotype441 @pq @scheglov @srawlins

Previous Discussion

Another thought: Maybe we should treat all of __, ___ etc. as non-binding declarations too. It's not necessary, but I'm not sure I'd want someone to write __ as a name, just to not have it be non-binding, and still not give the thing a real name.

It's a little iffy, because then they should probably work the same in patterns too. And that's an unrelated breaking change.

(Still think it would be prettier.)

Originally posted by @lrhn in #55661

I considered that when I was first writing the proposal. You're point about it being inconsistent with patterns is, I think, a good one.

Also, we ultimately want to get to a world where people don't use __, ___, etc. So I think scoping wildcards to only be a single _ helps us go in that direction.

Originally posted by @munificent in #55661

@stereotype441
Copy link
Member

Also, we ultimately want to get to a world where people don't use __, ___, etc. So I think scoping wildcards to only be a single _ helps us go in that direction.

Personally I find this pretty compelling.

I think we should go one step further, and add a lint that encourages people to rename an unused parameter called __, ___, etc. to a single _ (once they upgrade to a language version that supports wildcard variables, of course). That would be a nice way to gently encourage people to make use of the new language feature in the way it's intended.

@natebosch
Copy link
Member

The most straightforward migration for a referenced _ is to rename it __, so it might be handy to leave that as an escape hatch.

I think we should go one step further, and add a lint that encourages people to rename an unused parameter called __, ___, etc. to a single _ (once they upgrade to a language version that supports wildcard variables, of course). That would be a nice way to gently encourage people to make use of the new language feature in the way it's intended.

I think the proposed modification to the existing diagnostic in the first comment would be sufficient. We shouldn't need a new lint for this I think.

@lrhn
Copy link
Member

lrhn commented May 10, 2024

@munificent has a good point: it's not that we want people to use __ (or any longer all-underscore identifier) for anything.
We want all existing __ to be changed to _, unless it's actually referenced as a variable, which I don't expect anywhere.

So we don't need to support __ as a non-binding variable.

We could instead choose to just not allow __ at all, as a variable, or even as an identifier.
It's unlikely to break anything, but it does complicate identifier parsing, and it's not really a problem, if nobody uses the name anyway.

The path of least resistance is to just allow __ as a private identifier, and then never allow it on a code review.
If someone else wants a variable with that name, it's their readability that suffers.

@eernstg
Copy link
Member

eernstg commented May 13, 2024

The path of least resistance is to just allow __ as a private identifier, and then never allow it on a code review.
If someone else wants a variable with that name, it's their readability that suffers.

I like that.

We could say that __ etc. are private identifiers, with no special behaviors. A scope may have at most one binding for __ and at most one binding for ___, etc., and they can be referred from code locations where said names are accessible (just like every other private identifier).

We could lint all the underscore-only declared names with length at least 2, suggesting that _ is used instead. That would be useful during migration, catching all those names of the form __ etc. that were introduced simply to avoid a name clash with other wildcard-wannabe variables.

As a convention, a developer or an organization might then decide that a specific situation justifies the usage of such names as "soft" wildcards: They do need to be accessed for some specific reasons, but the expectation is that they should be ignored except for that specific usage that prevented them from being real wildcards in the first place.

One example could be that super.__ can be used to pass a positional parameter to a superinitializer, and at the same time indicate that nobody should need to use __ in any other way.

Another example could be that a wildcard is temporarily renamed to __ during debugging.

I think this scenario is more pragmatic than saying that "__, ___, ____ etc. are all wildcards". After all, the distinction won't be useful.

@lrhn
Copy link
Member

lrhn commented May 13, 2024

We could say that __ etc. are private identifiers, with no special behaviors.

Which is what we are already saying, so that's also the easy path.

(The lint + fix to _ is a good idea too.)

One example could be that super.__ can be used to pass a positional parameter to a superinitializer, and at the same time indicate that nobody should need to use __ in any other way.

Or just allow super._. 😉

@kallentu
Copy link
Member Author

kallentu commented May 13, 2024

Okay, so I see as a final decision:

We don't see us making __ and etc non-binding in the future.

We want __ and etc to still be binding and private identifiers.
This should be the current behaviour right now. Keep this.

What we do want to change:

  1. Change unused variable warnings to start triggering on __ and etc (and not singular _ since this should never be used)
  2. Add a lint + fix for all the underscore-only declared names with length at least 2, suggesting that _ is used instead. Or even change no_wildcard_variable_uses -- see [Wildcard Variables][lint] no_wildcard_variable_uses and the new wildcard variables feature. linter#4968 for related conversation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
specification wildcard-variables The wildcard-variables feature
Projects
None yet
Development

No branches or pull requests

5 participants