From 3ff12cbd8ce47d9b7b43b6d8811ef68bcaed66b8 Mon Sep 17 00:00:00 2001 From: Seo Sanghyeon Date: Thu, 1 Feb 2024 00:49:26 +0900 Subject: [PATCH 1/3] Removing trailing whitespace inside multiline strings is unsafe --- .../test/fixtures/pycodestyle/W291.py | 2 + .../ruff_linter/src/rules/pycodestyle/mod.rs | 1 + .../pycodestyle/rules/trailing_whitespace.rs | 10 +++- ...les__pycodestyle__tests__W291_W291.py.snap | 17 +++++++ crates/ruff_python_index/src/indexer.rs | 12 +++++ crates/ruff_python_index/src/lib.rs | 1 + .../ruff_python_index/src/multiline_ranges.rs | 47 +++++++++++++++++++ 7 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pycodestyle/W291.py create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W291_W291.py.snap create mode 100644 crates/ruff_python_index/src/multiline_ranges.rs diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W291.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W291.py new file mode 100644 index 0000000000000..4ccbd43194a34 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W291.py @@ -0,0 +1,2 @@ +'''trailing whitespace +inside a multiline string''' diff --git a/crates/ruff_linter/src/rules/pycodestyle/mod.rs b/crates/ruff_linter/src/rules/pycodestyle/mod.rs index 12bc583c20856..7cd5982fc9504 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/mod.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/mod.rs @@ -52,6 +52,7 @@ mod tests { #[test_case(Rule::SyntaxError, Path::new("E999.py"))] #[test_case(Rule::TabIndentation, Path::new("W19.py"))] #[test_case(Rule::TrailingWhitespace, Path::new("W29.py"))] + #[test_case(Rule::TrailingWhitespace, Path::new("W291.py"))] #[test_case(Rule::TrueFalseComparison, Path::new("E712.py"))] #[test_case(Rule::TypeComparison, Path::new("E721.py"))] #[test_case(Rule::UselessSemicolon, Path::new("E70.py"))] diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs index 7df8ee2eb5986..20787516a2e35 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -101,8 +101,16 @@ pub(crate) fn trailing_whitespace( return Some(diagnostic); } } else if settings.rules.enabled(Rule::TrailingWhitespace) { + // Removing trailing whitespace is not safe inside multiline strings. + let safe = !indexer.multiline_ranges().intersects(range); let mut diagnostic = Diagnostic::new(TrailingWhitespace, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_deletion(range))); + let edit = Edit::range_deletion(range); + let fix = if safe { + Fix::safe_edit(edit) + } else { + Fix::unsafe_edit(edit) + }; + diagnostic.set_fix(fix); return Some(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W291_W291.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W291_W291.py.snap new file mode 100644 index 0000000000000..441da196975e3 --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W291_W291.py.snap @@ -0,0 +1,17 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +W291.py:1:23: W291 [*] Trailing whitespace + | +1 | '''trailing whitespace + | ^ W291 +2 | inside a multiline string''' + | + = help: Remove trailing whitespace + +ℹ Unsafe fix +1 |-'''trailing whitespace + 1 |+'''trailing whitespace +2 2 | inside a multiline string''' + + diff --git a/crates/ruff_python_index/src/indexer.rs b/crates/ruff_python_index/src/indexer.rs index 27af11356b3dd..18ee7705555d0 100644 --- a/crates/ruff_python_index/src/indexer.rs +++ b/crates/ruff_python_index/src/indexer.rs @@ -11,6 +11,7 @@ use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::fstring_ranges::{FStringRanges, FStringRangesBuilder}; +use crate::multiline_ranges::{MultilineRanges, MultilineRangesBuilder}; use crate::CommentRangesBuilder; pub struct Indexer { @@ -21,6 +22,9 @@ pub struct Indexer { /// The range of all f-string in the source document. fstring_ranges: FStringRanges, + + /// The range of all multiline strings in the source document. + multiline_ranges: MultilineRanges, } impl Indexer { @@ -29,6 +33,7 @@ impl Indexer { let mut comment_ranges_builder = CommentRangesBuilder::default(); let mut fstring_ranges_builder = FStringRangesBuilder::default(); + let mut multiline_ranges_builder = MultilineRangesBuilder::default(); let mut continuation_lines = Vec::new(); // Token, end let mut prev_end = TextSize::default(); @@ -61,6 +66,7 @@ impl Indexer { comment_ranges_builder.visit_token(tok, *range); fstring_ranges_builder.visit_token(tok, *range); + multiline_ranges_builder.visit_token(tok, *range); match tok { Tok::Newline | Tok::NonLogicalNewline => { @@ -82,6 +88,7 @@ impl Indexer { comment_ranges: comment_ranges_builder.finish(), continuation_lines, fstring_ranges: fstring_ranges_builder.finish(), + multiline_ranges: multiline_ranges_builder.finish(), } } @@ -95,6 +102,11 @@ impl Indexer { &self.fstring_ranges } + /// Returns the byte offset ranges of multiline strings. + pub const fn multiline_ranges(&self) -> &MultilineRanges { + &self.multiline_ranges + } + /// Returns the line start positions of continuations (backslash). pub fn continuation_line_starts(&self) -> &[TextSize] { &self.continuation_lines diff --git a/crates/ruff_python_index/src/lib.rs b/crates/ruff_python_index/src/lib.rs index f2c22a77bf119..2a4660f0125cf 100644 --- a/crates/ruff_python_index/src/lib.rs +++ b/crates/ruff_python_index/src/lib.rs @@ -1,6 +1,7 @@ mod comment_ranges; mod fstring_ranges; mod indexer; +mod multiline_ranges; pub use comment_ranges::{tokens_and_ranges, CommentRangesBuilder}; pub use indexer::Indexer; diff --git a/crates/ruff_python_index/src/multiline_ranges.rs b/crates/ruff_python_index/src/multiline_ranges.rs new file mode 100644 index 0000000000000..84abf664a33c1 --- /dev/null +++ b/crates/ruff_python_index/src/multiline_ranges.rs @@ -0,0 +1,47 @@ +use ruff_python_parser::Tok; +use ruff_text_size::TextRange; + +/// Stores the range of all multiline strings in a file sorted by +/// [`TextRange::start`]. +pub struct MultilineRanges { + ranges: Vec, +} + +impl MultilineRanges { + /// Returns `true` if the given range is inside a multiline string. + pub fn intersects(&self, target: TextRange) -> bool { + self.ranges + .binary_search_by(|range| { + if range.contains_range(target) { + std::cmp::Ordering::Equal + } else if range.end() < target.start() { + std::cmp::Ordering::Less + } else { + std::cmp::Ordering::Greater + } + }) + .is_ok() + } +} + +#[derive(Default)] +pub(crate) struct MultilineRangesBuilder { + ranges: Vec, +} + +impl MultilineRangesBuilder { + pub(crate) fn visit_token(&mut self, token: &Tok, range: TextRange) { + match token { + Tok::String { triple_quoted, .. } => { + if *triple_quoted { + self.ranges.push(range) + } + } + _ => {} + } + } + + pub(crate) fn finish(self) -> MultilineRanges { + MultilineRanges { ranges: self.ranges } + } +} From 45084d4ec181845d7a981753019020edc6b9948e Mon Sep 17 00:00:00 2001 From: Seo Sanghyeon Date: Thu, 1 Feb 2024 02:39:51 +0900 Subject: [PATCH 2/3] Fix formatting, Clippy lints, and apply review comments --- .../pycodestyle/rules/trailing_whitespace.rs | 17 +++++++++-------- .../ruff_python_index/src/multiline_ranges.rs | 13 ++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs index 20787516a2e35..5939dcdf01f73 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_index::Indexer; use ruff_source_file::{Line, Locator}; @@ -104,13 +104,14 @@ pub(crate) fn trailing_whitespace( // Removing trailing whitespace is not safe inside multiline strings. let safe = !indexer.multiline_ranges().intersects(range); let mut diagnostic = Diagnostic::new(TrailingWhitespace, range); - let edit = Edit::range_deletion(range); - let fix = if safe { - Fix::safe_edit(edit) - } else { - Fix::unsafe_edit(edit) - }; - diagnostic.set_fix(fix); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_deletion(range), + if safe { + Applicability::Safe + } else { + Applicability::Unsafe + }, + )); return Some(diagnostic); } } diff --git a/crates/ruff_python_index/src/multiline_ranges.rs b/crates/ruff_python_index/src/multiline_ranges.rs index 84abf664a33c1..5f0bb64d2573e 100644 --- a/crates/ruff_python_index/src/multiline_ranges.rs +++ b/crates/ruff_python_index/src/multiline_ranges.rs @@ -31,17 +31,16 @@ pub(crate) struct MultilineRangesBuilder { impl MultilineRangesBuilder { pub(crate) fn visit_token(&mut self, token: &Tok, range: TextRange) { - match token { - Tok::String { triple_quoted, .. } => { - if *triple_quoted { - self.ranges.push(range) - } + if let Tok::String { triple_quoted, .. } = token { + if *triple_quoted { + self.ranges.push(range); } - _ => {} } } pub(crate) fn finish(self) -> MultilineRanges { - MultilineRanges { ranges: self.ranges } + MultilineRanges { + ranges: self.ranges, + } } } From bf8225b53271a243685790cbc4eccaa4b9b8bbac Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 31 Jan 2024 16:39:26 -0500 Subject: [PATCH 3/3] Move expression --- .../src/rules/pycodestyle/rules/trailing_whitespace.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs index 5939dcdf01f73..e1afe96f20860 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -101,15 +101,14 @@ pub(crate) fn trailing_whitespace( return Some(diagnostic); } } else if settings.rules.enabled(Rule::TrailingWhitespace) { - // Removing trailing whitespace is not safe inside multiline strings. - let safe = !indexer.multiline_ranges().intersects(range); let mut diagnostic = Diagnostic::new(TrailingWhitespace, range); diagnostic.set_fix(Fix::applicable_edit( Edit::range_deletion(range), - if safe { - Applicability::Safe - } else { + // Removing trailing whitespace is not safe inside multiline strings. + if indexer.multiline_ranges().intersects(range) { Applicability::Unsafe + } else { + Applicability::Safe }, )); return Some(diagnostic);