Skip to content

Commit 9de86f4

Browse files
flip1995flip1995
committed
Dogfood fixes
1 parent 567b65e commit 9de86f4

File tree

3 files changed

+114
-115
lines changed

3 files changed

+114
-115
lines changed

‎clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 81 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -202,91 +202,41 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
202202
};
203203

204204
let item_has_safety_comment = item_has_safety_comment(cx, item);
205-
match (&item.kind, item_has_safety_comment) {
206-
// lint unsafe impl without safety comment
207-
(
208-
ItemKind::Impl(Impl {
209-
of_trait: Some(of_trait),
210-
..
211-
}),
212-
HasSafetyComment::No,
213-
) if of_trait.safety.is_unsafe() => {
214-
if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
215-
&& !is_unsafe_from_proc_macro(cx, item.span)
216-
{
217-
let source_map = cx.tcx.sess.source_map();
218-
let span = if source_map.is_multiline(item.span) {
219-
source_map.span_until_char(item.span, '\n')
220-
} else {
221-
item.span
222-
};
223-
224-
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
225-
span_lint_and_then(
226-
cx,
227-
UNDOCUMENTED_UNSAFE_BLOCKS,
228-
span,
229-
"unsafe impl missing a safety comment",
230-
|diag| {
231-
diag.help("consider adding a safety comment on the preceding line");
232-
},
233-
);
234-
}
235-
},
236-
// lint safe impl with unnecessary safety comment
237-
(
238-
ItemKind::Impl(Impl {
239-
of_trait: Some(of_trait),
240-
..
241-
}),
242-
HasSafetyComment::Yes(pos),
243-
) if of_trait.safety.is_safe() => {
244-
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
245-
let (span, help_span) = mk_spans(pos);
246-
247-
span_lint_and_then(
248-
cx,
249-
UNNECESSARY_SAFETY_COMMENT,
250-
span,
251-
"impl has unnecessary safety comment",
252-
|diag| {
253-
diag.span_help(help_span, "consider removing the safety comment");
254-
},
255-
);
256-
}
257-
},
258-
(ItemKind::Impl(_), _) => {},
259-
// const and static items only need a safety comment if their body is an unsafe block, lint otherwise
260-
(&ItemKind::Const(.., body) | &ItemKind::Static(.., body), HasSafetyComment::Yes(pos)) => {
261-
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
262-
let body = cx.tcx.hir_body(body);
263-
if !matches!(
264-
body.value.kind, hir::ExprKind::Block(block, _)
265-
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
266-
) {
267-
let (span, help_span) = mk_spans(pos);
268-
269-
span_lint_and_then(
270-
cx,
271-
UNNECESSARY_SAFETY_COMMENT,
272-
span,
273-
format!(
274-
"{} has unnecessary safety comment",
275-
cx.tcx.def_descr(item.owner_id.to_def_id()),
276-
),
277-
|diag| {
278-
diag.span_help(help_span, "consider removing the safety comment");
279-
},
280-
);
281-
}
282-
}
283-
},
284-
// Aside from unsafe impls and consts/statics with an unsafe block, items in general
285-
// do not have safety invariants that need to be documented, so lint those.
286-
(_, HasSafetyComment::Yes(pos)) => {
287-
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
288-
let (span, help_span) = mk_spans(pos);
205+
match item_has_safety_comment {
206+
HasSafetyComment::Yes(pos) => check_has_safety_comment(cx, item, mk_spans(pos)),
207+
HasSafetyComment::No => check_has_no_safety_comment(cx, item),
208+
HasSafetyComment::Maybe => {},
209+
}
210+
}
211+
}
289212

