Auto merge of #9539 - Jarcho:ice_9445, r=flip1995

[rust.git] / clippy_lints / src / unnecessary_wraps.rs

diff --git a/clippy_lints/src/unnecessary_wraps.rs b/clippy_lints/src/unnecessary_wraps.rs
index f2f1410aed7421343bd812172c02f40d25167ca4..83ef3b0fac8741fb6f46cf8f1c2724f15ddba649 100644 (file)
--- a/clippy_lints/src/unnecessary_wraps.rs
+++ b/clippy_lints/src/unnecessary_wraps.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet;
-use clippy_utils::{contains_return, in_macro, is_lang_ctor, return_ty, visitors::find_all_ret_expressions};
+use clippy_utils::{contains_return, is_lang_ctor, return_ty, visitors::find_all_ret_expressions};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::FnKind;
@@ -8,20 +8,22 @@
 use rustc_hir::{Body, ExprKind, FnDecl, HirId, Impl, ItemKind, Node};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::symbol::sym;
 use rustc_span::Span;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for private functions that only return `Ok` or `Some`.
+    /// ### What it does
+    /// Checks for private functions that only return `Ok` or `Some`.
     ///
-    /// **Why is this bad?** It is not meaningful to wrap values when no `None` or `Err` is returned.
+    /// ### Why is this bad?
+    /// It is not meaningful to wrap values when no `None` or `Err` is returned.
     ///
-    /// **Known problems:** There can be false positives if the function signature is designed to
+    /// ### Known problems
+    /// There can be false positives if the function signature is designed to
     /// fit some external requirement.
     ///
-    /// **Example:**
-    ///
+    /// ### Example
     /// ```rust
     /// fn get_cool_number(a: bool, b: bool) -> Option<i32> {
     ///     if a && b {
@@ -47,12 +49,25 @@
     ///     }
     /// }
     /// ```
+    #[clippy::version = "1.50.0"]
     pub UNNECESSARY_WRAPS,
     pedantic,
     "functions that only return `Ok` or `Some`"
 }
 
-declare_lint_pass!(UnnecessaryWraps => [UNNECESSARY_WRAPS]);
+pub struct UnnecessaryWraps {
+    avoid_breaking_exported_api: bool,
+}
+
+impl_lint_pass!(UnnecessaryWraps => [UNNECESSARY_WRAPS]);
+
+impl UnnecessaryWraps {
+    pub fn new(avoid_breaking_exported_api: bool) -> Self {
+        Self {
+            avoid_breaking_exported_api,
+        }
+    }
+}
 
 impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps {
     fn check_fn(
@@ -66,13 +81,13 @@ fn check_fn(
     ) {
         // Abort if public function/method or closure.
         match fn_kind {
-            FnKind::ItemFn(.., visibility) | FnKind::Method(.., Some(visibility)) => {
-                if visibility.node.is_pub() {
+            FnKind::ItemFn(..) | FnKind::Method(..) => {
+                let def_id = cx.tcx.hir().local_def_id(hir_id);
+                if self.avoid_breaking_exported_api && cx.access_levels.is_exported(def_id) {
                     return;
                 }
             },
             FnKind::Closure => return,
-            FnKind::Method(..) => (),
         }
 
         // Abort if the method is implementing a trait or of it a trait method.
@@ -87,9 +102,9 @@ fn check_fn(
 
         // Get the wrapper and inner types, if can't, abort.
         let (return_type_label, lang_item, inner_type) = if let ty::Adt(adt_def, subst) = return_ty(cx, hir_id).kind() {
-            if cx.tcx.is_diagnostic_item(sym::option_type, adt_def.did) {
+            if cx.tcx.is_diagnostic_item(sym::Option, adt_def.did()) {
                 ("Option", OptionSome, subst.type_at(0))
-            } else if cx.tcx.is_diagnostic_item(sym::result_type, adt_def.did) {
+            } else if cx.tcx.is_diagnostic_item(sym::Result, adt_def.did()) {
                 ("Result", ResultOk, subst.type_at(0))
             } else {
                 return;
@@ -100,9 +115,9 @@ fn check_fn(
 
         // Check if all return expression respect the following condition and collect them.
         let mut suggs = Vec::new();
-        let can_sugg = find_all_ret_expressions(cx, &body.value, |ret_expr| {
+        let can_sugg = find_all_ret_expressions(cx, body.value, |ret_expr| {
             if_chain! {
-                if !in_macro(ret_expr.span);
+                if !ret_expr.span.from_expansion();
                 // Check if a function call.
                 if let ExprKind::Call(func, [arg]) = ret_expr.kind;
                 // Check if OPTION_SOME or RESULT_OK, depending on return type.
@@ -115,7 +130,7 @@ fn check_fn(
                         (
                             ret_expr.span,
                             if inner_type.is_unit() {
-                                "".to_string()
+                                String::new()
                             } else {
                                 snippet(cx, arg.span.source_callsite(), "..").to_string()
                             }
@@ -138,11 +153,8 @@ fn check_fn(
                 )
             } else {
                 (
-                    format!(
-                        "this function's return value is unnecessarily wrapped by `{}`",
-                        return_type_label
-                    ),
-                    format!("remove `{}` from the return type...", return_type_label),
+                    format!("this function's return value is unnecessarily wrapped by `{return_type_label}`"),
+                    format!("remove `{return_type_label}` from the return type..."),
                     inner_type.to_string(),
                     "...and then change returning expressions",
                 )