add ptr_offset_by_literal lint (#15606)

Related to
https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
(and very much modelled off of its implementation).

This lint is motivated by cleaning up the output of
[c2rust](https://c2rust.com/), which contains many `.offset` calls with
by literal. The lint also nicely handles offsets by zero, which are
unnecessary.

I'm aware that the feature freeze is still in place, but this was
top-of-mind now. I'll patiently wait until the freeze is lifted.

changelog: [`ptr_offset_by_literal`]: add `ptr_offset_by_literal` lint

CHANGELOG.md

@@ -6766,6 +6766,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

@@ -444,6 +444,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

@@ -94,6 +94,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;
@@ -1728,6 +1729,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,
+    pedantic,
+    "unneeded pointer offset"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of the `offset` pointer method with a `usize` casted to an
@@ -4803,6 +4838,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,
@@ -5426,6 +5462,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,138 @@
+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, Lit, 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> {
+    match expr.kind {
+        ExprKind::Lit(Lit {
+            node: LitKind::Int(packed_u128, _),
+            ..
+        }) => Some(packed_u128.get()),
+        _ => None,
+    }
+}
+
+// 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 = peel_parens_str(&text);
+    Some(text.trim_end_matches("isize").trim_end_matches('_').to_string())
+}
+
+fn peel_parens_str(snippet: &str) -> &str {
+    let mut s = snippet.trim();
+    while let Some(next) = s.strip_prefix("(").and_then(|suf| suf.strip_suffix(")")) {
+        s = next.trim();
+    }
+    s
+}
+
+#[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"),
+            },
+            // `ptr.offset(0)` is equivalent to `ptr`, so no adjustment is needed
+            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,50 @@
+#![warn(clippy::ptr_offset_by_literal)]
+#![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 32-bit systems.
+        let _ = ptr.add(2_147_483_647);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.sub(2_147_483_648);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.add(5_0);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.sub(5_0);
+        //~^ ptr_offset_by_literal
+
+        macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; }
+        offs!(ptr, 6);
+        offs!(ptr, var);
+    }
+}

tests/ui/ptr_offset_by_literal.rs

@@ -0,0 +1,50 @@
+#![warn(clippy::ptr_offset_by_literal)]
+#![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 32-bit systems.
+        let _ = ptr.offset(2_147_483_647isize);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-2_147_483_648isize);
+        //~^ ptr_offset_by_literal
+
+        let _ = ptr.offset(5_0__isize);
+        //~^ ptr_offset_by_literal
+        let _ = ptr.offset(-5_0__isize);
+        //~^ ptr_offset_by_literal
+
+        macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; }
+        offs!(ptr, 6);
+        offs!(ptr, var);
+    }
+}

tests/ui/ptr_offset_by_literal.stderr

@@ -0,0 +1,141 @@
+error: use of `offset` with zero
+  --> tests/ui/ptr_offset_by_literal.rs:12:17
+   |
+LL |         let _ = ptr.offset(0);
+   |                 ^^^----------
+   |                    |
+   |                    help: remove the call to `offset`
+   |
+   = note: `-D clippy::ptr-offset-by-literal` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::ptr_offset_by_literal)]`
+
+error: use of `offset` with zero
+  --> tests/ui/ptr_offset_by_literal.rs:14:17
+   |
+LL |         let _ = ptr.offset(-0);
+   |                 ^^^-----------
+   |                    |
+   |                    help: remove the call to `offset`
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:17:17
+   |
+LL |         let _ = ptr.offset(5);
+   |                 ^^^^^^^^^^^^^
+   |
+help: use `add` instead
+   |
+LL -         let _ = ptr.offset(5);
+LL +         let _ = ptr.add(5);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:19:17
+   |
+LL |         let _ = ptr.offset(-5);
+   |                 ^^^^^^^^^^^^^^
+   |
+help: use `sub` instead
+   |
+LL -         let _ = ptr.offset(-5);
+LL +         let _ = ptr.sub(5);
+   |
+
+error: use of `wrapping_offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:25:17
+   |
+LL |         let _ = ptr.wrapping_offset(5isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `wrapping_add` instead
+   |
+LL -         let _ = ptr.wrapping_offset(5isize);
+LL +         let _ = ptr.wrapping_add(5);
+   |
+
+error: use of `wrapping_offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:27:17
+   |
+LL |         let _ = ptr.wrapping_offset(-5isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `wrapping_sub` instead
+   |
+LL -         let _ = ptr.wrapping_offset(-5isize);
+LL +         let _ = ptr.wrapping_sub(5);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:30:17
+   |
+LL |         let _ = ptr.offset(-(5));
+   |                 ^^^^^^^^^^^^^^^^
+   |
+help: use `sub` instead
+   |
+LL -         let _ = ptr.offset(-(5));
+LL +         let _ = ptr.sub(5);
+   |
+
+error: use of `wrapping_offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:32:17
+   |
+LL |         let _ = ptr.wrapping_offset(-(5));
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `wrapping_sub` instead
+   |
+LL -         let _ = ptr.wrapping_offset(-(5));
+LL +         let _ = ptr.wrapping_sub(5);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:36:17
+   |
+LL |         let _ = ptr.offset(2_147_483_647isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `add` instead
+   |
+LL -         let _ = ptr.offset(2_147_483_647isize);
+LL +         let _ = ptr.add(2_147_483_647);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:38:17
+   |
+LL |         let _ = ptr.offset(-2_147_483_648isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `sub` instead
+   |
+LL -         let _ = ptr.offset(-2_147_483_648isize);
+LL +         let _ = ptr.sub(2_147_483_648);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:41:17
+   |
+LL |         let _ = ptr.offset(5_0__isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `add` instead
+   |
+LL -         let _ = ptr.offset(5_0__isize);
+LL +         let _ = ptr.add(5_0);
+   |
+
+error: use of `offset` with a literal
+  --> tests/ui/ptr_offset_by_literal.rs:43:17
+   |
+LL |         let _ = ptr.offset(-5_0__isize);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `sub` instead
+   |
+LL -         let _ = ptr.offset(-5_0__isize);
+LL +         let _ = ptr.sub(5_0);
+   |
+
+error: aborting due to 12 previous errors
+

tests/ui/zero_offset.rs

@@ -1,4 +1,4 @@
-#[allow(clippy::borrow_as_ptr)]
+#[allow(clippy::borrow_as_ptr, clippy::ptr_offset_by_literal)]
 fn main() {
     unsafe {
         let m = &mut () as *mut ();