Skip to content

Commit ea78f21

Browse files
flip1995GHA CI
authored andcommitted
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.
1 parent 43e3384 commit ea78f21

File tree

5 files changed

+60
-3
lines changed

5 files changed

+60
-3
lines changed

clippy_lints/src/doc/empty_line_after.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_errors::{Applicability, Diag, SuggestionStyle};
88
use rustc_hir::{ItemKind, Node};
99
use rustc_lexer::TokenKind;
1010
use rustc_lint::LateContext;
11-
use rustc_span::{ExpnKind, InnerSpan, Span, SpanData};
11+
use rustc_span::{BytePos, ExpnKind, InnerSpan, Span, SpanData};
1212

1313
use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR};
1414

@@ -144,6 +144,30 @@ impl<'a> Gap<'a> {
144144
prev_chunk,
145145
})
146146
}
147+
148+
fn contiguous_empty_lines(&self) -> Vec<Span> {
149+
let mut spans = Vec::new();
150+
151+
let mut prev_span = *self.empty_lines.first().expect("at least one empty line");
152+
153+
// The BytePos subtraction here is safe, as before an empty line, there must be at least one
154+
// attribute/comment.
155+
prev_span = prev_span.with_lo(prev_span.lo() - BytePos(1));
156+
for empty_line in self.empty_lines.iter().skip(1) {
157+
if empty_line.lo() - prev_span.hi() > BytePos(1) {
158+
// If the empty line doesn't immidiately follow the previous one, push the previous span...
159+
spans.push(prev_span);
160+
// ...and start a new one
161+
prev_span = empty_line.with_lo(empty_line.lo() - BytePos(1));
162+
} else {
163+
// Otherwise, extend the previous span
164+
prev_span = prev_span.with_hi(empty_line.hi());
165+
}
166+
}
167+
spans.push(prev_span);
168+
169+
spans
170+
}
147171
}
148172

149173
/// 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 {
192216
return false;
193217
};
194218
let empty_lines = || gaps.iter().flat_map(|gap| gap.empty_lines.iter().copied());
219+
let contiguous_empty_lines = || gaps.iter().flat_map(|gap| gap.contiguous_empty_lines());
195220
let mut has_comment = false;
196221
let mut has_attr = false;
197222
for gap in gaps {
@@ -227,7 +252,9 @@ fn check_gaps(cx: &LateContext<'_>, gaps: &[Gap<'_>]) -> bool {
227252

228253
diag.multipart_suggestion_with_style(
229254
format!("if the empty {lines} {are} unintentional remove {them}"),
230-
empty_lines().map(|empty_line| (empty_line, String::new())).collect(),
255+
contiguous_empty_lines()
256+
.map(|empty_lines| (empty_lines, String::new()))
257+
.collect(),
231258
Applicability::MaybeIncorrect,
232259
SuggestionStyle::HideCodeAlways,
233260
);

tests/ui/empty_line_after/outer_attribute.1.fixed

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ mod foo {}
5151
// Still lint cases where the empty line does not immediately follow the attribute
5252
fn comment_before_empty_line() {}
5353

54+
//~v empty_line_after_outer_attr
55+
#[allow(unused)]
56+
// This comment is isolated
57+
pub fn isolated_comment() {}
58+
5459
#[doc = "
5560
Returns the escaped value of the textual representation of
5661

tests/ui/empty_line_after/outer_attribute.2.fixed

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ mod foo {}
5454
// Still lint cases where the empty line does not immediately follow the attribute
5555
fn comment_before_empty_line() {}
5656

57+
//~v empty_line_after_outer_attr
58+
#[allow(unused)]
59+
// This comment is isolated
60+
pub fn isolated_comment() {}
61+
5762
#[doc = "
5863
Returns the escaped value of the textual representation of
5964

tests/ui/empty_line_after/outer_attribute.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ mod foo {}
6060

6161
fn comment_before_empty_line() {}
6262

63+
//~v empty_line_after_outer_attr
64+
#[allow(unused)]
65+
66+
// This comment is isolated
67+
68+
pub fn isolated_comment() {}
69+
6370
#[doc = "
6471
Returns the escaped value of the textual representation of
6572

tests/ui/empty_line_after/outer_attribute.stderr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,5 +99,18 @@ LL | fn comment_before_empty_line() {}
9999
|
100100
= help: if the empty line is unintentional remove it
101101

102-
error: aborting due to 8 previous errors
102+
error: empty lines after outer attribute
103+
--> tests/ui/empty_line_after/outer_attribute.rs:64:1
104+
|
105+
LL | / #[allow(unused)]
106+
LL | |
107+
LL | | // This comment is isolated
108+
LL | |
109+
| |_
110+
LL | pub fn isolated_comment() {}
111+
| ------------------------- the attribute applies to this function
112+
|
113+
= help: if the empty lines are unintentional remove them
114+
115+
error: aborting due to 9 previous errors
103116

0 commit comments

Comments
 (0)