about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-09-14 15:01:46 +0000
committerGitHub <noreply@github.com>2025-09-14 15:01:46 +0000
commit7a1268474100cba2c5def59fe893b59ed0709aab (patch)
treeb577f35837fc68e691bbf0ba92638c50d139befc
parente7421ccb1aa764b669f167b08ad95d90c67c6a91 (diff)
parentf981739fcfdbe3e69d5fd630c4390a33ffc41035 (diff)
downloadrust-7a1268474100cba2c5def59fe893b59ed0709aab.tar.gz
rust-7a1268474100cba2c5def59fe893b59ed0709aab.zip
Refactor `ptr_offset_with_cast` (#15613)
Putting this lint in `/methods` is simpler and probably slightly more
efficient.

changelog: [`ptr_offset_with_cast`]: respect MSRV when suggesting fix,
and lint even more cases
-rw-r--r--clippy_lints/src/declared_lints.rs2
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/methods/mod.rs49
-rw-r--r--clippy_lints/src/methods/ptr_offset_with_cast.rs82
-rw-r--r--clippy_lints/src/ptr_offset_with_cast.rs151
-rw-r--r--clippy_utils/src/msrvs.rs2
-rw-r--r--tests/ui/ptr_offset_with_cast.fixed22
-rw-r--r--tests/ui/ptr_offset_with_cast.rs22
-rw-r--r--tests/ui/ptr_offset_with_cast.stderr41
9 files changed, 209 insertions, 164 deletions
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index e610e655f15..1b19a8851ed 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -448,6 +448,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::OR_THEN_UNWRAP_INFO,
     crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
     crate::methods::PATH_ENDS_WITH_EXT_INFO,
+    crate::methods::PTR_OFFSET_WITH_CAST_INFO,
     crate::methods::RANGE_ZIP_WITH_LEN_INFO,
     crate::methods::READONLY_WRITE_LOCK_INFO,
     crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
@@ -624,7 +625,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::ptr::MUT_FROM_REF_INFO,
     crate::ptr::PTR_ARG_INFO,
     crate::ptr::PTR_EQ_INFO,
-    crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
     crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
     crate::pub_use::PUB_USE_INFO,
     crate::question_mark::QUESTION_MARK_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 01e296ea33c..c56fa257b06 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -302,7 +302,6 @@ mod permissions_set_readonly_false;
 mod pointers_in_nomem_asm_block;
 mod precedence;
 mod ptr;
-mod ptr_offset_with_cast;
 mod pub_underscore_fields;
 mod pub_use;
 mod question_mark;
@@ -593,7 +592,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(unwrap::Unwrap));
     store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
     store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf)));
-    store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast));
     store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));
     store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit));
     store.register_late_pass(move |_| Box::new(unnecessary_wraps::UnnecessaryWraps::new(conf)));
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 49ca81dafc2..d06b0ed64bd 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -91,6 +91,7 @@ mod or_fun_call;
 mod or_then_unwrap;
 mod path_buf_push_overwrite;
 mod path_ends_with_ext;
+mod ptr_offset_with_cast;
 mod range_zip_with_len;
 mod read_line_without_trim;
 mod readonly_write_lock;
@@ -1727,6 +1728,43 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
+    /// Checks for usage of the `offset` pointer method with a `usize` casted to an
+    /// `isize`.
+    ///
+    /// ### Why is this bad?
+    /// If we’re always increasing the pointer address, we can avoid the numeric
+    /// cast by using the `add` method instead.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let vec = vec![b'a', b'b', b'c'];
+    /// let ptr = vec.as_ptr();
+    /// let offset = 1_usize;
+    ///
+    /// unsafe {
+    ///     ptr.offset(offset as isize);
+    /// }
+    /// ```
+    ///
+    /// Could be written:
+    ///
+    /// ```no_run
+    /// let vec = vec![b'a', b'b', b'c'];
+    /// let ptr = vec.as_ptr();
+    /// let offset = 1_usize;
+    ///
+    /// unsafe {
+    ///     ptr.add(offset);
+    /// }
+    /// ```
+    #[clippy::version = "1.30.0"]
+    pub PTR_OFFSET_WITH_CAST,
+    complexity,
+    "unneeded pointer offset cast"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
     /// Checks for `FileType::is_file()`.
     ///
     /// ### Why restrict this?
