user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(ptr): split each lint into a separate module (#16001)

Browse source 
changelog: none
This commit is contained in:
Jason Newcomb 2025-11-01 05:37:59 +00:00 • committed by GitHub
commit a2c13c1153
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 490 additions and 434 deletions
Download patch file Download diff file Expand all files Collapse all files

49
clippy_lints/src/ptr/cmp_null.rs Normal file
View file

@ -0,0 +1,49 @@
use super::CMP_NULL;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::res::{MaybeDef, MaybeResPath};
use clippy_utils::sugg::Sugg;
use clippy_utils::{is_lint_allowed, sym};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: BinOpKind,
l: &Expr<'_>,
r: &Expr<'_>,
) -> bool {
let non_null_path_snippet = match (
is_lint_allowed(cx, CMP_NULL, expr.hir_id),
is_null_path(cx, l),
is_null_path(cx, r),
) {
(false, true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_paren(),
(false, false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_paren(),
_ => return false,
};
let invert = if op == BinOpKind::Eq { "" } else { "!" };
span_lint_and_sugg(
cx,
CMP_NULL,
expr.span,
"comparing with null is better expressed by the `.is_null()` method",
"try",
format!("{invert}{non_null_path_snippet}.is_null()",),
Applicability::MachineApplicable,
);
true
}
fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let ExprKind::Call(pathexp, []) = expr.kind {
matches!(
pathexp.basic_res().opt_diag_name(cx),
Some(sym::ptr_null | sym::ptr_null_mut)
)
} else {
false
}
}

202
clippy_lints/src/ptr/mod.rs Normal file
View file

@ -0,0 +1,202 @@
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, ImplItemKind, ItemKind, Node, TraitFn, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
mod cmp_null;
mod mut_from_ref;
mod ptr_arg;
mod ptr_eq;
declare_clippy_lint! {
/// ### What it does
/// This lint checks for function arguments of type `&String`, `&Vec`,
/// `&PathBuf`, and `Cow<_>`. It will also suggest you replace `.clone()` calls
/// with the appropriate `.to_owned()`/`to_string()` calls.
///
/// ### Why is this bad?
/// Requiring the argument to be of the specific type
/// makes the function less useful for no benefit; slices in the form of `&[T]`
/// or `&str` usually suffice and can be obtained from other types, too.
///
/// ### Known problems
/// There may be `fn(&Vec)`-typed references pointing to your function.
/// If you have them, you will get a compiler error after applying this lint's
/// suggestions. You then have the choice to undo your changes or change the
/// type of the reference.
///
/// Note that if the function is part of your public interface, there may be
/// other crates referencing it, of which you may not be aware. Carefully
/// deprecate the function before applying the lint suggestions in this case.
///
/// ### Example
/// ```ignore
/// fn foo(&Vec<u32>) { .. }
/// ```
///
/// Use instead:
/// ```ignore
/// fn foo(&[u32]) { .. }
/// ```
#[clippy::version = "pre 1.29.0"]
pub PTR_ARG,
style,
"fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively"
}
declare_clippy_lint! {
/// ### What it does
/// This lint checks for equality comparisons with `ptr::null`
///
/// ### Why is this bad?
/// It's easier and more readable to use the inherent
/// `.is_null()`
/// method instead
///
/// ### Example
/// ```rust,ignore
/// use std::ptr;
///
/// if x == ptr::null {
/// // ..
/// }
/// ```
///
/// Use instead:
/// ```rust,ignore
/// if x.is_null() {
/// // ..
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub CMP_NULL,
style,
"comparing a pointer to a null pointer, suggesting to use `.is_null()` instead"
}
declare_clippy_lint! {
/// ### What it does
/// This lint checks for functions that take immutable references and return
/// mutable ones. This will not trigger if no unsafe code exists as there
/// are multiple safe functions which will do this transformation
///
/// To be on the conservative side, if there's at least one mutable
/// reference with the output lifetime, this lint will not trigger.
///
/// ### Why is this bad?
/// Creating a mutable reference which can be repeatably derived from an
/// immutable reference is unsound as it allows creating multiple live
/// mutable references to the same object.
///
/// This [error](https://github.com/rust-lang/rust/issues/39465) actually
/// lead to an interim Rust release 1.15.1.
///
/// ### Known problems
/// This pattern is used by memory allocators to allow allocating multiple
/// objects while returning mutable references to each one. So long as
/// different mutable references are returned each time such a function may
/// be safe.
///
/// ### Example
/// ```ignore
/// fn foo(&Foo) -> &mut Bar { .. }
/// ```
#[clippy::version = "pre 1.29.0"]
pub MUT_FROM_REF,
correctness,
"fns that create mutable refs from immutable ref args"
}
declare_clippy_lint! {
/// ### What it does
/// Use `std::ptr::eq` when applicable
///
/// ### Why is this bad?
/// `ptr::eq` can be used to compare `&T` references
/// (which coerce to `*const T` implicitly) by their address rather than
/// comparing the values they point to.
///
/// ### Example
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(a as *const _ as usize == b as *const _ as usize);
/// ```
/// Use instead:
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(std::ptr::eq(a, b));
/// ```
#[clippy::version = "1.49.0"]
pub PTR_EQ,
style,
"use `std::ptr::eq` when comparing raw pointers"
}
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, PTR_EQ]);
impl<'tcx> LateLintPass<'tcx> for Ptr {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
if let TraitItemKind::Fn(sig, trait_method) = &item.kind {
if matches!(trait_method, TraitFn::Provided(_)) {
// Handled by `check_body`.
return;
}
mut_from_ref::check(cx, sig, None);
ptr_arg::check_trait_item(cx, item.owner_id, sig);
}
}
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'tcx>) {
let mut parents = cx.tcx.hir_parent_iter(body.value.hir_id);
let (item_id, sig, is_trait_item) = match parents.next() {
Some((_, Node::Item(i))) => {
if let ItemKind::Fn { sig, .. } = &i.kind {
(i.owner_id, sig, false)
} else {
return;
}
},
Some((_, Node::ImplItem(i))) => {
if !matches!(parents.next(),
Some((_, Node::Item(i))) if matches!(&i.kind, ItemKind::Impl(i) if i.of_trait.is_none())
) {
return;
}
if let ImplItemKind::Fn(sig, _) = &i.kind {
(i.owner_id, sig, false)
} else {
return;
}
},
Some((_, Node::TraitItem(i))) => {
if let TraitItemKind::Fn(sig, _) = &i.kind {
(i.owner_id, sig, true)
} else {
return;
}
},
_ => return,
};
mut_from_ref::check(cx, sig, Some(body));
ptr_arg::check_body(cx, body, item_id, sig, is_trait_item);
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Binary(op, l, r) = expr.kind
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
{
#[expect(
clippy::collapsible_if,
reason = "the outer `if`s check the HIR, the inner ones run lints"
)]
if !cmp_null::check(cx, expr, op.node, l, r) {
ptr_eq::check(cx, op.node, l, r, expr.span);
}
}
}
}

