Matching on Pythonic boolean symbols (True, False) should result in better diags

[edward-shen]

Code

fn main() {
    match false {
        True => println!("I am true!"),
        False => println!("I am false!"),
    }
}

Current output

Compiling playground v0.0.1 (/playground)
warning: unreachable pattern
 --> src/main.rs:4:9
  |
3 |         True => println!("I am true!"),
  |         ---- matches any value
4 |         False => println!("I am false!"),
  |         ^^^^^ unreachable pattern
  |
  = note: `#[warn(unreachable_patterns)]` on by default

warning: unused variable: `True`
 --> src/main.rs:3:9
  |
3 |         True => println!("I am true!"),
  |         ^^^^ help: if this is intentional, prefix it with an underscore: `_True`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `False`
 --> src/main.rs:4:9
  |
4 |         False => println!("I am false!"),
  |         ^^^^^ help: if this is intentional, prefix it with an underscore: `_False`

warning: variable `True` should have a snake case name
 --> src/main.rs:3:9
  |
3 |         True => println!("I am true!"),
  |         ^^^^
  |
  = note: `#[warn(non_snake_case)]` on by default
help: rename the identifier or convert it to a snake case raw identifier
  |
3 |         r#true => println!("I am true!"),
  |         ~~~~~~

warning: variable `False` should have a snake case name
 --> src/main.rs:4:9
  |
4 |         False => println!("I am false!"),
  |         ^^^^^
  |
help: rename the identifier or convert it to a snake case raw identifier
  |
4 |         r#false => println!("I am false!"),
  |         ~~~~~~~

warning: `playground` (bin "playground") generated 5 warnings (run `cargo fix --bin "playground"` to apply 2 suggestions)
    Finished dev [unoptimized + debuginfo] target(s) in 0.40s
     Running `target/debug/playground`

Desired output

error: `True` will unconditionally match both `true` and `false` values.
 --> src/main.rs:3:9
  |
3 |         True => println!("I am true!"),
  |         ^^^^
  |
note: to pattern match on booleans in Rust, use `true` or `false` instead (note the capitalization)
 --> src/main.rs:3:9
  |
3 |         true => println!("I am true!"),
  |         ^^^^
  |

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 df77afb)

[edward-shen]

cc @compiler-errors since this seems right up your alley.

[compiler-errors]

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.

[edward-shen]

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?

[edward-shen]

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?

[Noratrieb]

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.

[edward-shen]

My original intent was a very specific lint:

1. The item being pattern matched on is a bool.
2. Any pattern containing 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.