refactor ptr_offset_with_cast

Author: folkertdev

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,
@@ -625,7 +626,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,

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;
@@ -592,7 +591,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)));

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;
@@ -1725,6 +1726,43 @@ declare_clippy_lint! {
     "Check for offset calculations on raw pointers to zero-sized types"
 }
 
+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()`.
@@ -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,
@@ -4962,9 +5001,13 @@ impl Methods {
             match (name, args) {
                 (
                     sym::add | sym::offset | sym::sub | sym::wrapping_offset | sym::wrapping_add | sym::wrapping_sub,
-                    [_arg],
+                    [arg],
                 ) => {
                     zst_offset::check(cx, expr, recv);
+
+                    if matches!(name, sym::offset | sym::wrapping_offset) {
+                        ptr_offset_with_cast::check(cx, name, expr, recv, arg);
+                    }
                 },
                 (sym::all, [arg]) => {
                     unused_enumerate_index::check(cx, expr, recv, arg);

clippy_lints/src/methods/ptr_offset_with_cast.rs

@@ -0,0 +1,72 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::SpanRangeExt;
+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<'_>) {
+    let method = match method {
+        sym::offset => Method::Offset,
+        sym::wrapping_offset => Method::WrappingOffset,
+        _ => 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 is_expr_ty_usize(cx, lhs) => lhs,
+        _ => 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| {
+        if let Some(sugg) = build_suggestion(cx, method, recv, cast_lhs_expr) {
+            diag.span_suggestion(expr.span, "try", sugg, Applicability::MachineApplicable);
+        }
+    });
+}
+
+// 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
+}
+
+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"),
+        }
+    }
+}