Skip to content

Add new check_private_items config #11842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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