From e2e23c0045d94cc0d509cbb3c915faae74844ea8 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 9 Feb 2023 16:41:45 -0500 Subject: [PATCH] Add docs and update tests --- clippy_lints/src/lib.rs | 4 +-- clippy_lints/src/missing_doc.rs | 30 ++++++++++--------- clippy_lints/src/utils/conf.rs | 4 +-- .../pub_crate_missing_docs/clippy.toml | 2 +- .../pub_crate_missing_doc.rs | 19 ++++++++++++ .../pub_crate_missing_doc.stderr | 26 +++++++++++++++- .../pub_crate_missing_doc.stdout | 0 .../toml_unknown_key/conf_unknown_key.stderr | 2 +- 8 files changed, 66 insertions(+), 21 deletions(-) create mode 100644 tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stdout diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f5c417fffa8..2d04118c694 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -665,13 +665,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: )) }); let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::>(); - let only_check_missing_docs_in_crate_items = conf.only_check_missing_docs_in_crate_items; + let missing_docs_in_crate_items = conf.missing_docs_in_crate_items; store.register_late_pass(move |_| Box::new(doc::DocMarkdown::new(doc_valid_idents.clone()))); store.register_late_pass(|_| Box::new(neg_multiply::NegMultiply)); store.register_late_pass(|_| Box::new(mem_forget::MemForget)); store.register_late_pass(|_| Box::new(let_if_seq::LetIfSeq)); store.register_late_pass(|_| Box::new(mixed_read_write_in_expression::EvalOrderDependence)); - store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(only_check_missing_docs_in_crate_items))); + store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(missing_docs_in_crate_items))); store.register_late_pass(|_| Box::new(missing_inline::MissingInline)); store.register_late_pass(move |_| Box::new(exhaustive_items::ExhaustiveItems)); store.register_late_pass(|_| Box::new(match_result_ok::MatchResultOk)); diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index 3bce54bf2e9..a5d8a500cca 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -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 hir::def_id::LocalDefId; use if_chain::if_chain; use rustc_ast::ast::{self, MetaItem, MetaItemKind}; use rustc_hir as hir; @@ -35,7 +36,7 @@ } pub struct MissingDoc { - /// FIXME: docs + /// Whether to only check for missing docs in `pub(crate)` items. crate_items_only: bool, /// Stack of whether #[doc(hidden)] is set /// at each level which has lint attributes. @@ -79,6 +80,7 @@ fn has_include(meta: Option) -> bool { fn check_missing_docs_attrs( &self, cx: &LateContext<'_>, + def_id: LocalDefId, attrs: &[ast::Attribute], sp: Span, article: &'static str, @@ -99,6 +101,13 @@ fn check_missing_docs_attrs( return; } + if self.crate_items_only && def_id != CRATE_DEF_ID { + let vis = cx.tcx.visibility(def_id); + if vis != Visibility::Public && vis != Visibility::Restricted(CRATE_DEF_ID.into()) { + return; + } + } + let has_doc = attrs .iter() .any(|a| a.doc_str().is_some() || Self::has_include(a.meta())); @@ -127,17 +136,10 @@ fn exit_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [ast::Attribute]) fn check_crate(&mut self, cx: &LateContext<'tcx>) { let attrs = cx.tcx.hir().attrs(hir::CRATE_HIR_ID); - self.check_missing_docs_attrs(cx, attrs, cx.tcx.def_span(CRATE_DEF_ID), "the", "crate"); + self.check_missing_docs_attrs(cx, CRATE_DEF_ID, attrs, cx.tcx.def_span(CRATE_DEF_ID), "the", "crate"); } fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'_>) { - if self.crate_items_only { - let vis = cx.tcx.visibility(it.owner_id.to_def_id()); - if vis != Visibility::Public && vis != Visibility::Restricted(CRATE_DEF_ID.into()) { - return; - } - } - match it.kind { hir::ItemKind::Fn(..) => { // ignore main() @@ -170,7 +172,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'_>) { let attrs = cx.tcx.hir().attrs(it.hir_id()); if !is_from_proc_macro(cx, it) { - self.check_missing_docs_attrs(cx, attrs, it.span, article, desc); + self.check_missing_docs_attrs(cx, it.owner_id.def_id, attrs, it.span, article, desc); } } @@ -179,7 +181,7 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx hir::Tr let attrs = cx.tcx.hir().attrs(trait_item.hir_id()); if !is_from_proc_macro(cx, trait_item) { - self.check_missing_docs_attrs(cx, attrs, trait_item.span, article, desc); + self.check_missing_docs_attrs(cx, trait_item.owner_id.def_id, attrs, trait_item.span, article, desc); } } @@ -196,7 +198,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl 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, attrs, impl_item.span, article, desc); + self.check_missing_docs_attrs(cx, impl_item.owner_id.def_id, attrs, impl_item.span, article, desc); } } @@ -204,7 +206,7 @@ fn check_field_def(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::FieldDef<'_ if !sf.is_positional() { let attrs = cx.tcx.hir().attrs(sf.hir_id); if !is_from_proc_macro(cx, sf) { - self.check_missing_docs_attrs(cx, attrs, sf.span, "a", "struct field"); + self.check_missing_docs_attrs(cx, sf.def_id, attrs, sf.span, "a", "struct field"); } } } @@ -212,7 +214,7 @@ fn check_field_def(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::FieldDef<'_ 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, attrs, v.span, "a", "variant"); + self.check_missing_docs_attrs(cx, v.def_id, attrs, v.span, "a", "variant"); } } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index f81965b6432..86a17a40930 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -456,8 +456,8 @@ pub(crate) fn get_configuration_metadata() -> Vec { (suppress_restriction_lint_in_const: bool = false), /// Lint: MISSING_DOCS_IN_PRIVATE_ITEMS. /// - /// FIXME: docs - (only_check_missing_docs_in_crate_items: bool = false), + /// Whether to **only** check for missing docmuentation in `pub(crate)` items. + (missing_docs_in_crate_items: bool = false), } /// Search for the configuration file. diff --git a/tests/ui-toml/pub_crate_missing_docs/clippy.toml b/tests/ui-toml/pub_crate_missing_docs/clippy.toml index a2f1fc27da3..ec210a98783 100644 --- a/tests/ui-toml/pub_crate_missing_docs/clippy.toml +++ b/tests/ui-toml/pub_crate_missing_docs/clippy.toml @@ -1 +1 @@ -only-check-missing-docs-in-crate-items = true +missing-docs-in-crate-items = true diff --git a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs index 8eef92d20bf..f2edb737d59 100644 --- a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs +++ b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs @@ -24,6 +24,25 @@ pub(crate) fn sub_crate_no_docs() {} pub(super) fn sub_super_with_docs() {} pub(super) fn sub_super_no_docs() {} } + + /// some docs + pub(crate) struct CrateStructWithDocs { + /// some docs + pub(crate) crate_field_with_docs: (), + pub(crate) crate_field_no_docs: (), + /// some docs + priv_field_with_docs: (), + priv_field_no_docs: (), + } + + pub(crate) struct CrateStructNoDocs { + /// some docs + pub(crate) crate_field_with_docs: (), + pub(crate) crate_field_no_docs: (), + /// some docs + priv_field_with_docs: (), + priv_field_no_docs: (), + } } fn main() { diff --git a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr index 954ef2382f7..89927abd87b 100644 --- a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr +++ b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr @@ -18,5 +18,29 @@ error: missing documentation for a function LL | pub(crate) fn sub_crate_no_docs() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: missing documentation for a struct field + --> $DIR/pub_crate_missing_doc.rs:32:9 + | +LL | pub(crate) crate_field_no_docs: (), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing documentation for a struct + --> $DIR/pub_crate_missing_doc.rs:38:5 + | +LL | / pub(crate) struct CrateStructNoDocs { +LL | | /// some docs +LL | | pub(crate) crate_field_with_docs: (), +LL | | pub(crate) crate_field_no_docs: (), +... | +LL | | priv_field_no_docs: (), +LL | | } + | |_____^ + +error: missing documentation for a struct field + --> $DIR/pub_crate_missing_doc.rs:41:9 + | +LL | pub(crate) crate_field_no_docs: (), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stdout b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stdout new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index f4815df19ca..6a246afac76 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -33,8 +33,8 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie max-struct-bools max-suggested-slice-pattern-length max-trait-bounds + missing-docs-in-crate-items msrv - only-check-missing-docs-in-crate-items pass-by-value-size-limit single-char-binding-names-threshold standard-macro-braces