That would be great but the only way to do that is very broad, #![deny(unused_variables)]
How many hours of confused debugging has a typo in a match arm cost you recently? I'm curious, and you have my sympathy!
I think writing something for clippy is too advanced for a noob like me but the idea is very good
Maybe you missed: This match feature is awful - #17 by mroth
In a code from an old personal project, I was trying to understand why an error code returned by a Vulkan command was not the expected error. I forced a situation where a code was expected but for some reason it didn't work, it was always the same code returned. I even thought it could be a bug in the driver until I reviewed the code and noticed a strange warning of unused variable in a match case. I don't remember how much time was lost, it could have been days or hours.
I tried something like this before but it doesn't work, it seems like it's not allowed. I used it inside the TryFrom::try_from
an inner attribute is not permitted in this context
inner attributes, like #![no_std], annotate the item enclosing them, and are usually found at the beginning of source files
outer attributes, like #[test], annotate the item following them
You wrote an inner attribute #![deny(unused_variables)] instead of an outer attribute
1 Like
Elevate all warnings to errors. Put this in your CI.
cargo clippy --workspace --tests -- -D warnings
Now unused variables become errors and your CI fails, the PR is rejected, everyone is happy.
Even if you are using CI, you might want to configure your IDE to run clippy on save.
There are several other ways to elevate warnings to errors. This is just the one I have handy while on mobile.
3 Likes
I do agree and I sympathize with your complaint. However, unused variable warnings can also be a sign of other logic bugs. So in general they have to be dealt with before testing and debugging, or at least that's my approach.
9 Likes
Aaaah, so that was it. This will help a lot, thank you.
Your example made the problem much clearer than the code I wrote. I need to improve how I express myself.
1 Like
While I respect your wish not to boresight too hard on the details of your example, I will offer one suggestion, which will be useful (I hope) only in so far as your example reflects your real problem. Please humour me.
In your example, there are only two outcomes intended: for Metal::Gold, the outcome illustrated by your true example, and for all other enum constants, the outcome illustrated by your false example. If that is representative of your real problem, then I might be tempted to write this in terms of the matches! macro:
if matches!(metal, Metal::Gold) {
true
} else {
false
}
Since this is a smaller set of conditions, it's a bit more ergonomic (in my view, at least) to use the qualified names of the enum constants, which in turn avoids the risk that your pattern might inadvertently be a catch-all variable binding and not a constant item to match against.
On the other hand, it's often useful to have proof that every case is considered, even if the result is binary. Consider, for example, mapping graphics modes to some configuration value: even if there are only two possible values, it can be clearer to associate each mode with its associated value than to only list the values in one of the two cases arbitrarily. Plus this has the advantage of yelling at you if new modes are added to deliberately consider what that mode should do, as opposed to silently getting one option. if-else is a good option if there's a reasonable "default" behavior and only one other behavior.
6 Likes
No warning and even Clippy is fine with
fn main() {
let earth = "Earth";
let planet = "Jupiter";
let alive = true;
match (alive, planet) {
(liveliness, earth) if liveliness =>
println!("I live on {earth}, which is between Venus and Mars."),
_ =>
println!("I do not live on {earth}.")
}
}
And it prints:
I live on Jupiter, which is between Venus and Mars.
5 Likes
To be fair, OP has a good point. The pattern matching syntax in Rust does have a somewhat non-explicit nature in this area, meaning it behaves completely differently depending on whether the pattern token refers to a symbol that's in scope or not.
One nice thing about Rust is its explicitness. If you want an exclusive/mutable reference than you have to say mut, or if you want to convert between types, you have to make this clear in the code (unlike, say, JavaScript). So, the pattern matching does lack some explicitness.
An alternative design to be more explicit would require a more verbose syntax to distinguish between the two interpretations, so that's a tradeoff.
All considered, the "unused variable" warning does seem sufficient to catch the problem, if you keep good code hygiene and don't allow warnings to accumulate.
Seems that it would be better to expend energy on a more significant area where we can improve Rust.
5 Likes
Good find!
1 Like
While this is true on a technical level itās also only half true on a pragmatic level. Really, Rustās pattern matching is similar to other programming languages such as Haskell. Comparing to Haskell is quite instructive. While the same thing superficially works even there, e.g.
data Foo = Bar | Baz
-- ^^^^^ like `enum Foo { Bar, Baz }`
demo(x:: Foo) =
let baz = Baz in
case x of {
baz -> "baz";
Bar -> "bar";
}
main = do {
print(demo(Bar));
-- ^^^^^ prints "baz"
}
(^ code example uses nonstandard formatting conventions to help unfamiliar readers)
it doesnāt actually suffer the same issue of "non-explicit nature". Like in Rust, you can't match against a local variable, but can against a data constructor (like an enum variant) - however, the rules on when the pattern will or will not make a new binding is really really simple here actually: lowercase and uppercase identifiers (determined by the first letter) are completely different namespaces; lowercase is for variables, constants[1], functions, etc..., uppercase is for data constructors. So matching against baz will always introduce a new variable, regardless of all context, and matching against Bar will never introduce a new variable and will always error when Baz isnāt a thing (more specifically a data constructor ā alternatively a "pattern synonym"[2]) already in scope.
Anyways... with this precedent in mind, an alternative design for Rust would have simply been, essentially:
make
#![deny(nonstandard_style)]a hard error without any opt-out.
Then everything is very much explicit and clear, you see the pattern foo_bar and it's a new binding matching any value; you see the pattern FooBar or FOO_BAR and it's an enum variant/unit-struct or a constant, either way, upper-case, so an existing thing being matched against.
At least ignoring the bool values true and false, but at least syntax highlighting (and familiarity) should help with typos around those.
So regardingā¦
Iād argue, no, it doesnāt need to be more verbose, and a more predictable "alternative design" is already mostly in place in the form of nonstandard_style lints, anyway.
Even when youāre depending on other libraries that might not follow the conventions, nonstandard_style apparently still has you covered, e.g.:
#[allow(nonstandard_style, unused)]
mod library {
pub const x: u8 = 1;
pub enum Weird { A, B, c }
pub use Weird::*;
}
#[deny(nonstandard_style)]
mod user {
use super::library::*;
fn _f(w: Weird, n: u8) {
// accepted :-(
match w {
A => {}
c => {}
_ => {}
}
// denied! :-)
// error: constant in pattern `x`
// should have an upper case name
match n {
42 => {}
x => {}
_ => {}
}
}
}
oh, it looks like this actually only works half... oh well, perhaps we should make an issue asking for a non_camel_case_types-warning for patterns. 
Why is nonstandard_style not strictly enforced, anyways? I guess a major motivation can be interoperability in FFI. Incidentally, @keras was talking of FFI, tooā¦ I wouldnāt be surprised if the underlying issue here that resulted in an unsatisfactory low amount of warning was lowercase constants. [And if that wasnāt the issue here, itās the solution, as @mroth had hinted at already, a simple global #![deny(non_snake_case)] should catch all relevant cases and help make the warning less "easy" to ignore.]
Stillā¦ in a world where lowercase constants exist sometimes you'll want to match against them, after all. So coming back to the topic of "a more verbose syntax to distinguish between the two interpretations", here are some thoughts on what might already be possible
(click to expand section)
Explicitly marking variables
being more explicit about a variable isn't necessary, still. Even an external library containing lowercase globals does not in any way force you as the user to ever use upper-case variables.
Explicitly marking enum variants
thereās already a desire for inferring the type/prefix of enum variants without the need to import them. A sometimes mentioned possible syntax is e.g. _::Variant. So instead of
use TheEnum::*;
let x: TheEnum = ā¦;
match x {
Variant1 => ā¦,
Varianl2 => ā¦, // we could miss this typo
Variant3 => ā¦,
_ => ā¦,
}
you could do
use TheEnum::*;
// let x: TheEnum = ā¦; // no longer needed! ^^
match x {
_::Variant1 => ā¦,
_::Variant2 => ā¦,
_::Variant3 => ā¦,
_ => ā¦,
}
a bit verbose, and the _ weirdly aligns with the catch-all :-/ ~~no wonder itās not a feature yet (syntax is hard!)
Explicitly marking constants
we already have on nightly unstable support for const { ā¦ } patterns, so you could write your match arm as const {x} => ā¦ to make explicit that this instance of x isnāt a variable.
Also quite verbose though one might be motivated to use path-based alternatives insteadā¦
Paths
so as already mentioned, just donāt use ā¦ the problematic thing completely, match against m::x where m is the module we got x fromā¦ alternatively, one can target one's own re-export of the thing
#1 - self in a module
if your use path::to::constant::x; is on the module level, then matching against self::x works
#[allow(nonstandard_style, unused)]
mod library {
pub const x: u8 = 1;
}
#[deny(nonstandard_style)]
mod user {
use super::library::*;
fn _f(n: u8) {
match n {
42 => {}
self::x => {} // the nonstandard_style-error is also gone ^^
_ => {}
}
}
}
however, moving the use ā¦::library::*; inside of the f_ function breaks this. Perhaps weād want an additional path-prefix keyword for this meaning? Something like here::x? IDKā¦
Oh here's a fun thing I just found!! would you look at that! - a path can apparently always be given an additional ::<> at the end! Here, look at this!
#[warn(nonstandard_style)]
mod user {
fn _f(w: super::library::Weird, n: u8) {
use super::library::*;
match w {
A => {}
c::<> => {}
_ => {}
}
match n {
42 => {}
x::<> => {} // the nonstandard_style-error is still there :-/
_ => {}
}
}
}
and if you typo any of the things like c or x here, itās immediately a hard error, yes an actual hard error. Yes, I didnāt know that const items support the ::<> too.
2 remaining main issues: first, the case with constants, unlike was the case with self::x, for x::<> the lint about "constant in pattern x should have an upper case name" still fires, and secondly, unfortunately rustfmt will eat those trailing ::<> for breakfast.
Yeah, and they are a bit ugly still~
5 Likes
How many hours of confused debugging has a typo in a match arm cost you recently?
This wasn't recent for me, but it did cost me some time debugging when I was newer to rust. It's exacerbated when you have temporarily commented out some code and have unused variables as a result. So you're expecting some "unused variable" warnings.
So I agree that it could be a problem that makes you frustrated when you finally find out what it was. At the same time, though, rust didn't used to warn about non-snake case catch-all match arms, so it's much better now than it was before.
1 Like
Really? I mean, perhaps pre-1.0, I wonāt go deeper into checking that, but otherwise I think it always did warn.