Skip to content

Add new comment_within_doc lint #15260

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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 @@ -5779,6 +5779,7 @@ Released 2018-09-13
[`collapsible_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match
[`collapsible_str_replace`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_str_replace
[`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read
[`comment_within_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#comment_within_doc
[`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
[`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
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 @@ -112,6 +112,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::disallowed_names::DISALLOWED_NAMES_INFO,
crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
crate::disallowed_types::DISALLOWED_TYPES_INFO,
crate::doc::COMMENT_WITHIN_DOC_INFO,
crate::doc::DOC_BROKEN_LINK_INFO,
crate::doc::DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS_INFO,
crate::doc::DOC_INCLUDE_WITHOUT_CFG_INFO,
Expand Down
107 changes: 107 additions & 0 deletions clippy_lints/src/doc/comment_within_doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
use rustc_ast::token::CommentKind;
use rustc_ast::{AttrKind, AttrStyle};
use rustc_errors::Applicability;
use rustc_lexer::{TokenKind, tokenize};
use rustc_lint::{EarlyContext, LintContext};
use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, Span};

use clippy_utils::diagnostics::span_lint_and_then;

use super::COMMENT_WITHIN_DOC;

struct AttrInfo {
line: usize,
is_outer: bool,
span: Span,
file_span_pos: BytePos,
}

impl AttrInfo {
fn new(source_map: &SourceMap, attr: &rustc_ast::Attribute) -> Option<Self> {
let span_info = source_map.span_to_lines(attr.span).ok()?;
// If we cannot get the line for any reason, no point in building this item.
let line = span_info.lines.last()?.line_index;
Some(Self {
line,
is_outer: attr.style == AttrStyle::Outer,
span: attr.span,
file_span_pos: span_info.file.start_pos,
})
}
}

// Returns a `Vec` of `TokenKind` if the span only contains comments, otherwise returns `None`.
fn snippet_contains_only_comments(snippet: &str) -> Option<Vec<TokenKind>> {
let mut tokens = Vec::new();
for token in tokenize(snippet) {
match token.kind {
TokenKind::Whitespace => {},
TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => tokens.push(token.kind),
_ => return None,
}
}
Some(tokens)
}

pub(super) fn check(cx: &EarlyContext<'_>, attrs: &[rustc_ast::Attribute]) {
let mut stored_prev_attr = None;
let source_map = cx.sess().source_map();
for attr in attrs
.iter()
// We ignore `#[doc = "..."]` and `/** */` attributes.
.filter(|attr| matches!(attr.kind, AttrKind::DocComment(CommentKind::Line, _)))
{
let Some(attr) = AttrInfo::new(source_map, attr) else {
stored_prev_attr = None;
continue;
};
let Some(prev_attr) = stored_prev_attr else {
stored_prev_attr = Some(attr);
continue;
};
// First we check if they are from the same file and if they are the same kind of doc
// comments.
if attr.file_span_pos != prev_attr.file_span_pos || attr.is_outer != prev_attr.is_outer {
stored_prev_attr = Some(attr);
continue;
}
let diff = attr.line - (prev_attr.line + 1);
// Then we check if they follow each other.
if diff == 0 || diff > 1 {
// If there is no line between them or there are more than 1, we skip this check.
stored_prev_attr = Some(attr);
continue;
}
let span_between = prev_attr.span.between(attr.span);
// If there is one line between the two doc comments and it contains only one line comment,
// then we lint.
if diff == 1
&& let Ok(snippet) = source_map.span_to_snippet(span_between)
&& let Some(comments) = snippet_contains_only_comments(&snippet)
&& let &[TokenKind::LineComment { .. }] = comments.as_slice()
{
let offset_begin = snippet.trim_start().len();
let offset_end = snippet.trim_end().len();
let span = span_between
.with_hi(span_between.lo() + BytePos(offset_begin.try_into().unwrap()))
.with_hi(span_between.hi() - BytePos(offset_end.try_into().unwrap()));
let comment_kind = if attr.is_outer { '/' } else { '!' };
span_lint_and_then(
cx,
COMMENT_WITHIN_DOC,
span,
"code comment surrounded by doc comments",
|diag| {
diag.span_suggestion(
span.with_hi(span.lo() + BytePos(2)),
"did you mean to make it a doc comment?",
format!("//{comment_kind}"),
Applicability::MaybeIncorrect,
);
},
);
}
stored_prev_attr = Some(attr);
}
}
28 changes: 28 additions & 0 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::ops::Range;
use url::Url;

mod broken_link;
mod comment_within_doc;
mod doc_comment_double_space_linebreaks;
mod doc_suspicious_footnotes;
mod include_in_doc_without_cfg;
Expand Down Expand Up @@ -668,6 +669,31 @@ declare_clippy_lint! {
"looks like a link or footnote ref, but with no definition"
}

declare_clippy_lint! {
/// ### What it does
/// Checks if a code comment is surrounded by doc comments.
///
/// ### Why is this bad?
/// This is likely a typo, making the documentation miss a line.
///
/// ### Example
/// ```no_run
/// //! Doc
/// // oups
/// //! doc
/// ```
/// Use instead:
/// ```no_run
/// //! Doc
/// //! oups
/// //! doc
/// ```
#[clippy::version = "1.90.0"]
pub COMMENT_WITHIN_DOC,
pedantic,
"code comment surrounded by doc comments"
}

pub struct Documentation {
valid_idents: FxHashSet<String>,
check_private_items: bool,
Expand Down Expand Up @@ -702,11 +728,13 @@ impl_lint_pass!(Documentation => [
DOC_INCLUDE_WITHOUT_CFG,
DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS,
DOC_SUSPICIOUS_FOOTNOTES,
COMMENT_WITHIN_DOC,
]);

impl EarlyLintPass for Documentation {
fn check_attributes(&mut self, cx: &EarlyContext<'_>, attrs: &[rustc_ast::Attribute]) {
include_in_doc_without_cfg::check(cx, attrs);
comment_within_doc::check(cx, attrs);
}
}

Expand Down
23 changes: 23 additions & 0 deletions tests/ui/comment_within_doc.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![warn(clippy::comment_within_doc)]

//! Hello//!/ oups
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That "fix" is wrong, isn't it?

//! tadam
//~^^^ comment_within_doc

/// Hello//// oups
/// hehe
//~^^^ comment_within_doc
struct Bar;

mod b {
//! targe//! // oups
//! hello
//
/// nope/// // oups
/// yep
//~^^^ comment_within_doc
//~^^^^^^^^ comment_within_doc
struct Bar;
}

fn main() {}
27 changes: 27 additions & 0 deletions tests/ui/comment_within_doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![warn(clippy::comment_within_doc)]

//! Hello
// oups
//! tadam
//~^^^ comment_within_doc

/// Hello
// oups
/// hehe
//~^^^ comment_within_doc
struct Bar;

mod b {
//! targe
// oups
//! hello
//
/// nope
// oups
/// yep
//~^^^ comment_within_doc
//~^^^^^^^^ comment_within_doc
struct Bar;
}

fn main() {}
51 changes: 51 additions & 0 deletions tests/ui/comment_within_doc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
error: code comment surrounded by doc comments
--> tests/ui/comment_within_doc.rs:3:10
|
LL | //! Hello
| __________^
| |__________|
LL | || // oups
| || ^
| ||_|
| |_help: did you mean to make it a doc comment?: `//!`
|
|
= note: `-D clippy::comment-within-doc` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::comment_within_doc)]`

error: code comment surrounded by doc comments
--> tests/ui/comment_within_doc.rs:8:10
|
LL | /// Hello
| __________^
| |__________|
LL | || // oups
| || ^
| ||_|
| |_help: did you mean to make it a doc comment?: `///`
|

error: code comment surrounded by doc comments
--> tests/ui/comment_within_doc.rs:15:14
|
LL | //! targe
| ______________^
| |______________|
LL | || // oups
| ||_-__^
| |__|
| help: did you mean to make it a doc comment?: `//!`

error: code comment surrounded by doc comments
--> tests/ui/comment_within_doc.rs:19:13
|
LL | /// nope
| _____________^
| |_____________|
LL | || // oups
| ||_-__^
| |__|
| help: did you mean to make it a doc comment?: `///`

error: aborting due to 4 previous errors

Loading