Open edward-shen opened 1 year ago
cc @compiler-errors since this seems right up your alley.
I don't think this is possible to make into an error due to backwards-compatibility. After all, True
and False
are valid identifiers.
The best thing we could do is raise non_snake_case
to a deny-by-default level lint.
We have a similar deny-by-default lint, bindings_with_variant_name
, where we have valid identifiers but deny it anyways for similar reasons to this one.
We can't immediately make it an error in the 2021 edition, but could we introduce this in the 2021 edition as a warning and raise it to error in the 2024 edition?
Hm, looks like that lint was introduced pre-1.0 in #19115.
I guess we wouldn't need an RFC to add a new lint, but we would need one to raise it to a hard error then, yeah?
what exactly would you want to make a hard error? having multiple binding patterns in a match? that does sound kinda reasonable, it's always wrong to have that.
My original intent was a very specific lint:
bool
.True
or False
as a binding name.So the following would examples would trigger the proposed lint:
let True = false else { ... }
if let False = true { ... }
match true {
True | False => {}
}
But we could expand that to any item containing a bool, so the following would examples would trigger the expanded lint:
let Some(True) = Some(false) else { ... }
if let Some(False) = Some(true) { ... }
match true {
True | False => {}
}
That being said, I'm not sure how reasonable the expanded lint would be.
Regardless of which version of the lint we use, these examples would definitely not trigger:
// Not a bool
match 5 {
True => {}
}
enum DoFoo {
True(bool),
False(bool)
}
// No binding named `True` or `False`
match DoFoo::True(true) {
DoFoo::True(_) => (),
DoFoo::False(_) => (),
}
Though your idea is also a good idea; I'm just not sure if that's being exploited by some niche usage in proc macros.
Code
Current output
Desired output
Rationale and extra context
For new Rustaceans coming from Python, none of the warnings actually explain the logical error here and do not indicate that there is almost certainly a logical bug going on here.
I'm not sure if this is worthwhile to add a correctness lint (deny-by-default), but this is almost certainly will be confusing for folks who are expecting
True
to be a value, not a pattern.Other cases
This should probably error if a boolean is matched on
False
as well.Anything else?
Tested on Nightly version: 1.72.0-nightly (2023-06-12 df77afbcaf3365a32066)