75
clippy_lints/src/ptr/mut_from_ref.rs Normal file
View file

@ -0,0 +1,75 @@
use super::MUT_FROM_REF;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::contains_unsafe_block;
use rustc_errors::MultiSpan;
use rustc_hir::intravisit::Visitor;
use rustc_hir::{self as hir, Body, FnRetTy, FnSig, GenericArg, Lifetime, Mutability, TyKind};
use rustc_lint::LateContext;
use rustc_span::Span;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'_>, body: Option<&Body<'tcx>>) {
let FnRetTy::Return(ty) = sig.decl.output else { return };
for (out, mutability, out_span) in get_lifetimes(ty) {
if mutability != Some(Mutability::Mut) {
continue;
}
let out_region = cx.tcx.named_bound_var(out.hir_id);
// `None` if one of the types contains `&'a mut T` or `T<'a>`.
// Else, contains all the locations of `&'a T` types.
let args_immut_refs: Option<Vec<Span>> = sig
.decl
.inputs
.iter()
.flat_map(get_lifetimes)
.filter(|&(lt, _, _)| cx.tcx.named_bound_var(lt.hir_id) == out_region)
.map(|(_, mutability, span)| (mutability == Some(Mutability::Not)).then_some(span))
.collect();
if let Some(args_immut_refs) = args_immut_refs
&& !args_immut_refs.is_empty()
&& body.is_none_or(|body| sig.header.is_unsafe() || contains_unsafe_block(cx, body.value))
{
span_lint_and_then(
cx,
MUT_FROM_REF,
out_span,
"mutable borrow from immutable input(s)",
|diag| {
let ms = MultiSpan::from_spans(args_immut_refs);
diag.span_note(ms, "immutable borrow here");
},
);
}
}
}
struct LifetimeVisitor<'tcx> {
result: Vec<(&'tcx Lifetime, Option<Mutability>, Span)>,
}
impl<'tcx> Visitor<'tcx> for LifetimeVisitor<'tcx> {
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx, hir::AmbigArg>) {
if let TyKind::Ref(lt, ref m) = ty.kind {
self.result.push((lt, Some(m.mutbl), ty.span));
}
hir::intravisit::walk_ty(self, ty);
}
fn visit_generic_arg(&mut self, generic_arg: &'tcx GenericArg<'tcx>) {
if let GenericArg::Lifetime(lt) = generic_arg {
self.result.push((lt, None, generic_arg.span()));
}
hir::intravisit::walk_generic_arg(self, generic_arg);
}
}
/// Visit `ty` and collect the all the lifetimes appearing in it, implicit or not.
///
/// The second field of the vector's elements indicate if the lifetime is attached to a
/// shared reference, a mutable reference, or neither.
fn get_lifetimes<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Vec<(&'tcx Lifetime, Option<Mutability>, Span)> {
use hir::intravisit::VisitorExt as _;
let mut visitor = LifetimeVisitor { result: Vec::new() };
visitor.visit_ty_unambig(ty);
visitor.result
}

