From ea78f2116103df5f24ce3ec10cdd025bc13e40fd Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Sun, 22 Sep 2024 18:33:50 +0200 Subject: [PATCH] Use contiguous spans for empty_line_after_* suggestion Replacing an empty span (which an empty line is) with an empty string triggers a debug assertion in rustc. This fixes the debug assertion by using contiguous spans, with the same resulting suggestion. --- clippy_lints/src/doc/empty_line_after.rs | 31 +++++++++++++++++-- .../empty_line_after/outer_attribute.1.fixed | 5 +++ .../empty_line_after/outer_attribute.2.fixed | 5 +++ tests/ui/empty_line_after/outer_attribute.rs | 7 +++++ .../empty_line_after/outer_attribute.stderr | 15 ++++++++- 5 files changed, 60 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/doc/empty_line_after.rs b/clippy_lints/src/doc/empty_line_after.rs index 125b90770614..48bde4198e44 100644 --- a/clippy_lints/src/doc/empty_line_after.rs +++ b/clippy_lints/src/doc/empty_line_after.rs @@ -8,7 +8,7 @@ use rustc_errors::{Applicability, Diag, SuggestionStyle}; use rustc_hir::{ItemKind, Node}; use rustc_lexer::TokenKind; use rustc_lint::LateContext; -use rustc_span::{ExpnKind, InnerSpan, Span, SpanData}; +use rustc_span::{BytePos, ExpnKind, InnerSpan, Span, SpanData}; use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR}; @@ -144,6 +144,30 @@ impl<'a> Gap<'a> { prev_chunk, }) } + + fn contiguous_empty_lines(&self) -> Vec { + let mut spans = Vec::new(); + + let mut prev_span = *self.empty_lines.first().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. + prev_span = prev_span.with_lo(prev_span.lo() - BytePos(1)); + for empty_line in self.empty_lines.iter().skip(1) { + if empty_line.lo() - prev_span.hi() > BytePos(1) { + // If the empty line doesn't immidiately follow the previous one, push the previous span... + spans.push(prev_span); + // ...and start a new one + prev_span = empty_line.with_lo(empty_line.lo() - BytePos(1)); + } else { + // Otherwise, extend the previous span + prev_span = prev_span.with_hi(empty_line.hi()); + } + } + spans.push(prev_span); + + spans + } } /// If the node the attributes/docs apply to is the first in the module/crate suggest converting @@ -192,6 +216,7 @@ fn check_gaps(cx: &LateContext<'_>, gaps: &[Gap<'_>]) -> bool { return false; }; let empty_lines = || gaps.iter().flat_map(|gap| gap.empty_lines.iter().copied()); + let contiguous_empty_lines = || gaps.iter().flat_map(|gap| gap.contiguous_empty_lines()); let mut has_comment = false; let mut has_attr = false; for gap in gaps { @@ -227,7 +252,9 @@ fn check_gaps(cx: &LateContext<'_>, gaps: &[Gap<'_>]) -> bool { diag.multipart_suggestion_with_style( format!("if the empty {lines} {are} unintentional remove {them}"), - empty_lines().map(|empty_line| (empty_line, String::new())).collect(), + contiguous_empty_lines() + .map(|empty_lines| (empty_lines, String::new())) + .collect(), Applicability::MaybeIncorrect, SuggestionStyle::HideCodeAlways, ); diff --git a/tests/ui/empty_line_after/outer_attribute.1.fixed b/tests/ui/empty_line_after/outer_attribute.1.fixed index cd7ea24b6be3..36d80a2c95bf 100644 --- a/tests/ui/empty_line_after/outer_attribute.1.fixed +++ b/tests/ui/empty_line_after/outer_attribute.1.fixed @@ -51,6 +51,11 @@ mod foo {} // Still lint cases where the empty line does not immediately follow the attribute fn comment_before_empty_line() {} +//~v empty_line_after_outer_attr +#[allow(unused)] +// This comment is isolated +pub fn isolated_comment() {} + #[doc = " Returns the escaped value of the textual representation of diff --git a/tests/ui/empty_line_after/outer_attribute.2.fixed b/tests/ui/empty_line_after/outer_attribute.2.fixed index 1b044d2fcde4..0e8e4129e858 100644 --- a/tests/ui/empty_line_after/outer_attribute.2.fixed +++ b/tests/ui/empty_line_after/outer_attribute.2.fixed @@ -54,6 +54,11 @@ mod foo {} // Still lint cases where the empty line does not immediately follow the attribute fn comment_before_empty_line() {} +//~v empty_line_after_outer_attr +#[allow(unused)] +// This comment is isolated +pub fn isolated_comment() {} + #[doc = " Returns the escaped value of the textual representation of diff --git a/tests/ui/empty_line_after/outer_attribute.rs b/tests/ui/empty_line_after/outer_attribute.rs index 81e1a7ab8ed5..1295088ac00e 100644 --- a/tests/ui/empty_line_after/outer_attribute.rs +++ b/tests/ui/empty_line_after/outer_attribute.rs @@ -60,6 +60,13 @@ mod foo {} fn comment_before_empty_line() {} +//~v empty_line_after_outer_attr +#[allow(unused)] + +// This comment is isolated + +pub fn isolated_comment() {} + #[doc = " Returns the escaped value of the textual representation of diff --git a/tests/ui/empty_line_after/outer_attribute.stderr b/tests/ui/empty_line_after/outer_attribute.stderr index b73ebb4f6623..958b40424a92 100644 --- a/tests/ui/empty_line_after/outer_attribute.stderr +++ b/tests/ui/empty_line_after/outer_attribute.stderr @@ -99,5 +99,18 @@ LL | fn comment_before_empty_line() {} | = help: if the empty line is unintentional remove it -error: aborting due to 8 previous errors +error: empty lines after outer attribute + --> tests/ui/empty_line_after/outer_attribute.rs:64:1 + | +LL | / #[allow(unused)] +LL | | +LL | | // This comment is isolated +LL | | + | |_ +LL | pub fn isolated_comment() {} + | ------------------------- the attribute applies to this function + | + = help: if the empty lines are unintentional remove them + +error: aborting due to 9 previous errors