Skip to content

Commit

Permalink
Auto merge of rust-lang#12433 - J-ZhengLi:issue12197, r=dswij
Browse files Browse the repository at this point in the history 
fix [`missing_docs_in_private_items`] on some proc macros

fixes: rust-lang#12197

---

changelog: [`missing_docs_in_private_items`] support manually search for docs as fallback method
  • Loading branch information
@bors
bors committed Mar 8, 2024
2 parents 93f0a9a + adc91e4 commit 0b4b684
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 24 deletions.
64 changes: 59 additions & 5 deletions clippy_lints/src/missing_doc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use clippy_utils::attrs::is_doc_hidden;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_from_proc_macro;
use clippy_utils::source::snippet_opt;
use rustc_ast::ast::{self, MetaItem, MetaItemKind};
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
Expand All @@ -32,13 +33,22 @@ declare_clippy_lint! {
"detects missing documentation for private members"
}

macro_rules! note_prev_span_then_ret {
($prev_span:expr, $span:expr) => {{
$prev_span = Some($span);
return;
}};
}

pub struct MissingDoc {
/// Whether to **only** check for missing documentation in items visible within the current
/// crate. For example, `pub(crate)` items.
crate_items_only: bool,
/// Stack of whether #[doc(hidden)] is set
/// at each level which has lint attributes.
doc_hidden_stack: Vec<bool>,
/// Used to keep tracking of the previous item, field or variants etc, to get the search span.
prev_span: Option<Span>,
}

impl Default for MissingDoc {
Expand All @@ -54,6 +64,7 @@ impl MissingDoc {
Self {
crate_items_only,
doc_hidden_stack: vec![false],
prev_span: None,
}
}

Expand Down Expand Up @@ -108,7 +119,8 @@ impl MissingDoc {

let has_doc = attrs
.iter()
.any(|a| a.doc_str().is_some() || Self::has_include(a.meta()));
.any(|a| a.doc_str().is_some() || Self::has_include(a.meta()))
|| matches!(self.search_span(sp), Some(span) if span_to_snippet_contains_docs(cx, span));

if !has_doc {
span_lint(
Expand All @@ -119,6 +131,32 @@ impl MissingDoc {
);
}
}

/// Return a span to search for doc comments manually.
///
/// # Example
/// ```ignore
/// fn foo() { ... }
/// ^^^^^^^^^^^^^^^^ prev_span
/// ↑
/// | search_span |
/// ↓
/// fn bar() { ... }
/// ^^^^^^^^^^^^^^^^ cur_span
/// ```
fn search_span(&self, cur_span: Span) -> Option<Span> {
let prev_span = self.prev_span?;
let start_pos = if prev_span.contains(cur_span) {
// In case when the prev_span is an entire struct, or enum,
// and the current span is a field, or variant, we need to search from
// the starting pos of the previous span.
prev_span.lo()
} else {
prev_span.hi()
};
let search_span = cur_span.with_lo(start_pos).with_hi(cur_span.lo());
Some(search_span)
}
}

impl_lint_pass!(MissingDoc => [MISSING_DOCS_IN_PRIVATE_ITEMS]);
Expand All @@ -138,14 +176,18 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
self.check_missing_docs_attrs(cx, CRATE_DEF_ID, attrs, cx.tcx.def_span(CRATE_DEF_ID), "the", "crate");
}

fn check_crate_post(&mut self, _: &LateContext<'tcx>) {
self.prev_span = None;
}

fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'_>) {
match it.kind {
hir::ItemKind::Fn(..) => {
// ignore main()
if it.ident.name == sym::main {
let at_root = cx.tcx.local_parent(it.owner_id.def_id) == CRATE_DEF_ID;
if at_root {
return;
note_prev_span_then_ret!(self.prev_span, it.span);
}
}
},
Expand All @@ -164,7 +206,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
| hir::ItemKind::ForeignMod { .. }
| hir::ItemKind::GlobalAsm(..)
| hir::ItemKind::Impl { .. }
| hir::ItemKind::Use(..) => return,
| hir::ItemKind::Use(..) => note_prev_span_then_ret!(self.prev_span, it.span),
};

let (article, desc) = cx.tcx.article_and_description(it.owner_id.to_def_id());
Expand All @@ -173,6 +215,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
if !is_from_proc_macro(cx, it) {
self.check_missing_docs_attrs(cx, it.owner_id.def_id, attrs, it.span, article, desc);
}
self.prev_span = Some(it.span);
}

fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx hir::TraitItem<'_>) {
Expand All @@ -182,23 +225,25 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
if !is_from_proc_macro(cx, trait_item) {
self.check_missing_docs_attrs(cx, trait_item.owner_id.def_id, attrs, trait_item.span, article, desc);
}
self.prev_span = Some(trait_item.span);
}

fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) {
// If the method is an impl for a trait, don't doc.
if let Some(cid) = cx.tcx.associated_item(impl_item.owner_id).impl_container(cx.tcx) {
if cx.tcx.impl_trait_ref(cid).is_some() {
return;
note_prev_span_then_ret!(self.prev_span, impl_item.span);
}
} else {
return;
note_prev_span_then_ret!(self.prev_span, impl_item.span);
}

let (article, desc) = cx.tcx.article_and_description(impl_item.owner_id.to_def_id());
let attrs = cx.tcx.hir().attrs(impl_item.hir_id());
if !is_from_proc_macro(cx, impl_item) {
self.check_missing_docs_attrs(cx, impl_item.owner_id.def_id, attrs, impl_item.span, article, desc);
}
self.prev_span = Some(impl_item.span);
}

fn check_field_def(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::FieldDef<'_>) {
Expand All @@ -208,12 +253,21 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
self.check_missing_docs_attrs(cx, sf.def_id, attrs, sf.span, "a", "struct field");
}
}
self.prev_span = Some(sf.span);
}

fn check_variant(&mut self, cx: &LateContext<'tcx>, v: &'tcx hir::Variant<'_>) {
let attrs = cx.tcx.hir().attrs(v.hir_id);
if !is_from_proc_macro(cx, v) {
self.check_missing_docs_attrs(cx, v.def_id, attrs, v.span, "a", "variant");
}
self.prev_span = Some(v.span);
}
}

fn span_to_snippet_contains_docs(cx: &LateContext<'_>, search_span: Span) -> bool {
let Some(snippet) = snippet_opt(cx, search_span) else {
return false;
};
snippet.lines().rev().any(|line| line.trim().starts_with("///"))
}
30 changes: 24 additions & 6 deletions tests/ui/auxiliary/proc_macro_attr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use quote::{quote, quote_spanned};
use syn::spanned::Spanned;
use syn::token::Star;
use syn::{
parse_macro_input, parse_quote, FnArg, ImplItem, ItemFn, ItemImpl, ItemTrait, Lifetime, Pat, PatIdent, PatType,
Signature, TraitItem, Type,
parse_macro_input, parse_quote, FnArg, ImplItem, ItemFn, ItemImpl, ItemStruct, ItemTrait, Lifetime, Pat, PatIdent,
PatType, Signature, TraitItem, Type, Visibility,
};

