`matches!` accepts unusable bindings in the pattern

[Fishrock123]

Perhaps this is more a thing for clippy to detect, I'm not sure.

A co-worker of mine tried this code:

if !matches!(binding_to_uuid, binding_to_other_uuid) {
    // ...
}

I expected to see this happen:

Compiler should say that this is an obvious error, because there is no way to use the binding_to_other_uuid binding because it is internal to the match statement (and just shadows the actual binding_to_other_uuid).

Instead, this happened:

The matches! macro here expands to something that is never useful:

if !match binding_to_uuid {
    binding_to_other_uuid => true,
    _ => false,
} { ... }

Clippy suggests underscoring the binding_to_other_uuid because it's unused, but that's not super clear unless you are familiar with the macro expansion. Ideally providing variable bindings that are not used in a guard would be disallowed in matches! because they cannot otherwise be used.

Meta

rustc --version --verbose:

rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-unknown-linux-gnu
release: 1.52.1
LLVM version: 12.0.0

[SkiFire13]

Changing the definition of matches! to be something like the following would make cases like this not compile while also allowing const patterns.

macro_rules! matches {
    ($expression:expr, $( $pattern:pat )|+ $( if $guard: expr )? $(,)?) => {{
        #[deny(unreachable_patterns)]
        let tmp = match $expression {
            $( $pattern )|+ $( if $guard )? => true,
            _ => false
        };
        tmp
    }}
}

Playground example

However the error message is rather cryptic.

error: unreachable pattern
  --> src/main.rs:6:13
   |
6  |             _ => false
   |             ^ unreachable pattern
...
17 |     if matches!(1, a) {
   |        --------------
   |        |           |
   |        |           matches any value
   |        in this macro invocation
   |
note: the lint level is defined here
  --> src/main.rs:3:16
   |
3  |         #[deny(unreachable_patterns)]
   |                ^^^^^^^^^^^^^^^^^^^^
...
17 |     if matches!(1, a) {
   |        -------------- in this macro invocation
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

[inquisitivecrystal]

@rustbot label +A-patterns +T-libs

[jhpratt]

Not sure I'd really call this a bug…it is a valid pattern and it does match it. Clippy seems more appropriate for this imo.

[camsteffen]

Rust has the irrefutable_let_patterns lint so this could be irrefutable_matches_patterns.

[Fishrock123]

This is not quite just an irrefutable_matches_patterns. The example is, but this could also be matches!(a, Some(x)) rather than matches!(a, Some(_)), the latter which would be more correct.

[camsteffen]

That is caught by unused_variables?

[camsteffen]

Clippy does not have a mechanism to check if a pattern is refutable in the general case. So I think this should be caught be Rustc, possibly with the existing irrefutable_let_patterns lint (a small stretch since there is no let).

enum MyEnum {
    Variant,
}

let _ = matches!(MyEnum::Variant, MyEnum::Variant); // this should lint

[Fishrock123]

It is technically caught by unused_variables but the message could be much clearer in this case. It can be hard to see how the macro expands.

[camsteffen]

Do you have an idea for how it can be clearer? I don't think it should show how the macro expands - that's an implementation detail of the macro.

[Fishrock123]

Unfortunately it is contextual - but renaming is not the correct solution here. The binding is unusable. It should tell you to rid it or ignore it completely.

[camsteffen]

Adding a _ is a sensible suggestion since keeping the name can be good for code readability. It is not really unusable since you can use it in an if guard.