Skip to content

Commit c9b99e4

Browse files
committed
ruff_linter: adjust empty spans after line terminator more generally
Instead of doing this on a lint-by-lint basis, we now just do it right before rendering. This is more broadly applicable. Note that this doesn't fix the diagnostic rendering for the Python parser. But that's using a different path anyway (`annotate-snippets` is only used in tests).
1 parent 2ff2a54 commit c9b99e4

File tree

3 files changed

+46
-42
lines changed

3 files changed

+46
-42
lines changed

crates/ruff_linter/src/checkers/logical_lines.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use ruff_python_codegen::Stylist;
33
use ruff_python_index::Indexer;
44
use ruff_python_parser::{TokenKind, Tokens};
55
use ruff_source_file::LineRanges;
6-
use ruff_text_size::{Ranged, TextRange, TextSize};
6+
use ruff_text_size::{Ranged, TextRange};
77

88
use crate::line_width::IndentWidth;
99
use crate::registry::{AsRule, Rule};
@@ -161,13 +161,7 @@ pub(crate) fn check_logical_lines(
161161
let range = if first_token.kind() == TokenKind::Indent {
162162
first_token.range()
163163
} else {
164-
let mut range =
165-
TextRange::new(locator.line_start(first_token.start()), first_token.start());
166-
if range.is_empty() {
167-
let end = locator.ceil_char_boundary(range.start() + TextSize::from(1));
168-
range = TextRange::new(range.start(), end);
169-
}
170-
range
164+
TextRange::new(locator.line_start(first_token.start()), first_token.start())
171165
};
172166

173167
let indent_level = expand_indent(locator.slice(range), settings.tab_size);

crates/ruff_linter/src/message/text.rs

+38-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use crate::line_width::{IndentWidth, LineWidthBuilder};
1515
use crate::message::diff::Diff;
1616
use crate::message::{Emitter, EmitterContext, Message};
1717
use crate::settings::types::UnsafeFixes;
18+
use crate::Locator;
1819

1920
bitflags! {
2021
#[derive(Default)]
@@ -247,7 +248,8 @@ impl Display for MessageCodeFrame<'_> {
247248
let source = replace_whitespace_and_unprintable(
248249
source_code.slice(TextRange::new(start_offset, end_offset)),
249250
self.message.range() - start_offset,
250-
);
251+
)
252+
.fix_up_empty_spans_after_line_terminator();
251253

252254
let label = self
253255
.message
@@ -365,6 +367,41 @@ struct SourceCode<'a> {
365367
annotation_range: TextRange,
366368
}
367369

370+
impl<'a> SourceCode<'a> {
371+
/// This attempts to "fix up" the span on `SourceCode` in the case where
372+
/// it's an empty span immediately following a line terminator.
373+
///
374+
/// At present, `annotate-snippets` (both upstream and our vendored copy)
375+
/// will render annotations of such spans to point to the space immediately
376+
/// following the previous line. But ideally, this should point to the space
377+
/// immediately preceding the next line.
378+
///
379+
/// After attempting to fix `annotate-snippets` and giving up after a couple
380+
/// hours, this routine takes a different tact: it adjusts the span to be
381+
/// non-empty and it will cover the first codepoint of the following line.
382+
/// This forces `annotate-snippets` to point to the right place.
383+
///
384+
/// See also: <https://github.com/astral-sh/ruff/issues/15509>
385+
fn fix_up_empty_spans_after_line_terminator(self) -> SourceCode<'a> {
386+
if !self.annotation_range.is_empty()
387+
|| self.annotation_range.start() == TextSize::from(0)
388+
|| self.annotation_range.start() >= self.text.text_len()
389+
{
390+
return self;
391+
}
392+
if self.text.as_bytes()[self.annotation_range.start().to_usize() - 1] != b'\n' {
393+
return self;
394+
}
395+
let locator = Locator::new(&self.text);
396+
let start = self.annotation_range.start();
397+
let end = locator.ceil_char_boundary(start + TextSize::from(1));
398+
SourceCode {
399+
annotation_range: TextRange::new(start, end),
400+
..self
401+
}
402+
}
403+
}
404+
368405
#[cfg(test)]
369406
mod tests {
370407
use insta::assert_snapshot;

crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs

+6-33
Original file line numberDiff line numberDiff line change
@@ -223,20 +223,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
223223
// We report under-indentation on every line. This isn't great, but enables
224224
// fix.
225225
if (is_last || !is_blank) && line_indent_size < docstring_indent_size {
226-
// Previously, this used `TextRange::empty(line.start())`,
227-
// but this creates an offset immediately after the line
228-
// terminator. Probably, our renderer should create an
229-
// annotation that points to the beginning of the following
230-
// line. But it doesn't at present and this have proved
231-
// difficult to fix without regressing other cases. So for now,
232-
// we work around this by creating a range that points at the
233-
// first codepoint in the corresponding line. This makes the
234-
// renderer do what we want. ---AG
235-
let start = line.start();
236-
let end = checker
237-
.locator()
238-
.ceil_char_boundary(start + TextSize::from(1));
239-
let mut diagnostic = Diagnostic::new(UnderIndentation, TextRange::new(start, end));
226+
let mut diagnostic =
227+
Diagnostic::new(UnderIndentation, TextRange::empty(line.start()));
240228
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
241229
clean_space(docstring.indentation),
242230
TextRange::at(line.start(), line_indent.text_len()),
@@ -293,16 +281,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
293281

294282
// We report over-indentation on every line. This isn't great, but
295283
// enables the fix capability.
296-
//
297-
// Also, we ensure that our range points to the first character
298-
// of the line instead of the empty spance immediately
299-
// preceding the line. See above for how we handle under
300-
// indentation for more explanation. ---AG
301-
let start = line.start();
302-
let end = checker
303-
.locator()
304-
.ceil_char_boundary(start + TextSize::from(1));
305-
let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::new(start, end));
284+
let mut diagnostic =
285+
Diagnostic::new(OverIndentation, TextRange::empty(line.start()));
306286

307287
let edit = if indent.is_empty() {
308288
// Delete the entire indent.
@@ -344,15 +324,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
344324

345325
let is_indent_only = line_indent.len() == last.len();
346326
if last_line_over_indent > 0 && is_indent_only {
347-
// We ensure that our range points to the first character of
348-
// the line instead of the empty spance immediately preceding
349-
// the line. See above for how we handle under indentation for
350-
// more explanation. ---AG
351-
let start = last.start();
352-
let end = checker
353-
.locator()
354-
.ceil_char_boundary(start + TextSize::from(1));
355-
let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::new(start, end));
327+
let mut diagnostic =
328+
Diagnostic::new(OverIndentation, TextRange::empty(last.start()));
356329
let indent = clean_space(docstring.indentation);
357330
let range = TextRange::at(last.start(), line_indent.text_len());
358331
let edit = if indent.is_empty() {

0 commit comments

Comments
 (0)