@@ -4665,6 +4703,7 @@ impl_lint_pass!(Methods => [
     UNINIT_ASSUMED_INIT,
     MANUAL_SATURATING_ARITHMETIC,
     ZST_OFFSET,
+    PTR_OFFSET_WITH_CAST,
     FILETYPE_IS_FILE,
     OPTION_AS_REF_DEREF,
     UNNECESSARY_LAZY_EVALUATIONS,
@@ -4960,10 +4999,7 @@ impl Methods {
         // Handle method calls whose receiver and arguments may not come from expansion
         if let Some((name, recv, args, span, call_span)) = method_call(expr) {
             match (name, args) {
-                (
-                    sym::add | sym::offset | sym::sub | sym::wrapping_offset | sym::wrapping_add | sym::wrapping_sub,
-                    [_arg],
-                ) => {
+                (sym::add | sym::sub | sym::wrapping_add | sym::wrapping_sub, [_arg]) => {
                     zst_offset::check(cx, expr, recv);
                 },
                 (sym::all, [arg]) => {
@@ -5334,6 +5370,11 @@ impl Methods {
                     },
                     _ => iter_nth_zero::check(cx, expr, recv, n_arg),
                 },
+                (sym::offset | sym::wrapping_offset, [arg]) => {
+                    zst_offset::check(cx, expr, recv);
+
+                    ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv);
+                },
                 (sym::ok_or_else, [arg]) => {
                     unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
                 },
diff --git a/clippy_lints/src/methods/ptr_offset_with_cast.rs b/clippy_lints/src/methods/ptr_offset_with_cast.rs
new file mode 100644
index 00000000000..d19d3b8eb89
--- /dev/null
+++ b/clippy_lints/src/methods/ptr_offset_with_cast.rs
@@ -0,0 +1,82 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sym;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::Symbol;
+use std::fmt;
+
+use super::PTR_OFFSET_WITH_CAST;
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    method: Symbol,
+    expr: &Expr<'_>,
+    recv: &Expr<'_>,
+    arg: &Expr<'_>,
+    msrv: Msrv,
+) {
+    // `pointer::add` and `pointer::wrapping_add` are only stable since 1.26.0. These functions
+    // became const-stable in 1.61.0, the same version that `pointer::offset` became const-stable.
+    if !msrv.meets(cx, msrvs::POINTER_ADD_SUB_METHODS) {
+        return;
+    }
+
+    let method = match method {
+        sym::offset => Method::Offset,
+        sym::wrapping_offset => Method::WrappingOffset,
+        _ => return,
+    };
+
+    if !cx.typeck_results().expr_ty_adjusted(recv).is_raw_ptr() {
+        return;
+    }
+
+    // Check if the argument to the method call is a cast from usize.
+    let cast_lhs_expr = match arg.kind {
+        ExprKind::Cast(lhs, _) if cx.typeck_results().expr_ty(lhs).is_usize() => lhs,
+        _ => return,
+    };
+
+    let ExprKind::MethodCall(method_name, _, _, _) = expr.kind else {
+        return;
+    };
+
+    let msg = format!("use of `{method}` with a `usize` casted to an `isize`");
+    span_lint_and_then(cx, PTR_OFFSET_WITH_CAST, expr.span, msg, |diag| {
+        diag.multipart_suggestion(
+            format!("use `{}` instead", method.suggestion()),
+            vec![
+                (method_name.ident.span, method.suggestion().to_string()),
+                (arg.span.with_lo(cast_lhs_expr.span.hi()), String::new()),
+            ],
+            Applicability::MachineApplicable,
+        );
+    });
+}
+
+#[derive(Copy, Clone)]
+enum Method {
+    Offset,
+    WrappingOffset,
+}
+
+impl Method {
+    #[must_use]
+    fn suggestion(self) -> &'static str {
+        match self {
+            Self::Offset => "add",
+            Self::WrappingOffset => "wrapping_add",
+        }
+    }
+}
+
+impl fmt::Display for Method {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Offset => write!(f, "offset"),
+            Self::WrappingOffset => write!(f, "wrapping_offset"),
+        }
+    }
+}
diff --git a/clippy_lints/src/ptr_offset_with_cast.rs b/clippy_lints/src/ptr_offset_with_cast.rs
deleted file mode 100644
index d8d813f9846..00000000000
--- a/clippy_lints/src/ptr_offset_with_cast.rs
+++ /dev/null
@@ -1,151 +0,0 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
-use clippy_utils::source::SpanRangeExt;
-use clippy_utils::sym;
-use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::declare_lint_pass;
-use std::fmt;
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for usage of the `offset` pointer method with a `usize` casted to an
-    /// `isize`.
-    ///
-    /// ### Why is this bad?
-    /// If we’re always increasing the pointer address, we can avoid the numeric
-    /// cast by using the `add` method instead.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// let vec = vec![b'a', b'b', b'c'];
-    /// let ptr = vec.as_ptr();
-    /// let offset = 1_usize;
-    ///
-    /// unsafe {
-    ///     ptr.offset(offset as isize);
-    /// }
-    /// ```
-    ///
-    /// Could be written:
-    ///
-    /// ```no_run
-    /// let vec = vec![b'a', b'b', b'c'];
-    /// let ptr = vec.as_ptr();
-    /// let offset = 1_usize;
-    ///
-    /// unsafe {
-    ///     ptr.add(offset);
-    /// }
-    /// ```
-    #[clippy::version = "1.30.0"]
-    pub PTR_OFFSET_WITH_CAST,
-    complexity,
-    "unneeded pointer offset cast"
-}
-
-declare_lint_pass!(PtrOffsetWithCast => [PTR_OFFSET_WITH_CAST]);
-
-impl<'tcx> LateLintPass<'tcx> for PtrOffsetWithCast {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        // Check if the expressions is a ptr.offset or ptr.wrapping_offset method call
-        let Some((receiver_expr, arg_expr, method)) = expr_as_ptr_offset_call(cx, expr) else {
-            return;
-        };
-
-        // Check if the argument to the method call is a cast from usize
-        let Some(cast_lhs_expr) = expr_as_cast_from_usize(cx, arg_expr) else {
-            return;
-        };
-
-        let msg = format!("use of `{method}` with a `usize` casted to an `isize`");
-        if let Some(sugg) = build_suggestion(cx, method, receiver_expr, cast_lhs_expr) {
-            span_lint_and_sugg(
-                cx,
-                PTR_OFFSET_WITH_CAST,
-                expr.span,
-                msg,
-                "try",
-                sugg,
-                Applicability::MachineApplicable,
-            );
-        } else {
-            span_lint(cx, PTR_OFFSET_WITH_CAST, expr.span, msg);
-        }
-    }
-}
-
-// If the given expression is a cast from a usize, return the lhs of the cast
-fn expr_as_cast_from_usize<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
-    if let ExprKind::Cast(cast_lhs_expr, _) = expr.kind
-        && is_expr_ty_usize(cx, cast_lhs_expr)
-    {
-        return Some(cast_lhs_expr);
-    }
-    None
-}
-
-// If the given expression is a ptr::offset  or ptr::wrapping_offset method call, return the
-// receiver, the arg of the method call, and the method.
-fn expr_as_ptr_offset_call<'tcx>(
-    cx: &LateContext<'tcx>,
-    expr: &'tcx Expr<'_>,
-) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Method)> {
-    if let ExprKind::MethodCall(path_segment, arg_0, [arg_1], _) = &expr.kind
-        && is_expr_ty_raw_ptr(cx, arg_0)
-    {
-        if path_segment.ident.name == sym::offset {
-            return Some((arg_0, arg_1, Method::Offset));
-        }
-        if path_segment.ident.name == sym::wrapping_offset {
-            return Some((arg_0, arg_1, Method::WrappingOffset));
-        }
-    }
-    None
-}
-
-// Is the type of the expression a usize?
-fn is_expr_ty_usize(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    cx.typeck_results().expr_ty(expr) == cx.tcx.types.usize
-}
-
-// Is the type of the expression a raw pointer?
-fn is_expr_ty_raw_ptr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    cx.typeck_results().expr_ty(expr).is_raw_ptr()
-}
-
-fn build_suggestion(
-    cx: &LateContext<'_>,
-    method: Method,
-    receiver_expr: &Expr<'_>,
-    cast_lhs_expr: &Expr<'_>,
-) -> Option<String> {
-    let receiver = receiver_expr.span.get_source_text(cx)?;
-    let cast_lhs = cast_lhs_expr.span.get_source_text(cx)?;
-    Some(format!("{receiver}.{}({cast_lhs})", method.suggestion()))
-}
-
-#[derive(Copy, Clone)]
-enum Method {
-    Offset,
-    WrappingOffset,
-}
-
-impl Method {
-    #[must_use]
-    fn suggestion(self) -> &'static str {
-        match self {
-            Self::Offset => "add",
-            Self::WrappingOffset => "wrapping_add",
-        }
-    }
-}
-
-impl fmt::Display for Method {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Offset => write!(f, "offset"),
-            Self::WrappingOffset => write!(f, "wrapping_offset"),
-        }
-    }
-}
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
index 032c5e4c752..6e07ed9ffcc 100644
--- a/clippy_utils/src/msrvs.rs
+++ b/clippy_utils/src/msrvs.rs
@@ -73,7 +73,7 @@ msrv_aliases! {
     1,29,0 { ITER_FLATTEN }
     1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
     1,27,0 { ITERATOR_TRY_FOLD }
-    1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
+    1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
     1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
     1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
     1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }
