add ptr_offset_by_literal lint

CHANGELOG.md

@@ -6376,6 +6376,7 @@ Released 2018-09-13
 [`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
 [`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness
 [`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
+[`ptr_offset_by_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_by_literal
 [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
 [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
 [`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields

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_BY_LITERAL_INFO,
     crate::methods::PTR_OFFSET_WITH_CAST_INFO,
     crate::methods::RANGE_ZIP_WITH_LEN_INFO,
     crate::methods::READONLY_WRITE_LOCK_INFO,

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_by_literal;
 mod ptr_offset_with_cast;
 mod range_zip_with_len;
 mod read_line_without_trim;
@@ -1726,6 +1727,40 @@ 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 an integer
+    /// literal.
+    ///
+    /// ### Why is this bad?
+    /// The `add` and `sub` methods more accurately express the intent.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let vec = vec![b'a', b'b', b'c'];
+    /// let ptr = vec.as_ptr();
+    ///
+    /// unsafe {
+    ///     ptr.offset(-8);
+    /// }
+    /// ```
+    ///
+    /// Could be written:
+    ///
+    /// ```no_run
+    /// let vec = vec![b'a', b'b', b'c'];
+    /// let ptr = vec.as_ptr();
+    ///
+    /// unsafe {
+    ///     ptr.sub(8);
+    /// }
+    /// ```
+    #[clippy::version = "1.92.0"]
+    pub PTR_OFFSET_BY_LITERAL,
+    complexity,
+    "unneeded pointer offset"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of the `offset` pointer method with a `usize` casted to an
@@ -4703,6 +4738,7 @@ impl_lint_pass!(Methods => [
     UNINIT_ASSUMED_INIT,
     MANUAL_SATURATING_ARITHMETIC,
     ZST_OFFSET,
+    PTR_OFFSET_BY_LITERAL,
     PTR_OFFSET_WITH_CAST,
     FILETYPE_IS_FILE,
     OPTION_AS_REF_DEREF,
@@ -5374,6 +5410,7 @@ impl Methods {
                     zst_offset::check(cx, expr, recv);
 
                     ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv);
+                    ptr_offset_by_literal::check(cx, expr, self.msrv);
                 },
                 (sym::ok_or_else, [arg]) => {
                     unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");

clippy_lints/src/methods/ptr_offset_by_literal.rs

@@ -0,0 +1,151 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::SpanRangeExt;
+use clippy_utils::sym;
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, UnOp};
+use rustc_lint::LateContext;
+use std::cmp::Ordering;
+use std::fmt;
+
+use super::PTR_OFFSET_BY_LITERAL;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx 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 ExprKind::MethodCall(method_name, recv, [arg_expr], _) = expr.kind else {
+        return;
+    };
+
+    let method = match method_name.ident.name {
+        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 (negated) literal.
+    let Some((literal, literal_text)) = expr_as_literal(cx, arg_expr) else {
+        return;
+    };
+
+    match method.suggestion(literal) {
+        None => {
+            let msg = format!("use of `{method}` with zero");
+            span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| {
+                diag.span_suggestion(
+                    expr.span.with_lo(recv.span.hi()),
+                    format!("remove the call to `{method}`"),
+                    String::new(),
+                    Applicability::MachineApplicable,
+                );
+            });
+        },
+        Some(method_suggestion) => {
+            let msg = format!("use of `{method}` with a literal");
+            span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| {
+                diag.multipart_suggestion(
+                    format!("use `{method_suggestion}` instead"),
+                    vec![
+                        (method_name.ident.span, method_suggestion.to_string()),
+                        (arg_expr.span, literal_text),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            });
+        },
+    }
+}
+
+fn get_literal_bits<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<u128> {
+    let ExprKind::Lit(lit) = expr.kind else {
+        return None;
+    };
+
+    let LitKind::Int(packed_u128, _) = lit.node else {
+        return None;
+    };
+
+    Some(packed_u128.get())
+}
+
+// If the given expression is a (negated) literal, return its value.
+fn expr_as_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(i128, String)> {
+    if let Some(literal_bits) = get_literal_bits(expr) {
+        // The value must fit in a isize, so we can't have overflow here.
+        return Some((literal_bits.cast_signed(), format_isize_literal(cx, expr)?));
+    }
+
+    if let ExprKind::Unary(UnOp::Neg, inner) = expr.kind
+        && let Some(literal_bits) = get_literal_bits(inner)
+    {
+        return Some((-(literal_bits.cast_signed()), format_isize_literal(cx, inner)?));
+    }
+
+    None
+}
+
+fn format_isize_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<String> {
+    let text = expr.span.get_source_text(cx)?;
+    let text = match peel_parens_str(&text) {
+        Some((_, text, _)) => text,
+        None => &text,
+    };
+
+    Some(text.trim_end_matches("isize").trim_end_matches('_').to_string())
+}
+
+fn peel_parens_str(snippet: &str) -> Option<(usize, &str, usize)> {
+    let trimmed = snippet.trim();
+    if !(trimmed.starts_with('(') && trimmed.ends_with(')')) {
+        return None;
+    }
+
+    let trim_start = (snippet.len() - snippet.trim_start().len()) + 1;
+    let trim_end = (snippet.len() - snippet.trim_end().len()) + 1;
+
+    let inner = snippet.get(trim_start..snippet.len() - trim_end)?;
+    Some(match peel_parens_str(inner) {
+        None => (trim_start, inner, trim_end),
+        Some((start, inner, end)) => (trim_start + start, inner, trim_end + end),
+    })
+}
+
+#[derive(Copy, Clone)]
+enum Method {
+    Offset,
+    WrappingOffset,
+}
+
+impl Method {
+    fn suggestion(self, literal: i128) -> Option<&'static str> {
+        match Ord::cmp(&literal, &0) {
+            Ordering::Greater => match self {
+                Method::Offset => Some("add"),
+                Method::WrappingOffset => Some("wrapping_add"),
+            },
+            Ordering::Equal => None,
+            Ordering::Less => match self {
+                Method::Offset => Some("sub"),
+                Method::WrappingOffset => Some("wrapping_sub"),
+            },
+        }
+    }
+}
+
+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"),
+        }
+    }
+}

tests/ui/borrow_as_ptr.fixed

@@ -1,6 +1,6 @@
 //@aux-build:proc_macros.rs
 #![warn(clippy::borrow_as_ptr)]
-#![allow(clippy::useless_vec)]
+#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)]
 
 extern crate proc_macros;
 

tests/ui/borrow_as_ptr.rs

@@ -1,6 +1,6 @@
 //@aux-build:proc_macros.rs
 #![warn(clippy::borrow_as_ptr)]
-#![allow(clippy::useless_vec)]
+#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)]
 
 extern crate proc_macros;
 

tests/ui/crashes/ice-4579.rs

@@ -1,6 +1,6 @@
 //@ check-pass
 
-#![allow(clippy::single_match)]
+#![allow(clippy::single_match, clippy::ptr_offset_by_literal)]
 
 use std::ptr;
 

tests/ui/ptr_offset_by_literal.fixed

@@ -0,0 +1,45 @@
+#![allow(clippy::inconsistent_digit_grouping)]
+
+fn main() {
+    let arr = [b'a', b'b', b'c'];
+    let ptr = arr.as_ptr();
+
+    let var = 32;
+    const CONST: isize = 42;
+
+    unsafe {
+        let _ = ptr;
+        //~^ ptr_offset_by_literal
+        let _ = ptr;
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.add(5);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.sub(5);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(var);
+        let _ = ptr.offset(CONST);
+
+        let _ = ptr.wrapping_add(5);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.wrapping_sub(5);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.sub(5);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.wrapping_sub(5);
+        //~^ ptr_offset_by_literal
+
+        // isize::MAX and isize::MIN on 64-bit systems.
+        let _ = ptr.add(9_223_372_036_854_775_807);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.sub(9_223_372_036_854_775_808);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.add(5_0);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.sub(5_0);
+        //~^ ptr_offset_by_literal
+    }
+}

tests/ui/ptr_offset_by_literal.rs

@@ -0,0 +1,45 @@
+#![allow(clippy::inconsistent_digit_grouping)]
+
+fn main() {
+    let arr = [b'a', b'b', b'c'];
+    let ptr = arr.as_ptr();
+
+    let var = 32;
+    const CONST: isize = 42;
+
+    unsafe {
+        let _ = ptr.offset(0);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-0);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(5);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-5);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(var);
+        let _ = ptr.offset(CONST);
+
+        let _ = ptr.wrapping_offset(5isize);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.wrapping_offset(-5isize);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(-(5));
+        //~^ ptr_offset_by_literal
+        let _ = ptr.wrapping_offset(-(5));
+        //~^ ptr_offset_by_literal
+
+        // isize::MAX and isize::MIN on 64-bit systems.
+        let _ = ptr.offset(9_223_372_036_854_775_807isize);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-9_223_372_036_854_775_808isize);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(5_0__isize);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-5_0__isize);
+        //~^ ptr_offset_by_literal
+    }
+}