Auto merge of rust-lang#8292 - marekdownar:8239, r=xFrednet

issue rust-lang#8239: Printed hint for lint or_fun_call is cropped and does no… fixes rust-lang/rust-clippy#8239 changelog: [`or_fun_call`]: if suggestion contains more lines than MAX_SUGGESTION_HIGHLIGHT_LINES it is stripped to one line
rust-lang-ci · Jan 17, 2022 · 8d14c94 · 8d14c94
2 parents 72c59fd + 69d78ce
commit 8d14c94
Show file tree

Hide file tree

Showing 4 changed files with 171 additions and 7 deletions.
diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs
@@ -4,6 +4,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_mac
 use clippy_utils::ty::{implements_trait, match_type};
 use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
 use if_chain::if_chain;
+use rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
@@ -23,6 +24,7 @@ pub(super) fn check<'tcx>(
     args: &'tcx [hir::Expr<'_>],
 ) {
     /// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`.
+    #[allow(clippy::too_many_arguments)]
     fn check_unwrap_or_default(
         cx: &LateContext<'_>,
         name: &str,
@@ -31,6 +33,7 @@ pub(super) fn check<'tcx>(
         arg: &hir::Expr<'_>,
         or_has_args: bool,
         span: Span,
+        method_span: Span,
     ) -> bool {
         let is_default_default = || is_trait_item(cx, fun, sym::Default);
 
@@ -52,16 +55,27 @@ pub(super) fn check<'tcx>(
 
             then {
                 let mut applicability = Applicability::MachineApplicable;
+                let hint = "unwrap_or_default()";
+                let mut sugg_span = span;
+
+                let mut sugg: String = format!(
+                    "{}.{}",
+                    snippet_with_applicability(cx, self_expr.span, "..", &mut applicability),
+                    hint
+                );
+
+                if sugg.lines().count() > MAX_SUGGESTION_HIGHLIGHT_LINES {
+                    sugg_span = method_span.with_hi(span.hi());
+                    sugg = hint.to_string();
+                }
+
                 span_lint_and_sugg(
                     cx,
                     OR_FUN_CALL,
-                    span,
+                    sugg_span,
                     &format!("use of `{}` followed by a call to `{}`", name, path),
                     "try this",
-                    format!(
-                        "{}.unwrap_or_default()",
-                        snippet_with_applicability(cx, self_expr.span, "..", &mut applicability)
-                    ),
+                    sugg,
                     applicability,
                 );
 
@@ -164,7 +178,7 @@ pub(super) fn check<'tcx>(
         match inner_arg.kind {
             hir::ExprKind::Call(fun, or_args) => {
                 let or_has_args = !or_args.is_empty();
-                if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
+                if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) {
                     let fun_span = if or_has_args { None } else { Some(fun.span) };
                     check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
                 }

diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed
@@ -176,4 +176,52 @@ mod issue6675 {
     }
 }
 
+mod issue8239 {
+    fn more_than_max_suggestion_highest_lines_0() {
+        let frames = Vec::new();
+        frames
+            .iter()
+            .map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                acc.push_str(&f);
+                acc
+            })
+            .unwrap_or_default();
+    }
+
+    fn more_to_max_suggestion_highest_lines_1() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        iter.map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                let _ = "";
+                let _ = "";
+                acc.push_str(&f);
+                acc
+            })
+            .unwrap_or_default();
+    }
+
+    fn equal_to_max_suggestion_highest_lines() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        iter.map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                let _ = "";
+                acc.push_str(&f);
+                acc
+            }).unwrap_or_default();
+    }
+
+    fn less_than_max_suggestion_highest_lines() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        let map = iter.map(|f: &String| f.to_lowercase());
+        map.reduce(|mut acc, f| {
+            acc.push_str(&f);
+            acc
+        }).unwrap_or_default();
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs
@@ -176,4 +176,54 @@ mod issue6675 {
     }
 }
 
+mod issue8239 {
+    fn more_than_max_suggestion_highest_lines_0() {
+        let frames = Vec::new();
+        frames
+            .iter()
+            .map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                acc.push_str(&f);
+                acc
+            })
+            .unwrap_or(String::new());
+    }
+
+    fn more_to_max_suggestion_highest_lines_1() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        iter.map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                let _ = "";
+                let _ = "";
+                acc.push_str(&f);
+                acc
+            })
+            .unwrap_or(String::new());
+    }
+
+    fn equal_to_max_suggestion_highest_lines() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        iter.map(|f: &String| f.to_lowercase())
+            .reduce(|mut acc, f| {
+                let _ = "";
+                acc.push_str(&f);
+                acc
+            })
+            .unwrap_or(String::new());
+    }
+
+    fn less_than_max_suggestion_highest_lines() {
+        let frames = Vec::new();
+        let iter = frames.iter();
+        let map = iter.map(|f: &String| f.to_lowercase());
+        map.reduce(|mut acc, f| {
+            acc.push_str(&f);
+            acc
+        })
+        .unwrap_or(String::new());
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr
@@ -108,5 +108,57 @@ error: use of `unwrap_or` followed by a function call
 LL |         None.unwrap_or( unsafe { ptr_to_ref(s) }    );
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
 
-error: aborting due to 18 previous errors
+error: use of `unwrap_or` followed by a call to `new`
+  --> $DIR/or_fun_call.rs:189:14
+   |
+LL |             .unwrap_or(String::new());
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
+
+error: use of `unwrap_or` followed by a call to `new`
+  --> $DIR/or_fun_call.rs:202:14
+   |
+LL |             .unwrap_or(String::new());
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
+
+error: use of `unwrap_or` followed by a call to `new`
+  --> $DIR/or_fun_call.rs:208:9
+   |
+LL | /         iter.map(|f: &String| f.to_lowercase())
+LL | |             .reduce(|mut acc, f| {
+LL | |                 let _ = "";
+LL | |                 acc.push_str(&f);
+LL | |                 acc
+LL | |             })
+LL | |             .unwrap_or(String::new());
+   | |_____________________________________^
+   |
+help: try this
+   |
+LL ~         iter.map(|f: &String| f.to_lowercase())
+LL +             .reduce(|mut acc, f| {
+LL +                 let _ = "";
+LL +                 acc.push_str(&f);
+LL +                 acc
+LL ~             }).unwrap_or_default();
+   |
+
+error: use of `unwrap_or` followed by a call to `new`
+  --> $DIR/or_fun_call.rs:221:9
+   |
+LL | /         map.reduce(|mut acc, f| {
+LL | |             acc.push_str(&f);
+LL | |             acc
+LL | |         })
+LL | |         .unwrap_or(String::new());
+   | |_________________________________^
+   |
+help: try this
+   |
+LL ~         map.reduce(|mut acc, f| {
+LL +             acc.push_str(&f);
+LL +             acc
+LL ~         }).unwrap_or_default();
+   |
+
+error: aborting due to 22 previous errors