Dogfood fixes
This commit is contained in:
Philipp Krones
parent
567b65e537
commit
9de86f40d7
3 changed files with 114 additions and 115 deletions
|
|
@ -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>,
|
||||
|
|
|
|||
|
|
@ -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)| {
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue