From 5806bc915d9a6cd30ce78644b84ff884f26cf841 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 5 Jun 2024 17:55:14 +0200 Subject: [PATCH] Fix formatter instability for lines only consisting of zero-width characters (#11748) --- Cargo.lock | 1 - crates/ruff_formatter/src/printer/mod.rs | 20 +++++++++++++++- crates/ruff_python_formatter/Cargo.toml | 1 - .../ruff/docstring_non_visible_characters.py | 6 +++++ crates/ruff_python_formatter/src/verbatim.rs | 14 +---------- ...t@docstring_non_visible_characters.py.snap | 23 +++++++++++++++++++ 6 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_non_visible_characters.py create mode 100644 crates/ruff_python_formatter/tests/snapshots/format@docstring_non_visible_characters.py.snap diff --git a/Cargo.lock b/Cargo.lock index 162ca379af..6f75068cf2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2215,7 +2215,6 @@ dependencies = [ "static_assertions", "thiserror", "tracing", - "unicode-width", ] [[package]] diff --git a/crates/ruff_formatter/src/printer/mod.rs b/crates/ruff_formatter/src/printer/mod.rs index 8f4edd4e1c..916dc5cad5 100644 --- a/crates/ruff_formatter/src/printer/mod.rs +++ b/crates/ruff_formatter/src/printer/mod.rs @@ -124,7 +124,7 @@ impl<'a> Printer<'a> { self.flush_line_suffixes(queue, stack, Some(element)); } else { // Only print a newline if the current line isn't already empty - if self.state.line_width > 0 { + if !self.state.buffer[self.state.line_start..].is_empty() { self.push_marker(); self.print_char('\n'); } @@ -830,6 +830,7 @@ impl<'a> Printer<'a> { .push_str(self.options.line_ending.as_str()); self.state.line_width = 0; + self.state.line_start = self.state.buffer.len(); // Fit's only tests if groups up to the first line break fit. // The next group must re-measure if it still fits. @@ -872,12 +873,29 @@ enum FillPairLayout { /// position the printer currently is. #[derive(Default, Debug)] struct PrinterState<'a> { + /// The formatted output. buffer: String, + + /// The source markers that map source positions to formatted positions. source_markers: Vec, + + /// The next source position that should be flushed when writing the next text. pending_source_position: Option, + + /// The current indentation that should be written before the next text. pending_indent: Indention, + + /// Caches if the code up to the next newline has been measured to fit on a single line. + /// This is used to avoid re-measuring the same content multiple times. measured_group_fits: bool, + + /// The offset at which the current line in `buffer` starts. + line_start: usize, + + /// The accumulated unicode-width of all characters on the current line. line_width: u32, + + /// The line suffixes that should be printed at the end of the line. line_suffixes: LineSuffixes<'a>, verbatim_markers: Vec, group_modes: GroupModes, diff --git a/crates/ruff_python_formatter/Cargo.toml b/crates/ruff_python_formatter/Cargo.toml index a57e480130..befb78e31f 100644 --- a/crates/ruff_python_formatter/Cargo.toml +++ b/crates/ruff_python_formatter/Cargo.toml @@ -37,7 +37,6 @@ smallvec = { workspace = true } static_assertions = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } -unicode-width = { workspace = true } [dev-dependencies] ruff_formatter = { workspace = true } diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_non_visible_characters.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_non_visible_characters.py new file mode 100644 index 0000000000..0b816c10a4 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_non_visible_characters.py @@ -0,0 +1,6 @@ +# Regresssion test for https://github.com/astral-sh/ruff/issues/11724 + +''' + + +''' diff --git a/crates/ruff_python_formatter/src/verbatim.rs b/crates/ruff_python_formatter/src/verbatim.rs index 587f2d0690..df70e6e298 100644 --- a/crates/ruff_python_formatter/src/verbatim.rs +++ b/crates/ruff_python_formatter/src/verbatim.rs @@ -2,8 +2,6 @@ use std::borrow::Cow; use std::iter::FusedIterator; use std::slice::Iter; -use unicode_width::UnicodeWidthStr; - use ruff_formatter::{write, FormatError}; use ruff_python_ast::AnyNodeRef; use ruff_python_ast::Stmt; @@ -760,17 +758,7 @@ impl Format> for FormatVerbatimStatementRange { // Write the line separator that terminates the line, except if it is the last line (that isn't separated by a hard line break). if logical_line.has_trailing_newline { - // Insert an empty line if the text is non-empty but all characters have a width of zero. - // This is necessary to work around the fact that the Printer omits hard line breaks if the line width is 0. - // The alternative is to "fix" the printer and explicitly track the width and whether the line is empty. - // There's currently no use case for zero-width content outside of the verbatim context (and, form feeds are a Python specific speciality). - // It, therefore, feels wrong to add additional complexity to the very hot `Printer::print_char` function, - // to work around this special case. Therefore, work around the Printer behavior here, in the cold verbatim-formatting. - if f.context().source()[trimmed_line_range].width() == 0 { - empty_line().fmt(f)?; - } else { - hard_line_break().fmt(f)?; - } + hard_line_break().fmt(f)?; } } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@docstring_non_visible_characters.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@docstring_non_visible_characters.py.snap new file mode 100644 index 0000000000..9029655d5a --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@docstring_non_visible_characters.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_non_visible_characters.py +--- +## Input +```python +# Regresssion test for https://github.com/astral-sh/ruff/issues/11724 + +''' + + +''' +``` + +## Output +```python +# Regresssion test for https://github.com/astral-sh/ruff/issues/11724 + +""" + + +""" +```