From b0e37f3a9a5760ebc7fa75a5e353479bf00ecff8 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 3 Sep 2025 15:53:13 +0200 Subject: [PATCH] add `ptr_offset_by_literal` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 37 +++++ .../src/methods/ptr_offset_by_literal.rs | 151 ++++++++++++++++++ tests/ui/borrow_as_ptr.fixed | 2 +- tests/ui/borrow_as_ptr.rs | 2 +- tests/ui/crashes/ice-4579.rs | 2 +- tests/ui/ptr_offset_by_literal.fixed | 45 ++++++ tests/ui/ptr_offset_by_literal.rs | 45 ++++++ tests/ui/ptr_offset_by_literal.stderr | 141 ++++++++++++++++ tests/ui/zero_offset.rs | 2 +- 11 files changed, 425 insertions(+), 4 deletions(-) create mode 100644 clippy_lints/src/methods/ptr_offset_by_literal.rs create mode 100644 tests/ui/ptr_offset_by_literal.fixed create mode 100644 tests/ui/ptr_offset_by_literal.rs create mode 100644 tests/ui/ptr_offset_by_literal.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index eb2a76a81836..3fa733863640 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1b19a8851edd..f4386bb7d446 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_BY_LITERAL_INFO, crate::methods::PTR_OFFSET_WITH_CAST_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::READONLY_WRITE_LOCK_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d06b0ed64bdc..ff43ea910402 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_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"); diff --git a/clippy_lints/src/methods/ptr_offset_by_literal.rs b/clippy_lints/src/methods/ptr_offset_by_literal.rs new file mode 100644 index 000000000000..2eae69821316 --- /dev/null +++ b/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 { + 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 { + 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"), + } + } +} diff --git a/tests/ui/borrow_as_ptr.fixed b/tests/ui/borrow_as_ptr.fixed index bfe826508f36..a4689a7840ce 100644 --- a/tests/ui/borrow_as_ptr.fixed +++ b/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; diff --git a/tests/ui/borrow_as_ptr.rs b/tests/ui/borrow_as_ptr.rs index ce248f157c6e..d7468f37a3a4 100644 --- a/tests/ui/borrow_as_ptr.rs +++ b/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; diff --git a/tests/ui/crashes/ice-4579.rs b/tests/ui/crashes/ice-4579.rs index 14c8113e315b..a2592e74c66d 100644 --- a/tests/ui/crashes/ice-4579.rs +++ b/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; diff --git a/tests/ui/ptr_offset_by_literal.fixed b/tests/ui/ptr_offset_by_literal.fixed new file mode 100644 index 000000000000..1c4741c7a8c6 --- /dev/null +++ b/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 + } +} diff --git a/tests/ui/ptr_offset_by_literal.rs b/tests/ui/ptr_offset_by_literal.rs new file mode 100644 index 000000000000..461c29bf6ae9 --- /dev/null +++ b/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 + } +} diff --git a/tests/ui/ptr_offset_by_literal.stderr b/tests/ui/ptr_offset_by_literal.stderr new file mode 100644 index 000000000000..edd73f340428 --- /dev/null +++ b/tests/ui/ptr_offset_by_literal.stderr @@ -0,0 +1,141 @@ +error: use of `offset` with zero + --> tests/ui/ptr_offset_by_literal.rs:11: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:13: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:16: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:18: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:24: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:26: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:29: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:31: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:35:17 + | +LL | let _ = ptr.offset(9_223_372_036_854_775_807isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `add` instead + | +LL - let _ = ptr.offset(9_223_372_036_854_775_807isize); +LL + let _ = ptr.add(9_223_372_036_854_775_807); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:37:17 + | +LL | let _ = ptr.offset(-9_223_372_036_854_775_808isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `sub` instead + | +LL - let _ = ptr.offset(-9_223_372_036_854_775_808isize); +LL + let _ = ptr.sub(9_223_372_036_854_775_808); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:40: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:42: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 + diff --git a/tests/ui/zero_offset.rs b/tests/ui/zero_offset.rs index bedb09536c53..5a9c3ac9248f 100644 --- a/tests/ui/zero_offset.rs +++ b/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 ();