Skip to content

Commit

Permalink
Auto merge of #12635 - Alexendoo:doc-check-attributes, r=Jarcho
Browse files Browse the repository at this point in the history 
Use `check_attributes` in doc lints

Ensures we catch all the places that doc comments could occur, found one that we were currently missing - docs on `extern` items

changelog: none
  • Loading branch information
@bors
bors committed Apr 12, 2024
2 parents 6b1c828 + d4a8f61 commit 0d84f00
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 90 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/doc/missing_headers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::{DocHeaders, MISSING_ERRORS_DOC, MISSING_PANICS_DOC, MISSING_SAFETY_D
pub fn check(
cx: &LateContext<'_>,
owner_id: OwnerId,
sig: &FnSig<'_>,
sig: FnSig<'_>,
headers: DocHeaders,
body_id: Option<BodyId>,
panic_span: Option<Span>,
Expand Down
149 changes: 61 additions & 88 deletions clippy_lints/src/doc/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::Visitable;
use clippy_utils::{is_entrypoint_fn, method_chain_args};
use clippy_utils::{is_entrypoint_fn, is_trait_impl_item, method_chain_args};
use pulldown_cmark::Event::{
Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text,
};
use pulldown_cmark::Tag::{BlockQuote, CodeBlock, Heading, Item, Link, Paragraph};
use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options};
use rustc_ast::ast::Attribute;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{AnonConst, Expr};
use rustc_hir::{AnonConst, Expr, ImplItemKind, ItemKind, Node, TraitItemKind, Unsafety};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::nested_filter;
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -366,15 +365,13 @@ declare_clippy_lint! {
#[derive(Clone)]
pub struct Documentation {
valid_idents: FxHashSet<String>,
in_trait_impl: bool,
check_private_items: bool,
}

impl Documentation {
pub fn new(valid_idents: &[String], check_private_items: bool) -> Self {
Self {
valid_idents: valid_idents.iter().cloned().collect(),
in_trait_impl: false,
check_private_items,
}
}
Expand All @@ -394,36 +391,72 @@ impl_lint_pass!(Documentation => [
]);

impl<'tcx> LateLintPass<'tcx> for Documentation {
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
let attrs = cx.tcx.hir().attrs(hir::CRATE_HIR_ID);
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_variant(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::Variant<'tcx>) {
let attrs = cx.tcx.hir().attrs(variant.hir_id);
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_field_def(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::FieldDef<'tcx>) {
let attrs = cx.tcx.hir().attrs(variant.hir_id);
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
let attrs = cx.tcx.hir().attrs(item.hir_id());
fn check_attributes(&mut self, cx: &LateContext<'tcx>, attrs: &'tcx [Attribute]) {
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
return;
};

match item.kind {
hir::ItemKind::Fn(ref sig, _, body_id) => {
if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
match cx.tcx.hir_node(cx.last_node_with_lint_attrs) {
Node::Item(item) => match item.kind {
ItemKind::Fn(sig, _, body_id) => {
if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
let body = cx.tcx.hir().body(body_id);

let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
missing_headers::check(
cx,
item.owner_id,
sig,
headers,
Some(body_id),
panic_span,
self.check_private_items,
);
}
},
ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
(false, Unsafety::Unsafe) => span_lint(
cx,
MISSING_SAFETY_DOC,
cx.tcx.def_span(item.owner_id),
"docs for unsafe trait missing `# Safety` section",
),
(true, Unsafety::Normal) => span_lint(
cx,
UNNECESSARY_SAFETY_DOC,
cx.tcx.def_span(item.owner_id),
"docs for safe trait have unnecessary `# Safety` section",
),
_ => (),
},
_ => (),
},
Node::TraitItem(trait_item) => {
if let TraitItemKind::Fn(sig, ..) = trait_item.kind
&& !in_external_macro(cx.tcx.sess, trait_item.span)
{
missing_headers::check(
cx,
trait_item.owner_id,
sig,
headers,
None,
None,
self.check_private_items,
);
}
},
Node::ImplItem(impl_item) => {
if let ImplItemKind::Fn(sig, body_id) = impl_item.kind
&& !in_external_macro(cx.tcx.sess, impl_item.span)
&& !is_trait_impl_item(cx, impl_item.hir_id())
{
let body = cx.tcx.hir().body(body_id);

let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(impl_item.owner_id), body.value);
missing_headers::check(
cx,
item.owner_id,
impl_item.owner_id,
sig,
headers,
Some(body_id),
Expand All @@ -432,67 +465,7 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
);
}
},
hir::ItemKind::Impl(impl_) => {
self.in_trait_impl = impl_.of_trait.is_some();
},
hir::ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
(false, hir::Unsafety::Unsafe) => span_lint(
cx,
MISSING_SAFETY_DOC,
cx.tcx.def_span(item.owner_id),
"docs for unsafe trait missing `# Safety` section",
),
(true, hir::Unsafety::Normal) => span_lint(
cx,
UNNECESSARY_SAFETY_DOC,
cx.tcx.def_span(item.owner_id),
"docs for safe trait have unnecessary `# Safety` section",
),
_ => (),
},
_ => (),
}
}

fn check_item_post(&mut self, _cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
if let hir::ItemKind::Impl { .. } = item.kind {
self.in_trait_impl = false;
}
}

fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
return;
};
if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind {
if !in_external_macro(cx.tcx.sess, item.span) {
missing_headers::check(cx, item.owner_id, sig, headers, None, None, self.check_private_items);
}
}
}

fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
return;
};
if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
return;
}
if let hir::ImplItemKind::Fn(ref sig, body_id) = item.kind {
let body = cx.tcx.hir().body(body_id);

let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
missing_headers::check(
cx,
item.owner_id,
sig,
headers,
Some(body_id),
panic_span,
self.check_private_items,
);
_ => {},
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/doc/doc-fixable.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,8 @@ fn parenthesized_word() {}
/// OSes
/// UXes
fn plural_acronym_test() {}

extern {
/// `foo()`
fn in_extern();
}
5 changes: 5 additions & 0 deletions tests/ui/doc/doc-fixable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,8 @@ fn parenthesized_word() {}
/// OSes
/// UXes
fn plural_acronym_test() {}

extern {
/// foo()
fn in_extern();
}
13 changes: 12 additions & 1 deletion tests/ui/doc/doc-fixable.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,5 +352,16 @@ help: try
LL | /// `ABes`
| ~~~~~~

error: aborting due to 32 previous errors
error: item in documentation is missing backticks
--> tests/ui/doc/doc-fixable.rs:240:9
|
LL | /// foo()
| ^^^^^
|
help: try
|
LL | /// `foo()`
| ~~~~~~~

error: aborting due to 33 previous errors

0 comments on commit 0d84f00

Please sign in to comment.