#[proc_macro_attribute]
Expand Down Expand Up @@ -101,9 +101,7 @@ pub fn fake_main(_attr: TokenStream, item: TokenStream) -> TokenStream {
let mut item = parse_macro_input!(item as ItemFn);
let span = item.block.brace_token.span;

if item.sig.asyncness.is_some() {
item.sig.asyncness = None;
}
item.sig.asyncness = None;

let crate_name = quote! { fake_crate };
let block = item.block;
Expand All @@ -128,7 +126,7 @@ pub fn fake_main(_attr: TokenStream, item: TokenStream) -> TokenStream {

#[proc_macro_attribute]
pub fn fake_desugar_await(_args: TokenStream, input: TokenStream) -> TokenStream {
let mut async_fn = syn::parse_macro_input!(input as syn::ItemFn);
let mut async_fn = parse_macro_input!(input as syn::ItemFn);

for stmt in &mut async_fn.block.stmts {
if let syn::Stmt::Expr(syn::Expr::Match(syn::ExprMatch { expr: scrutinee, .. }), _) = stmt {
Expand All @@ -145,3 +143,23 @@ pub fn fake_desugar_await(_args: TokenStream, input: TokenStream) -> TokenStream

quote!(#async_fn).into()
}

#[proc_macro_attribute]
pub fn rewrite_struct(_args: TokenStream, input: TokenStream) -> TokenStream {
let mut item_struct = parse_macro_input!(input as syn::ItemStruct);
// remove struct attributes including doc comments.
item_struct.attrs = vec![];
if let Visibility::Public(token) = item_struct.vis {
// set vis to `pub(crate)` to trigger `missing_docs_in_private_items` lint.
let new_vis: Visibility = syn::parse_quote_spanned!(token.span() => pub(crate));
item_struct.vis = new_vis;
}
if let syn::Fields::Named(fields) = &mut item_struct.fields {
for field in &mut fields.named {
// remove all attributes from fields as well.
field.attrs = vec![];
}
}

quote!(#item_struct).into()
}
12 changes: 12 additions & 0 deletions tests/ui/missing_doc.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@needs-asm-support
//@aux-build: proc_macros.rs
//@aux-build: proc_macro_attr.rs

#![warn(clippy::missing_docs_in_private_items)]
// When denying at the crate level, be sure to not get random warnings from the
Expand All @@ -8,6 +9,8 @@
//! Some garbage docs for the crate here
#![doc = "More garbage"]

#[macro_use]
extern crate proc_macro_attr;
extern crate proc_macros;

use proc_macros::with_span;
Expand Down Expand Up @@ -112,3 +115,12 @@ with_span!(span pub enum FooPm3 { A, B(u32), C { field: u32 }});
with_span!(span pub fn foo_pm() {});
with_span!(span pub static FOO_PM: u32 = 0;);
with_span!(span pub const FOO2_PM: u32 = 0;);

// issue #12197
// Undocumented field originated inside of spanned proc-macro attribute
/// Some dox for struct.
#[rewrite_struct]
pub struct Test {
/// Dox
a: u8,
}
26 changes: 13 additions & 13 deletions tests/ui/missing_doc.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: missing documentation for a type alias
--> tests/ui/missing_doc.rs:16:1
--> tests/ui/missing_doc.rs:19:1
|
LL | type Typedef = String;
| ^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -8,19 +8,19 @@ LL | type Typedef = String;
= help: to override `-D warnings` add `#[allow(clippy::missing_docs_in_private_items)]`

error: missing documentation for a module
--> tests/ui/missing_doc.rs:19:1
--> tests/ui/missing_doc.rs:22:1
|
LL | mod module_no_dox {}
| ^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a function
--> tests/ui/missing_doc.rs:25:1
--> tests/ui/missing_doc.rs:28:1
|
LL | fn foo3() {}
| ^^^^^^^^^^^^

error: missing documentation for an enum
--> tests/ui/missing_doc.rs:39:1
--> tests/ui/missing_doc.rs:42:1
|
LL | / enum Baz {
LL | | BazA { a: isize, b: isize },
Expand All @@ -29,43 +29,43 @@ LL | | }
| |_^

error: missing documentation for a variant
--> tests/ui/missing_doc.rs:40:5
--> tests/ui/missing_doc.rs:43:5
|
LL | BazA { a: isize, b: isize },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a struct field
--> tests/ui/missing_doc.rs:40:12
--> tests/ui/missing_doc.rs:43:12
|
LL | BazA { a: isize, b: isize },
| ^^^^^^^^

error: missing documentation for a struct field
--> tests/ui/missing_doc.rs:40:22
--> tests/ui/missing_doc.rs:43:22
|
LL | BazA { a: isize, b: isize },
| ^^^^^^^^

error: missing documentation for a variant
--> tests/ui/missing_doc.rs:41:5
--> tests/ui/missing_doc.rs:44:5
|
LL | BarB,
| ^^^^

error: missing documentation for a constant
--> tests/ui/missing_doc.rs:65:1
--> tests/ui/missing_doc.rs:68:1
|
LL | const FOO: u32 = 0;
| ^^^^^^^^^^^^^^^^^^^

error: missing documentation for a static
--> tests/ui/missing_doc.rs:74:1
--> tests/ui/missing_doc.rs:77:1
|
LL | static BAR: u32 = 0;
| ^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a module
--> tests/ui/missing_doc.rs:83:1
--> tests/ui/missing_doc.rs:86:1
|
LL | / mod internal_impl {
LL | | /// dox
Expand All @@ -77,13 +77,13 @@ LL | | }
| |_^

error: missing documentation for a function
--> tests/ui/missing_doc.rs:88:5
--> tests/ui/missing_doc.rs:91:5
|
LL | fn undocumented3() {}
| ^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a function
--> tests/ui/missing_doc.rs:94:9
--> tests/ui/missing_doc.rs:97:9
|
LL | fn also_undocumented2() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit 0b4b684

Please sign in to comment.