Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 23:20:39 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #5998 - deg4uss3r:master, r=yaahc

Browse source 
Add map_err_ignore lint

In a large code base a lot of times errors are ignored by using something like:

```rust
foo.map_err(|_| Some::Enum)?;
```

This drops the original error in favor of a enum that will not have the original error's context. This lint helps catch throwing away the original error in favor of an enum without its context.

---

*Please keep the line below*
changelog: Added map_err_ignore lint
This commit is contained in:
bors 2020-09-14 19:56:47 +00:00
commit f82e84c894
8 changed files with 214 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -1676,6 +1676,7 @@ Released 2018-09-13
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
[`map_err_ignore`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_err_ignore
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or

4
clippy_lints/src/lib.rs
View file

@ -231,6 +231,7 @@ mod main_recursion;
mod manual_async_fn;
mod manual_non_exhaustive;
mod map_clone;
mod map_err_ignore;
mod map_identity;
mod map_unit_fn;
mod match_on_vec_items;
@ -627,6 +628,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&manual_async_fn::MANUAL_ASYNC_FN,
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
&map_clone::MAP_CLONE,
&map_err_ignore::MAP_ERR_IGNORE,
&map_identity::MAP_IDENTITY,
&map_unit_fn::OPTION_MAP_UNIT_FN,
&map_unit_fn::RESULT_MAP_UNIT_FN,
@ -920,6 +922,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
store.register_late_pass(|| box methods::Methods);
store.register_late_pass(|| box map_clone::MapClone);
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
store.register_late_pass(|| box shadow::Shadow);
store.register_late_pass(|| box types::LetUnitValue);
store.register_late_pass(|| box types::UnitCmp);
@ -1186,6 +1189,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
LintId::of(&loops::EXPLICIT_ITER_LOOP),
LintId::of(&macro_use::MACRO_USE_IMPORTS),
LintId::of(&map_err_ignore::MAP_ERR_IGNORE),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),

147
clippy_lints/src/map_err_ignore.rs Normal file
View file

@ -0,0 +1,147 @@
use crate::utils::span_lint_and_help;
use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:** Checks for instances of `map_err(|_| Some::Enum)`
///
/// **Why is this bad?** This map_err throws away the original error rather than allowing the enum to contain and report the cause of the error
///
/// **Known problems:** None.
///
/// **Example:**
/// Before:
/// ```rust
/// use std::fmt;
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible,
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {}
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(|_| Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
///
/// After:
/// ```rust
/// use std::{fmt, num::ParseIntError};
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible(ParseIntError),
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible(_) => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {
/// fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
/// match self {
/// Error::Indivisible(source) => Some(source),
/// _ => None,
/// }
/// }
/// }
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
pub MAP_ERR_IGNORE,
pedantic,
"`map_err` should not ignore the original error"
}
declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);
impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
// do not try to lint if this is from a macro or desugaring
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if e.span.from_expansion() {
return;
}
// check if this is a method call (e.g. x.foo())
if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = e.kind {
// only work if the method name is `map_err` and there are only 2 arguments (e.g. x.map_err(|_|[1]
// Enum::Variant[2]))
if method.ident.as_str() == "map_err" && args.len() == 2 {
// make sure the first argument is a closure, and grab the CaptureRef, body_id, and body_span fields
if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind {
// check if this is by Reference (meaning there's no move statement)
if capture == CaptureBy::Ref {
// Get the closure body to check the parameters and values
let closure_body = cx.tcx.hir().body(body_id);
// make sure there's only one parameter (`|_|`)
if closure_body.params.len() == 1 {
// make sure that parameter is the wild token (`_`)
if let PatKind::Wild = closure_body.params[0].pat.kind {
// span the area of the closure capture and warn that the
// original error will be thrown away
span_lint_and_help(
cx,
MAP_ERR_IGNORE,
body_span,
"`map_err(|_|...` ignores the original error",
None,
"Consider wrapping the error in an enum variant",
);
}
}
}
}
}
}
}
}

7
src/lintlist/mod.rs
View file

