From 1ea7bddbdfc7fea5686f26047cc42f120b12ba68 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Sun, 4 Aug 2024 13:14:23 +0000 Subject: [PATCH] Remove `multispan_sugg[_with_applicability]` --- clippy_lints/src/loops/for_kv_map.rs | 7 ++-- .../src/loops/manual_while_let_some.rs | 7 ++-- clippy_lints/src/loops/needless_range_loop.rs | 11 ++--- .../src/loops/unused_enumerate_index.rs | 7 ++-- clippy_lints/src/manual_strip.rs | 22 +++++----- clippy_lints/src/matches/match_ref_pats.rs | 8 +++- .../src/methods/bind_instead_of_map.rs | 19 ++++----- .../src/methods/unused_enumerate_index.rs | 7 ++-- clippy_lints/src/needless_pass_by_value.rs | 11 +++-- clippy_lints/src/operators/op_ref.rs | 6 +-- clippy_lints/src/semicolon_block.rs | 12 +++--- .../internal_lints/metadata_collector.rs | 40 ++++++------------- clippy_utils/src/diagnostics.rs | 29 -------------- tests/ui/bind_instead_of_map_multipart.stderr | 10 ++--- 14 files changed, 80 insertions(+), 116 deletions(-) diff --git a/clippy_lints/src/loops/for_kv_map.rs b/clippy_lints/src/loops/for_kv_map.rs index 6922533fb..185d834be 100644 --- a/clippy_lints/src/loops/for_kv_map.rs +++ b/clippy_lints/src/loops/for_kv_map.rs @@ -1,8 +1,9 @@ use super::FOR_KV_MAP; -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{pat_is_wild, sugg}; +use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind}; use rustc_lint::LateContext; use rustc_middle::ty; @@ -40,13 +41,13 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx format!("you seem to want to iterate on a map's {kind}s"), |diag| { let map = sugg::Sugg::hir(cx, arg, "map"); - multispan_sugg( - diag, + diag.multipart_suggestion( "use the corresponding method", vec![ (pat_span, snippet(cx, new_pat_span, kind).into_owned()), (arg_span, format!("{}.{kind}s{mutbl}()", map.maybe_par())), ], + Applicability::MachineApplicable, ); }, ); diff --git a/clippy_lints/src/loops/manual_while_let_some.rs b/clippy_lints/src/loops/manual_while_let_some.rs index b00a082bb..57434f355 100644 --- a/clippy_lints/src/loops/manual_while_let_some.rs +++ b/clippy_lints/src/loops/manual_while_let_some.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::{match_def_path, paths, SpanlessEq}; use rustc_errors::Applicability; @@ -38,11 +38,10 @@ fn report_lint(cx: &LateContext<'_>, pop_span: Span, pop_stmt_kind: PopStmt<'_>, }; let loop_replacement = format!("while let Some({}) = {}.pop()", pat, snippet(cx, receiver_span, "..")); - multispan_sugg_with_applicability( - diag, + diag.multipart_suggestion( "consider using a `while..let` loop", + vec![(loop_span, loop_replacement), (pop_span, pop_replacement)], Applicability::MachineApplicable, - [(loop_span, loop_replacement), (pop_span, pop_replacement)], ); }, ); diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs index de7ec81bc..e18e43746 100644 --- a/clippy_lints/src/loops/needless_range_loop.rs +++ b/clippy_lints/src/loops/needless_range_loop.rs @@ -1,11 +1,12 @@ use super::NEEDLESS_RANGE_LOOP; -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::has_iter_method; use clippy_utils::visitors::is_local_used; use clippy_utils::{contains_name, higher, is_integer_const, sugg, SpanlessEq}; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{BinOpKind, BorrowKind, Closure, Expr, ExprKind, HirId, Mutability, Pat, PatKind, QPath}; @@ -145,8 +146,7 @@ pub(super) fn check<'tcx>( arg.span, format!("the loop variable `{}` is used to index `{indexed}`", ident.name), |diag| { - multispan_sugg( - diag, + diag.multipart_suggestion( "consider using an iterator and enumerate()", vec![ (pat.span, format!("({}, )", ident.name)), @@ -155,6 +155,7 @@ pub(super) fn check<'tcx>( format!("{indexed}.{method}().enumerate(){method_1}{method_2}"), ), ], + Applicability::HasPlaceholders, ); }, ); @@ -171,10 +172,10 @@ pub(super) fn check<'tcx>( arg.span, format!("the loop variable `{}` is only used to index `{indexed}`", ident.name), |diag| { - multispan_sugg( - diag, + diag.multipart_suggestion( "consider using an iterator", vec![(pat.span, "".to_string()), (arg.span, repl)], + Applicability::HasPlaceholders, ); }, ); diff --git a/clippy_lints/src/loops/unused_enumerate_index.rs b/clippy_lints/src/loops/unused_enumerate_index.rs index 40ccfec02..51e21aa97 100644 --- a/clippy_lints/src/loops/unused_enumerate_index.rs +++ b/clippy_lints/src/loops/unused_enumerate_index.rs @@ -1,7 +1,8 @@ use super::UNUSED_ENUMERATE_INDEX; -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::{pat_is_wild, sugg}; +use rustc_errors::Applicability; use rustc_hir::def::DefKind; use rustc_hir::{Expr, ExprKind, Pat, PatKind}; use rustc_lint::LateContext; @@ -28,13 +29,13 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_ "you seem to use `.enumerate()` and immediately discard the index", |diag| { let base_iter = sugg::Sugg::hir(cx, self_arg, "base iter"); - multispan_sugg( - diag, + diag.multipart_suggestion( "remove the `.enumerate()` call", vec![ (pat.span, snippet(cx, elem.span, "..").into_owned()), (arg.span, base_iter.to_string()), ], + Applicability::MachineApplicable, ); }, ); diff --git a/clippy_lints/src/manual_strip.rs b/clippy_lints/src/manual_strip.rs index 686ecccf8..324ad2575 100644 --- a/clippy_lints/src/manual_strip.rs +++ b/clippy_lints/src/manual_strip.rs @@ -1,11 +1,12 @@ use clippy_config::msrvs::{self, Msrv}; use clippy_config::Conf; use clippy_utils::consts::{constant, Constant}; -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::usage::mutated_variables; use clippy_utils::{eq_expr_value, higher, match_def_path, paths}; use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind}; @@ -14,6 +15,7 @@ use rustc_middle::ty; use rustc_session::impl_lint_pass; use rustc_span::source_map::Spanned; use rustc_span::Span; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -108,19 +110,19 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip { format!("stripping a {kind_word} manually"), |diag| { diag.span_note(test_span, format!("the {kind_word} was tested here")); - multispan_sugg( - diag, + diag.multipart_suggestion( format!("try using the `strip_{kind_word}` method"), - vec![( + iter::once(( test_span, format!( "if let Some() = {}.strip_{kind_word}({}) ", snippet(cx, target_arg.span, ".."), snippet(cx, pattern.span, "..") ), - )] - .into_iter() - .chain(strippings.into_iter().map(|span| (span, "".into()))), + )) + .chain(strippings.into_iter().map(|span| (span, "".into()))) + .collect(), + Applicability::HasPlaceholders, ); }, ); @@ -183,9 +185,9 @@ fn peel_ref<'a>(expr: &'a Expr<'_>) -> &'a Expr<'a> { } } -// Find expressions where `target` is stripped using the length of `pattern`. -// We'll suggest replacing these expressions with the result of the `strip_{prefix,suffix}` -// method. +/// Find expressions where `target` is stripped using the length of `pattern`. +/// We'll suggest replacing these expressions with the result of the `strip_{prefix,suffix}` +/// method. fn find_stripping<'tcx>( cx: &LateContext<'tcx>, strip_kind: StripKind, diff --git a/clippy_lints/src/matches/match_ref_pats.rs b/clippy_lints/src/matches/match_ref_pats.rs index aba4c85c5..5445ee1f0 100644 --- a/clippy_lints/src/matches/match_ref_pats.rs +++ b/clippy_lints/src/matches/match_ref_pats.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{snippet, walk_span_to_context}; use clippy_utils::sugg::Sugg; use core::iter::once; @@ -54,7 +54,11 @@ where span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| { if !expr.span.from_expansion() { - multispan_sugg(diag, msg, first_sugg.chain(remaining_suggs)); + diag.multipart_suggestion( + msg, + first_sugg.chain(remaining_suggs).collect(), + Applicability::MachineApplicable, + ); } }); } diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index 20f3722e1..2a8a5fcc3 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -1,5 +1,5 @@ use super::{contains_return, BIND_INSTEAD_OF_MAP}; -use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::peel_blocks; use clippy_utils::source::{snippet, snippet_with_context}; use clippy_utils::visitors::find_all_ret_expressions; @@ -136,15 +136,16 @@ impl BindInsteadOfMap { return false; }; span_lint_and_then(cx, BIND_INSTEAD_OF_MAP, expr.span, msg, |diag| { - multispan_sugg_with_applicability( - diag, - "try", + diag.multipart_suggestion( + format!("use `{}` instead", self.good_method_name), + std::iter::once((span, self.good_method_name.into())) + .chain( + suggs + .into_iter() + .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())), + ) + .collect(), Applicability::MachineApplicable, - std::iter::once((span, self.good_method_name.into())).chain( - suggs - .into_iter() - .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())), - ), ); }); true diff --git a/clippy_lints/src/methods/unused_enumerate_index.rs b/clippy_lints/src/methods/unused_enumerate_index.rs index 8b8a965b9..3004d9c42 100644 --- a/clippy_lints/src/methods/unused_enumerate_index.rs +++ b/clippy_lints/src/methods/unused_enumerate_index.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_hir_and_then}; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::{snippet, snippet_opt}; use clippy_utils::{expr_or_init, is_trait_method, pat_is_wild}; use rustc_errors::Applicability; @@ -97,10 +97,8 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, enumerate_span, "you seem to use `.enumerate()` and immediately discard the index", |diag| { - multispan_sugg_with_applicability( - diag, + diag.multipart_suggestion( "remove the `.enumerate()` call", - Applicability::MachineApplicable, vec![ (closure_param.span, new_closure_param), ( @@ -108,6 +106,7 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, String::new(), ), ], + Applicability::MachineApplicable, ); }, ); diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index f2e00cef7..a0bbf6b14 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_self; use clippy_utils::ptr::get_spans; use clippy_utils::source::{snippet, snippet_opt}; @@ -278,9 +278,12 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { } } - let spans = vec![(input.span, format!("&{}", snippet(cx, input.span, "_")))]; - - multispan_sugg(diag, "consider taking a reference instead", spans); + diag.span_suggestion( + input.span, + "consider taking a reference instead", + format!("&{}", snippet(cx, input.span, "_")), + Applicability::MaybeIncorrect, + ); }; span_lint_and_then( diff --git a/clippy_lints/src/operators/op_ref.rs b/clippy_lints/src/operators/op_ref.rs index c2b27c9b2..82b9d10fb 100644 --- a/clippy_lints/src/operators/op_ref.rs +++ b/clippy_lints/src/operators/op_ref.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::get_enclosing_block; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; @@ -64,10 +64,10 @@ pub(crate) fn check<'tcx>( |diag| { let lsnip = snippet(cx, l.span, "...").to_string(); let rsnip = snippet(cx, r.span, "...").to_string(); - multispan_sugg( - diag, + diag.multipart_suggestion( "use the values directly", vec![(left.span, lsnip), (right.span, rsnip)], + Applicability::MachineApplicable, ); }, ); diff --git a/clippy_lints/src/semicolon_block.rs b/clippy_lints/src/semicolon_block.rs index 7615c2127..09f1c1123 100644 --- a/clippy_lints/src/semicolon_block.rs +++ b/clippy_lints/src/semicolon_block.rs @@ -1,5 +1,5 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use rustc_errors::Applicability; use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -92,11 +92,10 @@ impl SemicolonBlock { semi_span, "consider moving the `;` inside the block for consistent formatting", |diag| { - multispan_sugg_with_applicability( - diag, + diag.multipart_suggestion( "put the `;` here", + vec![(remove_span, String::new()), (insert_span, ";".to_owned())], Applicability::MachineApplicable, - [(remove_span, String::new()), (insert_span, ";".to_owned())], ); }, ); @@ -124,11 +123,10 @@ impl SemicolonBlock { block.span, "consider moving the `;` outside the block for consistent formatting", |diag| { - multispan_sugg_with_applicability( - diag, + diag.multipart_suggestion( "put the `;` here", + vec![(remove_span, String::new()), (insert_span, ";".to_owned())], Applicability::MachineApplicable, - [(remove_span, String::new()), (insert_span, ";".to_owned())], ); }, ); diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 2fc66c560..7f9173df0 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -85,10 +85,6 @@ const SUGGESTION_DIAG_METHODS: [(&str, bool); 9] = [ ("tool_only_multipart_suggestion", true), ("span_suggestions", true), ]; -const SUGGESTION_FUNCTIONS: [&[&str]; 2] = [ - &["clippy_utils", "diagnostics", "multispan_sugg"], - &["clippy_utils", "diagnostics", "multispan_sugg_with_applicability"], -]; const DEPRECATED_LINT_TYPE: [&str; 3] = ["clippy_lints", "deprecated_lints", "ClippyDeprecatedLint"]; /// The index of the applicability name of `paths::APPLICABILITY_VALUES` @@ -1060,33 +1056,21 @@ impl<'a, 'hir> Visitor<'hir> for IsMultiSpanScanner<'a, 'hir> { return; } - match &expr.kind { - ExprKind::Call(fn_expr, _args) => { - let found_function = SUGGESTION_FUNCTIONS - .iter() - .any(|func_path| match_function_call(self.cx, fn_expr, func_path).is_some()); - if found_function { - // These functions are all multi part suggestions - self.add_single_span_suggestion(); - } - }, - ExprKind::MethodCall(path, recv, _, _arg_span) => { - let (self_ty, _) = walk_ptrs_ty_depth(self.cx.typeck_results().expr_ty(recv)); - if match_type(self.cx, self_ty, &paths::DIAG) { - let called_method = path.ident.name.as_str().to_string(); - for (method_name, is_multi_part) in &SUGGESTION_DIAG_METHODS { - if *method_name == called_method { - if *is_multi_part { - self.add_multi_part_suggestion(); - } else { - self.add_single_span_suggestion(); - } - break; + if let ExprKind::MethodCall(path, recv, _, _arg_span) = &expr.kind { + let (self_ty, _) = walk_ptrs_ty_depth(self.cx.typeck_results().expr_ty(recv)); + if match_type(self.cx, self_ty, &paths::DIAG) { + let called_method = path.ident.name.as_str().to_string(); + for (method_name, is_multi_part) in &SUGGESTION_DIAG_METHODS { + if *method_name == called_method { + if *is_multi_part { + self.add_multi_part_suggestion(); + } else { + self.add_single_span_suggestion(); } + break; } } - }, - _ => {}, + } } intravisit::walk_expr(self, expr); diff --git a/clippy_utils/src/diagnostics.rs b/clippy_utils/src/diagnostics.rs index 0641d37cd..4877fb65d 100644 --- a/clippy_utils/src/diagnostics.rs +++ b/clippy_utils/src/diagnostics.rs @@ -330,32 +330,3 @@ pub fn span_lint_and_sugg( diag.span_suggestion(sp, help.into(), sugg, applicability); }); } - -/// Create a suggestion made from several `span → replacement`. -/// -/// Note: in the JSON format (used by `compiletest_rs`), the help message will -/// appear once per -/// replacement. In human-readable format though, it only appears once before -/// the whole suggestion. -pub fn multispan_sugg(diag: &mut Diag<'_, ()>, help_msg: impl Into, sugg: I) -where - I: IntoIterator, -{ - multispan_sugg_with_applicability(diag, help_msg, Applicability::Unspecified, sugg); -} - -/// Create a suggestion made from several `span → replacement`. -/// -/// rustfix currently doesn't support the automatic application of suggestions with -/// multiple spans. This is tracked in issue [rustfix#141](https://github.com/rust-lang/rustfix/issues/141). -/// Suggestions with multiple spans will be silently ignored. -pub fn multispan_sugg_with_applicability( - diag: &mut Diag<'_, ()>, - help_msg: impl Into, - applicability: Applicability, - sugg: I, -) where - I: IntoIterator, -{ - diag.multipart_suggestion(help_msg.into(), sugg.into_iter().collect(), applicability); -} diff --git a/tests/ui/bind_instead_of_map_multipart.stderr b/tests/ui/bind_instead_of_map_multipart.stderr index b15857c32..2adaecc96 100644 --- a/tests/ui/bind_instead_of_map_multipart.stderr +++ b/tests/ui/bind_instead_of_map_multipart.stderr @@ -9,7 +9,7 @@ note: the lint level is defined here | LL | #![deny(clippy::bind_instead_of_map)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: try +help: use `map` instead | LL | let _ = Some("42").map(|s| if s.len() < 42 { 0 } else { s.len() }); | ~~~ ~ ~~~~~~~ @@ -20,7 +20,7 @@ error: using `Result.and_then(|x| Ok(y))`, which is more succinctly expressed as LL | let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Ok(0) } else { Ok(s.len()) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: try +help: use `map` instead | LL | let _ = Ok::<_, ()>("42").map(|s| if s.len() < 42 { 0 } else { s.len() }); | ~~~ ~ ~~~~~~~ @@ -31,7 +31,7 @@ error: using `Result.or_else(|x| Err(y))`, which is more succinctly expressed as LL | let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Err(s.len() + 20) } else { Err(s.len()) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: try +help: use `map_err` instead | LL | let _ = Err::<(), _>("42").map_err(|s| if s.len() < 42 { s.len() + 20 } else { s.len() }); | ~~~~~~~ ~~~~~~~~~~~~ ~~~~~~~ @@ -48,7 +48,7 @@ LL | | } LL | | }); | |______^ | -help: try +help: use `map` instead | LL ~ Some("42").map(|s| { LL | if { @@ -82,7 +82,7 @@ error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed LL | let _ = Some("").and_then(|s| if s.len() == 20 { Some(m!()) } else { Some(Some(20)) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: try +help: use `map` instead | LL | let _ = Some("").map(|s| if s.len() == 20 { m!() } else { Some(20) }); | ~~~ ~~~~ ~~~~~~~~