diff --git a/tests/ui/ptr_offset_with_cast.fixed b/tests/ui/ptr_offset_with_cast.fixed
index 4fe9dcf46c3..42d1abeaa05 100644
--- a/tests/ui/ptr_offset_with_cast.fixed
+++ b/tests/ui/ptr_offset_with_cast.fixed
@@ -1,4 +1,4 @@
-#![allow(clippy::unnecessary_cast, clippy::useless_vec)]
+#![expect(clippy::unnecessary_cast, clippy::useless_vec, clippy::needless_borrow)]
 
 fn main() {
     let vec = vec![b'a', b'b', b'c'];
@@ -18,5 +18,25 @@ fn main() {
         //~^ ptr_offset_with_cast
         let _ = ptr.wrapping_offset(offset_isize as isize);
         let _ = ptr.wrapping_offset(offset_u8 as isize);
+
+        let _ = S.offset(offset_usize as isize);
+        let _ = S.wrapping_offset(offset_usize as isize);
+
+        let _ = (&ptr).add(offset_usize);
+        //~^ ptr_offset_with_cast
+        let _ = (&ptr).wrapping_add(offset_usize);
+        //~^ ptr_offset_with_cast
+    }
+}
+
+#[derive(Clone, Copy)]
+struct S;
+
+impl S {
+    fn offset(self, _: isize) -> Self {
+        self
+    }
+    fn wrapping_offset(self, _: isize) -> Self {
+        self
     }
 }
