I had opened this as an issue on the Rust repo itself Allow pattern matching after guard clause · Issue #45978 · rust-lang/rust · GitHub . I'll post what's there as of this writing and I'll let the conversation continue normally from there.
Original Post
I've opened a separate issue for a Rust style guide on Guard Clause. What this issue is about is that Rust currently doesn't recognize when a path has already been handled by a Guard Clause.
I want to do the following:
use std::fs::File;
let file = File::open("program.cfg");
if file.is_err() { return Err(Error::ConfigLoadFail); }
let Ok(f) = file;
let Ok(f) = file
is a nice usage of pattern matching. But Rust complains as it doesn't know I've already accounted for the error path:
error[E0005]: refutable pattern in local binding: `Err(_)` not covered
--> src/lib.rs:166:7
|
166 | let Ok(f) = file;
| ^^^^^ pattern `Err(_)` not covered
error: aborting due to previous error
I really look forward to using pattern matching with guard clauses rather than using unwrap()
which I find a bit of an eye sore.
I love Rust, keep up the good work
kennytm commented:
You could write this as:
let f = match file {
Ok(f) => f,
Err(_) => return Err(Error::ConfigLoadFail),
};
or, since file
is a Result
, not instance of an arbitrary type:
let f = file.map_err(|_| Error::ConfigLoadFail)?;
If we supported rust-lang/rfcs#1303 (let ... else
) this could be written as
let Ok(f) = file else {
return Err(Error::ConfigLoadFail);
};
I think the compiler should never support the original code by OP as this means we need to inspect the implementation of is_err
to determine it will always return false
on the Ok
branch to determine the let Ok(f) = ...
is irrefutable.
I'm not sure if we should allow:
if let Err(_) = file {
return Err(Error::ConfigLoadFail);
}
let Ok(f) = file;
this looks reasonable, but should require a proper RFC to clarify the control flow interaction.
I responded:
I'm not sure how the internals for Rust checking paths are implemented, but I'm suggesting this more as a relative Guard Clause thing rather than a Result
thing.
In the example above the is a conditional check on file
with a immediate exit with return
in the code block. The very next item within the same scope is the same item file
being pattern matched for assignment. So this looks like it wouldn't be too hard to implement.
What I mean by not strictly a Result
thing is, for example, imagine if File.open
returned an Option
instead. Then my recommendation would look like:
if file.is_none() { return Err(Error::ConfigLoadFail); }
let Some(f) = file;
I do love the map_err
example you provided. I didn't know about that. I think my feature request would be more readable and beginner friendly rather than using map_err
to achieve the same result.
kennytm commented:
What I mean is that the compiler should not be allowed to peek into the definition of is_err()
or is_none()
to know that let Ok(f) =
or let Some(f) =
is acceptable, i.e.
if file.is_none() { return; }
let Some(f) = file;
// Unacceptable, the signature `fn is_none(&self) -> bool` tells nothing
// about the typestate of `file`
but
if let None = file { return; }
let Some(f) = file;
// Acceptable, we know that `Option` can only be
// `None` or `Some(_)`, and the `None` case will not reach here
// so we are sure that `file` can only be `Some(_)`
// and thus the pattern is irrefutable.
my last response:
if let None = file { return; } let Some(f) = file;
Yes. I find that as very acceptable. Thank you.