Skip to content
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

Empty docs #12342

Merged
merged 17 commits into from
Feb 26, 2024
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 @@ -5158,6 +5158,7 @@ Released 2018-09-13
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
[`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute
[`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
[`empty_docs`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_docs
[`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop
[`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
[`empty_enum_variants_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum_variants_with_brackets
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::disallowed_types::DISALLOWED_TYPES_INFO,
crate::doc::DOC_LINK_WITH_QUOTES_INFO,
crate::doc::DOC_MARKDOWN_INFO,
crate::doc::EMPTY_DOCS_INFO,
crate::doc::MISSING_ERRORS_DOC_INFO,
crate::doc::MISSING_PANICS_DOC_INFO,
crate::doc::MISSING_SAFETY_DOC_INFO,
Expand Down
61 changes: 54 additions & 7 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use rustc_middle::hir::nested_filter;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_resolve::rustdoc::{
add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment,
add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, span_of_fragments,
DocFragment,
};
use rustc_session::impl_lint_pass;
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -338,6 +339,30 @@ declare_clippy_lint! {
"suspicious usage of (outer) doc comments"
}

declare_clippy_lint! {
/// ### What it does
/// Detects documentation that is empty.
/// ### Why is this bad?
/// Empty docs clutter code without adding value, reducing readability and maintainability.
/// ### Example
/// ```no_run
/// ///
/// fn returns_true() -> bool {
/// true
/// }
/// ```
/// Use instead:
/// ```no_run
/// fn returns_true() -> bool {
/// true
/// }
/// ```
#[clippy::version = "1.78.0"]
pub EMPTY_DOCS,
suspicious,
"docstrings exist but documentation is empty"
}

#[derive(Clone)]
pub struct Documentation {
valid_idents: FxHashSet<String>,
Expand All @@ -364,7 +389,8 @@ impl_lint_pass!(Documentation => [
NEEDLESS_DOCTEST_MAIN,
TEST_ATTR_IN_DOCTEST,
UNNECESSARY_SAFETY_DOC,
SUSPICIOUS_DOC_COMMENTS
SUSPICIOUS_DOC_COMMENTS,
EMPTY_DOCS,
]);

impl<'tcx> LateLintPass<'tcx> for Documentation {
Expand All @@ -373,11 +399,22 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_variant(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::Variant<'tcx>) {
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
let attrs = cx.tcx.hir().attrs(variant.hir_id);
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_field_def(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::FieldDef<'tcx>) {
let attrs = cx.tcx.hir().attrs(variant.hir_id);
check_attrs(cx, &self.valid_idents, attrs);
}

fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
return;
};

match item.kind {
hir::ItemKind::Fn(ref sig, _, body_id) => {
if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
Expand Down Expand Up @@ -502,13 +539,23 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
suspicious_doc_comments::check(cx, attrs);

let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true);
let mut doc = String::new();
for fragment in &fragments {
add_doc_fragment(&mut doc, fragment);
}
let mut doc = fragments.iter().fold(String::new(), |mut acc, fragment| {
add_doc_fragment(&mut acc, fragment);
acc
});
doc.pop();

if doc.is_empty() {
if doc.trim().is_empty() {
if let Some(span) = span_of_fragments(&fragments) {
span_lint_and_help(
cx,
EMPTY_DOCS,
span,
"empty doc comment",
None,
"consider removing or filling it",
);
}
return Some(DocHeaders::default());
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_is_ascii_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ enum CharRange {
LowerChar,
/// 'A'..='Z' | b'A'..=b'Z'
UpperChar,
/// AsciiLower | AsciiUpper
/// `AsciiLower` | `AsciiUpper`
FullChar,
/// '0..=9'
Digit,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ enum OffendingFilterExpr<'tcx> {
},
/// `.filter(|enum| matches!(enum, Enum::A(_)))`
Matches {
/// The DefId of the variant being matched
/// The `DefId` of the variant being matched
variant_def_id: hir::def_id::DefId,
},
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ impl QuestionMark {
enum IfBlockType<'hir> {
/// An `if x.is_xxx() { a } else { b } ` expression.
///
/// Contains: caller (x), caller_type, call_sym (is_xxx), if_then (a), if_else (b)
/// Contains: `caller (x), caller_type, call_sym (is_xxx), if_then (a), if_else (b)`
IfIs(&'hir Expr<'hir>, Ty<'hir>, Symbol, &'hir Expr<'hir>),
/// An `if let Xxx(a) = b { c } else { d }` expression.
///
/// Contains: let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c),
/// if_else (d)
/// Contains: `let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c),
/// if_else (d)`
IfLet(
Res,
Ty<'hir>,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ declare_lint_pass!(SlowVectorInit => [SLOW_VECTOR_INITIALIZATION]);
/// assigned to a variable. For example, `let mut vec = Vec::with_capacity(0)` or
/// `vec = Vec::with_capacity(0)`
struct VecAllocation<'tcx> {
/// HirId of the variable
/// `HirId` of the variable
local_id: HirId,

/// Reference to the expression which allocates the vector
Expand Down
67 changes: 67 additions & 0 deletions tests/ui/empty_docs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#![allow(unused)]
#![warn(clippy::empty_docs)]
mod outer {
//!

/// this is a struct
struct Bananas {
/// count
count: usize,
}

///
enum Warn {
///
A,
B,
}

enum DontWarn {
/// i
A,
B,
}

#[doc = ""]
fn warn_about_this() {}

#[doc = ""]
#[doc = ""]
fn this_doesn_warn() {}

#[doc = "a fine function"]
fn this_is_fine() {}

///
mod inner {
///
fn dont_warn_inner_outer() {
//!w
}

fn this_is_ok() {
//!
//! inside the function
}

fn warn() {
/*! */
}

fn dont_warn() {
/*! dont warn me */
}

trait NoDoc {
///
fn some() {}
}
}

union Unite {
/// lint y
x: i32,
///
y: i32,
}
}
77 changes: 77 additions & 0 deletions tests/ui/empty_docs.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
error: empty doc comment
--> tests/ui/empty_docs.rs:4:5
|
LL | //!
| ^^^
|
= help: consider removing or filling it
= note: `-D clippy::empty-docs` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::empty_docs)]`

error: empty doc comment
--> tests/ui/empty_docs.rs:12:5
|
LL | ///
| ^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:14:9
|
LL | ///
| ^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:25:5
|
LL | #[doc = ""]
| ^^^^^^^^^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:28:5
|
LL | / #[doc = ""]
LL | | #[doc = ""]
| |_______________^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:35:5
|
LL | ///
| ^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:48:13
|
LL | /*! */
| ^^^^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:56:13
|
LL | ///
| ^^^
|
= help: consider removing or filling it

error: empty doc comment
--> tests/ui/empty_docs.rs:64:9
|
LL | ///
| ^^^
|
= help: consider removing or filling it

error: aborting due to 9 previous errors

7 changes: 6 additions & 1 deletion tests/ui/semicolon_if_nothing_returned.fixed
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
//@aux-build:proc_macro_attr.rs

#![warn(clippy::semicolon_if_nothing_returned)]
#![allow(clippy::redundant_closure, clippy::uninlined_format_args, clippy::needless_late_init)]
#![allow(
clippy::redundant_closure,
clippy::uninlined_format_args,
clippy::needless_late_init,
clippy::empty_docs
)]

#[macro_use]
extern crate proc_macro_attr;
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/semicolon_if_nothing_returned.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
//@aux-build:proc_macro_attr.rs

#![warn(clippy::semicolon_if_nothing_returned)]
#![allow(clippy::redundant_closure, clippy::uninlined_format_args, clippy::needless_late_init)]
#![allow(
clippy::redundant_closure,
clippy::uninlined_format_args,
clippy::needless_late_init,
clippy::empty_docs
)]

#[macro_use]
extern crate proc_macro_attr;
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/semicolon_if_nothing_returned.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: consider adding a `;` to the last statement for consistent formatting
--> tests/ui/semicolon_if_nothing_returned.rs:13:5
--> tests/ui/semicolon_if_nothing_returned.rs:18:5
|
LL | println!("Hello")
| ^^^^^^^^^^^^^^^^^ help: add a `;` here: `println!("Hello");`
Expand All @@ -8,25 +8,25 @@ LL | println!("Hello")
= help: to override `-D warnings` add `#[allow(clippy::semicolon_if_nothing_returned)]`

error: consider adding a `;` to the last statement for consistent formatting
--> tests/ui/semicolon_if_nothing_returned.rs:17:5
--> tests/ui/semicolon_if_nothing_returned.rs:22:5
|
LL | get_unit()
| ^^^^^^^^^^ help: add a `;` here: `get_unit();`

error: consider adding a `;` to the last statement for consistent formatting
--> tests/ui/semicolon_if_nothing_returned.rs:22:5
--> tests/ui/semicolon_if_nothing_returned.rs:27:5
|
LL | y = x + 1
| ^^^^^^^^^ help: add a `;` here: `y = x + 1;`

error: consider adding a `;` to the last statement for consistent formatting
--> tests/ui/semicolon_if_nothing_returned.rs:28:9
--> tests/ui/semicolon_if_nothing_returned.rs:33:9
|
LL | hello()
| ^^^^^^^ help: add a `;` here: `hello();`

error: consider adding a `;` to the last statement for consistent formatting
--> tests/ui/semicolon_if_nothing_returned.rs:39:9
--> tests/ui/semicolon_if_nothing_returned.rs:44:9
|
LL | ptr::drop_in_place(s.as_mut_ptr())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());`
Expand Down
Loading