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

To avoid false positives, the `range_plus_one` and `range_minus_one` lints
will restrict themselves to situations where the iterator types can be easily
switched from exclusive to inclusive or vice-versa. This includes situations
where the range is used as an iterator, or is used for indexing. Also,
when the range is used as a function or method call argument and the
parameter type is generic over traits implemented by both kind of
ranges, the lint will trigger.

On the other hand, assignments of the range to variables, including
automatically typed ones or wildcards, will no longer trigger the lint.
However, the cases where such an assignment would benefit from the lint
are probably rare.

Author: samueltardieu

clippy_lints/src/cognitive_complexity.rs

@@ -97,7 +97,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,17 @@ 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::{
+    fn_def_id, get_parent_expr, higher, is_in_const_context, is_integer_const, is_path_lang_item, path_to_local,
+};
 use rustc_ast::ast::RangeLimits;
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, HirId};
+use rustc_hir::{BinOpKind, Expr, ExprKind, HirId, LangItem};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty;
+use rustc_middle::ty::{self, ClauseKind, PredicatePolarity};
 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 +26,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 +42,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 +74,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,6 +347,88 @@ fn check_range_bounds<'a, 'tcx>(cx: &'a LateContext<'tcx>, ex: &'a Expr<'_>) ->
     None
 }
 
+/// Check whether `expr` could switch range types without breaking the typing requirements. This is
+/// the case when `expr` is used as an iterator for example, or as a slice or `&str` index.
+fn can_switch_ranges(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let Some(parent_expr) = get_parent_expr(cx, expr) 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 == expr.hir_id
+        && is_path_lang_item(cx, func, LangItem::IntoIterIntoIter)
+    {
+        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 == expr.hir_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 == expr.hir_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 one, otherwise modifying just one of them it 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 != 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.
+    if let ExprKind::Index(_, index, _) = parent_expr.kind
+        && index.hir_id == expr.hir_id
+    {
+        return true;
+    }
+
+    false
+}
+
 // 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()
@@ -353,6 +438,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
             limits: RangeLimits::HalfOpen,
         }) = higher::Range::hir(expr)
         && let Some(y) = y_plus_one(cx, end)
+        && can_switch_ranges(cx, expr)
     {
         let span = expr.span;
         span_lint_and_then(
@@ -391,6 +477,7 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
             limits: RangeLimits::Closed,
         }) = higher::Range::hir(expr)
         && let Some(y) = y_minus_one(cx, end)