493
clippy_lints/src/ptr.rs → clippy_lints/src/ptr/ptr_arg.rs
View file

@ -1,24 +1,22 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::res::{MaybeDef, MaybeResPath};
use super::PTR_ARG;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::res::MaybeResPath;
use clippy_utils::source::SpanRangeExt;
use clippy_utils::sugg::Sugg;
use clippy_utils::visitors::contains_unsafe_block;
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, std_or_core, sym};
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, sym};
use hir::LifetimeKind;
use rustc_abi::ExternAbi;
use rustc_errors::{Applicability, MultiSpan};
use rustc_errors::Applicability;
use rustc_hir::hir_id::{HirId, HirIdMap};
use rustc_hir::intravisit::{Visitor, walk_expr};
use rustc_hir::{
self as hir, AnonConst, BinOpKind, BindingMode, Body, Expr, ExprKind, FnRetTy, FnSig, GenericArg, ImplItemKind,
ItemKind, Lifetime, Mutability, Node, Param, PatKind, QPath, TraitFn, TraitItem, TraitItemKind, TyKind,
self as hir, AnonConst, BindingMode, Body, Expr, ExprKind, FnSig, GenericArg, Lifetime, Mutability, Node, OwnerId,
Param, PatKind, QPath, TyKind,
};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::LateContext;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Binder, ClauseKind, ExistentialPredicate, List, PredicateKind, Ty};
use rustc_session::declare_lint_pass;
use rustc_span::Span;
use rustc_span::symbol::Symbol;
use rustc_trait_selection::infer::InferCtxtExt as _;
@ -27,260 +25,65 @@ use std::{fmt, iter};
use crate::vec::is_allowed_vec_method;
declare_clippy_lint! {
/// ### What it does
/// This lint checks for function arguments of type `&String`, `&Vec`,
/// `&PathBuf`, and `Cow<_>`. It will also suggest you replace `.clone()` calls
/// with the appropriate `.to_owned()`/`to_string()` calls.
///
/// ### Why is this bad?
/// Requiring the argument to be of the specific type
/// makes the function less useful for no benefit; slices in the form of `&[T]`
/// or `&str` usually suffice and can be obtained from other types, too.
///
/// ### Known problems
/// There may be `fn(&Vec)`-typed references pointing to your function.
/// If you have them, you will get a compiler error after applying this lint's
/// suggestions. You then have the choice to undo your changes or change the
/// type of the reference.
///
/// Note that if the function is part of your public interface, there may be
/// other crates referencing it, of which you may not be aware. Carefully
/// deprecate the function before applying the lint suggestions in this case.
///
/// ### Example
/// ```ignore
/// fn foo(&Vec<u32>) { .. }
/// ```
///
/// Use instead:
/// ```ignore
/// fn foo(&[u32]) { .. }
/// ```
#[clippy::version = "pre 1.29.0"]
pub PTR_ARG,
style,
"fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively"
}
declare_clippy_lint! {
/// ### What it does
/// This lint checks for equality comparisons with `ptr::null`
///
/// ### Why is this bad?
/// It's easier and more readable to use the inherent
/// `.is_null()`
/// method instead
///
/// ### Example
/// ```rust,ignore
/// use std::ptr;
///
/// if x == ptr::null {
/// // ..
/// }
/// ```
///
/// Use instead:
/// ```rust,ignore
/// if x.is_null() {
/// // ..
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub CMP_NULL,
style,
"comparing a pointer to a null pointer, suggesting to use `.is_null()` instead"
}
declare_clippy_lint! {
/// ### What it does
/// This lint checks for functions that take immutable references and return
/// mutable ones. This will not trigger if no unsafe code exists as there
/// are multiple safe functions which will do this transformation
///
/// To be on the conservative side, if there's at least one mutable
/// reference with the output lifetime, this lint will not trigger.
///
/// ### Why is this bad?
/// Creating a mutable reference which can be repeatably derived from an
/// immutable reference is unsound as it allows creating multiple live
/// mutable references to the same object.
///
/// This [error](https://github.com/rust-lang/rust/issues/39465) actually
/// lead to an interim Rust release 1.15.1.
///
/// ### Known problems
/// This pattern is used by memory allocators to allow allocating multiple
/// objects while returning mutable references to each one. So long as
/// different mutable references are returned each time such a function may
/// be safe.
///
/// ### Example
/// ```ignore
/// fn foo(&Foo) -> &mut Bar { .. }
/// ```
#[clippy::version = "pre 1.29.0"]
pub MUT_FROM_REF,
correctness,
"fns that create mutable refs from immutable ref args"
}
declare_clippy_lint! {
/// ### What it does
/// Use `std::ptr::eq` when applicable
///
/// ### Why is this bad?
/// `ptr::eq` can be used to compare `&T` references
/// (which coerce to `*const T` implicitly) by their address rather than
/// comparing the values they point to.
///
/// ### Example
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(a as *const _ as usize == b as *const _ as usize);
/// ```
/// Use instead:
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(std::ptr::eq(a, b));
/// ```
#[clippy::version = "1.49.0"]
pub PTR_EQ,
style,
"use `std::ptr::eq` when comparing raw pointers"
}
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, PTR_EQ]);
impl<'tcx> LateLintPass<'tcx> for Ptr {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
if let TraitItemKind::Fn(sig, trait_method) = &item.kind {
if matches!(trait_method, TraitFn::Provided(_)) {
// Handled by check body.
return;
}
check_mut_from_ref(cx, sig, None);
if !matches!(sig.header.abi, ExternAbi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
for arg in check_fn_args(
cx,
cx.tcx.fn_sig(item.owner_id).instantiate_identity().skip_binder(),
sig.decl.inputs,
&[],
)
.filter(|arg| arg.mutability() == Mutability::Not)
{
span_lint_hir_and_then(cx, PTR_ARG, arg.emission_id, arg.span, arg.build_msg(), |diag| {
diag.span_suggestion(
arg.span,
"change this to",
format!("{}{}", arg.ref_prefix, arg.deref_ty.display(cx)),
Applicability::Unspecified,
);
});
}
}
pub(super) fn check_body<'tcx>(
cx: &LateContext<'tcx>,
body: &Body<'tcx>,
item_id: OwnerId,
sig: &FnSig<'tcx>,
is_trait_item: bool,
) {
if !matches!(sig.header.abi, ExternAbi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'tcx>) {
let mut parents = cx.tcx.hir_parent_iter(body.value.hir_id);
let (item_id, sig, is_trait_item) = match parents.next() {
Some((_, Node::Item(i))) => {
if let ItemKind::Fn { sig, .. } = &i.kind {
(i.owner_id, sig, false)
} else {
return;
}
},
Some((_, Node::ImplItem(i))) => {
if !matches!(parents.next(),
Some((_, Node::Item(i))) if matches!(&i.kind, ItemKind::Impl(i) if i.of_trait.is_none())
) {
return;
}
if let ImplItemKind::Fn(sig, _) = &i.kind {
(i.owner_id, sig, false)
} else {
return;
}
},
Some((_, Node::TraitItem(i))) => {
if let TraitItemKind::Fn(sig, _) = &i.kind {
(i.owner_id, sig, true)
} else {
return;
}
},
_ => return,
};
let decl = sig.decl;
let sig = cx.tcx.fn_sig(item_id).instantiate_identity().skip_binder();
let lint_args: Vec<_> = check_fn_args(cx, sig, decl.inputs, body.params)
.filter(|arg| !is_trait_item || arg.mutability() == Mutability::Not)
.collect();
let results = check_ptr_arg_usage(cx, body, &lint_args);
check_mut_from_ref(cx, sig, Some(body));
if !matches!(sig.header.abi, ExternAbi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
let decl = sig.decl;
let sig = cx.tcx.fn_sig(item_id).instantiate_identity().skip_binder();
let lint_args: Vec<_> = check_fn_args(cx, sig, decl.inputs, body.params)
.filter(|arg| !is_trait_item || arg.mutability() == Mutability::Not)
.collect();
let results = check_ptr_arg_usage(cx, body, &lint_args);
for (result, args) in iter::zip(&results, &lint_args).filter(|(r, _)| !r.skip) {
span_lint_hir_and_then(cx, PTR_ARG, args.emission_id, args.span, args.build_msg(), |diag| {
diag.multipart_suggestion(
"change this to",
iter::once((args.span, format!("{}{}", args.ref_prefix, args.deref_ty.display(cx))))
.chain(result.replacements.iter().map(|r| {
(
r.expr_span,
format!("{}{}", r.self_span.get_source_text(cx).unwrap(), r.replacement),
)
}))
.collect(),
Applicability::Unspecified,
);
});
}
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Binary(op, l, r) = expr.kind
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
{
let non_null_path_snippet = match (
is_lint_allowed(cx, CMP_NULL, expr.hir_id),
is_null_path(cx, l),
is_null_path(cx, r),
) {
(false, true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_paren(),
(false, false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_paren(),
_ => return check_ptr_eq(cx, expr, op.node, l, r),
};
let invert = if op.node == BinOpKind::Eq { "" } else { "!" };
span_lint_and_sugg(
cx,
CMP_NULL,
expr.span,
"comparing with null is better expressed by the `.is_null()` method",
"try",
format!("{invert}{non_null_path_snippet}.is_null()",),
Applicability::MachineApplicable,
for (result, args) in iter::zip(&results, &lint_args).filter(|(r, _)| !r.skip) {
span_lint_hir_and_then(cx, PTR_ARG, args.emission_id, args.span, args.build_msg(), |diag| {
diag.multipart_suggestion(
"change this to",
iter::once((args.span, format!("{}{}", args.ref_prefix, args.deref_ty.display(cx))))
.chain(result.replacements.iter().map(|r| {
(
r.expr_span,
format!("{}{}", r.self_span.get_source_text(cx).unwrap(), r.replacement),
)
}))
.collect(),
Applicability::Unspecified,
);
}
});
}
}
pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item_id: OwnerId, sig: &FnSig<'tcx>) {
if !matches!(sig.header.abi, ExternAbi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
for arg in check_fn_args(
cx,
cx.tcx.fn_sig(item_id).instantiate_identity().skip_binder(),
sig.decl.inputs,
&[],
)
.filter(|arg| arg.mutability() == Mutability::Not)
{
span_lint_hir_and_then(cx, PTR_ARG, arg.emission_id, arg.span, arg.build_msg(), |diag| {
diag.span_suggestion(
arg.span,
"change this to",
format!("{}{}", arg.ref_prefix, arg.deref_ty.display(cx)),
Applicability::Unspecified,
);
});
}
}
@ -393,10 +196,7 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
hir_tys: &'tcx [hir::Ty<'tcx>],
params: &'tcx [Param<'tcx>],
) -> impl Iterator<Item = PtrArg<'tcx>> + 'cx {
fn_sig
.inputs()
.iter()
.zip(hir_tys.iter())
iter::zip(fn_sig.inputs(), hir_tys)
.enumerate()
.filter_map(move |(i, (ty, hir_ty))| {
if let ty::Ref(_, ty, mutability) = *ty.kind()
@ -499,41 +299,6 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
})
}
fn check_mut_from_ref<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'_>, body: Option<&Body<'tcx>>) {
let FnRetTy::Return(ty) = sig.decl.output else { return };
for (out, mutability, out_span) in get_lifetimes(ty) {
if mutability != Some(Mutability::Mut) {
continue;
}
let out_region = cx.tcx.named_bound_var(out.hir_id);
// `None` if one of the types contains `&'a mut T` or `T<'a>`.
// Else, contains all the locations of `&'a T` types.
let args_immut_refs: Option<Vec<Span>> = sig
.decl
.inputs
.iter()
.flat_map(get_lifetimes)
.filter(|&(lt, _, _)| cx.tcx.named_bound_var(lt.hir_id) == out_region)
.map(|(_, mutability, span)| (mutability == Some(Mutability::Not)).then_some(span))
.collect();
if let Some(args_immut_refs) = args_immut_refs
&& !args_immut_refs.is_empty()
&& body.is_none_or(|body| sig.header.is_unsafe() || contains_unsafe_block(cx, body.value))
{
span_lint_and_then(
cx,
MUT_FROM_REF,
out_span,
"mutable borrow from immutable input(s)",
|diag| {
let ms = MultiSpan::from_spans(args_immut_refs);
diag.span_note(ms, "immutable borrow here");
},
);
}
}
}
#[expect(clippy::too_many_lines)]
fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[PtrArg<'tcx>]) -> Vec<PtrArgResult> {
struct V<'cx, 'tcx> {
@ -658,11 +423,11 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
match param.pat.kind {
PatKind::Binding(BindingMode::NONE, id, ident, None)
if !is_lint_allowed(cx, PTR_ARG, param.hir_id)
// Let's not lint for the current parameter. The user may still intend to mutate
// (or, if not mutate, then perhaps call a method that's not otherwise available
// for) the referenced value behind the parameter with the underscore being only
// temporary.
&& !ident.name.as_str().starts_with('_') =>
// Let's not lint for the current parameter. The user may still intend to mutate
// (or, if not mutate, then perhaps call a method that's not otherwise available
// for) the referenced value behind the parameter with the underscore being only
// temporary.
&& !ident.name.as_str().starts_with('_') =>
{
Some((id, i))
},
@ -708,123 +473,3 @@ fn matches_preds<'tcx>(
.must_apply_modulo_regions(),
})
}
struct LifetimeVisitor<'tcx> {
result: Vec<(&'tcx Lifetime, Option<Mutability>, Span)>,
}
impl<'tcx> Visitor<'tcx> for LifetimeVisitor<'tcx> {
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx, hir::AmbigArg>) {
if let TyKind::Ref(lt, ref m) = ty.kind {
self.result.push((lt, Some(m.mutbl), ty.span));
}
hir::intravisit::walk_ty(self, ty);
}
fn visit_generic_arg(&mut self, generic_arg: &'tcx GenericArg<'tcx>) {
if let GenericArg::Lifetime(lt) = generic_arg {
self.result.push((lt, None, generic_arg.span()));
}
hir::intravisit::walk_generic_arg(self, generic_arg);
}
}
/// Visit `ty` and collect the all the lifetimes appearing in it, implicit or not.
///
/// The second field of the vector's elements indicate if the lifetime is attached to a
/// shared reference, a mutable reference, or neither.
fn get_lifetimes<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Vec<(&'tcx Lifetime, Option<Mutability>, Span)> {
use hir::intravisit::VisitorExt as _;
let mut visitor = LifetimeVisitor { result: Vec::new() };
visitor.visit_ty_unambig(ty);
visitor.result
}
fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let ExprKind::Call(pathexp, []) = expr.kind {
matches!(
pathexp.basic_res().opt_diag_name(cx),
Some(sym::ptr_null | sym::ptr_null_mut)
)
} else {
false
}
}
fn check_ptr_eq<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: BinOpKind,
left: &'tcx Expr<'_>,
right: &'tcx Expr<'_>,
) {
if expr.span.from_expansion() {
return;
}
// Remove one level of usize conversion if any
let (left, right, usize_peeled) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) {
(Some(lhs), Some(rhs)) => (lhs, rhs, true),
_ => (left, right, false),
};
// This lint concerns raw pointers
let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
if !left_ty.is_raw_ptr() || !right_ty.is_raw_ptr() {
return;
}
let ((left_var, left_casts_peeled), (right_var, right_casts_peeled)) =
(peel_raw_casts(cx, left, left_ty), peel_raw_casts(cx, right, right_ty));
if !(usize_peeled || left_casts_peeled || right_casts_peeled) {
return;
}
let mut app = Applicability::MachineApplicable;
let left_snip = Sugg::hir_with_context(cx, left_var, expr.span.ctxt(), "_", &mut app);
let right_snip = Sugg::hir_with_context(cx, right_var, expr.span.ctxt(), "_", &mut app);
{
let Some(top_crate) = std_or_core(cx) else { return };
let invert = if op == BinOpKind::Eq { "" } else { "!" };
span_lint_and_sugg(
cx,
PTR_EQ,
expr.span,
format!("use `{top_crate}::ptr::eq` when comparing raw pointers"),
"try",
format!("{invert}{top_crate}::ptr::eq({left_snip}, {right_snip})"),
app,
);
}
}
// If the given expression is a cast to a usize, return the lhs of the cast
// E.g., `foo as *const _ as usize` returns `foo as *const _`.
fn expr_as_cast_to_usize<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
if !cast_expr.span.from_expansion()
&& cx.typeck_results().expr_ty(cast_expr) == cx.tcx.types.usize
&& let ExprKind::Cast(expr, _) = cast_expr.kind
{
Some(expr)
} else {
None
}
}
// Peel raw casts if the remaining expression can be coerced to it, and whether casts have been
// peeled or not.
fn peel_raw_casts<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expr_ty: Ty<'tcx>) -> (&'tcx Expr<'tcx>, bool) {
if !expr.span.from_expansion()
&& let ExprKind::Cast(inner, _) = expr.kind
&& let ty::RawPtr(target_ty, _) = expr_ty.kind()
&& let inner_ty = cx.typeck_results().expr_ty(inner)
&& let ty::RawPtr(inner_target_ty, _) | ty::Ref(_, inner_target_ty, _) = inner_ty.kind()
&& target_ty == inner_target_ty
{
(peel_raw_casts(cx, inner, inner_ty).0, true)
} else {
(expr, false)
}
}