213+
fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, help_span): (Span, Span)) {
214+
match &item.kind {
215+
ItemKind::Impl(Impl {
216+
of_trait: Some(of_trait),
217+
..
218+
}) if of_trait.safety.is_safe() => {
219+
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
220+
span_lint_and_then(
221+
cx,
222+
UNNECESSARY_SAFETY_COMMENT,
223+
span,
224+
"impl has unnecessary safety comment",
225+
|diag| {
226+
diag.span_help(help_span, "consider removing the safety comment");
227+
},
228+
);
229+
}
230+
},
231+
ItemKind::Impl(_) => {},
232+
// const and static items only need a safety comment if their body is an unsafe block, lint otherwise
233+
&ItemKind::Const(.., body) | &ItemKind::Static(.., body) => {
234+
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
235+
let body = cx.tcx.hir_body(body);
236+
if !matches!(
237+
body.value.kind, hir::ExprKind::Block(block, _)
238+
if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
239+
) {
290240
span_lint_and_then(
291241
cx,
292242
UNNECESSARY_SAFETY_COMMENT,
@@ -300,12 +250,56 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
300250
},
301251
);
302252
}
303-
},
304-
_ => (),
305-
}
253+
}
254+
},
255+
// Aside from unsafe impls and consts/statics with an unsafe block, items in general
256+
// do not have safety invariants that need to be documented, so lint those.
257+
_ => {
258+
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
259+
span_lint_and_then(
260+
cx,
261+
UNNECESSARY_SAFETY_COMMENT,
262+
span,
263+
format!(
264+
"{} has unnecessary safety comment",
265+
cx.tcx.def_descr(item.owner_id.to_def_id()),
266+
),
267+
|diag| {
268+
diag.span_help(help_span, "consider removing the safety comment");
269+
},
270+
);
271+
}
272+
},
306273
}
307274
}
275+
fn check_has_no_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) {
276+
if let ItemKind::Impl(Impl {
277+
of_trait: Some(of_trait),
278+
..
279+
}) = item.kind
280+
&& of_trait.safety.is_unsafe()
281+
&& !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
282+
&& !is_unsafe_from_proc_macro(cx, item.span)
283+
{
284+
let source_map = cx.tcx.sess.source_map();
285+
let span = if source_map.is_multiline(item.span) {
286+
source_map.span_until_char(item.span, '\n')
287+
} else {
288+
item.span
289+
};
308290

291+
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
292+
span_lint_and_then(
293+
cx,
294+
UNDOCUMENTED_UNSAFE_BLOCKS,
295+
span,
296+
"unsafe impl missing a safety comment",
297+
|diag| {
298+
diag.help("consider adding a safety comment on the preceding line");
299+
},
300+
);
301+
}
302+
}
309303
fn expr_has_unnecessary_safety_comment<'tcx>(
310304
cx: &LateContext<'tcx>,
311305
expr: &'tcx hir::Expr<'tcx>,

‎clippy_lints/src/unnested_or_patterns.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,14 +284,14 @@ fn transform_with_focus_on_idx(alternatives: &mut ThinVec<Box<Pat>>, focus_idx:
284284
|k, ps1, idx| matches!(
285285
k,
286286
TupleStruct(qself2, path2, ps2)
287-
if eq_maybe_qself(qself1.as_ref(), qself2.as_ref())
287+
if eq_maybe_qself(qself1.as_deref(), qself2.as_deref())
288288
&& eq_path(path1, path2) && eq_pre_post(ps1, ps2, idx)
289289
),
290290
|k| always_pat!(k, TupleStruct(_, _, ps) => ps),
291291
),
292292
// Transform a record pattern `S { fp_0, ..., fp_n }`.
293293
Struct(qself1, path1, fps1, rest1) => {
294-
extend_with_struct_pat(qself1.as_ref(), path1, fps1, *rest1, start, alternatives)
294+
extend_with_struct_pat(qself1.as_deref(), path1, fps1, *rest1, start, alternatives)
295295
},
296296
};
297297

