Skip to content

Commit e49a299

Browse files
RickyRicky
authored andcommitted
Working map_err_ignore lint
1 parent 066f105 commit e49a299

File tree

2 files changed

+113
-0
lines changed

2 files changed

+113
-0
lines changed

clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ mod main_recursion;
230230
mod manual_async_fn;
231231
mod manual_non_exhaustive;
232232
mod map_clone;
233+
mod map_err_ignore;
233234
mod map_identity;
234235
mod map_unit_fn;
235236
mod match_on_vec_items;
@@ -624,6 +625,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
624625
&manual_async_fn::MANUAL_ASYNC_FN,
625626
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
626627
&map_clone::MAP_CLONE,
628+
&map_err_ignore::MAP_ERR_IGNORE,
627629
&map_identity::MAP_IDENTITY,
628630
&map_unit_fn::OPTION_MAP_UNIT_FN,
629631
&map_unit_fn::RESULT_MAP_UNIT_FN,
@@ -916,6 +918,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
916918
store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
917919
store.register_late_pass(|| box methods::Methods);
918920
store.register_late_pass(|| box map_clone::MapClone);
921+
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
919922
store.register_late_pass(|| box shadow::Shadow);
920923
store.register_late_pass(|| box types::LetUnitValue);
921924
store.register_late_pass(|| box types::UnitCmp);
@@ -1327,6 +1330,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13271330
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
13281331
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
13291332
LintId::of(&map_clone::MAP_CLONE),
1333+
LintId::of(&map_err_ignore::MAP_ERR_IGNORE),
13301334
LintId::of(&map_identity::MAP_IDENTITY),
13311335
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
13321336
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
@@ -1534,6 +1538,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15341538
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
15351539
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
15361540
LintId::of(&map_clone::MAP_CLONE),
1541+
LintId::of(&map_err_ignore::MAP_ERR_IGNORE),
15371542
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
15381543
LintId::of(&matches::MATCH_LIKE_MATCHES_MACRO),
15391544
LintId::of(&matches::MATCH_OVERLAPPING_ARM),

clippy_lints/src/map_err_ignore.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
use crate::utils::span_lint_and_sugg;
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{Expr, ExprKind, CaptureBy, PatKind, QPath};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// **What it does:** Checks for instances of `map_err(|_| Some::Enum)`
9+
///
10+
/// **Why is this bad?** This map_err throws away the original error rather than allowing the enum to bubble the original error
11+
///
12+
/// **Known problems:** None.
13+
///
14+
/// **Example:**
15+
///
16+
/// ```rust
17+
/// enum Errors {
18+
/// Ignore
19+
///}
20+
///fn main() -> Result<(), Errors> {
21+
///
22+
/// let x = u32::try_from(-123_i32);
23+
///
24+
/// println!("{:?}", x.map_err(|_| Errors::Ignore));
25+
///
26+
/// Ok(())
27+
///}
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// enum Errors {
32+
/// WithContext(TryFromIntError)
33+
///}
34+
///fn main() -> Result<(), Errors> {
35+
///
36+
/// let x = u32::try_from(-123_i32);
37+
///
38+
/// println!("{:?}", x.map_err(|e| Errors::WithContext(e)));
39+
///
40+
/// Ok(())
41+
///}
42+
/// ```
43+
pub MAP_ERR_IGNORE,
44+
style,
45+
"`map_err` should not ignore the original error"
46+
}
47+
48+
declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);
49+
50+
impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
51+
// do not try to lint if this is from a macro or desugaring
52+
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
53+
if e.span.from_expansion() {
54+
return;
55+
}
56+
57+
// check if this is a method call (e.g. x.foo())
58+
if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = e.kind {
59+
// only work if the method name is `map_err` and there are only 2 arguments (e.g. x.map_err(|_|[1] Enum::Variant[2]))
60+
if method.ident.as_str() == "map_err" && args.len() == 2 {
61+
// make sure the first argument is a closure, and grab the CaptureRef, body_id, and body_span fields
62+
if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind {
63+
// check if this is by Reference (meaning there's no move statement)
64+
if capture == CaptureBy::Ref {
65+
// Get the closure body to check the parameters and values
66+
let closure_body = cx.tcx.hir().body(body_id);
67+
// make sure there's only one parameter (`|_|`)
68+
if closure_body.params.len() == 1 {
69+
// make sure that parameter is the wild token (`_`)
70+
if let PatKind::Wild = closure_body.params[0].pat.kind {
71+
// Check the value of the closure to see if we can build the enum we are throwing away the error for
72+
// make sure this is a Path
73+
if let ExprKind::Path(q_path) = &closure_body.value.kind {
74+
// this should be a resolved path, only keep the path field
75+
if let QPath::Resolved(_, path) = q_path {
76+
// finally get the idents for each path segment collect them as a string and join them with the path separator ("::"")
77+
let closure_fold: String = path.segments.iter().map(|x| x.ident.as_str().to_string()).collect::<Vec<String>>().join("::");
78+
//Span the body of the closure (the |...| bit) and suggest the fix by taking the error and encapsulating it in the enum
79+
span_lint_and_sugg(
80+
cx,
81+
MAP_ERR_IGNORE,
82+
body_span,
83+
"`map_err` has thrown away the original error",
84+
"Allow the error enum to encapsulate the original error",
85+
format!("|e| {}(e)", closure_fold),
86+
Applicability::HasPlaceholders,
87+
);
88+
}
89+
} else {
90+
//If we cannot build the enum in a human readable way just suggest not throwing way the error
91+
span_lint_and_sugg(
92+
cx,
93+
MAP_ERR_IGNORE,
94+
body_span,
95+
"`map_err` has thrown away the original error",
96+
"Allow the error enum to encapsulate the original error",
97+
"|e|".to_string(),
98+
Applicability::HasPlaceholders,
99+
);
100+
}
101+
}
102+
}
103+
}
104+
}
105+
}
106+
}
107+
}
108+
}

0 commit comments

Comments
 (0)