Propose to exchange ranges only when it is safe to do so

clippy_lints/src/cognitive_complexity.rs

@@ -103,7 +103,6 @@ impl CognitiveComplexity {
                 FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
                 FnKind::Closure => {
                     let header_span = body_span.with_hi(decl.output.span().lo());
-                    #[expect(clippy::range_plus_one)]
                     if let Some(range) = header_span.map_range(cx, |_, src, range| {
                         let mut idxs = src.get(range.clone())?.match_indices('|');
                         Some(range.start + idxs.next()?.0..range.start + idxs.next()?.0 + 1)

clippy_lints/src/doc/mod.rs

@@ -1159,7 +1159,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
     headers
 }
 
-#[expect(clippy::range_plus_one)] // inclusive ranges aren't the same type
 fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> {
     if range.end < range.start {
         return None;

clippy_lints/src/methods/manual_inspect.rs

@@ -100,7 +100,6 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name:
             match x {
                 UseKind::Return(s) => edits.push((s.with_leading_whitespace(cx).with_ctxt(s.ctxt()), String::new())),
                 UseKind::Borrowed(s) => {
-                    #[expect(clippy::range_plus_one)]
                     let range = s.map_range(cx, |_, src, range| {
                         let src = src.get(range.clone())?;
                         let trimmed = src.trim_start_matches([' ', '\t', '\n', '\r', '(']);

clippy_lints/src/ranges.rs

@@ -4,15 +4,20 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_the
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::{SpanRangeExt, snippet, snippet_with_applicability};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{get_parent_expr, higher, is_in_const_context, is_integer_const, path_to_local};
+use clippy_utils::ty::implements_trait;
+use clippy_utils::{
+    expr_use_ctxt, fn_def_id, get_parent_expr, higher, is_in_const_context, is_integer_const, is_path_lang_item,
+    path_to_local,
+};
+use rustc_ast::Mutability;
 use rustc_ast::ast::RangeLimits;
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, HirId};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty;
+use rustc_hir::{BinOpKind, Expr, ExprKind, HirId, LangItem, Node};
+use rustc_lint::{LateContext, LateLintPass, Lint};
+use rustc_middle::ty::{self, ClauseKind, GenericArgKind, PredicatePolarity, Ty};
 use rustc_session::impl_lint_pass;
-use rustc_span::Span;
 use rustc_span::source_map::Spanned;
+use rustc_span::{Span, sym};
 use std::cmp::Ordering;
 
 declare_clippy_lint! {
@@ -24,6 +29,12 @@ declare_clippy_lint! {
     /// The code is more readable with an inclusive range
     /// like `x..=y`.
     ///
+    /// ### Limitations
+    /// The lint is conservative and will trigger only when switching
+    /// from an exclusive to an inclusive range is provably safe from
+    /// a typing point of view. This corresponds to situations where
+    /// the range is used as an iterator, or for indexing.
+    ///
     /// ### Known problems
     /// Will add unnecessary pair of parentheses when the
     /// expression is not wrapped in a pair but starts with an opening parenthesis
@@ -34,11 +45,6 @@ declare_clippy_lint! {
     /// exclusive ranges, because they essentially add an extra branch that
     /// LLVM may fail to hoist out of the loop.
     ///
-    /// This will cause a warning that cannot be fixed if the consumer of the
-    /// range only accepts a specific range type, instead of the generic
-    /// `RangeBounds` trait
-    /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
-    ///
     /// ### Example
     /// ```no_run
     /// # let x = 0;
@@ -71,11 +77,11 @@ declare_clippy_lint! {
     /// The code is more readable with an exclusive range
     /// like `x..y`.
     ///
-    /// ### Known problems
-    /// This will cause a warning that cannot be fixed if
-    /// the consumer of the range only accepts a specific range type, instead of
-    /// the generic `RangeBounds` trait
-    /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
+    /// ### Limitations
+    /// The lint is conservative and will trigger only when switching
+    /// from an inclusive to an exclusive range is provably safe from
+    /// a typing point of view. This corresponds to situations where
+    /// the range is used as an iterator, or for indexing.
     ///
     /// ### Example
     /// ```no_run
@@ -344,70 +350,188 @@ fn check_range_bounds<'a, 'tcx>(cx: &'a LateContext<'tcx>, ex: &'a Expr<'_>) ->
     None
 }
 
-// exclusive range plus one: `x..(y+1)`
-fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
-    if expr.span.can_be_used_for_suggestions()
-        && let Some(higher::Range {
-            start,
-            end: Some(end),
-            limits: RangeLimits::HalfOpen,
-        }) = higher::Range::hir(expr)
-        && let Some(y) = y_plus_one(cx, end)
+/// Check whether `expr` could switch range types without breaking the typing requirements. This is
+/// generally the case when `expr` is used as an iterator for example, or as a slice or `&str`
+/// index.
+///
+/// FIXME: Note that the current implementation may still return false positives. A proper fix would
+/// check that the obligations are still satisfied after switching the range type.
+fn can_switch_ranges<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    original: RangeLimits,
+    inner_ty: Ty<'tcx>,
+) -> bool {
+    let use_ctxt = expr_use_ctxt(cx, expr);
+    let (Node::Expr(parent_expr), false) = (use_ctxt.node, use_ctxt.is_ty_unified) else {
+        return false;
+    };
+
+    // Check if `expr` is the argument of a compiler-generated `IntoIter::into_iter(expr)`
+    if let ExprKind::Call(func, [arg]) = parent_expr.kind
+        && arg.hir_id == use_ctxt.child_id
+        && is_path_lang_item(cx, func, LangItem::IntoIterIntoIter)
     {
-        let span = expr.span;
-        span_lint_and_then(
-            cx,
-            RANGE_PLUS_ONE,
-            span,
-            "an inclusive range would be more readable",
-            |diag| {
-                let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_paren().to_string());
-                let end = Sugg::hir(cx, y, "y").maybe_paren();
-                match span.with_source_text(cx, |src| src.starts_with('(') && src.ends_with(')')) {
-                    Some(true) => {
-                        diag.span_suggestion(span, "use", format!("({start}..={end})"), Applicability::MaybeIncorrect);
-                    },
-                    Some(false) => {
-                        diag.span_suggestion(
-                            span,
-                            "use",
-                            format!("{start}..={end}"),
-                            Applicability::MachineApplicable, // snippet
-                        );
-                    },
-                    None => {},
-                }
-            },
-        );
+        return true;
+    }
+
+    // Check if `expr` is used as the receiver of a method of the `Iterator`, `IntoIterator`,
+    // or `RangeBounds` traits.
+    if let ExprKind::MethodCall(_, receiver, _, _) = parent_expr.kind
+        && receiver.hir_id == use_ctxt.child_id
+        && let Some(method_did) = cx.typeck_results().type_dependent_def_id(parent_expr.hir_id)
+        && let Some(trait_did) = cx.tcx.trait_of_item(method_did)
+        && matches!(
+            cx.tcx.get_diagnostic_name(trait_did),
+            Some(sym::Iterator | sym::IntoIterator | sym::RangeBounds)
+        )
+    {
+        return true;
+    }
+
+    // Check if `expr` is an argument of a call which requires an `Iterator`, `IntoIterator`,
+    // or `RangeBounds` trait.
+    if let ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
+        && let Some(id) = fn_def_id(cx, parent_expr)
+        && let Some(arg_idx) = args.iter().position(|e| e.hir_id == use_ctxt.child_id)
+    {
+        let input_idx = if matches!(parent_expr.kind, ExprKind::MethodCall(..)) {
+            arg_idx + 1
+        } else {
+            arg_idx
+        };
+        let inputs = cx
+            .tcx
+            .liberate_late_bound_regions(id, cx.tcx.fn_sig(id).instantiate_identity())
+            .inputs();
+        let expr_ty = inputs[input_idx];
+        // Check that the `expr` type is present only once, otherwise modifying just one of them might be
+        // risky if they are referenced using the same generic type for example.
+        if inputs.iter().enumerate().all(|(n, ty)|
+                                         n == input_idx
+                                         || !ty.walk().any(|arg| matches!(arg.unpack(),
+                                                                          GenericArgKind::Type(ty) if ty == expr_ty)))
+            // Look for a clause requiring `Iterator`, `IntoIterator`, or `RangeBounds`, and resolving to `expr_type`.
+            && cx
+                .tcx
+                .param_env(id)
+                .caller_bounds()
+                .into_iter()
+                .any(|p| {
+                    if let ClauseKind::Trait(t) = p.kind().skip_binder()
+                        && t.polarity == PredicatePolarity::Positive
+                        && matches!(
+                            cx.tcx.get_diagnostic_name(t.trait_ref.def_id),
+                            Some(sym::Iterator | sym::IntoIterator | sym::RangeBounds)
+                        )
+                    {
+                        t.self_ty() == expr_ty
+                    } else {
+                        false
+                    }
+                })
+        {
+            return true;
+        }
+    }
+
+    // Check if `expr` is used for indexing, and if the switched range type could be used
+    // as well.
+    if let ExprKind::Index(outer_expr, index, _) = parent_expr.kind
+        && index.hir_id == expr.hir_id
+        // Build the switched range type (for example `RangeInclusive<usize>`).
+        && let Some(switched_range_def_id) = match original {
+            RangeLimits::HalfOpen => cx.tcx.lang_items().range_inclusive_struct(),
+            RangeLimits::Closed => cx.tcx.lang_items().range_struct(),
+        }
+        && let switched_range_ty = cx
+            .tcx
+            .type_of(switched_range_def_id)
+            .instantiate(cx.tcx, &[inner_ty.into()])
+        // Check that the switched range type can be used for indexing the original expression
+        // through the `Index` or `IndexMut` trait.
+        && let ty::Ref(_, outer_ty, mutability) = cx.typeck_results().expr_ty_adjusted(outer_expr).kind()
+        && let Some(index_def_id) = match mutability {
+            Mutability::Not => cx.tcx.lang_items().index_trait(),
+            Mutability::Mut => cx.tcx.lang_items().index_mut_trait(),
+        }
+       && implements_trait(cx, *outer_ty, index_def_id, &[switched_range_ty.into()])
+    // We could also check that the associated item of the `index_def_id` trait with the switched range type
+    // return the same type, but it is reasonable to expect so. We can't check that the result is identical
+    // in both `Index<Range<…>>` and `Index<RangeInclusive<…>>` anyway.
+    {
+        return true;
     }
+
+    false
+}
+
+// exclusive range plus one: `x..(y+1)`
+fn check_exclusive_range_plus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+    check_range_switch(
+        cx,
+        expr,
+        RangeLimits::HalfOpen,
+        y_plus_one,
+        RANGE_PLUS_ONE,
+        "an inclusive range would be more readable",
+        "..=",
+    );
 }
 
 // inclusive range minus one: `x..=(y-1)`
-fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
+fn check_inclusive_range_minus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+    check_range_switch(
+        cx,
+        expr,
+        RangeLimits::Closed,
+        y_minus_one,
+        RANGE_MINUS_ONE,
+        "an exclusive range would be more readable",
+        "..",
+    );
+}
+
+/// Check for a `kind` of range in `expr`, check for `predicate` on the end,
+/// and emit the `lint` with `msg` and the `operator`.
+fn check_range_switch<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    kind: RangeLimits,
+    predicate: impl for<'hir> FnOnce(&LateContext<'_>, &Expr<'hir>) -> Option<&'hir Expr<'hir>>,
+    lint: &'static Lint,
+    msg: &'static str,
+    operator: &str,
+) {
     if expr.span.can_be_used_for_suggestions()
         && let Some(higher::Range {
             start,
             end: Some(end),
-            limits: RangeLimits::Closed,
+            limits,
         }) = higher::Range::hir(expr)
-        && let Some(y) = y_minus_one(cx, end)
+        && limits == kind
+        && let Some(y) = predicate(cx, end)
+        && can_switch_ranges(cx, expr, kind, cx.typeck_results().expr_ty(y))
     {
-        span_lint_and_then(
-            cx,
-            RANGE_MINUS_ONE,
-            expr.span,
-            "an exclusive range would be more readable",
-            |diag| {
-                let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_paren().to_string());
-                let end = Sugg::hir(cx, y, "y").maybe_paren();
-                diag.span_suggestion(
-                    expr.span,
-                    "use",
-                    format!("{start}..{end}"),
-                    Applicability::MachineApplicable, // snippet
-                );
-            },
-        );
+        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| {
+                Sugg::hir_with_applicability(cx, x, "<x>", &mut app)
+                    .maybe_paren()
+                    .to_string()
+            });
+            let end = Sugg::hir_with_applicability(cx, y, "<y>", &mut app).maybe_paren();
+            match span.with_source_text(cx, |src| src.starts_with('(') && src.ends_with(')')) {
+                Some(true) => {
+                    diag.span_suggestion(span, "use", format!("({start}{operator}{end})"), app);
+                },
+                Some(false) => {
+                    diag.span_suggestion(span, "use", format!("{start}{operator}{end}"), app);
+                },
+                None => {},
+            }
+        });
     }
 }
 
@@ -494,7 +618,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
     }
 }
 
-fn y_plus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
+fn y_plus_one<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
     match expr.kind {
         ExprKind::Binary(
             Spanned {
@@ -515,7 +639,7 @@ fn y_plus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'
     }
 }
 
-fn y_minus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
+fn y_minus_one<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
     match expr.kind {
         ExprKind::Binary(
             Spanned {