87
clippy_lints/src/ptr/ptr_eq.rs Normal file
View file

@ -0,0 +1,87 @@
use super::PTR_EQ;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::std_or_core;
use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::Span;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
op: BinOpKind,
left: &'tcx Expr<'_>,
right: &'tcx Expr<'_>,
span: Span,
) {
if span.from_expansion() {
return;
}
// Remove one level of usize conversion if any
let (left, right, usize_peeled) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) {
(Some(lhs), Some(rhs)) => (lhs, rhs, true),
_ => (left, right, false),
};
// This lint concerns raw pointers
let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
if !left_ty.is_raw_ptr() || !right_ty.is_raw_ptr() {
return;
}
let ((left_var, left_casts_peeled), (right_var, right_casts_peeled)) =
(peel_raw_casts(cx, left, left_ty), peel_raw_casts(cx, right, right_ty));
if !(usize_peeled || left_casts_peeled || right_casts_peeled) {
return;
}
let mut app = Applicability::MachineApplicable;
let ctxt = span.ctxt();
let left_snip = Sugg::hir_with_context(cx, left_var, ctxt, "_", &mut app);
let right_snip = Sugg::hir_with_context(cx, right_var, ctxt, "_", &mut app);
{
let Some(top_crate) = std_or_core(cx) else { return };
let invert = if op == BinOpKind::Eq { "" } else { "!" };
span_lint_and_sugg(
cx,
PTR_EQ,
span,
format!("use `{top_crate}::ptr::eq` when comparing raw pointers"),
"try",
format!("{invert}{top_crate}::ptr::eq({left_snip}, {right_snip})"),
app,
);
}
}
// If the given expression is a cast to a usize, return the lhs of the cast
// E.g., `foo as *const _ as usize` returns `foo as *const _`.
fn expr_as_cast_to_usize<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
if !cast_expr.span.from_expansion()
&& cx.typeck_results().expr_ty(cast_expr) == cx.tcx.types.usize
&& let ExprKind::Cast(expr, _) = cast_expr.kind
{
Some(expr)
} else {
None
}
}
// Peel raw casts if the remaining expression can be coerced to it, and whether casts have been
// peeled or not.
fn peel_raw_casts<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expr_ty: Ty<'tcx>) -> (&'tcx Expr<'tcx>, bool) {
if !expr.span.from_expansion()
&& let ExprKind::Cast(inner, _) = expr.kind
&& let ty::RawPtr(target_ty, _) = expr_ty.kind()
&& let inner_ty = cx.typeck_results().expr_ty(inner)
&& let ty::RawPtr(inner_target_ty, _) | ty::Ref(_, inner_target_ty, _) = inner_ty.kind()
&& target_ty == inner_target_ty
{
(peel_raw_casts(cx, inner, inner_ty).0, true)
} else {
(expr, false)
}
}