@ -1172,6 +1172,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "entry",
},
Lint {
name: "map_err_ignore",
group: "pedantic",
desc: "`map_err` should not ignore the original error",
deprecation: None,
module: "map_err_ignore",
},
Lint {
name: "map_flatten",
group: "pedantic",

1
tests/ui/drop_ref.rs
View file

@ -1,5 +1,6 @@
#![warn(clippy::drop_ref)]
#![allow(clippy::toplevel_ref_arg)]
#![allow(clippy::map_err_ignore)]
use std::mem::drop;

36
tests/ui/drop_ref.stderr
View file

@ -1,108 +1,108 @@
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:9:5
--> $DIR/drop_ref.rs:10:5
|
LL | drop(&SomeStruct);
| ^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::drop-ref` implied by `-D warnings`
note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:9:10
--> $DIR/drop_ref.rs:10:10
|
LL | drop(&SomeStruct);
| ^^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:12:5
--> $DIR/drop_ref.rs:13:5
|
LL | drop(&owned1);
| ^^^^^^^^^^^^^
|
note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:12:10
--> $DIR/drop_ref.rs:13:10
|
LL | drop(&owned1);
| ^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:13:5
--> $DIR/drop_ref.rs:14:5
|
LL | drop(&&owned1);
| ^^^^^^^^^^^^^^
|
note: argument has type `&&SomeStruct`
--> $DIR/drop_ref.rs:13:10
--> $DIR/drop_ref.rs:14:10
|
LL | drop(&&owned1);
| ^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:14:5
--> $DIR/drop_ref.rs:15:5
|
LL | drop(&mut owned1);
| ^^^^^^^^^^^^^^^^^
|
note: argument has type `&mut SomeStruct`
--> $DIR/drop_ref.rs:14:10
--> $DIR/drop_ref.rs:15:10
|
LL | drop(&mut owned1);
| ^^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:18:5
--> $DIR/drop_ref.rs:19:5
|
LL | drop(reference1);
| ^^^^^^^^^^^^^^^^
|
note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:18:10
--> $DIR/drop_ref.rs:19:10
|
LL | drop(reference1);
| ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:21:5
--> $DIR/drop_ref.rs:22:5
|
LL | drop(reference2);
| ^^^^^^^^^^^^^^^^
|
note: argument has type `&mut SomeStruct`
--> $DIR/drop_ref.rs:21:10
--> $DIR/drop_ref.rs:22:10
|
LL | drop(reference2);
| ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:24:5
--> $DIR/drop_ref.rs:25:5
|
LL | drop(reference3);
| ^^^^^^^^^^^^^^^^
|
note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:24:10
--> $DIR/drop_ref.rs:25:10
|
LL | drop(reference3);
| ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:29:5
--> $DIR/drop_ref.rs:30:5
|
LL | drop(&val);
| ^^^^^^^^^^
|
note: argument has type `&T`
--> $DIR/drop_ref.rs:29:10
--> $DIR/drop_ref.rs:30:10
|
LL | drop(&val);
| ^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_ref.rs:37:5
--> $DIR/drop_ref.rs:38:5
|
LL | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:37:20
--> $DIR/drop_ref.rs:38:20
|
LL | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^

25
tests/ui/map_err.rs Normal file
View file

@ -0,0 +1,25 @@
#![warn(clippy::map_err_ignore)]
use std::convert::TryFrom;
use std::error::Error;
use std::fmt;
#[derive(Debug)]
enum Errors {
Ignored,
}
impl Error for Errors {}
impl fmt::Display for Errors {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Error")
}
}
fn main() -> Result<(), Errors> {
let x = u32::try_from(-123_i32);
println!("{:?}", x.map_err(|_| Errors::Ignored));
Ok(())
}

11
tests/ui/map_err.stderr Normal file
View file

@ -0,0 +1,11 @@
error: `map_err(|_|...` ignores the original error
--> $DIR/map_err.rs:22:32
|
LL | println!("{:?}", x.map_err(|_| Errors::Ignored));
| ^^^
|
= note: `-D clippy::map-err-ignore` implied by `-D warnings`
= help: Consider wrapping the error in an enum variant
error: aborting due to previous error