Skip to content

Commit

Permalink
Auto merge of #10303 - pvdrz:pub_crate_missing_docs, r=giraffate
Browse files Browse the repository at this point in the history
Add configuration to lint missing docs of `pub(crate)` items

Fixes this: #5736 (comment)

TODO:
- [x] Needs docs
- [x] Needs better names
- [x] Should `pub` items be checked to when this new option is enabled? I'm saying no because `missing_docs` already exists

`@flip1995` I'd like to get some input from you :)

---

changelog: Enhancement: [`missing_docs_in_private_items`]: Added new configuration `missing-docs-in-crate-items` to lint on items visible within the current crate. For example, `pub(crate)` items.
[#10303](#10303)
<!-- changelog_checked -->
  • Loading branch information
bors committed Feb 20, 2023
2 parents 574c8ae + 790f28b commit b1cf1e7
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 10 deletions.
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Please use that command to update the file and do not edit it by hand.
| [ignore-interior-mutability](#ignore-interior-mutability) | `["bytes::Bytes"]` |
| [allow-mixed-uninlined-format-args](#allow-mixed-uninlined-format-args) | `true` |
| [suppress-restriction-lint-in-const](#suppress-restriction-lint-in-const) | `false` |
| [missing-docs-in-crate-items](#missing-docs-in-crate-items) | `false` |

### arithmetic-side-effects-allowed
Suppress checking of the passed type names in all types of operations.
Expand Down Expand Up @@ -540,4 +541,13 @@ if no suggestion can be made.
* [indexing_slicing](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)


### missing-docs-in-crate-items
Whether to **only** check for missing documentation in items visible within the current
crate. For example, `pub(crate)` items.

**Default Value:** `false` (`bool`)

* [missing_docs_in_private_items](https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items)



3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,12 +668,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::<FxHashSet<_>>();
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(|_| Box::new(missing_doc::MissingDoc::new()));
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));
Expand Down
31 changes: 22 additions & 9 deletions clippy_lints/src/missing_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
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;
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::DefIdTree;
use rustc_middle::ty::{DefIdTree, Visibility};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::CRATE_DEF_ID;
use rustc_span::source_map::Span;
Expand All @@ -35,6 +36,9 @@ declare_clippy_lint! {
}

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>,
Expand All @@ -43,14 +47,15 @@ pub struct MissingDoc {
impl Default for MissingDoc {
#[must_use]
fn default() -> Self {
Self::new()
Self::new(false)
}
}

impl MissingDoc {
#[must_use]
pub fn new() -> Self {
pub fn new(crate_items_only: bool) -> Self {
Self {
crate_items_only,
doc_hidden_stack: vec![false],
}
}
Expand All @@ -76,6 +81,7 @@ impl MissingDoc {
fn check_missing_docs_attrs(
&self,
cx: &LateContext<'_>,
def_id: LocalDefId,
attrs: &[ast::Attribute],
sp: Span,
article: &'static str,
Expand All @@ -96,6 +102,13 @@ impl MissingDoc {
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()));
Expand Down Expand Up @@ -124,7 +137,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {

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<'_>) {
Expand Down Expand Up @@ -160,7 +173,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {

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);
}
}

Expand All @@ -169,7 +182,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {

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);
}
}

Expand All @@ -186,23 +199,23 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
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);
}
}

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");
}
}
}

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");
}
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,11 @@ define_Conf! {
/// configuration will cause restriction lints to trigger even
/// if no suggestion can be made.
(suppress_restriction_lint_in_const: bool = false),
/// Lint: MISSING_DOCS_IN_PRIVATE_ITEMS.
///
/// Whether to **only** check for missing documentation in items visible within the current
/// crate. For example, `pub(crate)` items.
(missing_docs_in_crate_items: bool = false),
}

/// Search for the configuration file.
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/pub_crate_missing_docs/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
missing-docs-in-crate-items = true
59 changes: 59 additions & 0 deletions tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//! this is crate
#![allow(missing_docs)]
#![warn(clippy::missing_docs_in_private_items)]

/// this is mod
mod my_mod {
/// some docs
fn priv_with_docs() {}
fn priv_no_docs() {}
/// some docs
pub(crate) fn crate_with_docs() {}
pub(crate) fn crate_no_docs() {}
/// some docs
pub(super) fn super_with_docs() {}
pub(super) fn super_no_docs() {}

mod my_sub {
/// some docs
fn sub_priv_with_docs() {}
fn sub_priv_no_docs() {}
/// some docs
pub(crate) fn sub_crate_with_docs() {}
pub(crate) fn sub_crate_no_docs() {}
/// some 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: (),
}
}

/// some docs
type CrateTypedefWithDocs = String;
type CrateTypedefNoDocs = String;
/// some docs
pub type PubTypedefWithDocs = String;
pub type PubTypedefNoDocs = String;

fn main() {
my_mod::crate_with_docs();
my_mod::crate_no_docs();
}
52 changes: 52 additions & 0 deletions tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
error: missing documentation for a function
--> $DIR/pub_crate_missing_doc.rs:12:5
|
LL | pub(crate) fn crate_no_docs() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::missing-docs-in-private-items` implied by `-D warnings`

error: missing documentation for a function
--> $DIR/pub_crate_missing_doc.rs:15:5
|
LL | pub(super) fn super_no_docs() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a function
--> $DIR/pub_crate_missing_doc.rs:23:9
|
LL | pub(crate) fn sub_crate_no_docs() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a struct field
--> $DIR/pub_crate_missing_doc.rs:33:9
|
LL | pub(crate) crate_field_no_docs: (),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a struct
--> $DIR/pub_crate_missing_doc.rs:39: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:42:9
|
LL | pub(crate) crate_field_no_docs: (),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a type alias
--> $DIR/pub_crate_missing_doc.rs:51:1
|
LL | type CrateTypedefNoDocs = String;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 7 previous errors

1 change: 1 addition & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ 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
pass-by-value-size-limit
single-char-binding-names-threshold
Expand Down

0 comments on commit b1cf1e7

Please sign in to comment.