3
tests/ui/cmp_null.fixed
View file

@ -1,5 +1,4 @@
#![warn(clippy::cmp_null)]
#![allow(unused_mut)]
use std::ptr;
@ -18,7 +17,7 @@ fn main() {
}
let mut y = 0;
let mut m: *mut usize = &mut y;
let m: *mut usize = &mut y;
if m.is_null() {
//~^ cmp_null

3
tests/ui/cmp_null.rs
View file

@ -1,5 +1,4 @@
#![warn(clippy::cmp_null)]
#![allow(unused_mut)]
use std::ptr;
@ -18,7 +17,7 @@ fn main() {
}
let mut y = 0;
let mut m: *mut usize = &mut y;
let m: *mut usize = &mut y;
if m == ptr::null_mut() {
//~^ cmp_null

12
tests/ui/cmp_null.stderr
View file

@ -1,5 +1,5 @@
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:9:8
--> tests/ui/cmp_null.rs:8:8
|
LL | if p == ptr::null() {
| ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
@ -8,31 +8,31 @@ LL | if p == ptr::null() {
= help: to override `-D warnings` add `#[allow(clippy::cmp_null)]`
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:14:8
--> tests/ui/cmp_null.rs:13:8
|
LL | if ptr::null() == p {
| ^^^^^^^^^^^^^^^^ help: try: `p.is_null()`
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:22:8
--> tests/ui/cmp_null.rs:21:8
|
LL | if m == ptr::null_mut() {
| ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:27:8
--> tests/ui/cmp_null.rs:26:8
|
LL | if ptr::null_mut() == m {
| ^^^^^^^^^^^^^^^^^^^^ help: try: `m.is_null()`
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:33:13
--> tests/ui/cmp_null.rs:32:13
|
LL | let _ = x as *const () == ptr::null();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(x as *const ()).is_null()`
error: comparing with null is better expressed by the `.is_null()` method
--> tests/ui/cmp_null.rs:39:19
--> tests/ui/cmp_null.rs:38:19
|
LL | debug_assert!(f != std::ptr::null_mut());
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!f.is_null()`