diff --git a/CHANGELOG.md b/CHANGELOG.md index 02f3188f8be0..d8f5d5328ac7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4159,6 +4159,7 @@ Released 2018-09-13 [`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec [`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_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr diff --git a/Cargo.toml b/Cargo.toml index e1b15cc49da4..1e80c8506ae9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,13 @@ path = "src/main.rs" name = "clippy-driver" path = "src/driver.rs" +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_driver = { path = "/Users/cgorski/repos/rust/compiler/rustc_driver" } +rustc_errors = { path = "/Users/cgorski/repos/rust/compiler/rustc_errors" } +rustc_interface = { path = "/Users/cgorski/repos/rust/compiler/rustc_interface" } +rustc_session = { path = "/Users/cgorski/repos/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/cgorski/repos/rust/compiler/rustc_span" } + [dependencies] clippy_lints = { path = "clippy_lints" } semver = "1.0" diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 38a87017635b..280aac6584a8 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -8,6 +8,28 @@ license = "MIT OR Apache-2.0" keywords = ["clippy", "lint", "plugin"] edition = "2021" +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_arena = { path = "/Users/cgorski/repos/rust/compiler/rustc_arena" } +rustc_ast = { path = "/Users/cgorski/repos/rust/compiler/rustc_ast" } +rustc_ast_pretty = { path = "/Users/cgorski/repos/rust/compiler/rustc_ast_pretty" } +rustc_data_structures = { path = "/Users/cgorski/repos/rust/compiler/rustc_data_structures" } +rustc_driver = { path = "/Users/cgorski/repos/rust/compiler/rustc_driver" } +rustc_errors = { path = "/Users/cgorski/repos/rust/compiler/rustc_errors" } +rustc_hir = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir" } +rustc_hir_analysis = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir_analysis" } +rustc_hir_pretty = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir_pretty" } +rustc_hir_typeck = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir_typeck" } +rustc_index = { path = "/Users/cgorski/repos/rust/compiler/rustc_index" } +rustc_infer = { path = "/Users/cgorski/repos/rust/compiler/rustc_infer" } +rustc_lexer = { path = "/Users/cgorski/repos/rust/compiler/rustc_lexer" } +rustc_lint = { path = "/Users/cgorski/repos/rust/compiler/rustc_lint" } +rustc_middle = { path = "/Users/cgorski/repos/rust/compiler/rustc_middle" } +rustc_parse = { path = "/Users/cgorski/repos/rust/compiler/rustc_parse" } +rustc_session = { path = "/Users/cgorski/repos/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/cgorski/repos/rust/compiler/rustc_span" } +rustc_target = { path = "/Users/cgorski/repos/rust/compiler/rustc_target" } +rustc_trait_selection = { path = "/Users/cgorski/repos/rust/compiler/rustc_trait_selection" } + [dependencies] cargo_metadata = "0.14" clippy_utils = { path = "../clippy_utils" } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2982460c9cfa..33e92f4c3e81 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -138,6 +138,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::drop_forget_ref::UNDROPPED_MANUALLY_DROPS_INFO, crate::duplicate_mod::DUPLICATE_MOD_INFO, crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO, + crate::empty_docs::EMPTY_DOCS_INFO, crate::empty_drop::EMPTY_DROP_INFO, crate::empty_enum::EMPTY_ENUM_INFO, crate::empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO, diff --git a/clippy_lints/src/empty_docs.rs b/clippy_lints/src/empty_docs.rs new file mode 100644 index 000000000000..64a29c7056a5 --- /dev/null +++ b/clippy_lints/src/empty_docs.rs @@ -0,0 +1,151 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use itertools::Itertools; +use rustc_ast::ast; +use rustc_ast::ast::{Attribute, Block, Item}; +use rustc_ast::token; +use rustc_ast::tokenstream; +use rustc_ast::AttrKind::{DocComment, Normal}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::BytePos; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for empty documentation + /// + /// ### Why is this bad? + /// It is unlikely there is any reason to have empty documentation for an entity + /// + /// ### Example + /// ```rust + /// /// + /// fn returns_true() { + /// true + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn returns_true() { + /// true + /// } + /// ``` + #[clippy::version = "1.68.0"] + pub EMPTY_DOCS, + suspicious, + "docstrings exist but documentation is empty" +} +declare_lint_pass!(EmptyDocs => [EMPTY_DOCS]); + +const MAX_CONTEXT_LEN: BytePos = BytePos(50); + +impl EarlyLintPass for EmptyDocs { + fn check_item(&mut self, ex: &EarlyContext<'_>, item: &Item) { + if item.span.from_expansion() { + return; + } + self.process_attributes(ex, item.span, &item.attrs.to_vec()); + self.process_into_item(ex, &item); + } +} + +impl EmptyDocs { + fn process_into_item(self, ex: &EarlyContext<'_>, item: &Item) { + match &item.kind { + ast::ItemKind::Struct(ast::VariantData::Struct(field_list, _), _) => { + for field_def in field_list { + self.process_attributes(ex, field_def.span, &field_def.attrs.to_vec()); + } + }, + ast::ItemKind::Union(ast::VariantData::Struct(field_list, _), _) => { + for field_def in field_list { + self.process_attributes(ex, field_def.span, &field_def.attrs.to_vec()); + } + }, + ast::ItemKind::Enum(enum_def, _) => { + for variant in &enum_def.variants { + self.process_attributes(ex, variant.span, &variant.attrs.to_vec()); + } + }, + ast::ItemKind::Fn(fn_def) => { + if let Some(block) = &fn_def.body { + self.process_into_block(ex, &block); + } + }, + _ => {}, + } + } + + fn process_into_block(self, ex: &EarlyContext<'_>, block: &Block) { + for statement in &block.stmts { + let statement_span = statement.span; + match &statement.kind { + ast::StmtKind::Local(local) => { + self.process_attributes(ex, statement_span, &local.attrs.to_vec()); + }, + _ => {}, + } + } + } + + fn process_attributes(self, ex: &EarlyContext<'_>, parent_span: Span, attributes: &Vec) { + for (is_doc_comment, doc_string_group) in &attributes.iter().group_by(|a| match &a.kind { + DocComment(..) => true, + Normal(normal_attr) if is_normal_attr_a_doc(&normal_attr) => true, + _ => false, + }) { + let doc_string_group = doc_string_group.collect::>(); + if is_doc_comment { + let empty_attributes = &doc_string_group + .iter() + .filter(|a| match &a.kind { + DocComment(_, comment_text) => comment_text.as_str().trim().is_empty(), + Normal(normal_attr) => is_normal_attr_doc_empty(&normal_attr), + }) + .collect::>(); + if empty_attributes.len() == doc_string_group.len() { + if !empty_attributes.iter().any(|a| a.span.from_expansion()) { + let lo_span = empty_attributes + .get(0) + .expect("should always have an element") + .span + .data(); + let hi_help_span = empty_attributes + .get(empty_attributes.len() - 1) + .expect("should always have an element") + .span + .data(); + let hi_span_adjusted = std::cmp::min(parent_span.data().hi, hi_help_span.hi + MAX_CONTEXT_LEN); + let span = Span::new(lo_span.lo, hi_span_adjusted, lo_span.ctxt, lo_span.parent); + let help_span = Span::new(lo_span.lo, hi_help_span.hi, lo_span.ctxt, lo_span.parent); + span_lint_and_help( + ex, + EMPTY_DOCS, + span, + "documentation is empty", + Some(help_span), + "consider adding documentation or removing the docstring(s)", + ); + } + } + } + } + } +} + +fn is_normal_attr_a_doc(normal_attr: &ast::NormalAttr) -> bool { + if let Some(segment) = normal_attr.item.path.segments.get(0) { + if segment.ident.as_str() == "doc" { true } else { false } + } else { + false + } +} + +fn is_normal_attr_doc_empty(normal_attr: &ast::NormalAttr) -> bool { + let ast::AttrArgs::Delimited(delim_args) = &normal_attr.item.args else {return false;}; + let Some(tree) = delim_args.tokens.trees().nth(0) else {return false;}; + let tokenstream::TokenTree::Token(token, _) = tree else {return false;}; + let token::TokenKind::Literal(lit) = token.kind else {return false;}; + + lit.symbol.as_str().is_empty() +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dcd8ca81ae87..233fa79339de 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -109,6 +109,7 @@ mod double_parens; mod drop_forget_ref; mod duplicate_mod; mod else_if_without_else; +mod empty_docs; mod empty_drop; mod empty_enum; mod empty_structs_with_brackets; @@ -908,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck)); store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse)); store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef)); + store.register_early_pass(|| Box::new(empty_docs::EmptyDocs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index ac6a566b9cd3..7bca1ca921c5 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -4,6 +4,26 @@ version = "0.1.68" edition = "2021" publish = false +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_ast = { path = "/Users/cgorski/repos/rust/compiler/rustc_ast" } +rustc_ast_pretty = { path = "/Users/cgorski/repos/rust/compiler/rustc_ast_pretty" } +rustc_attr = { path = "/Users/cgorski/repos/rust/compiler/rustc_attr" } +rustc_data_structures = { path = "/Users/cgorski/repos/rust/compiler/rustc_data_structures" } +rustc_errors = { path = "/Users/cgorski/repos/rust/compiler/rustc_errors" } +rustc_hir = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir" } +rustc_hir_typeck = { path = "/Users/cgorski/repos/rust/compiler/rustc_hir_typeck" } +rustc_index = { path = "/Users/cgorski/repos/rust/compiler/rustc_index" } +rustc_infer = { path = "/Users/cgorski/repos/rust/compiler/rustc_infer" } +rustc_lexer = { path = "/Users/cgorski/repos/rust/compiler/rustc_lexer" } +rustc_lint = { path = "/Users/cgorski/repos/rust/compiler/rustc_lint" } +rustc_middle = { path = "/Users/cgorski/repos/rust/compiler/rustc_middle" } +rustc_mir_dataflow = { path = "/Users/cgorski/repos/rust/compiler/rustc_mir_dataflow" } +rustc_parse_format = { path = "/Users/cgorski/repos/rust/compiler/rustc_parse_format" } +rustc_session = { path = "/Users/cgorski/repos/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/cgorski/repos/rust/compiler/rustc_span" } +rustc_target = { path = "/Users/cgorski/repos/rust/compiler/rustc_target" } +rustc_trait_selection = { path = "/Users/cgorski/repos/rust/compiler/rustc_trait_selection" } + [dependencies] arrayvec = { version = "0.7", default-features = false } if_chain = "1.0" diff --git a/tests/ui/empty_docs.rs b/tests/ui/empty_docs.rs new file mode 100644 index 000000000000..4e1dfe77cc52 --- /dev/null +++ b/tests/ui/empty_docs.rs @@ -0,0 +1,81 @@ +#![allow(unused)] +#![warn(clippy::empty_docs)] + + +#[doc("this is a doc")] +fn attr_doc() { + +} + +#[doc("")] +fn blank_attr_doc() { + +} + +/// +fn main() { + +} + +/// +/// +fn double_empty_line_should_fire_1() { + +} + +/// This should not trigger +fn test_should_not_fire() { + +} + +/// Test function with no docs on let +fn no_docs_on_let() { + /// + let no_docs = 1; + + /// Here are docs + let docs = 2; +} + +/// +/// This also should not trigger +fn test_should_not_fire_2(){ + +} + +/// docs +struct StructDocs; + +/// +struct StructNoDocs; + +struct Struct { + /// A field + a_field: u32, + /// + no_doc_field: u32, + /// + + /// + more_no_docs:u32, +} + +union UnionFieldTest { + /// A field + a_union_field: u32, + /// + no_doc_union_field: u32, + /// + + /// + more_no_union_docs:u32, +} + +enum ThisIsAnEnum { + /// This variant has a docstring with text + ThisDoes, + /// + ThisDoesNot +} + + diff --git a/tests/ui/empty_docs.stderr b/tests/ui/empty_docs.stderr new file mode 100644 index 000000000000..1b2a6bba39b2 --- /dev/null +++ b/tests/ui/empty_docs.stderr @@ -0,0 +1,168 @@ +error: documentation is empty + --> $DIR/empty_docs.rs:10:1 + | +LL | / #[doc("")] +LL | | fn blank_attr_doc() { +LL | | +LL | | } + | |_^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:10:1 + | +LL | #[doc("")] + | ^^^^^^^^^^ + = note: `-D clippy::empty-docs` implied by `-D warnings` + +error: documentation is empty + --> $DIR/empty_docs.rs:15:1 + | +LL | / /// +LL | | fn main() { +LL | | +LL | | } + | |_^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:15:1 + | +LL | /// + | ^^^ + +error: documentation is empty + --> $DIR/empty_docs.rs:20:1 + | +LL | / /// +LL | | /// +LL | | fn double_empty_line_should_fire_1() { +LL | | +LL | | } + | |_^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:20:1 + | +LL | / /// +LL | | /// + | |___^ + +error: documentation is empty + --> $DIR/empty_docs.rs:33:5 + | +LL | / /// +LL | | let no_docs = 1; + | |____________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:33:5 + | +LL | /// + | ^^^ + +error: documentation is empty + --> $DIR/empty_docs.rs:49:1 + | +LL | / /// +LL | | struct StructNoDocs; + | |____________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:49:1 + | +LL | /// + | ^^^ + +error: documentation is empty + --> $DIR/empty_docs.rs:55:5 + | +LL | / /// +LL | | no_doc_field: u32, + | |_____________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:55:5 + | +LL | /// + | ^^^ + +error: documentation is empty + --> $DIR/empty_docs.rs:57:5 + | +LL | / /// +LL | | +LL | | /// +LL | | more_no_docs:u32, + | |____________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:57:5 + | +LL | / /// +LL | | +LL | | /// + | |_______^ + +error: documentation is empty + --> $DIR/empty_docs.rs:66:5 + | +LL | / /// +LL | | no_doc_union_field: u32, + | |___________________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:66:5 + | +LL | /// + | ^^^ + +error: documentation is empty + --> $DIR/empty_docs.rs:68:5 + | +LL | / /// +LL | | +LL | | /// +LL | | more_no_union_docs:u32, + | |__________________________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:68:5 + | +LL | / /// +LL | | +LL | | /// + | |_______^ + +error: documentation is empty + --> $DIR/empty_docs.rs:77:5 + | +LL | / /// +LL | | ThisDoesNot + | |_______________^ + | +help: consider adding documentation or removing the docstring(s) + --> $DIR/empty_docs.rs:77:5 + | +LL | /// + | ^^^ + +error: invalid `doc` attribute + --> $DIR/empty_docs.rs:5:7 + | +LL | #[doc("this is a doc")] + | ^^^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #82730 + = note: `-D invalid-doc-attributes` implied by `-D warnings` + +error: invalid `doc` attribute + --> $DIR/empty_docs.rs:10:7 + | +LL | #[doc("")] + | ^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #82730 + +error: aborting due to 12 previous errors +