bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #136657 - jdonszelmann:empty-line-after, r=y21,flip1995,WaffleLapkin

Browse source 
Make empty-line-after an early clippy lint

r? ```@y21```

95% a refiling of https://github.com/rust-lang/rust-clippy/pull/13658 but for correctness it needed 2 extra methods in `rust_lint` which made it much easier to apply on `rust-lang/rust` than `rust-lang/rust-clippy`.

Commits have been thoroughly reviewed on `rust-lang/clippy already`. The last two review comments there (about using `Option` and popping for assoc items have been applied here.
This commit is contained in:
Matthias Krüger 2025-02-08 21:37:26 +01:00 • committed by GitHub
commit dbcd74e6d9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 621 additions and 472 deletions
Download patch file Download diff file Expand all files Collapse all files

8
compiler/rustc_lint/src/early.rs
View file

@ -246,6 +246,14 @@ impl<'ast, 'ecx, 'tcx, T: EarlyLintPass> ast_visit::Visitor<'ast>
}
}
ast_visit::walk_assoc_item(cx, item, ctxt);
match ctxt {
ast_visit::AssocCtxt::Trait => {
lint_callback!(cx, check_trait_item_post, item);
}
ast_visit::AssocCtxt::Impl => {
lint_callback!(cx, check_impl_item_post, item);
}
}
});
}

2
compiler/rustc_lint/src/passes.rs
View file

@ -162,7 +162,9 @@ macro_rules! early_lint_methods {
c: rustc_span::Span,
d_: rustc_ast::NodeId);
fn check_trait_item(a: &rustc_ast::AssocItem);
fn check_trait_item_post(a: &rustc_ast::AssocItem);
fn check_impl_item(a: &rustc_ast::AssocItem);
fn check_impl_item_post(a: &rustc_ast::AssocItem);
fn check_variant(a: &rustc_ast::Variant);
fn check_attribute(a: &rustc_ast::Attribute);
fn check_attributes(a: &[rustc_ast::Attribute]);

4
src/tools/clippy/clippy_lints/src/declared_lints.rs
View file

@ -144,8 +144,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::doc::DOC_NESTED_REFDEFS_INFO,
crate::doc::DOC_OVERINDENTED_LIST_ITEMS_INFO,
crate::doc::EMPTY_DOCS_INFO,
crate::doc::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
crate::doc::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
crate::doc::MISSING_ERRORS_DOC_INFO,
crate::doc::MISSING_PANICS_DOC_INFO,
crate::doc::MISSING_SAFETY_DOC_INFO,
@ -162,6 +160,8 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO,
crate::empty_drop::EMPTY_DROP_INFO,
crate::empty_enum::EMPTY_ENUM_INFO,
crate::empty_line_after::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
crate::empty_line_after::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
crate::empty_with_brackets::EMPTY_ENUM_VARIANTS_WITH_BRACKETS_INFO,
crate::empty_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO,
crate::endian_bytes::BIG_ENDIAN_BYTES_INFO,

345
src/tools/clippy/clippy_lints/src/doc/empty_line_after.rs
View file