diff --git a/tests/ui/ptr_offset_with_cast.rs b/tests/ui/ptr_offset_with_cast.rs
index a1fb892733d..6d06a6af1fa 100644
--- a/tests/ui/ptr_offset_with_cast.rs
+++ b/tests/ui/ptr_offset_with_cast.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::unnecessary_cast, clippy::useless_vec)]
+#![expect(clippy::unnecessary_cast, clippy::useless_vec, clippy::needless_borrow)]
 
 fn main() {
     let vec = vec![b'a', b'b', b'c'];
@@ -18,5 +18,25 @@ fn main() {
         //~^ ptr_offset_with_cast
         let _ = ptr.wrapping_offset(offset_isize as isize);
         let _ = ptr.wrapping_offset(offset_u8 as isize);
+
+        let _ = S.offset(offset_usize as isize);
+        let _ = S.wrapping_offset(offset_usize as isize);
+
+        let _ = (&ptr).offset(offset_usize as isize);
+        //~^ ptr_offset_with_cast
+        let _ = (&ptr).wrapping_offset(offset_usize as isize);
+        //~^ ptr_offset_with_cast
+    }
+}
+
+#[derive(Clone, Copy)]
+struct S;
+
+impl S {
+    fn offset(self, _: isize) -> Self {
+        self
+    }
+    fn wrapping_offset(self, _: isize) -> Self {
+        self
     }
 }
diff --git a/tests/ui/ptr_offset_with_cast.stderr b/tests/ui/ptr_offset_with_cast.stderr
index dcd5e027d18..022b3286c93 100644
--- a/tests/ui/ptr_offset_with_cast.stderr
+++ b/tests/ui/ptr_offset_with_cast.stderr
@@ -2,16 +2,51 @@ error: use of `offset` with a `usize` casted to an `isize`
   --> tests/ui/ptr_offset_with_cast.rs:12:17
    |
 LL |         let _ = ptr.offset(offset_usize as isize);
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::ptr-offset-with-cast` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::ptr_offset_with_cast)]`
+help: use `add` instead
+   |
+LL -         let _ = ptr.offset(offset_usize as isize);
+LL +         let _ = ptr.add(offset_usize);
+   |
 
 error: use of `wrapping_offset` with a `usize` casted to an `isize`
   --> tests/ui/ptr_offset_with_cast.rs:17:17
    |
 LL |         let _ = ptr.wrapping_offset(offset_usize as isize);
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `wrapping_add` instead
+   |
+LL -         let _ = ptr.wrapping_offset(offset_usize as isize);
+LL +         let _ = ptr.wrapping_add(offset_usize);
+   |
+
+error: use of `offset` with a `usize` casted to an `isize`
+  --> tests/ui/ptr_offset_with_cast.rs:25:17
+   |
+LL |         let _ = (&ptr).offset(offset_usize as isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `add` instead
+   |
+LL -         let _ = (&ptr).offset(offset_usize as isize);
+LL +         let _ = (&ptr).add(offset_usize);
+   |
+
+error: use of `wrapping_offset` with a `usize` casted to an `isize`
+  --> tests/ui/ptr_offset_with_cast.rs:27:17
+   |
+LL |         let _ = (&ptr).wrapping_offset(offset_usize as isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `wrapping_add` instead
+   |
+LL -         let _ = (&ptr).wrapping_offset(offset_usize as isize);
+LL +         let _ = (&ptr).wrapping_add(offset_usize);
+   |
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors