about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2025-08-22 14:26:24 +0200
committerPhilipp Krones <hello@philkrones.com>2025-08-22 14:26:24 +0200
commit9de86f40d7e1a2cbcc308e39fdbc7447d691c527 (patch)
tree670b6e49e46794c62a8793066797014b9dd0d90e
parent567b65e537159d4cca44673df2a64852bf15d3c9 (diff)
downloadrust-9de86f40d7e1a2cbcc308e39fdbc7447d691c527.tar.gz
rust-9de86f40d7e1a2cbcc308e39fdbc7447d691c527.zip
Dogfood fixes
-rw-r--r--clippy_lints/src/undocumented_unsafe_blocks.rs168
-rw-r--r--clippy_lints/src/unnested_or_patterns.rs8
-rw-r--r--clippy_utils/src/ast_utils/mod.rs53
3 files changed, 114 insertions, 115 deletions
diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs
index 9c56e0eb9a1..114d1fb471e 100644
--- a/clippy_lints/src/undocumented_unsafe_blocks.rs
+++ b/clippy_lints/src/undocumented_unsafe_blocks.rs
@@ -202,91 +202,41 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
         };
 
         let item_has_safety_comment = item_has_safety_comment(cx, item);
-        match (&item.kind, item_has_safety_comment) {
-            // lint unsafe impl without safety comment
-            (
-                ItemKind::Impl(Impl {
-                    of_trait: Some(of_trait),
-                    ..
-                }),
-                HasSafetyComment::No,
-            ) if of_trait.safety.is_unsafe() => {
-                if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
-                    && !is_unsafe_from_proc_macro(cx, item.span)
-                {
-                    let source_map = cx.tcx.sess.source_map();
-                    let span = if source_map.is_multiline(item.span) {
-                        source_map.span_until_char(item.span, '\n')
-                    } else {
-                        item.span
-                    };
-
-                    #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
-                    span_lint_and_then(
-                        cx,
-                        UNDOCUMENTED_UNSAFE_BLOCKS,
-                        span,
-                        "unsafe impl missing a safety comment",
-                        |diag| {
-                            diag.help("consider adding a safety comment on the preceding line");
-                        },
-                    );
-                }
-            },
-            // lint safe impl with unnecessary safety comment
-            (
-                ItemKind::Impl(Impl {
-                    of_trait: Some(of_trait),
-                    ..
-                }),
-                HasSafetyComment::Yes(pos),
-            ) if of_trait.safety.is_safe() => {
-                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
-                    let (span, help_span) = mk_spans(pos);
-
-                    span_lint_and_then(
-                        cx,
-                        UNNECESSARY_SAFETY_COMMENT,
-                        span,
-                        "impl has unnecessary safety comment",
-                        |diag| {
-                            diag.span_help(help_span, "consider removing the safety comment");
-                        },
-                    );
-                }
-            },
-            (ItemKind::Impl(_), _) => {},
-            // const and static items only need a safety comment if their body is an unsafe block, lint otherwise
-            (&ItemKind::Const(.., body) | &ItemKind::Static(.., body), HasSafetyComment::Yes(pos)) => {
-                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
-                    let body = cx.tcx.hir_body(body);
-                    if !matches!(
-                        body.value.kind, hir::ExprKind::Block(block, _)
-                        if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
-                    ) {
-                        let (span, help_span) = mk_spans(pos);
-
-                        span_lint_and_then(
-                            cx,
-                            UNNECESSARY_SAFETY_COMMENT,
-                            span,
-                            format!(
-                                "{} has unnecessary safety comment",
-                                cx.tcx.def_descr(item.owner_id.to_def_id()),
-                            ),
-                            |diag| {
-                                diag.span_help(help_span, "consider removing the safety comment");
-                            },
-                        );
-                    }
-                }
-            },
-            // Aside from unsafe impls and consts/statics with an unsafe block, items in general
-            // do not have safety invariants that need to be documented, so lint those.
-            (_, HasSafetyComment::Yes(pos)) => {
-                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
-                    let (span, help_span) = mk_spans(pos);
+        match item_has_safety_comment {
+            HasSafetyComment::Yes(pos) => check_has_safety_comment(cx, item, mk_spans(pos)),
+            HasSafetyComment::No => check_has_no_safety_comment(cx, item),
+            HasSafetyComment::Maybe => {},
+        }
+    }
+}
 
+fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, help_span): (Span, Span)) {
+    match &item.kind {
+        ItemKind::Impl(Impl {
+            of_trait: Some(of_trait),
+            ..
+        }) if of_trait.safety.is_safe() => {
+            if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
+                span_lint_and_then(
+                    cx,
+                    UNNECESSARY_SAFETY_COMMENT,
+                    span,
+                    "impl has unnecessary safety comment",
+                    |diag| {
+                        diag.span_help(help_span, "consider removing the safety comment");
+                    },
+                );
+            }
+        },
+        ItemKind::Impl(_) => {},
+        // const and static items only need a safety comment if their body is an unsafe block, lint otherwise
+        &ItemKind::Const(.., body) | &ItemKind::Static(.., body) => {
+            if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
+                let body = cx.tcx.hir_body(body);
+                if !matches!(
+                    body.value.kind, hir::ExprKind::Block(block, _)
+                    if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
+                ) {
                     span_lint_and_then(
                         cx,
                         UNNECESSARY_SAFETY_COMMENT,
@@ -300,12 +250,56 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
                         },
                     );
                 }
-            },
-            _ => (),
-        }
+            }
+        },
+        // Aside from unsafe impls and consts/statics with an unsafe block, items in general
+        // do not have safety invariants that need to be documented, so lint those.
+        _ => {
+            if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
+                span_lint_and_then(
+                    cx,
+                    UNNECESSARY_SAFETY_COMMENT,
+                    span,
+                    format!(
+                        "{} has unnecessary safety comment",
+                        cx.tcx.def_descr(item.owner_id.to_def_id()),
+                    ),
+                    |diag| {
+                        diag.span_help(help_span, "consider removing the safety comment");
+                    },
+                );
+            }
+        },
     }
 }
+fn check_has_no_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) {
+    if let ItemKind::Impl(Impl {
+        of_trait: Some(of_trait),
+        ..
+    }) = item.kind
+        && of_trait.safety.is_unsafe()
+        && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
+        && !is_unsafe_from_proc_macro(cx, item.span)
+    {
+        let source_map = cx.tcx.sess.source_map();
+        let span = if source_map.is_multiline(item.span) {
+            source_map.span_until_char(item.span, '\n')
+        } else {
+            item.span
+        };
 
+        #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
+        span_lint_and_then(
+            cx,
+            UNDOCUMENTED_UNSAFE_BLOCKS,
+            span,
+            "unsafe impl missing a safety comment",
+            |diag| {
+                diag.help("consider adding a safety comment on the preceding line");
+            },
+        );
+    }
+}
 fn expr_has_unnecessary_safety_comment<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'tcx>,
diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs
index e9ad578da2f..8b278d98a30 100644
--- a/clippy_lints/src/unnested_or_patterns.rs
+++ b/clippy_lints/src/unnested_or_patterns.rs
@@ -284,14 +284,14 @@ fn transform_with_focus_on_idx(alternatives: &mut ThinVec<Box<Pat>>, focus_idx:
             |k, ps1, idx| matches!(
                 k,
                 TupleStruct(qself2, path2, ps2)
-                    if eq_maybe_qself(qself1.as_ref(), qself2.as_ref())
+                    if eq_maybe_qself(qself1.as_deref(), qself2.as_deref())
                        && eq_path(path1, path2) && eq_pre_post(ps1, ps2, idx)
             ),
             |k| always_pat!(k, TupleStruct(_, _, ps) => ps),
         ),
         // Transform a record pattern `S { fp_0, ..., fp_n }`.
         Struct(qself1, path1, fps1, rest1) => {
-            extend_with_struct_pat(qself1.as_ref(), path1, fps1, *rest1, start, alternatives)
+            extend_with_struct_pat(qself1.as_deref(), path1, fps1, *rest1, start, alternatives)
         },
     };
 
@@ -304,7 +304,7 @@ fn transform_with_focus_on_idx(alternatives: &mut ThinVec<Box<Pat>>, focus_idx:
 /// So when we fixate on some `ident_k: pat_k`, we try to find `ident_k` in the other pattern
 /// and check that all `fp_i` where `i ∈ ((0...n) \ k)` between two patterns are equal.
 fn extend_with_struct_pat(
-    qself1: Option<&Box<ast::QSelf>>,
+    qself1: Option<&ast::QSelf>,
     path1: &ast::Path,
     fps1: &mut [ast::PatField],
     rest1: ast::PatFieldsRest,
@@ -319,7 +319,7 @@ fn extend_with_struct_pat(
             |k| {
                 matches!(k, Struct(qself2, path2, fps2, rest2)
                 if rest1 == *rest2 // If one struct pattern has `..` so must the other.
-                && eq_maybe_qself(qself1, qself2.as_ref())
+                && eq_maybe_qself(qself1, qself2.as_deref())
                 && eq_path(path1, path2)
                 && fps1.len() == fps2.len()
                 && fps1.iter().enumerate().all(|(idx_1, fp1)| {
diff --git a/clippy_utils/src/ast_utils/mod.rs b/clippy_utils/src/ast_utils/mod.rs
index 45e955adfdb..40c00568a3b 100644
--- a/clippy_utils/src/ast_utils/mod.rs
+++ b/clippy_utils/src/ast_utils/mod.rs
@@ -41,21 +41,23 @@ pub fn eq_pat(l: &Pat, r: &Pat) -> bool {
             b1 == b2 && eq_id(*i1, *i2) && both(s1.as_deref(), s2.as_deref(), eq_pat)
         },
         (Range(lf, lt, le), Range(rf, rt, re)) => {
-            eq_expr_opt(lf.as_ref(), rf.as_ref())
-                && eq_expr_opt(lt.as_ref(), rt.as_ref())
+            eq_expr_opt(lf.as_deref(), rf.as_deref())
+                && eq_expr_opt(lt.as_deref(), rt.as_deref())
                 && eq_range_end(&le.node, &re.node)
         },
         (Box(l), Box(r))
         | (Ref(l, Mutability::Not), Ref(r, Mutability::Not))
         | (Ref(l, Mutability::Mut), Ref(r, Mutability::Mut)) => eq_pat(l, r),
         (Tuple(l), Tuple(r)) | (Slice(l), Slice(r)) => over(l, r, |l, r| eq_pat(l, r)),
-        (Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
+        (Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
         (TupleStruct(lqself, lp, lfs), TupleStruct(rqself, rp, rfs)) => {
-            eq_maybe_qself(lqself.as_ref(), rqself.as_ref()) && eq_path(lp, rp) && over(lfs, rfs, |l, r| eq_pat(l, r))
+            eq_maybe_qself(lqself.as_deref(), rqself.as_deref())
+                && eq_path(lp, rp)
+                && over(lfs, rfs, |l, r| eq_pat(l, r))
         },
         (Struct(lqself, lp, lfs, lr), Struct(rqself, rp, rfs, rr)) => {
             lr == rr
-                && eq_maybe_qself(lqself.as_ref(), rqself.as_ref())
+                && eq_maybe_qself(lqself.as_deref(), rqself.as_deref())
                 && eq_path(lp, rp)
                 && unordered_over(lfs, rfs, eq_field_pat)
         },
@@ -82,11 +84,11 @@ pub fn eq_field_pat(l: &PatField, r: &PatField) -> bool {
         && over(&l.attrs, &r.attrs, eq_attr)
 }
 
-pub fn eq_qself(l: &Box<QSelf>, r: &Box<QSelf>) -> bool {
+pub fn eq_qself(l: &QSelf, r: &QSelf) -> bool {
     l.position == r.position && eq_ty(&l.ty, &r.ty)
 }
 
-pub fn eq_maybe_qself(l: Option<&Box<QSelf>>, r: Option<&Box<QSelf>>) -> bool {
+pub fn eq_maybe_qself(l: Option<&QSelf>, r: Option<&QSelf>) -> bool {
     match (l, r) {
         (Some(l), Some(r)) => eq_qself(l, r),
         (None, None) => true,
@@ -129,8 +131,8 @@ pub fn eq_generic_arg(l: &GenericArg, r: &GenericArg) -> bool {
     }
 }
 
-pub fn eq_expr_opt(l: Option<&Box<Expr>>, r: Option<&Box<Expr>>) -> bool {
-    both(l, r, |l, r| eq_expr(l, r))
+pub fn eq_expr_opt(l: Option<&Expr>, r: Option<&Expr>) -> bool {
+    both(l, r, eq_expr)
 }
 
 pub fn eq_struct_rest(l: &StructRest, r: &StructRest) -> bool {
@@ -177,7 +179,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
         (Cast(l, lt), Cast(r, rt)) | (Type(l, lt), Type(r, rt)) => eq_expr(l, r) && eq_ty(lt, rt),
         (Let(lp, le, _, _), Let(rp, re, _, _)) => eq_pat(lp, rp) && eq_expr(le, re),
         (If(lc, lt, le), If(rc, rt, re)) => {
-            eq_expr(lc, rc) && eq_block(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref())
+            eq_expr(lc, rc) && eq_block(lt, rt) && eq_expr_opt(le.as_deref(), re.as_deref())
         },
         (While(lc, lt, ll), While(rc, rt, rl)) => {
             eq_label(ll.as_ref(), rl.as_ref()) && eq_expr(lc, rc) && eq_block(lt, rt)
@@ -201,9 +203,11 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
         (Loop(lt, ll, _), Loop(rt, rl, _)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lt, rt),
         (Block(lb, ll), Block(rb, rl)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lb, rb),
         (TryBlock(l), TryBlock(r)) => eq_block(l, r),
-        (Yield(l), Yield(r)) => eq_expr_opt(l.expr(), r.expr()) && l.same_kind(r),
-        (Ret(l), Ret(r)) => eq_expr_opt(l.as_ref(), r.as_ref()),
-        (Break(ll, le), Break(rl, re)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_ref(), re.as_ref()),
+        (Yield(l), Yield(r)) => eq_expr_opt(l.expr().map(Box::as_ref), r.expr().map(Box::as_ref)) && l.same_kind(r),
+        (Ret(l), Ret(r)) => eq_expr_opt(l.as_deref(), r.as_deref()),
+        (Break(ll, le), Break(rl, re)) => {
+            eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_deref(), re.as_deref())
+        },
         (Continue(ll), Continue(rl)) => eq_label(ll.as_ref(), rl.as_ref()),
         (Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => {
             eq_expr(l1, r1) && eq_expr(l2, r2)
@@ -240,13 +244,13 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
         },
         (Gen(lc, lb, lk, _), Gen(rc, rb, rk, _)) => lc == rc && eq_block(lb, rb) && lk == rk,
         (Range(lf, lt, ll), Range(rf, rt, rl)) => {
-            ll == rl && eq_expr_opt(lf.as_ref(), rf.as_ref()) && eq_expr_opt(lt.as_ref(), rt.as_ref())
+            ll == rl && eq_expr_opt(lf.as_deref(), rf.as_deref()) && eq_expr_opt(lt.as_deref(), rt.as_deref())
         },
         (AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re),
-        (Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
+        (Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
         (MacCall(l), MacCall(r)) => eq_mac_call(l, r),
         (Struct(lse), Struct(rse)) => {
-            eq_maybe_qself(lse.qself.as_ref(), rse.qself.as_ref())
+            eq_maybe_qself(lse.qself.as_deref(), rse.qself.as_deref())
                 && eq_path(&lse.path, &rse.path)
                 && eq_struct_rest(&lse.rest, &rse.rest)
                 && unordered_over(&lse.fields, &rse.fields, eq_field)
@@ -278,8 +282,8 @@ pub fn eq_field(l: &ExprField, r: &ExprField) -> bool {
 pub fn eq_arm(l: &Arm, r: &Arm) -> bool {
     l.is_placeholder == r.is_placeholder
         && eq_pat(&l.pat, &r.pat)
-        && eq_expr_opt(l.body.as_ref(), r.body.as_ref())
-        && eq_expr_opt(l.guard.as_ref(), r.guard.as_ref())
+        && eq_expr_opt(l.body.as_deref(), r.body.as_deref())
+        && eq_expr_opt(l.guard.as_deref(), r.guard.as_deref())
         && over(&l.attrs, &r.attrs, eq_attr)
 }
 
@@ -347,7 +351,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
                 safety: rs,
                 define_opaque: _,
             }),
-        ) => eq_id(*li, *ri) && lm == rm && ls == rs && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()),
+        ) => eq_id(*li, *ri) && lm == rm && ls == rs && eq_ty(lt, rt) && eq_expr_opt(le.as_deref(), re.as_deref()),
         (
             Const(box ConstItem {
                 defaultness: ld,
@@ -370,7 +374,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
                 && eq_id(*li, *ri)
                 && eq_generics(lg, rg)
                 && eq_ty(lt, rt)
-                && eq_expr_opt(le.as_ref(), re.as_ref())
+                && eq_expr_opt(le.as_deref(), re.as_deref())
         },
         (
             Fn(box ast::Fn {
@@ -525,7 +529,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool {
                 safety: rs,
                 define_opaque: _,
             }),
-        ) => eq_id(*li, *ri) && eq_ty(lt, rt) && lm == rm && eq_expr_opt(le.as_ref(), re.as_ref()) && ls == rs,
+        ) => eq_id(*li, *ri) && eq_ty(lt, rt) && lm == rm && eq_expr_opt(le.as_deref(), re.as_deref()) && ls == rs,
         (
             Fn(box ast::Fn {
                 defaultness: ld,
@@ -607,7 +611,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool {
                 && eq_id(*li, *ri)
                 && eq_generics(lg, rg)
                 && eq_ty(lt, rt)
-                && eq_expr_opt(le.as_ref(), re.as_ref())
+                && eq_expr_opt(le.as_deref(), re.as_deref())
         },
         (
             Fn(box ast::Fn {
@@ -723,7 +727,8 @@ pub fn eq_fn_header(l: &FnHeader, r: &FnHeader) -> bool {
 pub fn eq_opt_fn_contract(l: &Option<Box<FnContract>>, r: &Option<Box<FnContract>>) -> bool {
     match (l, r) {
         (Some(l), Some(r)) => {
-            eq_expr_opt(l.requires.as_ref(), r.requires.as_ref()) && eq_expr_opt(l.ensures.as_ref(), r.ensures.as_ref())
+            eq_expr_opt(l.requires.as_deref(), r.requires.as_deref())
+                && eq_expr_opt(l.ensures.as_deref(), r.ensures.as_deref())
         },
         (None, None) => true,
         (Some(_), None) | (None, Some(_)) => false,
@@ -841,7 +846,7 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool {
                 && eq_fn_decl(&l.decl, &r.decl)
         },
         (Tup(l), Tup(r)) => over(l, r, |l, r| eq_ty(l, r)),
-        (Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
+        (Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
         (TraitObject(lg, ls), TraitObject(rg, rs)) => ls == rs && over(lg, rg, eq_generic_bound),
         (ImplTrait(_, lg), ImplTrait(_, rg)) => over(lg, rg, eq_generic_bound),
         (Typeof(l), Typeof(r)) => eq_expr(&l.value, &r.value),