diff --git a/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs b/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs index f60387fe86f7..216ca7dbc27f 100644 --- a/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs +++ b/clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs @@ -1,7 +1,7 @@ use crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::snippet_with_context; use clippy_utils::sugg::Sugg; use clippy_utils::{eager_or_lazy, higher, std_or_core, usage}; use rustc_ast::LitKind; @@ -10,12 +10,13 @@ use rustc_errors::Applicability; use rustc_hir::{Body, Closure, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_span::Span; +use rustc_span::{Span, SyntaxContext}; fn extract_count_with_applicability( cx: &LateContext<'_>, range: higher::Range<'_>, applicability: &mut Applicability, + ctxt: SyntaxContext, ) -> Option { let start = range.start?; let end = range.end?; @@ -40,7 +41,7 @@ fn extract_count_with_applicability( }; return Some(format!("{count}")); } - let end_snippet = Sugg::hir_with_applicability(cx, end, "...", applicability) + let end_snippet = Sugg::hir_with_context(cx, end, ctxt, "...", applicability) .maybe_paren() .into_string(); if lower_bound == 0 { @@ -74,45 +75,23 @@ pub(super) fn check( value: body_expr, } = body_hir && !usage::BindingUsageFinder::are_params_used(cx, body_hir) - && let Some(count) = extract_count_with_applicability(cx, range, &mut applicability) + && let ctxt = ex.span.ctxt() + && let Some(count) = extract_count_with_applicability(cx, range, &mut applicability, ctxt) && let Some(exec_context) = std_or_core(cx) { - let method_to_use_name; - let new_span; - let use_take; - - if eager_or_lazy::switch_to_eager_eval(cx, body_expr) { + let (method_to_use_name, new_span, use_take) = if eager_or_lazy::switch_to_eager_eval(cx, body_expr) { if msrv.meets(cx, msrvs::REPEAT_N) { - method_to_use_name = "repeat_n"; - let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability); - new_span = (arg.span, format!("{body_snippet}, {count}")); - use_take = false; + let (body_snippet, _) = snippet_with_context(cx, body_expr.span, ctxt, "..", &mut applicability); + ("repeat_n", (arg.span, format!("{body_snippet}, {count}")), false) } else { - method_to_use_name = "repeat"; - let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability); - new_span = (arg.span, body_snippet.to_string()); - use_take = true; + let (body_snippet, _) = snippet_with_context(cx, body_expr.span, ctxt, "..", &mut applicability); + ("repeat", (arg.span, body_snippet.to_string()), true) } } else if msrv.meets(cx, msrvs::REPEAT_WITH) { - method_to_use_name = "repeat_with"; - new_span = (param.span, String::new()); - use_take = true; + ("repeat_with", (param.span, String::new()), true) } else { return; - } - - // We need to provide nonempty parts to diag.multipart_suggestion so we - // collate all our parts here and then remove those that are empty. - let mut parts = vec![ - ( - ex.span.with_hi(method_name_span.hi()), - format!("{exec_context}::iter::{method_to_use_name}"), - ), - new_span, - ]; - if use_take { - parts.push((ex.span.shrink_to_hi(), format!(".take({count})"))); - } + }; span_lint_and_then( cx, @@ -120,6 +99,19 @@ pub(super) fn check( ex.span, "map of a closure that does not depend on its parameter over a range", |diag| { + // We need to provide nonempty parts to diag.multipart_suggestion so we + // collate all our parts here and then remove those that are empty. + let mut parts = vec![ + ( + ex.span.with_hi(method_name_span.hi()), + format!("{exec_context}::iter::{method_to_use_name}"), + ), + new_span, + ]; + if use_take { + parts.push((ex.span.shrink_to_hi(), format!(".take({count})"))); + } + diag.multipart_suggestion( if use_take { format!("remove the explicit range and use `{method_to_use_name}` and `take`") diff --git a/tests/ui/map_with_unused_argument_over_ranges.fixed b/tests/ui/map_with_unused_argument_over_ranges.fixed index 254c46ecae38..97cb72748839 100644 --- a/tests/ui/map_with_unused_argument_over_ranges.fixed +++ b/tests/ui/map_with_unused_argument_over_ranges.fixed @@ -89,3 +89,17 @@ fn msrv_1_82() { std::iter::repeat(3).take(10); //~^ map_with_unused_argument_over_ranges } + +fn wrongly_unmangled_macros() { + macro_rules! test { + ($val:expr) => { + ($val * 2 + 1) + }; + } + + std::iter::repeat_with(|| do_something()).take(test!(10)); + //~^ map_with_unused_argument_over_ranges + + std::iter::repeat_n(test!(3), 10); + //~^ map_with_unused_argument_over_ranges +} diff --git a/tests/ui/map_with_unused_argument_over_ranges.rs b/tests/ui/map_with_unused_argument_over_ranges.rs index 05c8d64f8012..f0222f407b8c 100644 --- a/tests/ui/map_with_unused_argument_over_ranges.rs +++ b/tests/ui/map_with_unused_argument_over_ranges.rs @@ -89,3 +89,17 @@ fn msrv_1_82() { (0..10).map(|_| 3); //~^ map_with_unused_argument_over_ranges } + +fn wrongly_unmangled_macros() { + macro_rules! test { + ($val:expr) => { + ($val * 2 + 1) + }; + } + + (0..test!(10)).map(|_| do_something()); + //~^ map_with_unused_argument_over_ranges + + (0..10).map(|_| test!(3)); + //~^ map_with_unused_argument_over_ranges +} diff --git a/tests/ui/map_with_unused_argument_over_ranges.stderr b/tests/ui/map_with_unused_argument_over_ranges.stderr index e5c93ceac02a..ba72a6b9d89f 100644 --- a/tests/ui/map_with_unused_argument_over_ranges.stderr +++ b/tests/ui/map_with_unused_argument_over_ranges.stderr @@ -223,5 +223,29 @@ LL - (0..10).map(|_| 3); LL + std::iter::repeat(3).take(10); | -error: aborting due to 18 previous errors +error: map of a closure that does not depend on its parameter over a range + --> tests/ui/map_with_unused_argument_over_ranges.rs:100:5 + | +LL | (0..test!(10)).map(|_| do_something()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the explicit range and use `repeat_with` and `take` + | +LL - (0..test!(10)).map(|_| do_something()); +LL + std::iter::repeat_with(|| do_something()).take(test!(10)); + | + +error: map of a closure that does not depend on its parameter over a range + --> tests/ui/map_with_unused_argument_over_ranges.rs:103:5 + | +LL | (0..10).map(|_| test!(3)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the explicit range and use `repeat_n` + | +LL - (0..10).map(|_| test!(3)); +LL + std::iter::repeat_n(test!(3), 10); + | + +error: aborting due to 20 previous errors