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 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
20 changes: 20 additions & 0 deletions clippy_lints/src/doc/empty_docs.rs
@@ -0,0 +1,20 @@
use super::EMPTY_DOCS;
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_ast::Attribute;
use rustc_lint::LateContext;
use rustc_resolve::rustdoc::{attrs_to_doc_fragments, span_of_fragments};

// TODO: Adjust the parameters as necessary
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
pub(super) fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true);
if let Some(span) = span_of_fragments(&fragments) {
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
span_lint_and_help(
cx,
EMPTY_DOCS,
span,
"empty doc comment",
None,
"consider removing or filling it",
);
}
}
52 changes: 45 additions & 7 deletions clippy_lints/src/doc/mod.rs
Expand Up @@ -17,7 +17,7 @@ use rustc_hir::{AnonConst, Expr};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_middle::ty::{self};
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
use rustc_resolve::rustdoc::{
add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment,
};
Expand All @@ -27,6 +27,7 @@ use rustc_span::{sym, Span};
use std::ops::Range;
use url::Url;

mod empty_docs;
mod link_with_quotes;
mod markdown;
mod missing_headers;
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?
/// It is unlikely that there is any reason to have empty documentation for an item
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
/// ### Example
/// ```rs
/// ///
/// fn returns_true() -> bool {
/// true
/// }
/// ```
/// Use instead:
/// ```rs
lucarlig marked this conversation as resolved.
Show resolved Hide resolved
/// 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,14 @@ 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() {
empty_docs::check(cx, attrs);
return Some(DocHeaders::default());
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_is_ascii_check.rs
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
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
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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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