From 8135ab8a22f4aa6ad071574798397aa131377ca2 Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 3 Dec 2020 16:11:52 -0500 Subject: [PATCH 1/5] Moved map_err_ignore to restriction and updated help message --- clippy_lints/src/map_err_ignore.rs | 4 ++-- tests/ui/map_err.stderr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/map_err_ignore.rs b/clippy_lints/src/map_err_ignore.rs index 5298e16a04d9..324a11f140a8 100644 --- a/clippy_lints/src/map_err_ignore.rs +++ b/clippy_lints/src/map_err_ignore.rs @@ -99,7 +99,7 @@ declare_clippy_lint! { /// } /// ``` pub MAP_ERR_IGNORE, - pedantic, + restriction, "`map_err` should not ignore the original error" } @@ -135,7 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { body_span, "`map_err(|_|...` ignores the original error", None, - "Consider wrapping the error in an enum variant", + "Consider wrapping the error in an enum variant for more error context, or using a named wildcard (`.map_err(|_ignored| ...`) to intentionally ignore the error", ); } } diff --git a/tests/ui/map_err.stderr b/tests/ui/map_err.stderr index 390d7ce2e4e7..8193f7cfb8e0 100644 --- a/tests/ui/map_err.stderr +++ b/tests/ui/map_err.stderr @@ -5,7 +5,7 @@ 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 + = help: Consider wrapping the error in an enum variant for more error context, or using a named wildcard (`.map_err(|_ignored| ...`) to intentionally ignore the error error: aborting due to previous error From f633ef6bba70b91f0415316fe10bb1f9c016ba33 Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 3 Dec 2020 17:22:03 -0500 Subject: [PATCH 2/5] didn't update lint correctly --- clippy_lints/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 167e5b6b87fe..013406347f21 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1214,6 +1214,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&integer_division::INTEGER_DIVISION), LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE), LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION), + LintId::of(&map_err_ignore::MAP_ERR_IGNORE), LintId::of(&matches::REST_PAT_IN_FULLY_BOUND_STRUCTS), LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM), LintId::of(&mem_forget::MEM_FORGET), @@ -1280,7 +1281,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::EXPLICIT_ITER_LOOP), LintId::of(¯o_use::MACRO_USE_IMPORTS), LintId::of(&manual_ok_or::MANUAL_OK_OR), - 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_SAME_ARMS), From 75482f917d7fc65190274bd577dcc2e30ba34aff Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 3 Dec 2020 17:44:50 -0500 Subject: [PATCH 3/5] Apply suggestions from code review updated help message for the user Co-authored-by: Jane Lusby --- clippy_lints/src/map_err_ignore.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/map_err_ignore.rs b/clippy_lints/src/map_err_ignore.rs index 324a11f140a8..f3c0515b9bcd 100644 --- a/clippy_lints/src/map_err_ignore.rs +++ b/clippy_lints/src/map_err_ignore.rs @@ -133,9 +133,9 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { cx, MAP_ERR_IGNORE, body_span, - "`map_err(|_|...` ignores the original error", + "`map_err(|_|...` wildcard pattern discards the original error", None, - "Consider wrapping the error in an enum variant for more error context, or using a named wildcard (`.map_err(|_ignored| ...`) to intentionally ignore the error", + "Consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`)", ); } } From 4bc33d37225db3dd3ab68bb53e855fcf794047db Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 3 Dec 2020 17:49:27 -0500 Subject: [PATCH 4/5] Update the stderr message in ui tests --- tests/ui/map_err.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/map_err.stderr b/tests/ui/map_err.stderr index 8193f7cfb8e0..8ee2941790d3 100644 --- a/tests/ui/map_err.stderr +++ b/tests/ui/map_err.stderr @@ -1,11 +1,11 @@ -error: `map_err(|_|...` ignores the original error +error: `map_err(|_|...` wildcard pattern discards the original error --> $DIR/map_err.rs:23: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 for more error context, or using a named wildcard (`.map_err(|_ignored| ...`) to intentionally ignore the error + = help: Consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`) error: aborting due to previous error From 5f821fbcf1687c4476c117bcab5a3b2a4a977d4c Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 3 Dec 2020 19:41:44 -0500 Subject: [PATCH 5/5] Added test to make sure ignoring the error with a named wildcard value works --- tests/ui/map_err.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/ui/map_err.rs b/tests/ui/map_err.rs index 05b9949f1021..00e037843f8c 100644 --- a/tests/ui/map_err.rs +++ b/tests/ui/map_err.rs @@ -22,5 +22,9 @@ fn main() -> Result<(), Errors> { println!("{:?}", x.map_err(|_| Errors::Ignored)); + // Should not warn you because you explicitly ignore the parameter + // using a named wildcard value + println!("{:?}", x.map_err(|_foo| Errors::Ignored)); + Ok(()) }