@ -1,345 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{SpanRangeExt, snippet_indent};
use clippy_utils::tokenize_with_text;
use itertools::Itertools;
use rustc_ast::AttrStyle;
use rustc_ast::token::CommentKind;
use rustc_errors::{Applicability, Diag, SuggestionStyle};
use rustc_hir::{AttrKind, Attribute, ItemKind, Node};
use rustc_lexer::TokenKind;
use rustc_lint::LateContext;
use rustc_span::{BytePos, ExpnKind, InnerSpan, Span, SpanData};
use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR};
#[derive(Debug, PartialEq, Clone, Copy)]
enum StopKind {
Attr,
Doc(CommentKind),
}
impl StopKind {
fn is_doc(self) -> bool {
matches!(self, StopKind::Doc(_))
}
}
#[derive(Debug)]
struct Stop {
span: Span,
kind: StopKind,
first: usize,
last: usize,
}
impl Stop {
fn convert_to_inner(&self) -> (Span, String) {
let inner = match self.kind {
// #|[...]
StopKind::Attr => InnerSpan::new(1, 1),
// /// or /**
// ^ ^
StopKind::Doc(_) => InnerSpan::new(2, 3),
};
(self.span.from_inner(inner), "!".into())
}
fn comment_out(&self, cx: &LateContext<'_>, suggestions: &mut Vec<(Span, String)>) {
match self.kind {
StopKind::Attr => {
if cx.tcx.sess.source_map().is_multiline(self.span) {
suggestions.extend([
(self.span.shrink_to_lo(), "/* ".into()),
(self.span.shrink_to_hi(), " */".into()),
]);
} else {
suggestions.push((self.span.shrink_to_lo(), "// ".into()));
}
},
StopKind::Doc(CommentKind::Line) => suggestions.push((self.span.shrink_to_lo(), "// ".into())),
StopKind::Doc(CommentKind::Block) => {
// /** outer */ /*! inner */
// ^ ^
let asterisk = self.span.from_inner(InnerSpan::new(1, 2));
suggestions.push((asterisk, String::new()));
},
}
}
fn from_attr(cx: &LateContext<'_>, attr: &Attribute) -> Option<Self> {
let SpanData { lo, hi, .. } = attr.span.data();
let file = cx.tcx.sess.source_map().lookup_source_file(lo);
Some(Self {
span: attr.span,
kind: match attr.kind {
AttrKind::Normal(_) => StopKind::Attr,
AttrKind::DocComment(comment_kind, _) => StopKind::Doc(comment_kind),
},
first: file.lookup_line(file.relative_position(lo))?,
last: file.lookup_line(file.relative_position(hi))?,
})
}
}
/// Represents a set of attrs/doc comments separated by 1 or more empty lines
///
/// ```ignore
/// /// chunk 1 docs
/// // not an empty line so also part of chunk 1
/// #[chunk_1_attrs] // <-- prev_stop
///
/// /* gap */
///
/// /// chunk 2 docs // <-- next_stop
/// #[chunk_2_attrs]
/// ```
struct Gap<'a> {
/// The span of individual empty lines including the newline at the end of the line
empty_lines: Vec<Span>,
has_comment: bool,
next_stop: &'a Stop,
prev_stop: &'a Stop,
/// The chunk that includes [`prev_stop`](Self::prev_stop)
prev_chunk: &'a [Stop],
}
impl<'a> Gap<'a> {
fn new(cx: &LateContext<'_>, prev_chunk: &'a [Stop], next_chunk: &'a [Stop]) -> Option<Self> {
let prev_stop = prev_chunk.last()?;
let next_stop = next_chunk.first()?;
let gap_span = prev_stop.span.between(next_stop.span);
let gap_snippet = gap_span.get_source_text(cx)?;
let mut has_comment = false;
let mut empty_lines = Vec::new();
for (token, source, inner_span) in tokenize_with_text(&gap_snippet) {
match token {
TokenKind::BlockComment {
doc_style: None,
terminated: true,
}
| TokenKind::LineComment { doc_style: None } => has_comment = true,
TokenKind::Whitespace => {
let newlines = source.bytes().positions(|b| b == b'\n');
empty_lines.extend(
newlines
.tuple_windows()
.map(|(a, b)| InnerSpan::new(inner_span.start + a + 1, inner_span.start + b))
.map(|inner_span| gap_span.from_inner(inner_span)),
);
},
// Ignore cfg_attr'd out attributes as they may contain empty lines, could also be from macro
// shenanigans
_ => return None,
}
}
(!empty_lines.is_empty()).then_some(Self {
empty_lines,
has_comment,
next_stop,
prev_stop,
prev_chunk,
})
}
fn contiguous_empty_lines(&self) -> impl Iterator<Item = Span> + '_ {
self.empty_lines
// The `+ BytePos(1)` means "next line", because each empty line span is "N:1-N:1".
.chunk_by(|a, b| a.hi() + BytePos(1) == b.lo())
.map(|chunk| {
let first = chunk.first().expect("at least one empty line");
let last = chunk.last().expect("at least one empty line");
// The BytePos subtraction here is safe, as before an empty line, there must be at least one
// attribute/comment. The span needs to start at the end of the previous line.
first.with_lo(first.lo() - BytePos(1)).with_hi(last.hi())
})
}
}
/// If the node the attributes/docs apply to is the first in the module/crate suggest converting
/// them to inner attributes/docs
fn suggest_inner(cx: &LateContext<'_>, diag: &mut Diag<'_, ()>, kind: StopKind, gaps: &[Gap<'_>]) {
let Some(owner) = cx.last_node_with_lint_attrs.as_owner() else {
return;
};
let parent_desc = match cx.tcx.parent_hir_node(owner.into()) {
Node::Item(item)
if let ItemKind::Mod(parent_mod) = item.kind
&& let [first, ..] = parent_mod.item_ids
&& first.owner_id == owner =>
{
"parent module"
},
Node::Crate(crate_mod)
if let Some(first) = crate_mod
.item_ids
.iter()
.map(|&id| cx.tcx.hir().item(id))
// skip prelude imports
.find(|item| !matches!(item.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
&& first.owner_id == owner =>
{
"crate"
},
_ => return,
};
diag.multipart_suggestion_verbose(
match kind {
StopKind::Attr => format!("if the attribute should apply to the {parent_desc} use an inner attribute"),
StopKind::Doc(_) => format!("if the comment should document the {parent_desc} use an inner doc comment"),
},
gaps.iter()
.flat_map(|gap| gap.prev_chunk)
.map(Stop::convert_to_inner)
.collect(),
Applicability::MaybeIncorrect,
);
}
fn check_gaps(cx: &LateContext<'_>, gaps: &[Gap<'_>]) -> bool {
let Some(first_gap) = gaps.first() else {
return false;
};
let empty_lines = || gaps.iter().flat_map(|gap| gap.empty_lines.iter().copied());
let contiguous_empty_lines = || gaps.iter().flat_map(Gap::contiguous_empty_lines);
let mut has_comment = false;
let mut has_attr = false;
for gap in gaps {
has_comment |= gap.has_comment;
if !has_attr {
has_attr = gap.prev_chunk.iter().any(|stop| stop.kind == StopKind::Attr);
}
}
let kind = first_gap.prev_stop.kind;
let (lint, kind_desc) = match kind {
StopKind::Attr => (EMPTY_LINE_AFTER_OUTER_ATTR, "outer attribute"),
StopKind::Doc(_) => (EMPTY_LINE_AFTER_DOC_COMMENTS, "doc comment"),
};
let (lines, are, them) = if empty_lines().nth(1).is_some() {
("lines", "are", "them")
} else {
("line", "is", "it")
};
span_lint_and_then(
cx,
lint,
first_gap.prev_stop.span.to(empty_lines().last().unwrap()),
format!("empty {lines} after {kind_desc}"),
|diag| {
if let Some(owner) = cx.last_node_with_lint_attrs.as_owner() {
let def_id = owner.to_def_id();
let def_descr = cx.tcx.def_descr(def_id);
diag.span_label(
cx.tcx.def_span(def_id),
match kind {
StopKind::Attr => format!("the attribute applies to this {def_descr}"),
StopKind::Doc(_) => format!("the comment documents this {def_descr}"),
},
);
}
diag.multipart_suggestion_with_style(
format!("if the empty {lines} {are} unintentional remove {them}"),
contiguous_empty_lines()
.map(|empty_lines| (empty_lines, String::new()))
.collect(),
Applicability::MaybeIncorrect,
SuggestionStyle::HideCodeAlways,
);
if has_comment && kind.is_doc() {
// Likely doc comments that applied to some now commented out code
//
// /// Old docs for Foo
// // struct Foo;
let mut suggestions = Vec::new();
for stop in gaps.iter().flat_map(|gap| gap.prev_chunk) {
stop.comment_out(cx, &mut suggestions);
}
let name = match cx.tcx.hir().opt_name(cx.last_node_with_lint_attrs) {
Some(name) => format!("`{name}`"),
None => "this".into(),
};
diag.multipart_suggestion_verbose(
format!("if the doc comment should not document {name} comment it out"),
suggestions,
Applicability::MaybeIncorrect,
);
} else {
suggest_inner(cx, diag, kind, gaps);
}
if kind == StopKind::Doc(CommentKind::Line)
&& gaps
.iter()
.all(|gap| !gap.has_comment && gap.next_stop.kind == StopKind::Doc(CommentKind::Line))
{
// Commentless empty gaps between line doc comments, possibly intended to be part of the markdown
let indent = snippet_indent(cx, first_gap.prev_stop.span).unwrap_or_default();
diag.multipart_suggestion_verbose(
format!("if the documentation should include the empty {lines} include {them} in the comment"),
empty_lines()
.map(|empty_line| (empty_line, format!("{indent}///")))
.collect(),
Applicability::MaybeIncorrect,
);
}
},
);
kind.is_doc()
}
/// Returns `true` if [`EMPTY_LINE_AFTER_DOC_COMMENTS`] triggered, used to skip other doc comment
/// lints where they would be confusing
///
/// [`EMPTY_LINE_AFTER_OUTER_ATTR`] is also here to share an implementation but does not return
/// `true` if it triggers
pub(super) fn check(cx: &LateContext<'_>, attrs: &[Attribute]) -> bool {
let mut outer = attrs
.iter()
.filter(|attr| attr.style == AttrStyle::Outer && !attr.span.from_expansion())
.map(|attr| Stop::from_attr(cx, attr))
.collect::<Option<Vec<_>>>()
.unwrap_or_default();
if outer.is_empty() {
return false;
}
// Push a fake attribute Stop for the item itself so we check for gaps between the last outer
// attr/doc comment and the item they apply to
let span = cx.tcx.hir().span(cx.last_node_with_lint_attrs);
if !span.from_expansion()
&& let Ok(line) = cx.tcx.sess.source_map().lookup_line(span.lo())
{
outer.push(Stop {
span,
kind: StopKind::Attr,
first: line.line,
// last doesn't need to be accurate here, we don't compare it with anything
last: line.line,
});
}
let mut gaps = Vec::new();
let mut last = 0;
for pos in outer
.array_windows()
.positions(|[a, b]| b.first.saturating_sub(a.last) > 1)
{
// we want to be after the first stop in the window
let pos = pos + 1;
if let Some(gap) = Gap::new(cx, &outer[last..pos], &outer[pos..]) {
last = pos;
gaps.push(gap);
}
}
check_gaps(cx, &gaps)
}

81
src/tools/clippy/clippy_lints/src/doc/mod.rs
View file

@ -33,7 +33,6 @@ use rustc_span::{Span, sym};
use std::ops::Range;
use url::Url;
mod empty_line_after;
mod include_in_doc_without_cfg;
mod link_with_quotes;
mod markdown;
@ -491,82 +490,6 @@ declare_clippy_lint! {
"ensure the first documentation paragraph is short"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for empty lines after outer attributes
///
/// ### Why is this bad?
/// The attribute may have meant to be an inner attribute (`#![attr]`). If
/// it was meant to be an outer attribute (`#[attr]`) then the empty line
/// should be removed
///
/// ### Example
/// ```no_run
/// #[allow(dead_code)]
///
/// fn not_quite_good_code() {}
/// ```
///
/// Use instead:
/// ```no_run
/// // Good (as inner attribute)
/// #![allow(dead_code)]
///
/// fn this_is_fine() {}
///
/// // or
///
/// // Good (as outer attribute)
/// #[allow(dead_code)]
/// fn this_is_fine_too() {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub EMPTY_LINE_AFTER_OUTER_ATTR,
suspicious,
"empty line after outer attribute"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for empty lines after doc comments.
///
/// ### Why is this bad?
/// The doc comment may have meant to be an inner doc comment, regular
/// comment or applied to some old code that is now commented out. If it was
/// intended to be a doc comment, then the empty line should be removed.
///
/// ### Example
/// ```no_run
/// /// Some doc comment with a blank line after it.
///
/// fn f() {}
///
/// /// Docs for `old_code`
/// // fn old_code() {}
///
/// fn new_code() {}
/// ```
///
/// Use instead:
/// ```no_run
/// //! Convert it to an inner doc comment
///
/// // Or a regular comment
///
/// /// Or remove the empty line
/// fn f() {}
///
/// // /// Docs for `old_code`
/// // fn old_code() {}
///
/// fn new_code() {}
/// ```
#[clippy::version = "1.70.0"]
pub EMPTY_LINE_AFTER_DOC_COMMENTS,
suspicious,
"empty line after doc comments"
}
declare_clippy_lint! {
/// ### What it does
/// Checks if included files in doc comments are included only for `cfg(doc)`.
@ -650,8 +573,6 @@ impl_lint_pass!(Documentation => [
EMPTY_DOCS,
DOC_LAZY_CONTINUATION,
DOC_OVERINDENTED_LIST_ITEMS,
EMPTY_LINE_AFTER_OUTER_ATTR,
EMPTY_LINE_AFTER_DOC_COMMENTS,
TOO_LONG_FIRST_DOC_PARAGRAPH,
DOC_INCLUDE_WITHOUT_CFG,
]);
@ -784,7 +705,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
}
include_in_doc_without_cfg::check(cx, attrs);
if suspicious_doc_comments::check(cx, attrs) || empty_line_after::check(cx, attrs) || is_doc_hidden(attrs) {
if suspicious_doc_comments::check(cx, attrs) || is_doc_hidden(attrs) {
return None;
}

492
src/tools/clippy/clippy_lints/src/empty_line_after.rs Normal file
View file

@ -0,0 +1,492 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{SpanRangeExt, snippet_indent};
use clippy_utils::tokenize_with_text;
use itertools::Itertools;
use rustc_ast::token::CommentKind;
use rustc_ast::{AssocItemKind, AttrKind, AttrStyle, Attribute, Crate, Item, ItemKind, ModKind, NodeId};
use rustc_errors::{Applicability, Diag, SuggestionStyle};
use rustc_lexer::TokenKind;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::impl_lint_pass;
use rustc_span::symbol::kw;
use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol};
declare_clippy_lint! {
/// ### What it does
/// Checks for empty lines after outer attributes
///
/// ### Why is this bad?
/// The attribute may have meant to be an inner attribute (`#![attr]`). If
/// it was meant to be an outer attribute (`#[attr]`) then the empty line
/// should be removed
///
/// ### Example
/// ```no_run
/// #[allow(dead_code)]
///
/// fn not_quite_good_code() {}
/// ```
///
/// Use instead:
/// ```no_run
/// // Good (as inner attribute)
/// #![allow(dead_code)]
///
/// fn this_is_fine() {}
///
/// // or
///
/// // Good (as outer attribute)
/// #[allow(dead_code)]
/// fn this_is_fine_too() {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub EMPTY_LINE_AFTER_OUTER_ATTR,
suspicious,
"empty line after outer attribute"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for empty lines after doc comments.
///
/// ### Why is this bad?
/// The doc comment may have meant to be an inner doc comment, regular
/// comment or applied to some old code that is now commented out. If it was
/// intended to be a doc comment, then the empty line should be removed.
///
/// ### Example
/// ```no_run
/// /// Some doc comment with a blank line after it.
///
/// fn f() {}
///
/// /// Docs for `old_code`
/// // fn old_code() {}
///
/// fn new_code() {}
/// ```
///
/// Use instead:
/// ```no_run
/// //! Convert it to an inner doc comment
///
/// // Or a regular comment
///
/// /// Or remove the empty line
/// fn f() {}
///
/// // /// Docs for `old_code`
/// // fn old_code() {}
///
/// fn new_code() {}
/// ```
#[clippy::version = "1.70.0"]
pub EMPTY_LINE_AFTER_DOC_COMMENTS,
suspicious,
"empty line after doc comments"
}
#[derive(Debug)]
struct ItemInfo {
kind: &'static str,
name: Symbol,
span: Span,
mod_items: Option<NodeId>,
}
pub struct EmptyLineAfter {
items: Vec<ItemInfo>,
}
impl_lint_pass!(EmptyLineAfter => [
EMPTY_LINE_AFTER_OUTER_ATTR,
EMPTY_LINE_AFTER_DOC_COMMENTS,
]);
impl EmptyLineAfter {
pub fn new() -> Self {
Self { items: Vec::new() }
}
}
#[derive(Debug, PartialEq, Clone, Copy)]
enum StopKind {
Attr,
Doc(CommentKind),
}
impl StopKind {
fn is_doc(self) -> bool {
matches!(self, StopKind::Doc(_))
}
}
#[derive(Debug)]
struct Stop {
span: Span,
kind: StopKind,
first: usize,
last: usize,
}
impl Stop {
fn convert_to_inner(&self) -> (Span, String) {
let inner = match self.kind {
// #![...]
StopKind::Attr => InnerSpan::new(1, 1),
// /// or /**
// ^ ^
StopKind::Doc(_) => InnerSpan::new(2, 3),
};
(self.span.from_inner(inner), "!".into())
}
fn comment_out(&self, cx: &EarlyContext<'_>, suggestions: &mut Vec<(Span, String)>) {
match self.kind {
StopKind::Attr => {
if cx.sess().source_map().is_multiline(self.span) {
suggestions.extend([
(self.span.shrink_to_lo(), "/* ".into()),
(self.span.shrink_to_hi(), " */".into()),
]);
} else {
suggestions.push((self.span.shrink_to_lo(), "// ".into()));
}
},
StopKind::Doc(CommentKind::Line) => suggestions.push((self.span.shrink_to_lo(), "// ".into())),
StopKind::Doc(CommentKind::Block) => {
// /** outer */ /*! inner */
// ^ ^
let asterisk = self.span.from_inner(InnerSpan::new(1, 2));
suggestions.push((asterisk, String::new()));
},
}
}
fn from_attr(cx: &EarlyContext<'_>, attr: &Attribute) -> Option<Self> {
let SpanData { lo, hi, .. } = attr.span.data();
let file = cx.sess().source_map().lookup_source_file(lo);
Some(Self {
span: attr.span,
kind: match attr.kind {
AttrKind::Normal(_) => StopKind::Attr,
AttrKind::DocComment(comment_kind, _) => StopKind::Doc(comment_kind),
},
first: file.lookup_line(file.relative_position(lo))?,
last: file.lookup_line(file.relative_position(hi))?,
})
}
}
/// Represents a set of attrs/doc comments separated by 1 or more empty lines
///
/// ```ignore
/// /// chunk 1 docs
/// // not an empty line so also part of chunk 1
/// #[chunk_1_attrs] // <-- prev_stop
///
/// /* gap */
///
/// /// chunk 2 docs // <-- next_stop
/// #[chunk_2_attrs]
/// ```
struct Gap<'a> {
/// The span of individual empty lines including the newline at the end of the line
empty_lines: Vec<Span>,
has_comment: bool,
next_stop: &'a Stop,
prev_stop: &'a Stop,
/// The chunk that includes [`prev_stop`](Self::prev_stop)
prev_chunk: &'a [Stop],
}
impl<'a> Gap<'a> {
fn new(cx: &EarlyContext<'_>, prev_chunk: &'a [Stop], next_chunk: &'a [Stop]) -> Option<Self> {
let prev_stop = prev_chunk.last()?;
let next_stop = next_chunk.first()?;
let gap_span = prev_stop.span.between(next_stop.span);
let gap_snippet = gap_span.get_source_text(cx)?;
let mut has_comment = false;
let mut empty_lines = Vec::new();
for (token, source, inner_span) in tokenize_with_text(&gap_snippet) {
match token {
TokenKind::BlockComment {
doc_style: None,
terminated: true,
}
| TokenKind::LineComment { doc_style: None } => has_comment = true,
TokenKind::Whitespace => {
let newlines = source.bytes().positions(|b| b == b'\n');
empty_lines.extend(
newlines
.tuple_windows()
.map(|(a, b)| InnerSpan::new(inner_span.start + a + 1, inner_span.start + b))
.map(|inner_span| gap_span.from_inner(inner_span)),
);
},
// Ignore cfg_attr'd out attributes as they may contain empty lines, could also be from macro
// shenanigans
_ => return None,
}
}
(!empty_lines.is_empty()).then_some(Self {
empty_lines,
has_comment,
next_stop,
prev_stop,
prev_chunk,
})
}
fn contiguous_empty_lines(&self) -> impl Iterator<Item = Span> + '_ {
self.empty_lines
// The `+ BytePos(1)` means "next line", because each empty line span is "N:1-N:1".
.chunk_by(|a, b| a.hi() + BytePos(1) == b.lo())
.map(|chunk| {
let first = chunk.first().expect("at least one empty line");
let last = chunk.last().expect("at least one empty line");
// The BytePos subtraction here is safe, as before an empty line, there must be at least one
// attribute/comment. The span needs to start at the end of the previous line.
first.with_lo(first.lo() - BytePos(1)).with_hi(last.hi())
})
}
}
impl EmptyLineAfter {
fn check_gaps(&self, cx: &EarlyContext<'_>, gaps: &[Gap<'_>], id: NodeId) {
let Some(first_gap) = gaps.first() else {
return;
};
let empty_lines = || gaps.iter().flat_map(|gap| gap.empty_lines.iter().copied());
let contiguous_empty_lines = || gaps.iter().flat_map(Gap::contiguous_empty_lines);
let mut has_comment = false;
let mut has_attr = false;
for gap in gaps {
has_comment |= gap.has_comment;
if !has_attr {
has_attr = gap.prev_chunk.iter().any(|stop| stop.kind == StopKind::Attr);
}
}
let kind = first_gap.prev_stop.kind;
let (lint, kind_desc) = match kind {
StopKind::Attr => (EMPTY_LINE_AFTER_OUTER_ATTR, "outer attribute"),
StopKind::Doc(_) => (EMPTY_LINE_AFTER_DOC_COMMENTS, "doc comment"),
};
let (lines, are, them) = if empty_lines().nth(1).is_some() {
("lines", "are", "them")
} else {
("line", "is", "it")
};
span_lint_and_then(
cx,
lint,
first_gap.prev_stop.span.to(empty_lines().last().unwrap()),
format!("empty {lines} after {kind_desc}"),
|diag| {
let info = self.items.last().unwrap();
diag.span_label(info.span, match kind {
StopKind::Attr => format!("the attribute applies to this {}", info.kind),
StopKind::Doc(_) => format!("the comment documents this {}", info.kind),
});
diag.multipart_suggestion_with_style(
format!("if the empty {lines} {are} unintentional, remove {them}"),
contiguous_empty_lines()
.map(|empty_lines| (empty_lines, String::new()))
.collect(),
Applicability::MaybeIncorrect,
SuggestionStyle::HideCodeAlways,
);
if has_comment && kind.is_doc() {
// Likely doc comments that applied to some now commented out code
//
// /// Old docs for Foo
// // struct Foo;
let mut suggestions = Vec::new();
for stop in gaps.iter().flat_map(|gap| gap.prev_chunk) {
stop.comment_out(cx, &mut suggestions);
}
diag.multipart_suggestion_verbose(
format!("if the doc comment should not document `{}` comment it out", info.name),
suggestions,
Applicability::MaybeIncorrect,
);
} else {
self.suggest_inner(diag, kind, gaps, id);
}
if kind == StopKind::Doc(CommentKind::Line)
&& gaps
.iter()
.all(|gap| !gap.has_comment && gap.next_stop.kind == StopKind::Doc(CommentKind::Line))
{
// Commentless empty gaps between line doc comments, possibly intended to be part of the markdown
let indent = snippet_indent(cx, first_gap.prev_stop.span).unwrap_or_default();
diag.multipart_suggestion_verbose(
format!("if the documentation should include the empty {lines} include {them} in the comment"),
empty_lines()
.map(|empty_line| (empty_line, format!("{indent}///")))
.collect(),
Applicability::MaybeIncorrect,
);
}
},
);
}
/// If the node the attributes/docs apply to is the first in the module/crate suggest converting
/// them to inner attributes/docs
fn suggest_inner(&self, diag: &mut Diag<'_, ()>, kind: StopKind, gaps: &[Gap<'_>], id: NodeId) {
if let Some(parent) = self.items.iter().rev().nth(1)
&& (parent.kind == "module" || parent.kind == "crate")
&& parent.mod_items == Some(id)
{
let desc = if parent.kind == "module" {
"parent module"
} else {
parent.kind
};
diag.multipart_suggestion_verbose(
match kind {
StopKind::Attr => format!("if the attribute should apply to the {desc} use an inner attribute"),
StopKind::Doc(_) => format!("if the comment should document the {desc} use an inner doc comment"),
},
gaps.iter()
.flat_map(|gap| gap.prev_chunk)
.map(Stop::convert_to_inner)
.collect(),
Applicability::MaybeIncorrect,
);
}
}
fn check_item_kind(
&mut self,
cx: &EarlyContext<'_>,
kind: &ItemKind,
ident: &Ident,
span: Span,
attrs: &[Attribute],
id: NodeId,
) {
self.items.push(ItemInfo {
kind: kind.descr(),
name: ident.name,
span: if span.contains(ident.span) {
span.with_hi(ident.span.hi())
} else {
span.with_hi(span.lo())
},
mod_items: match kind {
ItemKind::Mod(_, ModKind::Loaded(items, _, _, _)) => items
.iter()
.filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
.map(|i| i.id)
.next(),
_ => None,
},
});
let mut outer = attrs
.iter()
.filter(|attr| attr.style == AttrStyle::Outer && !attr.span.from_expansion())
.map(|attr| Stop::from_attr(cx, attr))
.collect::<Option<Vec<_>>>()
.unwrap_or_default();
if outer.is_empty() {
return;
}
// Push a fake attribute Stop for the item itself so we check for gaps between the last outer
// attr/doc comment and the item they apply to
let span = self.items.last().unwrap().span;
if !span.from_expansion()
&& let Ok(line) = cx.sess().source_map().lookup_line(span.lo())
{
outer.push(Stop {
span,
kind: StopKind::Attr,
first: line.line,
// last doesn't need to be accurate here, we don't compare it with anything
last: line.line,
});
}
let mut gaps = Vec::new();
let mut last = 0;
for pos in outer
.array_windows()
.positions(|[a, b]| b.first.saturating_sub(a.last) > 1)
{
// we want to be after the first stop in the window
let pos = pos + 1;
if let Some(gap) = Gap::new(cx, &outer[last..pos], &outer[pos..]) {
last = pos;
gaps.push(gap);
}
}
self.check_gaps(cx, &gaps, id);
}
}
impl EarlyLintPass for EmptyLineAfter {
fn check_crate(&mut self, _: &EarlyContext<'_>, krate: &Crate) {
self.items.push(ItemInfo {
kind: "crate",
name: kw::Crate,
span: krate.spans.inner_span.with_hi(krate.spans.inner_span.lo()),
mod_items: krate
.items
.iter()
.filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
.map(|i| i.id)
.next(),
});
}
fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
self.items.pop();
}
fn check_impl_item_post(&mut self, _: &EarlyContext<'_>, _: &Item<AssocItemKind>) {
self.items.pop();
}
fn check_trait_item_post(&mut self, _: &EarlyContext<'_>, _: &Item<AssocItemKind>) {
self.items.pop();
}
fn check_impl_item(&mut self, cx: &EarlyContext<'_>, item: &Item<AssocItemKind>) {
self.check_item_kind(
cx,
&item.kind.clone().into(),
&item.ident,
item.span,
&item.attrs,
item.id,
);
}
fn check_trait_item(&mut self, cx: &EarlyContext<'_>, item: &Item<AssocItemKind>) {
self.check_item_kind(
cx,
&item.kind.clone().into(),
&item.ident,
item.span,
&item.attrs,
item.id,
);
}
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
self.check_item_kind(cx, &item.kind, &item.ident, item.span, &item.attrs, item.id);
}
}

