user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #146069 - camsteffen:range-desugar-span, r=SparrowLii

Browse source 
Mark desugared range expression spans with DesugaringKind::RangeExpr

This is a prerequisite to removing `QPath::LangItem` (rust-lang/rust#115178) because otherwise there would be no way to detect a range expression in the HIR.

There are some non-obvious Clippy changes so a Clippy team review would be good.
This commit is contained in:
bors 2025-10-27 02:50:35 +00:00
commit 05aaed9523
18 changed files with 80 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

1
clippy_lints/src/loops/manual_memcpy.rs
View file

@ -28,6 +28,7 @@ pub(super) fn check<'tcx>(
start: Some(start),
end: Some(end),
limits,
span: _,
}) = higher::Range::hir(arg)
// the var must be a single name
&& let PatKind::Binding(_, canonical_id, _, _) = pat.kind

1
clippy_lints/src/loops/manual_slice_fill.rs
View file

@ -31,6 +31,7 @@ pub(super) fn check<'tcx>(
start: Some(start),
end: Some(end),
limits: RangeLimits::HalfOpen,
span: _,
}) = higher::Range::hir(arg)
&& let ExprKind::Lit(Spanned {
node: LitKind::Int(Pu128(0), _),

9
clippy_lints/src/loops/needless_range_loop.rs
View file

@ -30,6 +30,7 @@ pub(super) fn check<'tcx>(
start: Some(start),
ref end,
limits,
span,
}) = higher::Range::hir(arg)
// the var must be a single name
&& let PatKind::Binding(_, canonical_id, ident, _) = pat.kind
@ -149,7 +150,7 @@ pub(super) fn check<'tcx>(
span_lint_and_then(
cx,
NEEDLESS_RANGE_LOOP,
arg.span,
span,
format!("the loop variable `{}` is used to index `{indexed}`", ident.name),
|diag| {
diag.multipart_suggestion(
@ -157,7 +158,7 @@ pub(super) fn check<'tcx>(
vec![
(pat.span, format!("({}, <item>)", ident.name)),
(
arg.span,
span,
format!("{indexed}.{method}().enumerate(){method_1}{method_2}"),
),
],
@ -175,12 +176,12 @@ pub(super) fn check<'tcx>(
span_lint_and_then(
cx,
NEEDLESS_RANGE_LOOP,
arg.span,
span,
format!("the loop variable `{}` is only used to index `{indexed}`", ident.name),
|diag| {
diag.multipart_suggestion(
"consider using an iterator",
vec![(pat.span, "<item>".to_string()), (arg.span, repl)],
vec![(pat.span, "<item>".to_string()), (span, repl)],
Applicability::HasPlaceholders,
);
},

1
clippy_lints/src/manual_is_ascii_check.rs
View file

@ -110,6 +110,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
start: Some(start),
end: Some(end),
limits: RangeLimits::Closed,
span: _,
}) = higher::Range::hir(receiver)
&& !matches!(cx.typeck_results().expr_ty(arg).peel_refs().kind(), ty::Param(_))
{

1
clippy_lints/src/methods/iter_next_slice.rs
View file

@ -30,6 +30,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, cal
start: Some(start_expr),
end: None,
limits: ast::RangeLimits::HalfOpen,
span: _,
}) = higher::Range::hir(index_expr)
&& let hir::ExprKind::Lit(start_lit) = &start_expr.kind
&& let ast::LitKind::Int(start_idx, _) = start_lit.node

4
clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs
View file

@ -63,7 +63,7 @@ pub(super) fn check(
receiver: &Expr<'_>,
arg: &Expr<'_>,
msrv: Msrv,
method_call_span: Span,
method_name_span: Span,
) {
let mut applicability = Applicability::MaybeIncorrect;
if let Some(range) = higher::Range::hir(receiver)
@ -105,7 +105,7 @@ pub(super) fn check(
// collate all our parts here and then remove those that are empty.
let mut parts = vec![
(
receiver.span.to(method_call_span),
ex.span.with_hi(method_name_span.hi()),
format!("{exec_context}::iter::{method_to_use_name}"),
),
new_span,

4
clippy_lints/src/methods/mod.rs
View file

@ -4854,8 +4854,8 @@ impl_lint_pass!(Methods => [
/// come from expansion.
pub fn method_call<'tcx>(recv: &'tcx Expr<'tcx>) -> Option<(Symbol, &'tcx Expr<'tcx>, &'tcx [Expr<'tcx>], Span, Span)> {
if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind
&& !args.iter().any(|e| e.span.from_expansion())
&& !receiver.span.from_expansion()
&& !args.iter().any(|e| e.range_span().unwrap_or(e.span).from_expansion())
&& !receiver.range_span().unwrap_or(receiver.span).from_expansion()
{
Some((path.ident.name, receiver, args, path.ident.span, call_span))
} else {

6
clippy_lints/src/no_effect.rs
View file

@ -122,7 +122,7 @@ impl NoEffect {
return true;
}
if expr.span.from_expansion() {
if expr.range_span().unwrap_or(expr.span).from_expansion() {
return false;
}
let expr = peel_blocks(expr);
@ -273,7 +273,7 @@ fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
if let StmtKind::Semi(expr) = stmt.kind
&& !stmt.span.in_external_macro(cx.sess().source_map())
&& let ctxt = stmt.span.ctxt()
&& expr.span.ctxt() == ctxt
&& expr.range_span().unwrap_or(expr.span).ctxt() == ctxt
&& let Some(reduced) = reduce_expression(cx, expr)
&& reduced.iter().all(|e| e.span.ctxt() == ctxt)
{
@ -330,7 +330,7 @@ fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
}
fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec<&'a Expr<'a>>> {
if expr.span.from_expansion() {
if expr.range_span().unwrap_or(expr.span).from_expansion() {
return None;
}
match expr.kind {

16
clippy_lints/src/ranges.rs
View file

@ -501,17 +501,18 @@ fn check_range_switch<'tcx>(
msg: &'static str,
operator: &str,
) {
if expr.span.can_be_used_for_suggestions()
&& let Some(higher::Range {
if let Some(range) = higher::Range::hir(expr)
&& let higher::Range {
start,
end: Some(end),
limits,
}) = higher::Range::hir(expr)
span,
} = range
&& span.can_be_used_for_suggestions()
&& limits == kind
&& let Some(y) = predicate(cx, end)
&& can_switch_ranges(cx, expr, kind, cx.typeck_results().expr_ty(y))
{
let span = expr.span;
span_lint_and_then(cx, lint, span, msg, |diag| {
let mut app = Applicability::MachineApplicable;
let start = start.map_or(String::new(), |x| {
@ -567,6 +568,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
start: Some(start),
end: Some(end),
limits,
span,
}) = higher::Range::hir(expr)
&& let ty = cx.typeck_results().expr_ty(start)
&& let ty::Int(_) | ty::Uint(_) = ty.kind()
@ -582,7 +584,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
span_lint(
cx,
REVERSED_EMPTY_RANGES,
expr.span,
span,
"this range is reversed and using it to index a slice will panic at run-time",
);
}
@ -591,7 +593,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
span_lint_and_then(
cx,
REVERSED_EMPTY_RANGES,
expr.span,
span,
"this range is empty so it will yield no values",
|diag| {
if ordering != Ordering::Equal {
@ -603,7 +605,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
};
diag.span_suggestion(
expr.span,
span,
"consider using the following if you are attempting to iterate over this \
range in reverse",
format!("({end_snippet}{dots}{start_snippet}).rev()"),

19
clippy_lints/src/unnecessary_struct_initialization.rs
View file

@ -6,6 +6,7 @@ use clippy_utils::{get_parent_expr, is_mutable};
use rustc_hir::{Expr, ExprField, ExprKind, Path, QPath, StructTailExpr, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::{DesugaringKind, Ident};
declare_clippy_lint! {
/// ### What it does
@ -52,7 +53,8 @@ impl LateLintPass<'_> for UnnecessaryStruct {
return;
};
if expr.span.from_expansion() {
let expr_span = expr.range_span().unwrap_or(expr.span);
if expr_span.from_expansion() {
// Prevent lint from hitting inside macro code
return;
}
@ -80,7 +82,7 @@ impl LateLintPass<'_> for UnnecessaryStruct {
span_lint_and_sugg(
cx,
UNNECESSARY_STRUCT_INITIALIZATION,
expr.span,
expr_span,
"unnecessary struct building",
"replace with",
snippet(cx, sugg, "..").into_owned(),
@ -130,7 +132,7 @@ fn same_path_in_all_fields<'tcx>(
// expression type matches
&& ty == cx.typeck_results().expr_ty(src_expr)
// field name matches
&& f.ident == ident
&& ident_without_range_desugaring(f.ident) == ident
// assigned from a path expression
&& let ExprKind::Path(QPath::Resolved(None, src_path)) = src_expr.kind
{
@ -197,3 +199,14 @@ fn path_matches_base(path: &Path<'_>, base: &Expr<'_>) -> bool {
};
path.res == base_path.res
}
fn ident_without_range_desugaring(ident: Ident) -> Ident {
if ident.span.desugaring_kind() == Some(DesugaringKind::RangeExpr) {
Ident {
span: ident.span.parent_callsite().unwrap(),
..ident
}
} else {
ident
}
}

8
clippy_utils/src/higher.rs
View file

@ -209,12 +209,14 @@ pub struct Range<'a> {
pub end: Option<&'a Expr<'a>>,
/// Whether the interval is open or closed.
pub limits: ast::RangeLimits,
pub span: Span
}
impl<'a> Range<'a> {
/// Higher a `hir` range to something similar to `ast::ExprKind::Range`.
#[expect(clippy::similar_names)]
pub fn hir(expr: &'a Expr<'_>) -> Option<Range<'a>> {
let span = expr.range_span()?;
match expr.kind {
ExprKind::Call(path, [arg1, arg2])
if matches!(
@ -226,6 +228,7 @@ impl<'a> Range<'a> {
start: Some(arg1),
end: Some(arg2),
limits: ast::RangeLimits::Closed,
span,
})
},
ExprKind::Struct(path, fields, StructTailExpr::None) => match (path, fields) {
@ -233,12 +236,14 @@ impl<'a> Range<'a> {
start: None,
end: None,
limits: ast::RangeLimits::HalfOpen,
span,
}),
(QPath::LangItem(hir::LangItem::RangeFrom, ..), [field]) if field.ident.name == sym::start => {
Some(Range {
start: Some(field.expr),
end: None,
limits: ast::RangeLimits::HalfOpen,
span,
})
},
(QPath::LangItem(hir::LangItem::Range, ..), [field1, field2]) => {
@ -251,6 +256,7 @@ impl<'a> Range<'a> {
start: Some(start),
end: Some(end),
limits: ast::RangeLimits::HalfOpen,
span,
})
},
(QPath::LangItem(hir::LangItem::RangeToInclusive, ..), [field]) if field.ident.name == sym::end => {
@ -258,12 +264,14 @@ impl<'a> Range<'a> {
start: None,
end: Some(field.expr),
limits: ast::RangeLimits::Closed,
span,
})
},
(QPath::LangItem(hir::LangItem::RangeTo, ..), [field]) if field.ident.name == sym::end => Some(Range {
start: None,
end: Some(field.expr),
limits: ast::RangeLimits::HalfOpen,
span,
}),
_ => None,
},

2
clippy_utils/src/lib.rs
View file

@ -1282,7 +1282,7 @@ pub fn is_else_clause_in_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
/// If the given `Expr` is not some kind of range, the function returns `false`.
pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Option<&Path<'_>>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
if let Some(Range { start, end, limits }) = Range::hir(expr) {
if let Some(Range { start, end, limits, .. }) = Range::hir(expr) {
let start_is_none_or_min = start.is_none_or(|start| {
if let rustc_ty::Adt(_, subst) = ty.kind()
&& let bnd_ty = subst.type_at(0)

12
clippy_utils/src/source.rs
View file

@ -13,8 +13,8 @@ use rustc_middle::ty::TyCtxt;
use rustc_session::Session;
use rustc_span::source_map::{SourceMap, original_sp};
use rustc_span::{
BytePos, DUMMY_SP, FileNameDisplayPreference, Pos, RelativeBytePos, SourceFile, SourceFileAndLine, Span, SpanData,
SyntaxContext, hygiene,
BytePos, DesugaringKind, DUMMY_SP, FileNameDisplayPreference, Pos, RelativeBytePos, SourceFile, SourceFileAndLine,
Span, SpanData, SyntaxContext, hygiene,
};
use std::borrow::Cow;
use std::fmt;
@ -670,6 +670,14 @@ fn snippet_with_context_sess<'a>(
default: &'a str,
applicability: &mut Applicability,
) -> (Cow<'a, str>, bool) {
// If it is just range desugaring, use the desugaring span since it may include parenthesis.
if span.desugaring_kind() == Some(DesugaringKind::RangeExpr) && span.parent_callsite().unwrap().ctxt() == outer {
return (
snippet_with_applicability_sess(sess, span, default, applicability),
false,
)
}
let (span, is_macro_call) = walk_span_to_context(span, outer).map_or_else(
|| {
// The span is from a macro argument, and the outer context is the macro using the argument

16
tests/ui/range_plus_minus_one.stderr
View file

@ -32,22 +32,22 @@ LL | for _ in 1..ONE + ONE {}
| ^^^^^^^^^^^^ help: use: `1..=ONE`
error: an inclusive range would be more readable
--> tests/ui/range_plus_minus_one.rs:70:5
--> tests/ui/range_plus_minus_one.rs:70:6
|
LL | (1..10 + 1).for_each(|_| {});
| ^^^^^^^^^^^ help: use: `(1..=10)`
| ^^^^^^^^^ help: use: `1..=10`
error: an inclusive range would be more readable
--> tests/ui/range_plus_minus_one.rs:75:5
--> tests/ui/range_plus_minus_one.rs:75:6
|
LL | (1..10 + 1).into_iter().for_each(|_| {});
| ^^^^^^^^^^^ help: use: `(1..=10)`
| ^^^^^^^^^ help: use: `1..=10`
error: an inclusive range would be more readable
--> tests/ui/range_plus_minus_one.rs:80:17
--> tests/ui/range_plus_minus_one.rs:80:18
|
LL | let _ = (1..10 + 1).start_bound();
| ^^^^^^^^^^^ help: use: `(1..=10)`
| ^^^^^^^^^ help: use: `1..=10`
error: an inclusive range would be more readable
--> tests/ui/range_plus_minus_one.rs:86:16
@ -80,10 +80,10 @@ LL | a[0..2 + 1][0] = 1;
| ^^^^^^^^ help: use: `0..=2`
error: an exclusive range would be more readable
--> tests/ui/range_plus_minus_one.rs:180:5
--> tests/ui/range_plus_minus_one.rs:180:6
|
LL | (1..=n - 1).sum()
| ^^^^^^^^^^^ help: use: `(1..n)`
| ^^^^^^^^^ help: use: `1..n`
|
= note: `-D clippy::range-minus-one` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::range_minus_one)]`

4
tests/ui/reversed_empty_ranges_fixable.fixed
View file

@ -6,9 +6,9 @@ const ANSWER: i32 = 42;
fn main() {
// These should be linted:
(21..=42).rev().for_each(|x| println!("{}", x));
((21..=42).rev()).for_each(|x| println!("{}", x));
//~^ reversed_empty_ranges
let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
let _ = ((21..ANSWER).rev()).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
//~^ reversed_empty_ranges
for _ in (-42..=-21).rev() {}

12
tests/ui/reversed_empty_ranges_fixable.stderr
View file

@ -1,27 +1,27 @@
error: this range is empty so it will yield no values
--> tests/ui/reversed_empty_ranges_fixable.rs:9:5
--> tests/ui/reversed_empty_ranges_fixable.rs:9:6
|
LL | (42..=21).for_each(|x| println!("{}", x));
| ^^^^^^^^^
| ^^^^^^^
|
= note: `-D clippy::reversed-empty-ranges` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::reversed_empty_ranges)]`
help: consider using the following if you are attempting to iterate over this range in reverse
|
LL - (42..=21).for_each(|x| println!("{}", x));
LL + (21..=42).rev().for_each(|x| println!("{}", x));
LL + ((21..=42).rev()).for_each(|x| println!("{}", x));
|
error: this range is empty so it will yield no values
--> tests/ui/reversed_empty_ranges_fixable.rs:11:13
--> tests/ui/reversed_empty_ranges_fixable.rs:11:14
|
LL | let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
| ^^^^^^^^^^^^
| ^^^^^^^^^^
|
help: consider using the following if you are attempting to iterate over this range in reverse
|
LL - let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
LL + let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
LL + let _ = ((21..ANSWER).rev()).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
|
error: this range is empty so it will yield no values

2
tests/ui/reversed_empty_ranges_loops_fixable.fixed
View file

@ -34,7 +34,7 @@ fn main() {
println!("{}", i);
}
for i in (0..10).rev().map(|x| x * 2) {
for i in ((0..10).rev()).map(|x| x * 2) {
//~^ reversed_empty_ranges
println!("{}", i);
}

6
tests/ui/reversed_empty_ranges_loops_fixable.stderr
View file

@ -37,15 +37,15 @@ LL + for i in (0..MAX_LEN).rev() {
|
error: this range is empty so it will yield no values
--> tests/ui/reversed_empty_ranges_loops_fixable.rs:37:14
--> tests/ui/reversed_empty_ranges_loops_fixable.rs:37:15
|
LL | for i in (10..0).map(|x| x * 2) {
| ^^^^^^^
| ^^^^^
|
help: consider using the following if you are attempting to iterate over this range in reverse
|
LL - for i in (10..0).map(|x| x * 2) {
LL + for i in (0..10).rev().map(|x| x * 2) {
LL + for i in ((0..10).rev()).map(|x| x * 2) {
|
error: this range is empty so it will yield no values