EmptyDocs #10152
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<Attribute>) { | ||
| 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::<Vec<_>>(); | ||
| 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::<Vec<_>>(); | ||
| 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 | ||
| } | ||
|
Comment on lines
+137
to
+141
There was a problem hiding this comment. style nits: normal_attr.item.path.segments.get(0).map_or(false, |segment| segment.ident.as_str == "doc") |
||
| } | ||
|
|
||
| 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() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
|
|
||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a similar clippy util to extract comments, although it takes all comments, not just doc coms.
Will something like that help here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some code that reassembles doc comments for some related lints, the lint could be moved into
doc.rsand check here-ish based onspans+docSorry to point that out after you've done the complicated part yourself already 😬
Not the first time this has happened with
doc.rs, maybe it would be worth renaming it todoc_comments.rsor something, but it'll still be easy to miss