2
src/tools/clippy/clippy_lints/src/lib.rs
View file

@ -126,6 +126,7 @@ mod duplicate_mod;
mod else_if_without_else;
mod empty_drop;
mod empty_enum;
mod empty_line_after;
mod empty_with_brackets;
mod endian_bytes;
mod entry;
@ -973,6 +974,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
store.register_early_pass(|| Box::new(empty_line_after::EmptyLineAfter::new()));
store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default());

16
src/tools/clippy/tests/ui/doc/unbalanced_ticks.rs
View file

@ -66,3 +66,19 @@ fn escape_3() {}
/// Backslashes ` \` within code blocks don't count.
fn escape_4() {}
trait Foo {
fn bar();
}
struct Bar;
impl Foo for Bar {
// NOTE: false positive
/// Returns an `Option<Month>` from a i64, assuming a 1-index, January = 1.
///
/// `Month::from_i64(n: i64)`: | `1` | `2` | ... | `12`
/// ---------------------------| -------------------- | --------------------- | ... | -----
/// ``: | Some(Month::January) | Some(Month::February) | ... |
/// Some(Month::December)
fn bar() {}
}

14
src/tools/clippy/tests/ui/doc/unbalanced_ticks.stderr
View file

@ -94,5 +94,17 @@ LL | /// Escaped \` ` backticks don't count, but unescaped backticks do.
|
= help: a backtick may be missing a pair
error: aborting due to 10 previous errors
error: backticks are unbalanced
--> tests/ui/doc/unbalanced_ticks.rs:79:9
|
LL | /// `Month::from_i64(n: i64)`: | `1` | `2` | ... | `12`
| _________^
LL | | /// ---------------------------| -------------------- | --------------------- | ... | -----
LL | | /// ``: | Some(Month::January) | Some(Month::February) | ... |
LL | | /// Some(Month::December)
| |_____________________________^
|
= help: a backtick may be missing a pair
error: aborting due to 11 previous errors

