Add clippy::result_as_ref_deref lint

Author: aleksanderkrauze

CHANGELOG.md

@@ -5880,6 +5880,7 @@ Released 2018-09-13
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
+[`result_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_as_ref_deref
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
 [`result_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_filter_map
 [`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

clippy_config/src/msrvs.rs

@@ -17,8 +17,8 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
-    1,81,0  { LINT_REASONS_STABILIZATION }
-    1,80,0 { BOX_INTO_ITER}
+    1,81,0 { LINT_REASONS_STABILIZATION }
+    1,80,0 { BOX_INTO_ITER }
     1,77,0 { C_STR_LITERALS }
     1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
     1,73,0 { MANUAL_DIV_CEIL }
@@ -37,7 +37,7 @@ msrv_aliases! {
     1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
     1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
     1,50,0 { BOOL_THEN, CLAMP }
-    1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST }
+    1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST, RESULT_AS_DEREF }
     1,46,0 { CONST_IF_MATCH }
     1,45,0 { STR_STRIP_PREFIX }
     1,43,0 { LOG2_10, LOG10_2, NUMERIC_ASSOCIATED_CONSTANTS }

clippy_lints/src/declared_lints.rs

@@ -444,6 +444,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
     crate::methods::REDUNDANT_AS_STR_INFO,
     crate::methods::REPEAT_ONCE_INFO,
+    crate::methods::RESULT_AS_REF_DEREF_INFO,
     crate::methods::RESULT_FILTER_MAP_INFO,
     crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
     crate::methods::SEARCH_IS_SOME_INFO,

clippy_lints/src/methods/option_as_ref_deref.rs renamed to clippy_lints/src/methods/manual_as_ref_deref.rs

@@ -9,9 +9,15 @@ use rustc_lint::LateContext;
 use rustc_middle::ty;
 use rustc_span::sym;
 
-use super::OPTION_AS_REF_DEREF;
+use super::{OPTION_AS_REF_DEREF, RESULT_AS_REF_DEREF};
 
-/// lint use of `_.as_ref().map(Deref::deref)` for `Option`s
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Target {
+    Option,
+    Result,
+}
+
+/// lint use of `_.as_ref().map(Deref::deref)` for `Option`s and `Result`s
 pub(super) fn check(
     cx: &LateContext<'_>,
     expr: &hir::Expr<'_>,
@@ -20,14 +26,23 @@ pub(super) fn check(
     is_mut: bool,
     msrv: &Msrv,
 ) {
-    if !msrv.meets(msrvs::OPTION_AS_DEREF) {
-        return;
-    }
-
     let same_mutability = |m| (is_mut && m == &hir::Mutability::Mut) || (!is_mut && m == &hir::Mutability::Not);
 
-    let option_ty = cx.typeck_results().expr_ty(as_ref_recv);
-    if !is_type_diagnostic_item(cx, option_ty, sym::Option) {
+    let target = 'target: {
+        let target_ty = cx.typeck_results().expr_ty(as_ref_recv);
+        if is_type_diagnostic_item(cx, target_ty, sym::Option) {
+            break 'target Target::Option;
+        }
+        if is_type_diagnostic_item(cx, target_ty, sym::Result) {
+            break 'target Target::Result;
+        }
+        return;
+    };
+
+    if target == Target::Option && !msrv.meets(msrvs::OPTION_AS_DEREF) {
+        return;
+    }
+    if target == Target::Result && !msrv.meets(msrvs::RESULT_AS_DEREF) {
         return;
     }
 
@@ -99,10 +114,20 @@ pub(super) fn check(
         let hint = format!("{}.{method_hint}()", snippet(cx, as_ref_recv.span, ".."));
         let suggestion = format!("consider using {method_hint}");
 
-        let msg = format!("called `{current_method}` on an `Option` value");
+        let target_name_with_article = match target {
+            Target::Option => "an `Option`",
+            Target::Result => "a `Result`",
+        };
+        let msg = format!("called `{current_method}` on {target_name_with_article} value");
+
+        let lint = match target {
+            Target::Option => OPTION_AS_REF_DEREF,
+            Target::Result => RESULT_AS_REF_DEREF,
+        };
+
         span_lint_and_sugg(
             cx,
-            OPTION_AS_REF_DEREF,
+            lint,
             expr.span,
             msg,
             suggestion,

clippy_lints/src/methods/mod.rs

@@ -52,6 +52,7 @@ mod iter_skip_zero;
 mod iter_with_drain;
 mod iterator_step_by_zero;
 mod join_absolute_paths;
+mod manual_as_ref_deref;
 mod manual_c_str_literals;
 mod manual_inspect;
 mod manual_is_variant_and;
@@ -76,7 +77,6 @@ mod obfuscated_if_else;
 mod ok_expect;
 mod open_options;
 mod option_as_ref_cloned;
-mod option_as_ref_deref;
 mod option_map_or_err_ok;
 mod option_map_or_none;
 mod option_map_unwrap_or;
@@ -1758,7 +1758,8 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for usage of `_.as_ref().map(Deref::deref)` or its aliases (such as String::as_str).
+    /// Checks for usage of `_.as_ref().map(Deref::deref)` or its aliases
+    /// (such as `String::as_str`) on `Option`.
     ///
     /// ### Why is this bad?
     /// Readability, this can be written more concisely as
@@ -1782,6 +1783,33 @@ declare_clippy_lint! {
     "using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `_.as_ref().map(Deref::deref)` or its aliases
+    /// (such as `String::as_str`) on `Result`.
+    ///
+    /// ### Why is this bad?
+    /// Readability, this can be written more concisely as
+    /// `_.as_deref()`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let res = Ok::<_, ()>("".to_string());
+    /// res.as_ref().map(String::as_str)
+    /// # ;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let res = OK::<_, ()>("".to_string());
+    /// res.as_deref()
+    /// # ;
+    /// ```
+    #[clippy::version = "1.83.0"]
+    pub RESULT_AS_REF_DEREF,
+    complexity,
+    "using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `iter().next()` on a Slice or an Array
@@ -4223,6 +4251,7 @@ impl_lint_pass!(Methods => [
     ZST_OFFSET,
     FILETYPE_IS_FILE,
     OPTION_AS_REF_DEREF,
+    RESULT_AS_REF_DEREF,
     UNNECESSARY_LAZY_EVALUATIONS,
     MAP_COLLECT_RESULT_UNIT,
     FROM_ITER_INSTEAD_OF_COLLECT,
@@ -4791,8 +4820,8 @@ impl Methods {
                     }
                     if let Some((name, recv2, args, span2, _)) = method_call(recv) {
                         match (name, args) {
-                            ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, &self.msrv),
-                            ("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, &self.msrv),
+                            ("as_mut", []) => manual_as_ref_deref::check(cx, expr, recv2, m_arg, true, &self.msrv),
+                            ("as_ref", []) => manual_as_ref_deref::check(cx, expr, recv2, m_arg, false, &self.msrv),
                             ("filter", [f_arg]) => {
                                 filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, false);
                             },

tests/ui/result_as_ref_deref.fixed

@@ -0,0 +1,53 @@
+#![allow(unused, clippy::redundant_clone, clippy::useless_vec)]
+#![warn(clippy::result_as_ref_deref)]
+
+use std::ffi::{CString, OsString};
+use std::ops::{Deref, DerefMut};
+use std::path::PathBuf;
+
+fn main() {
+    let mut res = Ok::<_, ()>(String::from("123"));
+
+    let _ = res.clone().as_deref().map(str::len);
+
+    #[rustfmt::skip]
+    let _ = res.clone().as_deref()
+        .map(str::len);
+
+    let _ = res.as_deref_mut();
+
+    let _ = res.as_deref();
+    let _ = res.as_deref();
+    let _ = res.as_deref_mut();
+    let _ = res.as_deref_mut();
+    let _ = Ok::<_, ()>(CString::new(vec![]).unwrap()).as_deref();
+    let _ = Ok::<_, ()>(OsString::new()).as_deref();
+    let _ = Ok::<_, ()>(PathBuf::new()).as_deref();
+    let _ = Ok::<_, ()>(Vec::<()>::new()).as_deref();
+    let _ = Ok::<_, ()>(Vec::<()>::new()).as_deref_mut();
+
+    let _ = res.as_deref();
+    let _ = res.clone().as_deref_mut().map(|x| x.len());
+
+    let vc = vec![String::new()];
+    let _ = Ok::<_, ()>(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
+
+    let _: Result<&str, &()> = Ok(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
+
+    let _ = res.as_deref();
+    let _ = res.as_deref_mut();
+
+    let _ = res.as_deref();
+}
+
+#[clippy::msrv = "1.46"]
+fn msrv_1_46() {
+    let res = Ok::<_, ()>(String::from("123"));
+    let _ = res.as_ref().map(String::as_str);
+}
+
+#[clippy::msrv = "1.47"]
+fn msrv_1_47() {
+    let res = Ok::<_, ()>(String::from("123"));
+    let _ = res.as_deref();
+}

tests/ui/result_as_ref_deref.rs

@@ -0,0 +1,58 @@
+#![allow(unused, clippy::redundant_clone, clippy::useless_vec)]
+#![warn(clippy::result_as_ref_deref)]
+
+use std::ffi::{CString, OsString};
+use std::ops::{Deref, DerefMut};
+use std::path::PathBuf;
+
+fn main() {
+    let mut res = Ok::<_, ()>(String::from("123"));
+
+    let _ = res.clone().as_ref().map(Deref::deref).map(str::len);
+
+    #[rustfmt::skip]
+    let _ = res.clone()
+        .as_ref().map(
+            Deref::deref
+        )
+        .map(str::len);
+
+    let _ = res.as_mut().map(DerefMut::deref_mut);
+
+    let _ = res.as_ref().map(String::as_str);
+    let _ = res.as_ref().map(|x| x.as_str());
+    let _ = res.as_mut().map(String::as_mut_str);
+    let _ = res.as_mut().map(|x| x.as_mut_str());
+    let _ = Ok::<_, ()>(CString::new(vec![]).unwrap())
+        .as_ref()
+        .map(CString::as_c_str);
+    let _ = Ok::<_, ()>(OsString::new()).as_ref().map(OsString::as_os_str);
+    let _ = Ok::<_, ()>(PathBuf::new()).as_ref().map(PathBuf::as_path);
+    let _ = Ok::<_, ()>(Vec::<()>::new()).as_ref().map(Vec::as_slice);
+    let _ = Ok::<_, ()>(Vec::<()>::new()).as_mut().map(Vec::as_mut_slice);
+
+    let _ = res.as_ref().map(|x| x.deref());
+    let _ = res.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len());
+
+    let vc = vec![String::new()];
+    let _ = Ok::<_, ()>(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
+
+    let _: Result<&str, &()> = Ok(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
+
+    let _ = res.as_ref().map(|x| &**x);
+    let _ = res.as_mut().map(|x| &mut **x);
+
+    let _ = res.as_ref().map(std::ops::Deref::deref);
+}
+
+#[clippy::msrv = "1.46"]
+fn msrv_1_46() {
+    let res = Ok::<_, ()>(String::from("123"));
+    let _ = res.as_ref().map(String::as_str);
+}
+
+#[clippy::msrv = "1.47"]
+fn msrv_1_47() {
+    let res = Ok::<_, ()>(String::from("123"));
+    let _ = res.as_ref().map(String::as_str);
+}