diff --git a/CHANGELOG.md b/CHANGELOG.md index 26dce1f9265..f0b01742deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index b6a35bb3e03..fb3ae2457e3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 2b4ce6ddfaa..9af34c3a7bf 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -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; @@ -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, @@ -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 { @@ -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>) { + 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)) { @@ -502,13 +539,23 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, 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()); } diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs index e433c5a3b32..338a299740a 100644 --- a/clippy_lints/src/manual_is_ascii_check.rs +++ b/clippy_lints/src/manual_is_ascii_check.rs @@ -74,7 +74,7 @@ enum CharRange { LowerChar, /// 'A'..='Z' | b'A'..=b'Z' UpperChar, - /// AsciiLower | AsciiUpper + /// `AsciiLower` | `AsciiUpper` FullChar, /// '0..=9' Digit, diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index e489899c19e..9b656531957 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -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, }, } diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 27ed5458ce2..1df2d5e4602 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -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>, diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index c4a5e48e855..4837f2858a6 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -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 diff --git a/tests/ui/empty_docs.rs b/tests/ui/empty_docs.rs new file mode 100644 index 00000000000..c47a603e71b --- /dev/null +++ b/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, + } +} diff --git a/tests/ui/empty_docs.stderr b/tests/ui/empty_docs.stderr new file mode 100644 index 00000000000..3f1d071fb13 --- /dev/null +++ b/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 + diff --git a/tests/ui/semicolon_if_nothing_returned.fixed b/tests/ui/semicolon_if_nothing_returned.fixed index cdfa5d9cc78..6c8c835d0e0 100644 --- a/tests/ui/semicolon_if_nothing_returned.fixed +++ b/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; diff --git a/tests/ui/semicolon_if_nothing_returned.rs b/tests/ui/semicolon_if_nothing_returned.rs index 315b7e4f383..2c2e4c02419 100644 --- a/tests/ui/semicolon_if_nothing_returned.rs +++ b/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; diff --git a/tests/ui/semicolon_if_nothing_returned.stderr b/tests/ui/semicolon_if_nothing_returned.stderr index 286cf512ed0..69e434b142c 100644 --- a/tests/ui/semicolon_if_nothing_returned.stderr +++ b/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");` @@ -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());`