From 282de2506a0714ff05bc86be3ae636f93fb40aa3 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Wed, 15 Jan 2025 13:04:21 -0500 Subject: [PATCH] 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). --- .../ruff_linter/src/checkers/logical_lines.rs | 10 +---- crates/ruff_linter/src/message/text.rs | 39 ++++++++++++++++++- .../src/rules/pydocstyle/rules/indent.rs | 39 +++---------------- 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/crates/ruff_linter/src/checkers/logical_lines.rs b/crates/ruff_linter/src/checkers/logical_lines.rs index 487327a397bfb1..1933889387b92e 100644 --- a/crates/ruff_linter/src/checkers/logical_lines.rs +++ b/crates/ruff_linter/src/checkers/logical_lines.rs @@ -3,7 +3,7 @@ use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_python_parser::{TokenKind, Tokens}; use ruff_source_file::LineRanges; -use ruff_text_size::{Ranged, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextRange}; use crate::line_width::IndentWidth; use crate::registry::{AsRule, Rule}; @@ -161,13 +161,7 @@ pub(crate) fn check_logical_lines( let range = if first_token.kind() == TokenKind::Indent { first_token.range() } else { - let mut range = - TextRange::new(locator.line_start(first_token.start()), first_token.start()); - if range.is_empty() { - let end = locator.ceil_char_boundary(range.start() + TextSize::from(1)); - range = TextRange::new(range.start(), end); - } - range + TextRange::new(locator.line_start(first_token.start()), first_token.start()) }; let indent_level = expand_indent(locator.slice(range), settings.tab_size); diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index 7ea602623b3d3b..108a77aad2e81e 100644 --- a/crates/ruff_linter/src/message/text.rs +++ b/crates/ruff_linter/src/message/text.rs @@ -15,6 +15,7 @@ use crate::line_width::{IndentWidth, LineWidthBuilder}; use crate::message::diff::Diff; use crate::message::{Emitter, EmitterContext, Message}; use crate::settings::types::UnsafeFixes; +use crate::Locator; bitflags! { #[derive(Default)] @@ -247,7 +248,8 @@ impl Display for MessageCodeFrame<'_> { let source = replace_whitespace_and_unprintable( source_code.slice(TextRange::new(start_offset, end_offset)), self.message.range() - start_offset, - ); + ) + .fix_up_empty_spans_after_line_terminator(); let label = self .message @@ -365,6 +367,41 @@ struct SourceCode<'a> { annotation_range: TextRange, } +impl<'a> SourceCode<'a> { + /// This attempts to "fix up" the span on `SourceCode` in the case where + /// it's an empty span immediately following a line terminator. + /// + /// At present, `annotate-snippets` (both upstream and our vendored copy) + /// will render annotations of such spans to point to the space immediately + /// following the previous line. But ideally, this should point to the space + /// immediately preceding the next line. + /// + /// After attempting to fix `annotate-snippets` and giving up after a couple + /// hours, this routine takes a different tact: it adjusts the span to be + /// non-empty and it will cover the first codepoint of the following line. + /// This forces `annotate-snippets` to point to the right place. + /// + /// See also: https://github.com/astral-sh/ruff/issues/15509 + fn fix_up_empty_spans_after_line_terminator(self) -> SourceCode<'a> { + if !self.annotation_range.is_empty() + || self.annotation_range.start() == TextSize::from(0) + || self.annotation_range.start() >= self.text.text_len() + { + return self; + } + if self.text.as_bytes()[self.annotation_range.start().to_usize() - 1] != b'\n' { + return self; + } + let locator = Locator::new(&self.text); + let start = self.annotation_range.start(); + let end = locator.ceil_char_boundary(start + TextSize::from(1)); + SourceCode { + annotation_range: TextRange::new(start, end), + ..self + } + } +} + #[cfg(test)] mod tests { use insta::assert_snapshot; diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs index 3a0809529b8e00..ae59026687db55 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs @@ -223,20 +223,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { // We report under-indentation on every line. This isn't great, but enables // fix. if (is_last || !is_blank) && line_indent_size < docstring_indent_size { - // Previously, this used `TextRange::empty(line.start())`, - // but this creates an offset immediately after the line - // terminator. Probably, our renderer should create an - // annotation that points to the beginning of the following - // line. But it doesn't at present and this have proved - // difficult to fix without regressing other cases. So for now, - // we work around this by creating a range that points at the - // first codepoint in the corresponding line. This makes the - // renderer do what we want. ---AG - let start = line.start(); - let end = checker - .locator() - .ceil_char_boundary(start + TextSize::from(1)); - let mut diagnostic = Diagnostic::new(UnderIndentation, TextRange::new(start, end)); + let mut diagnostic = + Diagnostic::new(UnderIndentation, TextRange::empty(line.start())); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( clean_space(docstring.indentation), TextRange::at(line.start(), line_indent.text_len()), @@ -293,16 +281,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { // We report over-indentation on every line. This isn't great, but // enables the fix capability. - // - // Also, we ensure that our range points to the first character - // of the line instead of the empty spance immediately - // preceding the line. See above for how we handle under - // indentation for more explanation. ---AG - let start = line.start(); - let end = checker - .locator() - .ceil_char_boundary(start + TextSize::from(1)); - let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::new(start, end)); + let mut diagnostic = + Diagnostic::new(OverIndentation, TextRange::empty(line.start())); let edit = if indent.is_empty() { // Delete the entire indent. @@ -344,15 +324,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { let is_indent_only = line_indent.len() == last.len(); if last_line_over_indent > 0 && is_indent_only { - // We ensure that our range points to the first character of - // the line instead of the empty spance immediately preceding - // the line. See above for how we handle under indentation for - // more explanation. ---AG - let start = last.start(); - let end = checker - .locator() - .ceil_char_boundary(start + TextSize::from(1)); - let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::new(start, end)); + let mut diagnostic = + Diagnostic::new(OverIndentation, TextRange::empty(last.start())); let indent = clean_space(docstring.indentation); let range = TextRange::at(last.start(), line_indent.text_len()); let edit = if indent.is_empty() {