about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2022-06-05 15:59:24 -0400
committerJason Newcomb <jsnewcomb@pm.me>2022-08-19 10:32:15 -0400
commit2f0ed0a0b1fa7928464b95e0cfd0883699cb9c33 (patch)
tree2ed207772077d5df969d7d48595670cec540df70
parent452395485b113b06c1a47313eeab75d374d97e9f (diff)
downloadrust-2f0ed0a0b1fa7928464b95e0cfd0883699cb9c33.tar.gz
rust-2f0ed0a0b1fa7928464b95e0cfd0883699cb9c33.zip
Move `MapErrIgnore` into `Methods` lint pass
-rw-r--r--clippy_lints/src/lib.register_lints.rs2
-rw-r--r--clippy_lints/src/lib.register_restriction.rs2
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/map_err_ignore.rs154
-rw-r--r--clippy_lints/src/methods/map_err_ignore.rs34
-rw-r--r--clippy_lints/src/methods/mod.rs104
6 files changed, 140 insertions, 158 deletions
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index c7ea7f703f2..67fb0d50931 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -246,7 +246,6 @@ store.register_lints(&[
     manual_rem_euclid::MANUAL_REM_EUCLID,
     manual_retain::MANUAL_RETAIN,
     manual_strip::MANUAL_STRIP,
-    map_err_ignore::MAP_ERR_IGNORE,
     map_unit_fn::OPTION_MAP_UNIT_FN,
     map_unit_fn::RESULT_MAP_UNIT_FN,
     match_result_ok::MATCH_RESULT_OK,
@@ -326,6 +325,7 @@ store.register_lints(&[
     methods::MANUAL_STR_REPEAT,
     methods::MAP_CLONE,
     methods::MAP_COLLECT_RESULT_UNIT,
+    methods::MAP_ERR_IGNORE,
     methods::MAP_FLATTEN,
     methods::MAP_IDENTITY,
     methods::MAP_UNWRAP_OR,
diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs
index 890ae2792ab..7fc5eef5f8a 100644
--- a/clippy_lints/src/lib.register_restriction.rs
+++ b/clippy_lints/src/lib.register_restriction.rs
@@ -30,7 +30,6 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(large_include_file::LARGE_INCLUDE_FILE),
     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::TRY_ERR),
     LintId::of(matches::WILDCARD_ENUM_MATCH_ARM),
@@ -39,6 +38,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(methods::EXPECT_USED),
     LintId::of(methods::FILETYPE_IS_FILE),
     LintId::of(methods::GET_UNWRAP),
+    LintId::of(methods::MAP_ERR_IGNORE),
     LintId::of(methods::UNWRAP_USED),
     LintId::of(misc_early::SEPARATED_LITERAL_SUFFIX),
     LintId::of(misc_early::UNNEEDED_FIELD_PATTERN),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 8f31e4b938c..179d630089f 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -273,7 +273,6 @@ mod manual_non_exhaustive;
 mod manual_rem_euclid;
 mod manual_retain;
 mod manual_strip;
-mod map_err_ignore;
 mod map_unit_fn;
 mod match_result_ok;
 mod matches;
@@ -636,7 +635,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             msrv,
         ))
     });
-    store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
     store.register_late_pass(|| Box::new(shadow::Shadow::default()));
     store.register_late_pass(|| Box::new(unit_types::UnitTypes));
     store.register_late_pass(|| Box::new(loops::Loops));
diff --git a/clippy_lints/src/map_err_ignore.rs b/clippy_lints/src/map_err_ignore.rs
deleted file mode 100644
index 1e542447c96..00000000000
--- a/clippy_lints/src/map_err_ignore.rs
+++ /dev/null
@@ -1,154 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use rustc_hir::{CaptureBy, Closure, 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
-    ///
-    /// ### 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))
-    ///             }
-    ///         })
-    /// }
-    /// ```
-    #[clippy::version = "1.48.0"]
-    pub MAP_ERR_IGNORE,
-    restriction,
-    "`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(method, [_, arg], _) = 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.name == sym!(map_err) {
-                // make sure the first argument is a closure, and grab the CaptureRef, BodyId, and fn_decl_span
-                // fields
-                if let ExprKind::Closure(&Closure {
-                    capture_clause,
-                    body,
-                    fn_decl_span,
-                    ..
-                }) = arg.kind
-                {
-                    // check if this is by Reference (meaning there's no move statement)
-                    if capture_clause == CaptureBy::Ref {
-                        // Get the closure body to check the parameters and values
-                        let closure_body = cx.tcx.hir().body(body);
-                        // 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,
-                                    fn_decl_span,
-                                    "`map_err(|_|...` wildcard pattern discards the original error",
-                                    None,
-                                    "consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`)",
-                                );
-                            }
-                        }
-                    }
-                }
-            }
-        }
-    }
-}
diff --git a/clippy_lints/src/methods/map_err_ignore.rs b/clippy_lints/src/methods/map_err_ignore.rs
new file mode 100644
index 00000000000..1fb6617145e
--- /dev/null
+++ b/clippy_lints/src/methods/map_err_ignore.rs
@@ -0,0 +1,34 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_hir::{CaptureBy, Closure, Expr, ExprKind, PatKind};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::MAP_ERR_IGNORE;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'_>, e: &Expr<'_>, arg: &'tcx Expr<'_>) {
+    if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
+        && let Some(impl_id) = cx.tcx.impl_of_method(method_id)
+        && is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id), sym::Result)
+        && let ExprKind::Closure(&Closure {
+            capture_clause: CaptureBy::Ref,
+            body,
+            fn_decl_span,
+            ..
+        }) = arg.kind
+        && let closure_body = cx.tcx.hir().body(body)
+        && let [param] = closure_body.params
+        && let PatKind::Wild = param.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,
+            fn_decl_span,
+            "`map_err(|_|...` wildcard pattern discards the original error",
+            None,
+            "consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`)",
+        );
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 02592da1aae..e327bdcf578 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -47,6 +47,7 @@ mod manual_saturating_arithmetic;
 mod manual_str_repeat;
 mod map_clone;
 mod map_collect_result_unit;
+mod map_err_ignore;
 mod map_flatten;
 mod map_identity;
 mod map_unwrap_or;
@@ -2541,6 +2542,106 @@ declare_clippy_lint! {
     "using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
 }
 
+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
+    ///
+    /// ### 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))
+    ///             }
+    ///         })
+    /// }
+    /// ```
+    #[clippy::version = "1.48.0"]
+    pub MAP_ERR_IGNORE,
+    restriction,
+    "`map_err` should not ignore the original error"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2651,6 +2752,7 @@ impl_lint_pass!(Methods => [
     GET_FIRST,
     MANUAL_OK_OR,
     MAP_CLONE,
+    MAP_ERR_IGNORE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2982,6 +3084,8 @@ impl Methods {
                 (name @ ("map" | "map_err"), [m_arg]) => {
                     if name == "map" {
                         map_clone::check(cx, expr, recv, m_arg, self.msrv);
+                    } else {
+                        map_err_ignore::check(cx, expr, m_arg);
                     }
                     if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) {
                         match (name, args) {