From 97cd46fa33a36bc1edcbc8eba2c142be5da34055 Mon Sep 17 00:00:00 2001 From: alex-semenyuk Date: Sun, 10 Jul 2022 13:11:19 +0300 Subject: [PATCH 1/3] Fix span for or_fun_call --- clippy_lints/src/methods/or_fun_call.rs | 23 ++--- tests/ui/or_fun_call.fixed | 12 ++- tests/ui/or_fun_call.stderr | 106 +++++------------------- 3 files changed, 37 insertions(+), 104 deletions(-) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 3d1208824fa..16e160e6a17 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::switch_to_lazy_eval; -use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; +use clippy_utils::source::{snippet, snippet_with_macro_callsite}; 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; @@ -28,10 +28,10 @@ fn check_unwrap_or_default( cx: &LateContext<'_>, name: &str, fun: &hir::Expr<'_>, - self_expr: &hir::Expr<'_>, 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,24 +52,15 @@ fn check_unwrap_or_default( || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { - let mut applicability = Applicability::MachineApplicable; - let hint = "unwrap_or_default()"; - let sugg_span = span; - - let sugg: String = format!( - "{}.{}", - snippet_with_applicability(cx, self_expr.span, "..", &mut applicability), - hint - ); - + let span_replace_word = method_span.with_hi(span.hi()); span_lint_and_sugg( cx, OR_FUN_CALL, - sugg_span, + span_replace_word, &format!("use of `{}` followed by a call to `{}`", name, path), "try this", - sugg, - applicability, + format!("unwrap_or_default()"), + Applicability::MachineApplicable, ); true @@ -171,7 +162,7 @@ fn check_general_case<'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, 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 index 123aed40251..fdb08d953ff 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -185,7 +185,8 @@ mod issue8239 { .reduce(|mut acc, f| { acc.push_str(&f); acc - }).unwrap_or_default(); + }) + .unwrap_or_default(); } fn more_to_max_suggestion_highest_lines_1() { @@ -197,7 +198,8 @@ mod issue8239 { let _ = ""; acc.push_str(&f); acc - }).unwrap_or_default(); + }) + .unwrap_or_default(); } fn equal_to_max_suggestion_highest_lines() { @@ -208,7 +210,8 @@ mod issue8239 { let _ = ""; acc.push_str(&f); acc - }).unwrap_or_default(); + }) + .unwrap_or_default(); } fn less_than_max_suggestion_highest_lines() { @@ -218,7 +221,8 @@ mod issue8239 { map.reduce(|mut acc, f| { acc.push_str(&f); acc - }).unwrap_or_default(); + }) + .unwrap_or_default(); } } diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index dfe15654bc3..4c5938ab88b 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -7,10 +7,10 @@ LL | with_constructor.unwrap_or(make()); = note: `-D clippy::or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:52:5 + --> $DIR/or_fun_call.rs:52:14 | LL | with_new.unwrap_or(Vec::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a function call --> $DIR/or_fun_call.rs:55:21 @@ -31,16 +31,16 @@ LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:64:5 + --> $DIR/or_fun_call.rs:64:24 | LL | with_default_trait.unwrap_or(Default::default()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:67:5 + --> $DIR/or_fun_call.rs:67:23 | LL | with_default_type.unwrap_or(u64::default()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a function call --> $DIR/or_fun_call.rs:70:18 @@ -49,16 +49,16 @@ LL | self_default.unwrap_or(::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(::default)` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:73:5 + --> $DIR/or_fun_call.rs:73:18 | LL | real_default.unwrap_or(::default()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `real_default.unwrap_or_default()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:76:5 + --> $DIR/or_fun_call.rs:76:14 | LL | with_vec.unwrap_or(vec![]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_vec.unwrap_or_default()` + | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a function call --> $DIR/or_fun_call.rs:79:21 @@ -109,90 +109,28 @@ LL | None.unwrap_or( unsafe { ptr_to_ref(s) } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:182:9 - | -LL | / frames -LL | | .iter() -LL | | .map(|f: &String| f.to_lowercase()) -LL | | .reduce(|mut acc, f| { -... | -LL | | }) -LL | | .unwrap_or(String::new()); - | |_____________________________________^ - | -help: try this - | -LL ~ frames -LL + .iter() -LL + .map(|f: &String| f.to_lowercase()) -LL + .reduce(|mut acc, f| { -LL + acc.push_str(&f); -LL + acc -LL ~ }).unwrap_or_default(); + --> $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:195:9 - | -LL | / iter.map(|f: &String| f.to_lowercase()) -LL | | .reduce(|mut acc, f| { -LL | | let _ = ""; -LL | | let _ = ""; -... | -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 + let _ = ""; -LL + acc.push_str(&f); -LL + acc -LL ~ }).unwrap_or_default(); + --> $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(); + --> $DIR/or_fun_call.rs:214: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: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(); + --> $DIR/or_fun_call.rs:225:10 | +LL | .unwrap_or(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: aborting due to 22 previous errors From 783992e69389bff1890adaca4cac5967c6724b40 Mon Sep 17 00:00:00 2001 From: alex-semenyuk Date: Sun, 10 Jul 2022 13:34:58 +0300 Subject: [PATCH 2/3] Fix dogfood issue for or_fun_call --- clippy_lints/src/methods/or_fun_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 16e160e6a17..5a356108434 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -59,7 +59,7 @@ fn check_unwrap_or_default( span_replace_word, &format!("use of `{}` followed by a call to `{}`", name, path), "try this", - format!("unwrap_or_default()"), + "unwrap_or_default()".to_string(), Applicability::MachineApplicable, ); From c3c4cda50b1b1b212eb02e5d822b3fd1479dc24c Mon Sep 17 00:00:00 2001 From: alex-semenyuk Date: Mon, 11 Jul 2022 10:34:01 +0300 Subject: [PATCH 3/3] Address review --- clippy_lints/src/methods/or_fun_call.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 5a356108434..6af134019a4 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -52,11 +52,10 @@ fn check_unwrap_or_default( || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { - let span_replace_word = method_span.with_hi(span.hi()); span_lint_and_sugg( cx, OR_FUN_CALL, - span_replace_word, + method_span.with_hi(span.hi()), &format!("use of `{}` followed by a call to `{}`", name, path), "try this", "unwrap_or_default()".to_string(),