@@ -304,7 +304,7 @@ fn transform_with_focus_on_idx(alternatives: &mut ThinVec<Box<Pat>>, focus_idx:
304304
/// So when we fixate on some `ident_k: pat_k`, we try to find `ident_k` in the other pattern
305305
/// and check that all `fp_i` where `i ∈ ((0...n) \ k)` between two patterns are equal.
306306
fn extend_with_struct_pat(
307-
qself1: Option<&Box<ast::QSelf>>,
307+
qself1: Option<&ast::QSelf>,
308308
path1: &ast::Path,
309309
fps1: &mut [ast::PatField],
310310
rest1: ast::PatFieldsRest,
@@ -319,7 +319,7 @@ fn extend_with_struct_pat(
319319
|k| {
320320
matches!(k, Struct(qself2, path2, fps2, rest2)
321321
if rest1 == *rest2 // If one struct pattern has `..` so must the other.
322-
&& eq_maybe_qself(qself1, qself2.as_ref())
322+
&& eq_maybe_qself(qself1, qself2.as_deref())
323323
&& eq_path(path1, path2)
324324
&& fps1.len() == fps2.len()
325325
&& fps1.iter().enumerate().all(|(idx_1, fp1)| {

‎clippy_utils/src/ast_utils/mod.rs

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,23 @@ pub fn eq_pat(l: &Pat, r: &Pat) -> bool {
4141
b1 == b2 && eq_id(*i1, *i2) && both(s1.as_deref(), s2.as_deref(), eq_pat)
4242
},
4343
(Range(lf, lt, le), Range(rf, rt, re)) => {
44-
eq_expr_opt(lf.as_ref(), rf.as_ref())
45-
&& eq_expr_opt(lt.as_ref(), rt.as_ref())
44+
eq_expr_opt(lf.as_deref(), rf.as_deref())
45+
&& eq_expr_opt(lt.as_deref(), rt.as_deref())
4646
&& eq_range_end(&le.node, &re.node)
4747
},
4848
(Box(l), Box(r))
4949
| (Ref(l, Mutability::Not), Ref(r, Mutability::Not))
5050
| (Ref(l, Mutability::Mut), Ref(r, Mutability::Mut)) => eq_pat(l, r),
5151
(Tuple(l), Tuple(r)) | (Slice(l), Slice(r)) => over(l, r, |l, r| eq_pat(l, r)),
52-
(Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
52+
(Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
5353
(TupleStruct(lqself, lp, lfs), TupleStruct(rqself, rp, rfs)) => {
54-
eq_maybe_qself(lqself.as_ref(), rqself.as_ref()) && eq_path(lp, rp) && over(lfs, rfs, |l, r| eq_pat(l, r))
54+
eq_maybe_qself(lqself.as_deref(), rqself.as_deref())
55+
&& eq_path(lp, rp)
56+
&& over(lfs, rfs, |l, r| eq_pat(l, r))
5557
},
5658
(Struct(lqself, lp, lfs, lr), Struct(rqself, rp, rfs, rr)) => {
5759
lr == rr
58-
&& eq_maybe_qself(lqself.as_ref(), rqself.as_ref())
60+
&& eq_maybe_qself(lqself.as_deref(), rqself.as_deref())
5961
&& eq_path(lp, rp)
6062
&& unordered_over(lfs, rfs, eq_field_pat)
6163
},
@@ -82,11 +84,11 @@ pub fn eq_field_pat(l: &PatField, r: &PatField) -> bool {
8284
&& over(&l.attrs, &r.attrs, eq_attr)
8385
}
8486

85-
pub fn eq_qself(l: &Box<QSelf>, r: &Box<QSelf>) -> bool {
87+
pub fn eq_qself(l: &QSelf, r: &QSelf) -> bool {
8688
l.position == r.position && eq_ty(&l.ty, &r.ty)
8789
}
8890

89-
pub fn eq_maybe_qself(l: Option<&Box<QSelf>>, r: Option<&Box<QSelf>>) -> bool {
91+
pub fn eq_maybe_qself(l: Option<&QSelf>, r: Option<&QSelf>) -> bool {
9092
match (l, r) {
9193
(Some(l), Some(r)) => eq_qself(l, r),
9294
(None, None) => true,
@@ -129,8 +131,8 @@ pub fn eq_generic_arg(l: &GenericArg, r: &GenericArg) -> bool {
129131
}
130132
}
131133

132-
pub fn eq_expr_opt(l: Option<&Box<Expr>>, r: Option<&Box<Expr>>) -> bool {
133-
both(l, r, |l, r| eq_expr(l, r))
134+
pub fn eq_expr_opt(l: Option<&Expr>, r: Option<&Expr>) -> bool {
135+
both(l, r, eq_expr)
134136
}
135137

136138
pub fn eq_struct_rest(l: &StructRest, r: &StructRest) -> bool {
@@ -177,7 +179,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
177179
(Cast(l, lt), Cast(r, rt)) | (Type(l, lt), Type(r, rt)) => eq_expr(l, r) && eq_ty(lt, rt),
178180
(Let(lp, le, _, _), Let(rp, re, _, _)) => eq_pat(lp, rp) && eq_expr(le, re),
179181
(If(lc, lt, le), If(rc, rt, re)) => {
180-
eq_expr(lc, rc) && eq_block(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref())
182+
eq_expr(lc, rc) && eq_block(lt, rt) && eq_expr_opt(le.as_deref(), re.as_deref())
181183
},
182184
(While(lc, lt, ll), While(rc, rt, rl)) => {
183185
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 {
201203
(Loop(lt, ll, _), Loop(rt, rl, _)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lt, rt),
202204
(Block(lb, ll), Block(rb, rl)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lb, rb),
203205
(TryBlock(l), TryBlock(r)) => eq_block(l, r),
204-
(Yield(l), Yield(r)) => eq_expr_opt(l.expr(), r.expr()) && l.same_kind(r),
205-
(Ret(l), Ret(r)) => eq_expr_opt(l.as_ref(), r.as_ref()),
206-
(Break(ll, le), Break(rl, re)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_ref(), re.as_ref()),
206+
(Yield(l), Yield(r)) => eq_expr_opt(l.expr().map(Box::as_ref), r.expr().map(Box::as_ref)) && l.same_kind(r),
207+
(Ret(l), Ret(r)) => eq_expr_opt(l.as_deref(), r.as_deref()),
208+
(Break(ll, le), Break(rl, re)) => {
209+
eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_deref(), re.as_deref())
210+
},
207211
(Continue(ll), Continue(rl)) => eq_label(ll.as_ref(), rl.as_ref()),
208212
(Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => {
209213
eq_expr(l1, r1) && eq_expr(l2, r2)
@@ -240,13 +244,13 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
240244
},
241245
(Gen(lc, lb, lk, _), Gen(rc, rb, rk, _)) => lc == rc && eq_block(lb, rb) && lk == rk,
242246
(Range(lf, lt, ll), Range(rf, rt, rl)) => {
243-
ll == rl && eq_expr_opt(lf.as_ref(), rf.as_ref()) && eq_expr_opt(lt.as_ref(), rt.as_ref())
247+
ll == rl && eq_expr_opt(lf.as_deref(), rf.as_deref()) && eq_expr_opt(lt.as_deref(), rt.as_deref())
244248
},
245249
(AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re),
246-
(Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
250+
(Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
247251
(MacCall(l), MacCall(r)) => eq_mac_call(l, r),
248252
(Struct(lse), Struct(rse)) => {
249-
eq_maybe_qself(lse.qself.as_ref(), rse.qself.as_ref())
253+
eq_maybe_qself(lse.qself.as_deref(), rse.qself.as_deref())
250254
&& eq_path(&lse.path, &rse.path)
251255
&& eq_struct_rest(&lse.rest, &rse.rest)
252256
&& unordered_over(&lse.fields, &rse.fields, eq_field)
@@ -278,8 +282,8 @@ pub fn eq_field(l: &ExprField, r: &ExprField) -> bool {
278282
pub fn eq_arm(l: &Arm, r: &Arm) -> bool {
279283
l.is_placeholder == r.is_placeholder
280284
&& eq_pat(&l.pat, &r.pat)
281-
&& eq_expr_opt(l.body.as_ref(), r.body.as_ref())
282-
&& eq_expr_opt(l.guard.as_ref(), r.guard.as_ref())
285+
&& eq_expr_opt(l.body.as_deref(), r.body.as_deref())
286+
&& eq_expr_opt(l.guard.as_deref(), r.guard.as_deref())
283287
&& over(&l.attrs, &r.attrs, eq_attr)
284288
}
285289

@@ -347,7 +351,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
347351
safety: rs,
348352
define_opaque: _,
349353
}),
350-
) => eq_id(*li, *ri) && lm == rm && ls == rs && eq_ty(lt, rt) && eq_expr_opt(le.as_ref(), re.as_ref()),
354+
) => eq_id(*li, *ri) && lm == rm && ls == rs && eq_ty(lt, rt) && eq_expr_opt(le.as_deref(), re.as_deref()),
351355
(
352356
Const(box ConstItem {
353357
defaultness: ld,
@@ -370,7 +374,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
370374
&& eq_id(*li, *ri)
371375
&& eq_generics(lg, rg)
372376
&& eq_ty(lt, rt)
373-
&& eq_expr_opt(le.as_ref(), re.as_ref())
377+
&& eq_expr_opt(le.as_deref(), re.as_deref())
374378
},
375379
(
376380
Fn(box ast::Fn {
@@ -525,7 +529,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool {
525529
safety: rs,
526530
define_opaque: _,
527531
}),
528-
) => eq_id(*li, *ri) && eq_ty(lt, rt) && lm == rm && eq_expr_opt(le.as_ref(), re.as_ref()) && ls == rs,
532+
) => eq_id(*li, *ri) && eq_ty(lt, rt) && lm == rm && eq_expr_opt(le.as_deref(), re.as_deref()) && ls == rs,
529533
(
530534
Fn(box ast::Fn {
531535
defaultness: ld,
@@ -607,7 +611,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool {
607611
&& eq_id(*li, *ri)
608612
&& eq_generics(lg, rg)
609613
&& eq_ty(lt, rt)
610-
&& eq_expr_opt(le.as_ref(), re.as_ref())
614+
&& eq_expr_opt(le.as_deref(), re.as_deref())
611615
},
612616
(
613617
Fn(box ast::Fn {
@@ -723,7 +727,8 @@ pub fn eq_fn_header(l: &FnHeader, r: &FnHeader) -> bool {
723727
pub fn eq_opt_fn_contract(l: &Option<Box<FnContract>>, r: &Option<Box<FnContract>>) -> bool {
724728
match (l, r) {
725729
(Some(l), Some(r)) => {
726-
eq_expr_opt(l.requires.as_ref(), r.requires.as_ref()) && eq_expr_opt(l.ensures.as_ref(), r.ensures.as_ref())
730+
eq_expr_opt(l.requires.as_deref(), r.requires.as_deref())
731+
&& eq_expr_opt(l.ensures.as_deref(), r.ensures.as_deref())
727732
},
728733
(None, None) => true,
729734
(Some(_), None) | (None, Some(_)) => false,
@@ -841,7 +846,7 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool {
841846
&& eq_fn_decl(&l.decl, &r.decl)
842847
},
843848
(Tup(l), Tup(r)) => over(l, r, |l, r| eq_ty(l, r)),
844-
(Path(lq, lp), Path(rq, rp)) => both(lq.as_ref(), rq.as_ref(), eq_qself) && eq_path(lp, rp),
849+
(Path(lq, lp), Path(rq, rp)) => both(lq.as_deref(), rq.as_deref(), eq_qself) && eq_path(lp, rp),
845850
(TraitObject(lg, ls), TraitObject(rg, rs)) => ls == rs && over(lg, rg, eq_generic_bound),
846851
(ImplTrait(_, lg), ImplTrait(_, rg)) => over(lg, rg, eq_generic_bound),
847852
(Typeof(l), Typeof(r)) => eq_expr(&l.value, &r.value),

0 commit comments

Comments
 (0)