Skip to content

Commit

Permalink
refractoring base on review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@J-ZhengLi
J-ZhengLi committed Dec 6, 2023
1 parent c6e1f5b commit 3657f5e
Showing 1 changed file with 38 additions and 93 deletions.
131 changes: 38 additions & 93 deletions clippy_lints/src/undocumented_unsafe_blocks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::walk_span_to_context;
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
use clippy_utils::{get_parent_node, is_lint_allowed};
use hir::OwnerId;
use rustc_data_structures::sync::Lrc;
use rustc_hir as hir;
use rustc_hir::{Block, BlockCheckMode, HirId, ItemKind, Node, OwnerNode, UnsafeSource};
use rustc_hir::{Block, BlockCheckMode, HirId, ItemKind, Node, UnsafeSource};
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -524,25 +525,34 @@ fn item_has_safety_comment(cx: &LateContext<'_>, hir_id: HirId, owner: hir::Owne
has_safety_comment => return has_safety_comment,
}

if owner.span().ctxt() != SyntaxContext::root() {
if owner.span().from_expansion() {
return HasSafetyComment::No;
}
if let Some(parent_node) = get_parent_node(cx.tcx, hir_id) {
let comment_start = match parent_node {
Node::Crate(parent_mod) => {
comment_start_pos_in_parent_context(cx, parent_mod.item_ids, parent_mod.spans.inner_span, owner)
},
Node::Crate(parent_mod) => comment_start_pos(
cx,
parent_mod.item_ids.iter().map(|id| id.owner_id),
parent_mod.spans.inner_span,
owner.def_id(),
),
Node::Item(parent_item) => {
match parent_item.kind {
ItemKind::Mod(parent_mod) => {
comment_start_pos_in_parent_context(cx, parent_mod.item_ids, parent_item.span, owner)
},
ItemKind::Mod(parent_mod) => comment_start_pos(
cx,
parent_mod.item_ids.iter().map(|id| id.owner_id),
parent_item.span,
owner.def_id(),
),
ItemKind::Trait(_, _, _, _, refs) => {
comment_start_pos_in_parent_context(cx, refs, parent_item.span, owner)
},
ItemKind::Impl(hir::Impl { items, .. }) => {
comment_start_pos_in_parent_context(cx, items, parent_item.span, owner)
comment_start_pos(cx, refs.iter().map(|r| r.id.owner_id), parent_item.span, owner.def_id())
},
ItemKind::Impl(hir::Impl { items, .. }) => comment_start_pos(
cx,
items.iter().map(|r| r.id.owner_id),
parent_item.span,
owner.def_id(),
),
// Doesn't support impls in this position. Pretend a comment was found.
_ => return HasSafetyComment::Maybe,
}
Expand Down Expand Up @@ -627,71 +637,20 @@ fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> H
HasSafetyComment::Maybe
}

trait CheckableItemId {
fn owner_id(&self) -> hir::OwnerId;
}

impl CheckableItemId for hir::ItemId {
fn owner_id(&self) -> hir::OwnerId {
self.owner_id
}
}
impl CheckableItemId for hir::ImplItemRef {
fn owner_id(&self) -> hir::OwnerId {
self.id.owner_id
}
}
impl CheckableItemId for hir::TraitItemRef {
fn owner_id(&self) -> hir::OwnerId {
self.id.owner_id
}
}

/// Search and return the starting [`BytePos`] of the comment above an 'item' in its context.
fn comment_start_pos_in_parent_context<T: CheckableItemId>(
fn comment_start_pos<I: Iterator<Item = OwnerId> + DoubleEndedIterator>(
cx: &LateContext<'_>,
ids: &[T],
mut siblings: I,
search_span: Span,
owner: OwnerNode<'_>,
owner_id: OwnerId,
) -> Option<BytePos> {
let map = cx.tcx.hir();
ids.iter().enumerate().find_map(|(idx, id)| {
if id.owner_id() == owner.def_id() {
if idx == 0 {
// mod A { /* comment */ unsafe impl T {} ... }
// ^------------------------------------------^ returns the start of this span
// ^---------------------^ finally checks comments in this range
walk_span_to_context(search_span, SyntaxContext::root()).map(Span::lo)
} else {
// some_item /* comment */ unsafe impl T {}
// ^-------^ returns the end of this span
// ^---------------^ finally checks comments in this range
match owner {
OwnerNode::Item(_) => {
let prev_item = map.item(hir::ItemId {
owner_id: ids[idx - 1].owner_id(),
});
walk_span_to_context(prev_item.span, SyntaxContext::root()).map(Span::lo)
},
OwnerNode::ImplItem(_) => {
let prev_impl_item = map.impl_item(hir::ImplItemId {
owner_id: ids[idx - 1].owner_id(),
});
walk_span_to_context(prev_impl_item.span, SyntaxContext::root()).map(Span::lo)
},
OwnerNode::TraitItem(_) => {
let prev_trait_item = map.trait_item(hir::TraitItemId {
owner_id: ids[idx - 1].owner_id(),
});
walk_span_to_context(prev_trait_item.span, SyntaxContext::root()).map(Span::lo)
},
_ => None,
}
}
} else {
None
}
})
let _ = siblings.rfind(|id| *id == owner_id);
if let Some(prev_sibling_id) = siblings.next_back() {
let prev_sibling_span = cx.tcx.hir().span(prev_sibling_id.into());
walk_span_to_context(prev_sibling_span, SyntaxContext::root()).map(Span::lo)
} else {
walk_span_to_context(search_span, SyntaxContext::root()).map(Span::lo)
}
}

fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> HasSafetyComment {
Expand Down Expand Up @@ -730,13 +689,8 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span

fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
let body = cx.enclosing_body?;
let map = cx.tcx.hir();
let mut span = map.body(body).value.span;
let mut is_const_or_static = false;
for (_, parent_node) in map.parent_iter(body.hir_id) {
for (_, parent_node) in cx.tcx.hir().parent_iter(body.hir_id) {
match parent_node {
Node::Expr(e) => span = e.span,
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::Local(_) => (),
Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
..
Expand All @@ -748,22 +702,13 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
| Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Const(..),
..
}) => is_const_or_static = true,
// Parent is a `mod xxx { .. }``
Node::Item(item) => {
match item.kind {
ItemKind::Mod(_) | ItemKind::Trait(..) | ItemKind::Impl(..) => span = item.span,
_ => (),
}
break;
},
Node::Crate(mod_) if is_const_or_static => {
span = mod_.spans.inner_span;
},
_ => break,
}) => {},
Node::Item(item) => return Some(item.span),
Node::Crate(mod_) => return Some(mod_.spans.inner_span),
_ => {},
}
}
Some(span)
None
}

fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
Expand Down

0 comments on commit 3657f5e

Please sign in to comment.