Skip to content

Commit

Permalink
Auto merge of rust-lang#11842 - GuillaumeGomez:check_private_items-co…
Browse files Browse the repository at this point in the history
…nfig, r=blyxyas

Add new `check_private_items` config

Fixes rust-lang#11742.

It adds a new configuration which allows for `MISSING_SAFETY_DOC`, `UNNECESSARY_SAFETY_DOC`, `MISSING_PANICS_DOC` and `MISSING_ERRORS_DOC` lints to run on private items.

changelog: [`missing_safety_doc`], [`unnecessary_safety_doc`], [`missing_panics_doc`], [`missing_errors_doc`]: Added the [`check-private-items`] configuration to enable lints on private items.
[rust-lang#11842](rust-lang#11842)

r? `@blyxyas`
  • Loading branch information
bors committed Nov 21, 2023
2 parents 72c0d80 + 5cdda53 commit 4d56366
Show file tree
Hide file tree
Showing 10 changed files with 171 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
<!-- end autogenerated links to configuration documentation -->
13 changes: 13 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)


4 changes: 4 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 8 additions & 6 deletions clippy_lints/src/doc/missing_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@ pub fn check(
headers: DocHeaders,
body_id: Option<BodyId>,
panic_span: Option<Span>,
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;
}
Expand Down
26 changes: 22 additions & 4 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,15 @@ declare_clippy_lint! {
pub struct DocMarkdown {
valid_idents: FxHashSet<String>,
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,
}
}
}
Expand Down Expand Up @@ -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_) => {
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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,
);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: _,
Expand Down Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/private-doc-errors/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
check-private-items = true
54 changes: 54 additions & 0 deletions tests/ui-toml/private-doc-errors/doc_lints.rs
Original file line number Diff line number Diff line change
@@ -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() {}
64 changes: 64 additions & 0 deletions tests/ui-toml/private-doc-errors/doc_lints.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4d56366

Please sign in to comment.