From 4c4152b01c6c8e2f036ca650cb0e6a6cfc374af3 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Sun, 19 Jun 2022 23:21:14 +0400 Subject: [PATCH 1/3] remove `span_lint_and_sugg_for_edges` from clippy utils --- .../clippy_lints/src/methods/map_flatten.rs | 11 +-- .../internal_lints/metadata_collector.rs | 1 - .../clippy/clippy_utils/src/diagnostics.rs | 91 +------------------ src/tools/clippy/tests/ui/map_flatten.stderr | 25 ++--- .../clippy/tests/ui/map_flatten_fixable.fixed | 2 - .../tests/ui/map_flatten_fixable.stderr | 54 ++--------- 6 files changed, 23 insertions(+), 161 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs index f447940ea3b..8ae84dbb3dc 100644 --- a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg_for_edges; +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_trait_method; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; @@ -14,17 +14,14 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) { if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) { let mut applicability = Applicability::MachineApplicable; - let help_msgs = [ - &format!("try replacing `map` with `{}`", method_to_use), - "and remove the `.flatten()`", - ]; + let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability); - span_lint_and_sugg_for_edges( + span_lint_and_sugg( cx, MAP_FLATTEN, expr.span.with_lo(map_span.lo()), &format!("called `map(..).flatten()` on `{}`", caller_ty_name), - &help_msgs, + &format!("try replacing `map` with `{}` and remove the `.flatten()`", method_to_use), format!("{}({})", method_to_use, closure_snippet), applicability, ); diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 99e9e3275ab..2564099f4db 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -112,7 +112,6 @@ macro_rules! RENAME_VALUE_TEMPLATE { &["clippy_utils", "diagnostics", "span_lint_and_sugg"], &["clippy_utils", "diagnostics", "span_lint_and_then"], &["clippy_utils", "diagnostics", "span_lint_hir_and_then"], - &["clippy_utils", "diagnostics", "span_lint_and_sugg_for_edges"], ]; const SUGGESTION_DIAGNOSTIC_BUILDER_METHODS: [(&str, bool); 9] = [ ("span_suggestion", false), diff --git a/src/tools/clippy/clippy_utils/src/diagnostics.rs b/src/tools/clippy/clippy_utils/src/diagnostics.rs index 39595f589c7..7f55db3b31f 100644 --- a/src/tools/clippy/clippy_utils/src/diagnostics.rs +++ b/src/tools/clippy/clippy_utils/src/diagnostics.rs @@ -8,7 +8,7 @@ //! Thank you! //! ~The `INTERNAL_METADATA_COLLECTOR` lint -use rustc_errors::{emitter::MAX_SUGGESTION_HIGHLIGHT_LINES, Applicability, Diagnostic, MultiSpan}; +use rustc_errors::{Applicability, Diagnostic, MultiSpan}; use rustc_hir::HirId; use rustc_lint::{LateContext, Lint, LintContext}; use rustc_span::source_map::Span; @@ -219,95 +219,6 @@ pub fn span_lint_and_sugg<'a, T: LintContext>( }); } -/// Like [`span_lint_and_sugg`] with a focus on the edges. The output will either -/// emit single span or multispan suggestion depending on the number of its lines. -/// -/// If the given suggestion string has more lines than the maximum display length defined by -/// [`MAX_SUGGESTION_HIGHLIGHT_LINES`][`rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES`], -/// this function will split the suggestion and span to showcase the change for the top and -/// bottom edge of the code. For normal suggestions, in one display window, the help message -/// will be combined with a colon. -/// -/// Multipart suggestions like the one being created here currently cannot be -/// applied by rustfix (See [rustfix#141](https://github.com/rust-lang/rustfix/issues/141)). -/// Testing rustfix with this lint emission function might require a file with -/// suggestions that can be fixed and those that can't. See -/// [clippy#8520](https://github.com/rust-lang/rust-clippy/pull/8520/files) for -/// an example and of this. -/// -/// # Example for a long suggestion -/// -/// ```text -/// error: called `map(..).flatten()` on `Option` -/// --> $DIR/map_flatten.rs:8:10 -/// | -/// LL | .map(|x| { -/// | __________^ -/// LL | | if x <= 5 { -/// LL | | Some(x) -/// LL | | } else { -/// ... | -/// LL | | }) -/// LL | | .flatten(); -/// | |__________________^ -/// | -/// = note: `-D clippy::map-flatten` implied by `-D warnings` -/// help: try replacing `map` with `and_then` -/// | -/// LL ~ .and_then(|x| { -/// LL + if x <= 5 { -/// LL + Some(x) -/// | -/// help: and remove the `.flatten()` -/// | -/// LL + None -/// LL + } -/// LL ~ }); -/// | -/// ``` -pub fn span_lint_and_sugg_for_edges( - cx: &LateContext<'_>, - lint: &'static Lint, - sp: Span, - msg: &str, - helps: &[&str; 2], - sugg: String, - applicability: Applicability, -) { - span_lint_and_then(cx, lint, sp, msg, |diag| { - let sugg_lines_count = sugg.lines().count(); - if sugg_lines_count > MAX_SUGGESTION_HIGHLIGHT_LINES { - let sm = cx.sess().source_map(); - if let (Ok(line_upper), Ok(line_bottom)) = - (sm.lookup_line(sp.lo()), sm.lookup_line(sp.hi())) - { - let split_idx = MAX_SUGGESTION_HIGHLIGHT_LINES / 2; - let span_upper = sm.span_until_char( - sp.with_hi(line_upper.sf.lines(|lines| lines[line_upper.line + split_idx])), - '\n', - ); - let span_bottom = sp.with_lo(line_bottom.sf.lines(|lines| lines[line_bottom.line - split_idx])); - - let sugg_lines_vec = sugg.lines().collect::>(); - let sugg_upper = sugg_lines_vec[..split_idx].join("\n"); - let sugg_bottom = sugg_lines_vec[sugg_lines_count - split_idx..].join("\n"); - - diag.span_suggestion(span_upper, helps[0], sugg_upper, applicability); - diag.span_suggestion(span_bottom, helps[1], sugg_bottom, applicability); - - return; - } - } - diag.span_suggestion_with_style( - sp, - &helps.join(", "), - sugg, - applicability, - rustc_errors::SuggestionStyle::ShowAlways, - ); - }); -} - /// Create a suggestion made from several `span → replacement`. /// /// Note: in the JSON format (used by `compiletest_rs`), the help message will diff --git a/src/tools/clippy/tests/ui/map_flatten.stderr b/src/tools/clippy/tests/ui/map_flatten.stderr index c9c60df838f..4b2630d6858 100644 --- a/src/tools/clippy/tests/ui/map_flatten.stderr +++ b/src/tools/clippy/tests/ui/map_flatten.stderr @@ -12,14 +12,12 @@ LL | | .flatten(); | |__________________^ | = note: `-D clippy::map-flatten` implied by `-D warnings` -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|x| { LL + if x <= 5 { LL + Some(x) - | -help: and remove the `.flatten()` - | +LL + } else { LL + None LL + } LL ~ }); @@ -38,14 +36,12 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|x| { LL + if x == 1 { LL + Ok(x) - | -help: and remove the `.flatten()` - | +LL + } else { LL + Err(0) LL + } LL ~ }); @@ -64,14 +60,13 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|res| { LL + if res > 0 { LL + do_something(); - | -help: and remove the `.flatten()` - | +LL + Ok(res) +LL + } else { LL + Err(0) LL + } LL ~ }); @@ -90,14 +85,12 @@ LL | | }) LL | | .flatten() | |__________________^ | -help: try replacing `map` with `filter_map` +help: try replacing `map` with `filter_map` and remove the `.flatten()` | LL ~ .filter_map(|some_value| { LL + if some_value > 3 { LL + Some(some_value) - | -help: and remove the `.flatten()` - | +LL + } else { LL + None LL + } LL + }) diff --git a/src/tools/clippy/tests/ui/map_flatten_fixable.fixed b/src/tools/clippy/tests/ui/map_flatten_fixable.fixed index 928e5bd509c..e9b41354c58 100644 --- a/src/tools/clippy/tests/ui/map_flatten_fixable.fixed +++ b/src/tools/clippy/tests/ui/map_flatten_fixable.fixed @@ -59,8 +59,6 @@ fn issue8878() { .and_then(|_| { // we need some newlines // so that the span is big enough -// we need some newlines -// so that the span is big enough // for a splitted output of the diagnostic Some("") // whitespace beforehand is important as well diff --git a/src/tools/clippy/tests/ui/map_flatten_fixable.stderr b/src/tools/clippy/tests/ui/map_flatten_fixable.stderr index 828e24acaad..f3b82ad08d0 100644 --- a/src/tools/clippy/tests/ui/map_flatten_fixable.stderr +++ b/src/tools/clippy/tests/ui/map_flatten_fixable.stderr @@ -2,79 +2,45 @@ error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:18:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)` | = note: `-D clippy::map-flatten` implied by `-D warnings` -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect(); - | ~~~~~~~~~~~~~~~~~~~~~ error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:19:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_ref).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_ref)` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:20:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:21:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:24:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `flat_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect(); - | ~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)` error: called `map(..).flatten()` on `Option` --> $DIR/map_flatten_fixable.rs:27:40 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); - | ^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `and_then`, and remove the `.flatten()` - | -LL | let _: Option<_> = (Some(Some(1))).and_then(|x| x); - | ~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Result` --> $DIR/map_flatten_fixable.rs:30:42 | LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); - | ^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `and_then`, and remove the `.flatten()` - | -LL | let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x); - | ~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Option` --> $DIR/map_flatten_fixable.rs:59:10 @@ -89,14 +55,12 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|_| { LL + // we need some newlines LL + // so that the span is big enough - | -help: and remove the `.flatten()` - | +LL + // for a splitted output of the diagnostic LL + Some("") LL + // whitespace beforehand is important as well LL ~ }); From e2512477a9faf2a86ddeba9c8538c84d9c948384 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 20 Jun 2022 00:25:07 +0400 Subject: [PATCH 2/3] remove last use of MAX_SUGGESTION_HIGHLIGHT_LINES --- .../clippy_lints/src/methods/or_fun_call.rs | 13 ++---- src/tools/clippy/tests/ui/or_fun_call.fixed | 6 +-- src/tools/clippy/tests/ui/or_fun_call.stderr | 46 ++++++++++++++++--- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs index 448dc4e6147..3d1208824fa 100644 --- a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs +++ b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs @@ -4,7 +4,6 @@ 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; @@ -33,7 +32,6 @@ fn check_unwrap_or_default( arg: &hir::Expr<'_>, or_has_args: bool, span: Span, - method_span: Span, ) -> bool { let is_default_default = || is_trait_item(cx, fun, sym::Default); @@ -56,19 +54,14 @@ fn check_unwrap_or_default( then { let mut applicability = Applicability::MachineApplicable; let hint = "unwrap_or_default()"; - let mut sugg_span = span; + let sugg_span = span; - let mut sugg: String = format!( + let 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, @@ -178,7 +171,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, method_span) { + if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.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/src/tools/clippy/tests/ui/or_fun_call.fixed b/src/tools/clippy/tests/ui/or_fun_call.fixed index 3208048e0d5..123aed40251 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.fixed +++ b/src/tools/clippy/tests/ui/or_fun_call.fixed @@ -185,8 +185,7 @@ 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() { @@ -198,8 +197,7 @@ mod issue8239 { let _ = ""; acc.push_str(&f); acc - }) - .unwrap_or_default(); + }).unwrap_or_default(); } fn equal_to_max_suggestion_highest_lines() { diff --git a/src/tools/clippy/tests/ui/or_fun_call.stderr b/src/tools/clippy/tests/ui/or_fun_call.stderr index 549b00ae3c4..dfe15654bc3 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.stderr +++ b/src/tools/clippy/tests/ui/or_fun_call.stderr @@ -109,16 +109,50 @@ 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:189:14 + --> $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(); | -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 + --> $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(); | -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 From ffb593bf4d0dfea864f8e47dfd30098e70b8e5f8 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 20 Jun 2022 00:25:51 +0400 Subject: [PATCH 3/3] remove MAX_SUGGESTION_HIGHLIGHT_LINES --- compiler/rustc_errors/src/emitter.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index a4cbc73978d..8b2a995f1c5 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -656,11 +656,6 @@ fn emit_diagnostic(&mut self, d: &Diagnostic) { } } -/// Maximum number of lines we will print for a multiline suggestion; arbitrary. -/// -/// This should be replaced with a more involved mechanism to output multiline suggestions that -/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided. -pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6; /// Maximum number of suggestions to be shown /// /// Arbitrary, but taken from trait import suggestion limit