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.

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,15 @@ 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::{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_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 +24,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 +40,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 +72,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 +345,41 @@ 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;
+    };
+
+    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;
+    }
+
+    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;
+    }
+
+    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 +389,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 +428,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,25 @@ 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
 }

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,25 @@ 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
 }

tests/ui/range_plus_minus_one.stderr

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