This match feature is awful

help
1

I don't know the name of this feature, but it allows you to use any name instead of a case, it's like a worse _ =>. The problem is that this feature allows the introduction of silent bugs, causing typos when comparing constants to go unnoticed, which has happened to me a few times when writing FFI code.

Is there any way to force an error in these cases without using #![deny(unused_variables)], which is extremely broad? Something like #![deny(unused_variables_in_match_arms)] would be ideal.

Edit: I'm looking for a way to force an error when match arm has a case that is a simple binding that was probably introduced by a typo, I intend to expose the module constants in the function scope to avoid typing the path every time I write a case.

Currently, I avoid introducing these bugs by using partial paths for FFI constants.
Even though using constants without path can be considered a problem, I believe that match working this way is another problem apart from bad programming practices.

The code is illustrative so please don't pay attention to the dumb ways used to show the match problem.

:warning::warning::warning:THIS CODE IS A DUMB EXAMPLE, DO NOT USE IT TO REPLY TO THE POST :warning::warning::warning:

    #[test]
    fn awful() {
        enum Metal {
            Iron, Gold, Silver,
        }

        use Metal::*;

        let metal = Silver;

        let is_gold = match metal {
            Iron => false,
            God => true, //<-typo, warning: unused variables
            Awful => false, //warning: unreachable patterns
            _ => false, //useless, warning: unreachable patterns
        };

        assert!(!is_gold); //panic: is_gold == true
    }
4 Likes
2

Don't import the enum, so that you are forced to use the enum's as the prefix:

    #[test]
    fn better() {
        enum Metal {
            Iron, Gold, Silver,
        }

        let metal = Mtal::Silver;

        let is_gold = match metal {
            Metal::Iron => false,
            Metal::God => true, //<-typo, shouldn't compile
            Metal::Awful => false, // same as above
            _ => false, // useful catch_all pattern
        };

        assert!(!is_gold); //panic: is_gold == true
    }
15 Likes
3

...did you read the post?

4

Yes... and I'm offering you a solution.

12 Likes
5

The typo represents a catch-all pattern and is interpreted as a variable named God -- reference.
As a result, the enum matches it.
The _ pattern is also a catch-all, but it is used when we don’t want to bind the matched value to a variable.

Hopefully, this clarifies your issue. To solve the problem, you should use an enum prefix to restrict this behaviour.

2 Likes
6 
#![deny(non_snake_case)]
#![deny(non_camel_case_types)]
#![deny(clippy::enum_glob_use)]
4 Likes
7

But I wrote in the post that I already use this solution and that is not what I am looking for through this post.

8

Maybe it's a misunderstanding on my side, but the only part in your original post that mentions your current workaround didn't sound to me like using the enum as the prefix:

Currently, I avoid introducing these bugs by using partial paths for FFI constants.
Even though using constants without path can be considered a problem, I believe that match working this way is another problem apart from bad programming practices.

1 Like
9

Oddly enough I just this minute finished watching "5 deadly Rust anti-patterns to avoid": https://www.youtube.com/watch?v=SWwTD2neodE where using wild cards on use is listed as an anti-pattern.

If your enum names are long and boring you can shorten them with use:

use Metal as M;

    let metal = M::Silver;

    let is_gold = match metal {
        M::Iron => false,
        M::God => true,    //<-typo, warning: unused variables
        M::Awful => false, //warning: unreachable patterns
        _ => false,        //useless, warning: unreachable patterns
    };

I don't feel that minimising typing should a primary goal though.

9 Likes
10

But I want to know how to make unused variables in match arms an error

11

So does being an anti-pattern make a feature that introduces silent bugs legitimate? And the fact that the code uses a enum wildcard is not relevant to the post, it's just a dumb example with dumb code. Let me know if my post is confusing and I will correct it.

12

Here's a similar topic from a few months ago, fwiw: Why does `match` allow arbitrary identifiers? - #12 by danvil.

1 Like
13

silent bugs

You mean, after you ignored six related warnings about unreachable patterns and case conventions, the compiler didn't have another warning for this exact case, and that makes it "silent"?

Conventions exist in part to make code obvious where the language itself may be unclear. This is such a case. The compiler is warning you that your code doesn't match the convention, which should always be a signal to read it more carefully. "Silent"?

17 Likes
14

What silent bugs? I at least get should have a snake case name warnings for your example. Which I like to upgrade to errors.

8 Likes
15

So would a better (albeit still dumb) example be something like:

let match_me_if_you_can = "SOME_CONSTANT-ish";
let s = "SOME_CONSTANT-ish";
let result = match s {
    "OTHER_CONSTANT-ish" => "huh!?",
    match_me_fi_you_can => "haha gotcha!",
};

Playground

I can contrive to get this down to one warning:

warning: unused variable: match_me_fi_you_can

Which is pretty much pointing at the exact problem.

I'm not aware of any way to get a more specific warning than that.

NOTE: The example above isn't good practice, in particular it does not mean that you can match against the value of the binding match_me_if_you_can, match will always introduce a new binding, matching any value. I.e. if we change the example to add a _ => ... branch at the end, we'll get an "unreachable pattern" compiler warning.

1 Like
16

You will have to agree with me that an "unused variable" warning in a case of a match is not a warning according to the severity of the bug introduced, right? Literally the purpose of "match" is being thrown away because of a case.

17

You can apply attributes in Rust only to a specific scope:

enum Foo {
    A,
    B,
}

fn foo(foo: Foo) {
    use Foo::*;

    let x = 123; // only a warning

    #[deny(unused_variables)]
    match foo {
        A => println!("A"),
        C => println!("B"), // compilation fails with error
    }
}
7 Likes
18

Thinking again, this sounds like something a clippy lint could detect. Maybe spotting the combination of:

  • match
  • That is unused
  • (Optionally) Improper case identifier as pattern
  • (Optionally) That is short levenshtein distance from a constant/enum variant that is in scope

And suggest that you may have a mistake in the match pattern that is creating a new binding that matches anything.

4 Likes
19

So this super buggy useful feature cannot be disabled or bypassed in any way. Thanks for the link.

20

Upgrade the warning to an error if you feel a warning is not serious enough.