+        && can_switch_ranges(cx, expr)
     {
         span_lint_and_then(
             cx,

tests/ui/range_plus_minus_one.fixed

@@ -45,15 +45,13 @@ fn main() {
     //~^ range_plus_one
     for _ in 0..=(1 + f()) {}
 
+    // Those are not linted, as in the general case we cannot be sure that the exact type won't be
+    // important.
     let _ = ..11 - 1;
-    let _ = ..11;
-    //~^ range_minus_one
-    let _ = ..11;
-    //~^ range_minus_one
-    let _ = (1..=11);
-    //~^ range_plus_one
-    let _ = ((f() + 1)..=f());
-    //~^ range_plus_one
+    let _ = ..=11 - 1;
+    let _ = ..=(11 - 1);
+    let _ = (1..11 + 1);
+    let _ = (f() + 1)..(f() + 1);
 
     const ONE: usize = 1;
     // integer consts are linted, too
@@ -65,4 +63,39 @@ fn main() {
 
     macro_plus_one!(5);
     macro_minus_one!(5);
+
+    // As an instance of `Iterator`
+    (1..=10).for_each(|_| {});
+    //~^ range_plus_one
+
+    // As an instance of `IntoIterator`
+    #[allow(clippy::useless_conversion)]
+    (1..=10).into_iter().for_each(|_| {});
+    //~^ range_plus_one
+
+    // As an instance of `RangeBounds`
+    {
+        use std::ops::RangeBounds;
+        let _ = (1..=10).start_bound();
+        //~^ range_plus_one
+    }
+
+    // As a `SliceIndex`
+    let a = [10, 20, 30];
+    let _ = &a[1..=1];
+    //~^ range_plus_one
+
+    // As method call argument
+    vec.drain(2..=3);
+    //~^ range_plus_one
+
+    // As function call argument
+    take_arg(10..=20);
+    //~^ range_plus_one
+
+    // Do not lint, as the same type is used for both parameters
+    take_args(10..20 + 1, 10..21);
 }
+
+fn take_arg<T: Iterator<Item = u32>>(_: T) {}
+fn take_args<T: Iterator<Item = u32>>(_: T, _: T) {}

tests/ui/range_plus_minus_one.rs

@@ -45,15 +45,13 @@ fn main() {
     //~^ range_plus_one
     for _ in 0..=(1 + f()) {}
 
+    // Those are not linted, as in the general case we cannot be sure that the exact type won't be
+    // important.
     let _ = ..11 - 1;
     let _ = ..=11 - 1;
-    //~^ range_minus_one
     let _ = ..=(11 - 1);
-    //~^ range_minus_one
     let _ = (1..11 + 1);
-    //~^ range_plus_one
     let _ = (f() + 1)..(f() + 1);
-    //~^ range_plus_one
 
     const ONE: usize = 1;
     // integer consts are linted, too
@@ -65,4 +63,39 @@ fn main() {
 
     macro_plus_one!(5);
     macro_minus_one!(5);
+
+    // As an instance of `Iterator`
+    (1..10 + 1).for_each(|_| {});
+    //~^ range_plus_one
+
+    // As an instance of `IntoIterator`
+    #[allow(clippy::useless_conversion)]
+    (1..10 + 1).into_iter().for_each(|_| {});
+    //~^ range_plus_one
+
+    // As an instance of `RangeBounds`
+    {
+        use std::ops::RangeBounds;
+        let _ = (1..10 + 1).start_bound();
+        //~^ range_plus_one
+    }
+
+    // As a `SliceIndex`
+    let a = [10, 20, 30];
+    let _ = &a[1..1 + 1];
+    //~^ range_plus_one
+
+    // As method call argument
+    vec.drain(2..3 + 1);
+    //~^ range_plus_one
+
+    // As function call argument
+    take_arg(10..20 + 1);
+    //~^ range_plus_one
+
+    // Do not lint, as the same type is used for both parameters
+    take_args(10..20 + 1, 10..21);
 }
+
+fn take_arg<T: Iterator<Item = u32>>(_: T) {}
+fn take_args<T: Iterator<Item = u32>>(_: T, _: T) {}

tests/ui/range_plus_minus_one.stderr

@@ -25,38 +25,47 @@ error: an inclusive range would be more readable
 LL |     for _ in 0..(1 + f()) {}
    |              ^^^^^^^^^^^^ help: use: `0..=f()`
 
-error: an exclusive range would be more readable
-  --> tests/ui/range_plus_minus_one.rs:49:13
+error: an inclusive range would be more readable
+  --> tests/ui/range_plus_minus_one.rs:58:14
    |
-LL |     let _ = ..=11 - 1;
-   |             ^^^^^^^^^ help: use: `..11`
+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:68:5
    |
-   = note: `-D clippy::range-minus-one` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::range_minus_one)]`
+LL |     (1..10 + 1).for_each(|_| {});
+   |     ^^^^^^^^^^^ help: use: `(1..=10)`
 
-error: an exclusive range would be more readable
-  --> tests/ui/range_plus_minus_one.rs:51:13
+error: an inclusive range would be more readable
+  --> tests/ui/range_plus_minus_one.rs:73:5
    |
-LL |     let _ = ..=(11 - 1);
-   |             ^^^^^^^^^^^ help: use: `..11`
+LL |     (1..10 + 1).into_iter().for_each(|_| {});
+   |     ^^^^^^^^^^^ help: use: `(1..=10)`
 
 error: an inclusive range would be more readable
-  --> tests/ui/range_plus_minus_one.rs:53:13
+  --> tests/ui/range_plus_minus_one.rs:79:17
    |
-LL |     let _ = (1..11 + 1);
-   |             ^^^^^^^^^^^ help: use: `(1..=11)`
+LL |         let _ = (1..10 + 1).start_bound();
+   |                 ^^^^^^^^^^^ help: use: `(1..=10)`
 
 error: an inclusive range would be more readable
-  --> tests/ui/range_plus_minus_one.rs:55:13
+  --> tests/ui/range_plus_minus_one.rs:85:16
    |
-LL |     let _ = (f() + 1)..(f() + 1);
-   |             ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())`
+LL |     let _ = &a[1..1 + 1];
+   |                ^^^^^^^^ help: use: `1..=1`
 
 error: an inclusive range would be more readable
-  --> tests/ui/range_plus_minus_one.rs:60:14
+  --> tests/ui/range_plus_minus_one.rs:89:15
    |
-LL |     for _ in 1..ONE + ONE {}
-   |              ^^^^^^^^^^^^ help: use: `1..=ONE`
+LL |     vec.drain(2..3 + 1);
+   |               ^^^^^^^^ help: use: `2..=3`
+
+error: an inclusive range would be more readable
+  --> tests/ui/range_plus_minus_one.rs:93:14
+   |
+LL |     take_arg(10..20 + 1);
+   |              ^^^^^^^^^^ help: use: `10..=20`
 
-error: aborting due to 9 previous errors
+error: aborting due to 11 previous errors