Prevent wildcard Err match arms

[manswami23]

What it does

Hi, I'm a beginner in Rust and have been playing around with error handling in my repo recently. One of the goals I'm trying to achieve is to ensure that all errors in my codebase are handled explicitly, either by propagating the error back to the caller or logging it.

We currently have the rule map_err_ignore that catches situations where we are throwing away the original error instead of propagating it.

I'm looking for something similar but that also covers when somebody directly matches on a Result, and similarly throws away the original error. So something like this should trigger a warning on the Err(_) match arm:

match get_result() {
    Ok(result) => {do something},
    Err(_) => {do something else}
}

I see that we have match_wild_err_arm, but I believe this only raises a warning when the body of the match arm includes a panic. However, let me know if there's already functionality through some other rule that accomplishes what I'm looking for.

Thanks!

Advantage

No response

Drawbacks

I could see drawbacks of the lint being that sometimes we really don't care about the error returned. I would probably keep it in the Pedantic category and allow it by default to avoid unnecessary warnings unless someone really wants to opt into it.

Example

match get_result() {
    Ok(result) => {do something},
    Err(_) => {do something else}
}

Could be written as:

match get_result() {
    Ok(result) => {do something},
    Err(e) => {
        println!("error happened: {}", e);
        // do something else
    } 
}

[profetia]

I think ignoring the error value is very common and reasonable in a lot of times.

[samueltardieu]

I see that we have match_wild_err_arm, but I believe this only raises a warning when the body of the match arm includes a panic. However, let me know if there's already functionality through some other rule that accomplishes what I'm looking for.

I don't think such a lint would be of use for many people. However, you can do the check at runtime by implementing your own error types (that you would put into the Err variant). Such type can implement Drop and panic when it is dropped without having gone through certain procedures (such as logging). Your logging method can set a flag indicating that the error has been properly logged for example. Another possibility would be to signal a error in the logs if it has not properly been logged before.

[manswami23]

@profetia , @samueltardieu i definitely agree that it's common someone wants to ignore the error value.

That being said, I feel the suggestion here is at least philosophically similar with the existing rule map_err_ignore, where if someone is ignoring an error value and returning some other error inside the mapping function, we have the option of warning them about it. But maybe I'm misunderstanding the original intent behind that rule?

[Jarcho]

We could extend map_err_ignore to catch anytime Err(_) gets turned into Err(T) rather than only linting on map_err. e.g. Err(_) => Err(MyError) or Err(_) => return Err(MyError).