diff --git a/CHANGELOG.md b/CHANGELOG.md index 748e283edffb..ad4430c45906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6798,6 +6798,7 @@ Released 2018-09-13 [`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive [`manual_noop_waker`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_noop_waker +[`manual_offset_from_unsigned`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_offset_from_unsigned [`manual_ok_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c164241673a3..0d29b150497f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -312,6 +312,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO, crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO, crate::manual_noop_waker::MANUAL_NOOP_WAKER_INFO, + crate::manual_offset_from_unsigned::MANUAL_OFFSET_FROM_UNSIGNED_INFO, crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO, crate::manual_pop_if::MANUAL_POP_IF_INFO, crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 72ee5cca0397..f886d91f67a2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -210,6 +210,7 @@ mod manual_let_else; mod manual_main_separator_str; mod manual_non_exhaustive; mod manual_noop_waker; +mod manual_offset_from_unsigned; mod manual_option_as_slice; mod manual_pop_if; mod manual_range_patterns; @@ -867,6 +868,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))), Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))), Box::new(|_| Box::new(byte_char_slices::ByteCharSlice)), + Box::new(|_| Box::new(manual_offset_from_unsigned::ManualOffsetFromUnsigned::default())), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_lints/src/manual_offset_from_unsigned.rs b/clippy_lints/src/manual_offset_from_unsigned.rs new file mode 100644 index 000000000000..1a4a780b413b --- /dev/null +++ b/clippy_lints/src/manual_offset_from_unsigned.rs @@ -0,0 +1,103 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::msrvs; +use clippy_utils::msrvs::Msrv; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::impl_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for patterns like `unsafe { ptr1.offset_from(ptr2) } as usize` and suggests + /// replacing them with `unsafe { ptr1.offset_from_unsigned(ptr2) }`. + /// + /// ### Why is this bad? + /// `offset_from_unsigned` specifically mentions the offset needing to be unsigned as + /// its additional safety precondition. Using it makes the code's intent clearer and + /// avoids the manual cast. + /// + /// ### Example + /// ```rust,no_run + /// # let (ptr1, ptr2) = (std::ptr::null::(), std::ptr::null::()); + /// unsafe { + /// let _ = ptr2.offset_from(ptr1) as usize; + /// } + /// ``` + /// + /// Use instead: + /// ```rust,no_run + /// # let (ptr1, ptr2) = (std::ptr::null::(), std::ptr::null::()); + /// unsafe { + /// let _ = ptr2.offset_from_unsigned(ptr1); + /// } + /// ``` + #[clippy::version = "1.87.0"] + pub MANUAL_OFFSET_FROM_UNSIGNED, + complexity, + "suggests using `offset_from_unsigned` instead of `offset_from(...) as usize`" +} + +impl_lint_pass!(ManualOffsetFromUnsigned => [MANUAL_OFFSET_FROM_UNSIGNED]); + +#[derive(Default)] +pub struct ManualOffsetFromUnsigned { + pub msrv: Msrv, +} + +impl<'tcx> LateLintPass<'tcx> for ManualOffsetFromUnsigned { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if !self.msrv.meets(cx, msrvs::PTR_OFFSET_FROM_UNSIGNED) { + return; + } + + if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind + && let cast_to_ty = cx.typeck_results().node_type(cast_ty.hir_id) + && cast_to_ty.is_usize() + { + let inner_expr = peel_blocks(cast_expr); + + if let ExprKind::MethodCall(path, receiver, [arg], _) = inner_expr.kind + && let method_name = path.ident.name.as_str() + && (method_name == "offset_from" || method_name == "byte_offset_from") + { + let receiver_ty = cx.typeck_results().expr_ty(receiver); + + let is_ptr = matches!(receiver_ty.kind(), ty::RawPtr(..)); + let is_non_null = receiver_ty + .ty_adt_def() + .is_some_and(|adt| cx.tcx.is_diagnostic_item(sym::NonNull, adt.did())); + + if is_ptr || is_non_null { + let mut app = Applicability::MaybeIncorrect; + let receiver_snip = snippet_with_applicability(cx, receiver.span, "..", &mut app); + let arg_snip = snippet_with_applicability(cx, arg.span, "..", &mut app); + + let msg = format!("manual conversion from `{method_name}` to `usize`"); + let sugg = format!("{receiver_snip}.{method_name}_unsigned({arg_snip})"); + + span_lint_and_sugg(cx, MANUAL_OFFSET_FROM_UNSIGNED, expr.span, msg, "use", sugg, app); + } + } + } + } +} + +fn peel_blocks<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> { + match expr.kind { + ExprKind::Block(block, _) => { + if let Some(final_expr) = block.expr { + peel_blocks(final_expr) + } else if block.stmts.len() == 1 + && let rustc_hir::StmtKind::Expr(stmt_expr) = block.stmts[0].kind + { + peel_blocks(stmt_expr) + } else { + expr + } + }, + _ => expr, + } +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index a56e729c70bb..3345739223bf 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -26,7 +26,8 @@ msrv_aliases! { 1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF } 1,91,0 { DURATION_FROM_MINUTES_HOURS } 1,88,0 { LET_CHAINS } - 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST } + 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST, + PTR_OFFSET_FROM_UNSIGNED } 1,86,0 { VEC_POP_IF } 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL, WAKER_NOOP } 1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR } diff --git a/manual_offset_from_unsigned b/manual_offset_from_unsigned new file mode 100755 index 000000000000..4ad8d390aab6 Binary files /dev/null and b/manual_offset_from_unsigned differ diff --git a/tests/ui/manual_offset_from_unsigned.fixed b/tests/ui/manual_offset_from_unsigned.fixed new file mode 100644 index 000000000000..30524aa0c71e --- /dev/null +++ b/tests/ui/manual_offset_from_unsigned.fixed @@ -0,0 +1,29 @@ +#![warn(clippy::manual_offset_from_unsigned)] +#![allow(unused_unsafe, clippy::unnecessary_cast)] + +use std::ptr::NonNull; + +fn main() { + let s = "hello world"; + let ptr1 = s.as_ptr(); + let ptr2 = unsafe { ptr1.add(1) }; + + unsafe { + let _ = ptr2.offset_from_unsigned(ptr1); + //~^ ERROR: manual conversion from `offset_from` to `usize` + + let _ = ptr2.byte_offset_from_unsigned(ptr1); + //~^ ERROR: manual conversion from `byte_offset_from` to `usize` + + let _ = ptr2.offset_from_unsigned(ptr1); + //~^ ERROR: manual conversion from `offset_from` to `usize` + + let nn1 = NonNull::new_unchecked(ptr1 as *mut u8); + let nn2 = NonNull::new_unchecked(ptr2 as *mut u8); + let _ = nn2.offset_from_unsigned(nn1); + //~^ ERROR: manual conversion from `offset_from` to `usize` + } + + let _ = unsafe { ptr2.offset_from(ptr1) }; + let _ = unsafe { ptr2.offset_from(ptr1) } as isize; +} diff --git a/tests/ui/manual_offset_from_unsigned.rs b/tests/ui/manual_offset_from_unsigned.rs new file mode 100644 index 000000000000..6132f3214a56 --- /dev/null +++ b/tests/ui/manual_offset_from_unsigned.rs @@ -0,0 +1,29 @@ +#![warn(clippy::manual_offset_from_unsigned)] +#![allow(unused_unsafe, clippy::unnecessary_cast)] + +use std::ptr::NonNull; + +fn main() { + let s = "hello world"; + let ptr1 = s.as_ptr(); + let ptr2 = unsafe { ptr1.add(1) }; + + unsafe { + let _ = ptr2.offset_from(ptr1) as usize; + //~^ ERROR: manual conversion from `offset_from` to `usize` + + let _ = ptr2.byte_offset_from(ptr1) as usize; + //~^ ERROR: manual conversion from `byte_offset_from` to `usize` + + let _ = { ptr2.offset_from(ptr1) } as usize; + //~^ ERROR: manual conversion from `offset_from` to `usize` + + let nn1 = NonNull::new_unchecked(ptr1 as *mut u8); + let nn2 = NonNull::new_unchecked(ptr2 as *mut u8); + let _ = nn2.offset_from(nn1) as usize; + //~^ ERROR: manual conversion from `offset_from` to `usize` + } + + let _ = unsafe { ptr2.offset_from(ptr1) }; + let _ = unsafe { ptr2.offset_from(ptr1) } as isize; +} diff --git a/tests/ui/manual_offset_from_unsigned.stderr b/tests/ui/manual_offset_from_unsigned.stderr new file mode 100644 index 000000000000..7e1e09f5ad1d --- /dev/null +++ b/tests/ui/manual_offset_from_unsigned.stderr @@ -0,0 +1,29 @@ +error: manual conversion from `offset_from` to `usize` + --> tests/ui/manual_offset_from_unsigned.rs:12:17 + | +LL | let _ = ptr2.offset_from(ptr1) as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `ptr2.offset_from_unsigned(ptr1)` + | + = note: `-D clippy::manual-offset-from-unsigned` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_offset_from_unsigned)]` + +error: manual conversion from `byte_offset_from` to `usize` + --> tests/ui/manual_offset_from_unsigned.rs:15:17 + | +LL | let _ = ptr2.byte_offset_from(ptr1) as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `ptr2.byte_offset_from_unsigned(ptr1)` + +error: manual conversion from `offset_from` to `usize` + --> tests/ui/manual_offset_from_unsigned.rs:18:17 + | +LL | let _ = { ptr2.offset_from(ptr1) } as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `ptr2.offset_from_unsigned(ptr1)` + +error: manual conversion from `offset_from` to `usize` + --> tests/ui/manual_offset_from_unsigned.rs:23:17 + | +LL | let _ = nn2.offset_from(nn1) as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `nn2.offset_from_unsigned(nn1)` + +error: aborting due to 4 previous errors +