9
src/tools/clippy/tests/ui/empty_line_after/doc_comments.1.fixed
View file

@ -132,4 +132,13 @@ pub struct BlockComment;
))]
fn empty_line_in_cfg_attr() {}
trait Foo {
fn bar();
}
impl Foo for LineComment {
/// comment on assoc item
fn bar() {}
}
fn main() {}

9
src/tools/clippy/tests/ui/empty_line_after/doc_comments.2.fixed
View file

@ -141,4 +141,13 @@ pub struct BlockComment;
))]
fn empty_line_in_cfg_attr() {}
trait Foo {
fn bar();
}
impl Foo for LineComment {
/// comment on assoc item
fn bar() {}
}
fn main() {}

10
src/tools/clippy/tests/ui/empty_line_after/doc_comments.rs
View file

@ -144,4 +144,14 @@ pub struct BlockComment;
))]
fn empty_line_in_cfg_attr() {}
trait Foo {
fn bar();
}
impl Foo for LineComment {
/// comment on assoc item
fn bar() {}
}
fn main() {}

51
src/tools/clippy/tests/ui/empty_line_after/doc_comments.stderr
View file

@ -5,11 +5,11 @@ LL | / /// for the crate
LL | |
| |_^
LL | fn first_in_crate() {}
| ------------------- the comment documents this function
| ----------------- the comment documents this function
|
= note: `-D clippy::empty-line-after-doc-comments` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::empty_line_after_doc_comments)]`
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the comment should document the crate use an inner doc comment
|
LL ~ //! Meant to be an
@ -24,9 +24,9 @@ LL | / /// for the module
LL | |
| |_^
LL | fn first_in_module() {}
| -------------------- the comment documents this function
| ------------------ the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the comment should document the parent module use an inner doc comment
|
LL ~ //! Meant to be an
@ -42,9 +42,9 @@ LL | |
| |_^
LL | /// Blank line
LL | fn indented() {}
| ------------- the comment documents this function
| ----------- the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the documentation should include the empty line include it in the comment
|
LL | ///
@ -57,9 +57,9 @@ LL | / /// This should produce a warning
LL | |
| |_^
LL | fn with_doc_and_newline() {}
| ------------------------- the comment documents this function
| ----------------------- the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty lines after doc comment
--> tests/ui/empty_line_after/doc_comments.rs:44:1
@ -72,9 +72,9 @@ LL | |
| |_^
...
LL | fn three_attributes() {}
| --------------------- the comment documents this function
| ------------------- the comment documents this function
|
= help: if the empty lines are unintentional remove them
= help: if the empty lines are unintentional, remove them
error: empty line after doc comment
--> tests/ui/empty_line_after/doc_comments.rs:56:5
@ -84,9 +84,9 @@ LL | | // fn old_code() {}
LL | |
| |_^
LL | fn new_code() {}
| ------------- the comment documents this function
| ----------- the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the doc comment should not document `new_code` comment it out
|
LL | // /// docs for `old_code`
@ -106,7 +106,7 @@ LL | |
LL | struct Multiple;
| --------------- the comment documents this struct
|
= help: if the empty lines are unintentional remove them
= help: if the empty lines are unintentional, remove them
help: if the doc comment should not document `Multiple` comment it out
|
LL ~ // /// Docs
@ -126,9 +126,9 @@ LL | | */
LL | |
| |_^
LL | fn first_in_module() {}
| -------------------- the comment documents this function
| ------------------ the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the comment should document the parent module use an inner doc comment
|
LL | /*!
@ -145,9 +145,9 @@ LL | |
| |_^
...
LL | fn new_code() {}
| ------------- the comment documents this function
| ----------- the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the doc comment should not document `new_code` comment it out
|
LL - /**
@ -163,13 +163,24 @@ LL | |
| |_^
LL | /// Docs for `new_code2`
LL | fn new_code2() {}
| -------------- the comment documents this function
| ------------ the comment documents this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the doc comment should not document `new_code2` comment it out
|
LL | // /// Docs for `old_code2`
| ++
error: aborting due to 10 previous errors
error: empty line after doc comment
--> tests/ui/empty_line_after/doc_comments.rs:152:5
|
LL | / /// comment on assoc item
LL | |
| |_^
LL | fn bar() {}
| ------ the comment documents this function
|
= help: if the empty line is unintentional, remove it
error: aborting due to 11 previous errors

30
src/tools/clippy/tests/ui/empty_line_after/outer_attribute.stderr
View file

@ -5,11 +5,11 @@ LL | / #[crate_type = "lib"]
LL | |
| |_^
LL | fn first_in_crate() {}
| ------------------- the attribute applies to this function
| ----------------- the attribute applies to this function
|
= note: `-D clippy::empty-line-after-outer-attr` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::empty_line_after_outer_attr)]`
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
help: if the attribute should apply to the crate use an inner attribute
|
LL | #![crate_type = "lib"]
@ -23,9 +23,9 @@ LL | |
| |_^
LL | /// some comment
LL | fn with_one_newline_and_comment() {}
| --------------------------------- the attribute applies to this function
| ------------------------------- the attribute applies to this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty line after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:23:1
@ -34,9 +34,9 @@ LL | / #[inline]
LL | |
| |_^
LL | fn with_one_newline() {}
| --------------------- the attribute applies to this function
| ------------------- the attribute applies to this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty lines after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:30:5
@ -46,9 +46,9 @@ LL | |
LL | |
| |_^
LL | fn with_two_newlines() {}
| ---------------------- the attribute applies to this function
| -------------------- the attribute applies to this function
|
= help: if the empty lines are unintentional remove them
= help: if the empty lines are unintentional, remove them
help: if the attribute should apply to the parent module use an inner attribute
|
LL | #![crate_type = "lib"]
@ -63,7 +63,7 @@ LL | |
LL | enum Baz {
| -------- the attribute applies to this enum
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty line after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:45:1
@ -74,7 +74,7 @@ LL | |
LL | struct Foo {
| ---------- the attribute applies to this struct
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty line after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:53:1
@ -85,7 +85,7 @@ LL | |
LL | mod foo {}
| ------- the attribute applies to this module
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty line after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:58:1
@ -95,9 +95,9 @@ LL | | // Still lint cases where the empty line does not immediately follow the
LL | |
| |_^
LL | fn comment_before_empty_line() {}
| ------------------------------ the attribute applies to this function
| ---------------------------- the attribute applies to this function
|
= help: if the empty line is unintentional remove it
= help: if the empty line is unintentional, remove it
error: empty lines after outer attribute
--> tests/ui/empty_line_after/outer_attribute.rs:64:1
@ -107,9 +107,9 @@ LL | / #[allow(unused)]
LL | |
| |_^
LL | pub fn isolated_comment() {}
| ------------------------- the attribute applies to this function
| ----------------------- the attribute applies to this function
|
= help: if the empty lines are unintentional remove them
= help: if the empty lines are unintentional, remove them
error: aborting due to 9 previous errors

1
src/tools/clippy/tests/ui/suspicious_doc_comments.fixed
View file

@ -1,5 +1,6 @@
#![allow(unused)]
#![warn(clippy::suspicious_doc_comments)]
#![allow(clippy::empty_line_after_doc_comments)]
//! Real module documentation.
//! Fake module documentation.

1
src/tools/clippy/tests/ui/suspicious_doc_comments.rs
View file

@ -1,5 +1,6 @@
#![allow(unused)]
#![warn(clippy::suspicious_doc_comments)]
#![allow(clippy::empty_line_after_doc_comments)]
//! Real module documentation.
///! Fake module documentation.

18
src/tools/clippy/tests/ui/suspicious_doc_comments.stderr
View file

@ -1,5 +1,5 @@
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:5:1
--> tests/ui/suspicious_doc_comments.rs:6:1
|
LL | ///! Fake module documentation.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -12,7 +12,7 @@ LL | //! Fake module documentation.
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:9:5
--> tests/ui/suspicious_doc_comments.rs:10:5
|
LL | ///! This module contains useful functions.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -23,7 +23,7 @@ LL | //! This module contains useful functions.
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:21:5
--> tests/ui/suspicious_doc_comments.rs:22:5
|
LL | / /**! This module contains useful functions.
LL | | */
@ -36,7 +36,7 @@ LL + */
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:35:5
--> tests/ui/suspicious_doc_comments.rs:36:5
|
LL | / ///! This module
LL | | ///! contains
@ -51,7 +51,7 @@ LL ~ //! useful functions.
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:43:5
--> tests/ui/suspicious_doc_comments.rs:44:5
|
LL | / ///! a
LL | | ///! b
@ -64,7 +64,7 @@ LL ~ //! b
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:51:5
--> tests/ui/suspicious_doc_comments.rs:52:5
|
LL | ///! a
| ^^^^^^
@ -75,7 +75,7 @@ LL | //! a
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:57:5
--> tests/ui/suspicious_doc_comments.rs:58:5
|
LL | / ///! a
LL | |
@ -90,7 +90,7 @@ LL ~ //! b
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:69:5
--> tests/ui/suspicious_doc_comments.rs:70:5
|
LL | ///! Very cool macro
| ^^^^^^^^^^^^^^^^^^^^
@ -101,7 +101,7 @@ LL | //! Very cool macro
|
error: this is an outer doc comment and does not apply to the parent module or crate
--> tests/ui/suspicious_doc_comments.rs:76:5
--> tests/ui/suspicious_doc_comments.rs:77:5
|
LL | ///! Huh.
| ^^^^^^^^^