From abd9deb9f46942e6615187a4a6d515340ceb4cc6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 19 Nov 2023 15:54:02 +0100 Subject: [PATCH 1/2] [`missing_safety_doc`], [`unnecessary_safety_doc`], [`missing_panics_doc`], [`missing_errors_doc`]: Added the [`check-private-items`] configuration to enable lints on private items. [#11842](https://github.com/rust-lang/rust-clippy/pull/11842) --- CHANGELOG.md | 1 + book/src/lint_configuration.md | 13 ++++++++++ clippy_config/src/conf.rs | 4 +++ clippy_lints/src/doc/missing_headers.rs | 14 +++++----- clippy_lints/src/doc/mod.rs | 26 ++++++++++++++++--- clippy_lints/src/lib.rs | 3 ++- .../toml_unknown_key/conf_unknown_key.stderr | 2 ++ 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f5ff168f6a..11a2d1e7ef9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5740,4 +5740,5 @@ Released 2018-09-13 [`absolute-paths-allowed-crates`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-allowed-crates [`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow +[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 841a5b6d0077..eaef074b8c5d 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -791,3 +791,16 @@ for _ in &mut *rmvec {} * [`explicit_iter_loop`](https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop) +## `check-private-items` + + +**Default Value:** `false` + +--- +**Affected lints:** +* [`missing_safety_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc) +* [`unnecessary_safety_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc) +* [`missing_panics_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc) +* [`missing_errors_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc) + + diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 47259776921b..064cb0426b42 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -543,6 +543,10 @@ define_Conf! { /// for _ in &mut *rmvec {} /// ``` (enforce_iter_loop_reborrow: bool = false), + /// Lint: MISSING_SAFETY_DOC, UNNECESSARY_SAFETY_DOC, MISSING_PANICS_DOC, MISSING_ERRORS_DOC + /// + /// Whether to also run the listed lints on private items. + (check_private_items: bool = false), } /// Search for the configuration file. diff --git a/clippy_lints/src/doc/missing_headers.rs b/clippy_lints/src/doc/missing_headers.rs index 703a100ef65c..4cbfa97a8a35 100644 --- a/clippy_lints/src/doc/missing_headers.rs +++ b/clippy_lints/src/doc/missing_headers.rs @@ -15,17 +15,19 @@ pub fn check( headers: DocHeaders, body_id: Option, panic_span: Option, + check_private_items: bool, ) { - if !cx.effective_visibilities.is_exported(owner_id.def_id) { + if !check_private_items && !cx.effective_visibilities.is_exported(owner_id.def_id) { return; // Private functions do not require doc comments } // do not lint if any parent has `#[doc(hidden)]` attribute (#7347) - if cx - .tcx - .hir() - .parent_iter(owner_id.into()) - .any(|(id, _node)| is_doc_hidden(cx.tcx.hir().attrs(id))) + if !check_private_items + && cx + .tcx + .hir() + .parent_iter(owner_id.into()) + .any(|(id, _node)| is_doc_hidden(cx.tcx.hir().attrs(id))) { return; } diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 2b8f0bc503be..607af6ba9052 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -310,13 +310,15 @@ declare_clippy_lint! { pub struct DocMarkdown { valid_idents: FxHashSet, in_trait_impl: bool, + check_private_items: bool, } impl DocMarkdown { - pub fn new(valid_idents: &[String]) -> Self { + 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, } } } @@ -349,7 +351,15 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { 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); + missing_headers::check( + cx, + item.owner_id, + sig, + headers, + Some(body_id), + panic_span, + self.check_private_items, + ); } }, hir::ItemKind::Impl(impl_) => { @@ -387,7 +397,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { }; 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); + missing_headers::check(cx, item.owner_id, sig, headers, None, None, self.check_private_items); } } } @@ -404,7 +414,15 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { 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); + missing_headers::check( + cx, + item.owner_id, + sig, + headers, + Some(body_id), + panic_span, + self.check_private_items, + ); } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 952625c78be1..abe768d50c7a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -563,6 +563,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { vec_box_size_threshold, verbose_bit_mask_threshold, warn_on_all_wildcard_imports, + check_private_items, blacklisted_names: _, cyclomatic_complexity_threshold: _, @@ -746,7 +747,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { avoid_breaking_exported_api, )) }); - store.register_late_pass(move |_| Box::new(doc::DocMarkdown::new(doc_valid_idents))); + store.register_late_pass(move |_| Box::new(doc::DocMarkdown::new(doc_valid_idents, check_private_items))); store.register_late_pass(|_| Box::new(neg_multiply::NegMultiply)); store.register_late_pass(|_| Box::new(let_if_seq::LetIfSeq)); store.register_late_pass(|_| Box::new(mixed_read_write_in_expression::EvalOrderDependence)); 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 2f9eaa5178c4..12828cf9dec5 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -21,6 +21,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect await-holding-invalid-types blacklisted-names cargo-ignore-publish + check-private-items cognitive-complexity-threshold cyclomatic-complexity-threshold disallowed-macros @@ -95,6 +96,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect await-holding-invalid-types blacklisted-names cargo-ignore-publish + check-private-items cognitive-complexity-threshold cyclomatic-complexity-threshold disallowed-macros From 5cdda53e4754e1d9b06a798ce16f74ef0567de26 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 19 Nov 2023 15:54:34 +0100 Subject: [PATCH 2/2] Add ui test for `check_private_items` config --- tests/ui-toml/private-doc-errors/clippy.toml | 1 + tests/ui-toml/private-doc-errors/doc_lints.rs | 54 ++++++++++++++++ .../private-doc-errors/doc_lints.stderr | 64 +++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 tests/ui-toml/private-doc-errors/clippy.toml create mode 100644 tests/ui-toml/private-doc-errors/doc_lints.rs create mode 100644 tests/ui-toml/private-doc-errors/doc_lints.stderr diff --git a/tests/ui-toml/private-doc-errors/clippy.toml b/tests/ui-toml/private-doc-errors/clippy.toml new file mode 100644 index 000000000000..8483b87c650a --- /dev/null +++ b/tests/ui-toml/private-doc-errors/clippy.toml @@ -0,0 +1 @@ +check-private-items = true diff --git a/tests/ui-toml/private-doc-errors/doc_lints.rs b/tests/ui-toml/private-doc-errors/doc_lints.rs new file mode 100644 index 000000000000..ae4c3f84c296 --- /dev/null +++ b/tests/ui-toml/private-doc-errors/doc_lints.rs @@ -0,0 +1,54 @@ +#![deny( + clippy::unnecessary_safety_doc, + clippy::missing_errors_doc, + clippy::missing_panics_doc +)] + +/// This is a private function, skip to match behavior with `missing_safety_doc`. +/// +/// # Safety +/// +/// Boo! +fn you_dont_see_me() { + //~^ ERROR: safe function's docs have unnecessary `# Safety` section + unimplemented!(); +} + +mod private_mod { + /// This is public but unexported function. + /// + /// # Safety + /// + /// Very safe! + pub fn only_crate_wide_accessible() -> Result<(), ()> { + //~^ ERROR: safe function's docs have unnecessary `# Safety` section + //~| ERROR: docs for function returning `Result` missing `# Errors` section + unimplemented!(); + } +} + +pub struct S; + +impl S { + /// Private, fine again to stay consistent with `missing_safety_doc`. + /// + /// # Safety + /// + /// Unnecessary! + fn private(&self) { + //~^ ERROR: safe function's docs have unnecessary `# Safety` section + //~| ERROR: docs for function which may panic missing `# Panics` section + panic!(); + } +} + +#[doc(hidden)] +pub mod __macro { + pub struct T; + impl T { + pub unsafe fn f() {} + //~^ ERROR: unsafe function's docs miss `# Safety` section + } +} + +fn main() {} diff --git a/tests/ui-toml/private-doc-errors/doc_lints.stderr b/tests/ui-toml/private-doc-errors/doc_lints.stderr new file mode 100644 index 000000000000..853367480498 --- /dev/null +++ b/tests/ui-toml/private-doc-errors/doc_lints.stderr @@ -0,0 +1,64 @@ +error: safe function's docs have unnecessary `# Safety` section + --> $DIR/doc_lints.rs:12:1 + | +LL | fn you_dont_see_me() { + | ^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/doc_lints.rs:2:5 + | +LL | clippy::unnecessary_safety_doc, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: safe function's docs have unnecessary `# Safety` section + --> $DIR/doc_lints.rs:23:5 + | +LL | pub fn only_crate_wide_accessible() -> Result<(), ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: docs for function returning `Result` missing `# Errors` section + --> $DIR/doc_lints.rs:23:5 + | +LL | pub fn only_crate_wide_accessible() -> Result<(), ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/doc_lints.rs:3:5 + | +LL | clippy::missing_errors_doc, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: safe function's docs have unnecessary `# Safety` section + --> $DIR/doc_lints.rs:38:5 + | +LL | fn private(&self) { + | ^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/doc_lints.rs:38:5 + | +LL | fn private(&self) { + | ^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/doc_lints.rs:41:9 + | +LL | panic!(); + | ^^^^^^^^ +note: the lint level is defined here + --> $DIR/doc_lints.rs:4:5 + | +LL | clippy::missing_panics_doc + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unsafe function's docs miss `# Safety` section + --> $DIR/doc_lints.rs:49:9 + | +LL | pub unsafe fn f() {} + | ^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::missing-safety-doc` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::missing_safety_doc)]` + +error: aborting due to 6 previous errors +