From 51bc758d189c62535a66ac8b719772513a45fe0a Mon Sep 17 00:00:00 2001 From: Evan Rittenhouse Date: Sun, 18 Jun 2023 19:57:35 -0500 Subject: [PATCH 1/4] Fix corner case with terminal backslash in W293 --- crates/ruff/src/checkers/physical_lines.rs | 9 ++++-- .../pycodestyle/rules/trailing_whitespace.rs | 30 +++++++++++++++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/checkers/physical_lines.rs b/crates/ruff/src/checkers/physical_lines.rs index 8934a75762f88..9c283e18db1a3 100644 --- a/crates/ruff/src/checkers/physical_lines.rs +++ b/crates/ruff/src/checkers/physical_lines.rs @@ -1,11 +1,10 @@ //! Lint rules based on checking physical lines. - use ruff_text_size::TextSize; use std::path::Path; use ruff_diagnostics::Diagnostic; use ruff_python_ast::source_code::{Indexer, Locator, Stylist}; -use ruff_python_whitespace::UniversalNewlines; +use ruff_python_whitespace::{Line, UniversalNewlines}; use crate::registry::Rule; use crate::rules::copyright::rules::missing_copyright_notice; @@ -58,6 +57,8 @@ pub(crate) fn check_physical_lines( let mut commented_lines_iter = indexer.comment_ranges().iter().peekable(); let mut doc_lines_iter = doc_lines.iter().peekable(); + let mut prev_line: Option = None; + for (index, line) in locator.contents().universal_newlines().enumerate() { while commented_lines_iter .next_if(|comment_range| line.range().contains_range(**comment_range)) @@ -146,7 +147,7 @@ pub(crate) fn check_physical_lines( } if enforce_trailing_whitespace || enforce_blank_line_contains_whitespace { - if let Some(diagnostic) = trailing_whitespace(&line, settings) { + if let Some(diagnostic) = trailing_whitespace(&line, &prev_line, settings) { diagnostics.push(diagnostic); } } @@ -156,6 +157,8 @@ pub(crate) fn check_physical_lines( diagnostics.push(diagnostic); } } + + prev_line = Some(line); } if enforce_no_newline_at_end_of_file { diff --git a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs index ff7082c9c79b9..c841f6a0faef8 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -72,7 +72,11 @@ impl AlwaysAutofixableViolation for BlankLineWithWhitespace { } /// W291, W293 -pub(crate) fn trailing_whitespace(line: &Line, settings: &Settings) -> Option { +pub(crate) fn trailing_whitespace( + line: &Line, + prev_line: &Option, + settings: &Settings, +) -> Option { let whitespace_len: TextSize = line .chars() .rev() @@ -85,10 +89,30 @@ pub(crate) fn trailing_whitespace(line: &Line, settings: &Settings) -> Option Date: Mon, 19 Jun 2023 08:48:23 -0500 Subject: [PATCH 2/4] Simplify calculation --- .../rules/pycodestyle/rules/trailing_whitespace.rs | 13 +++---------- ...uff__rules__pycodestyle__tests__W291_W29.py.snap | 6 +++--- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs index c841f6a0faef8..1f9035870cdb8 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -97,15 +97,9 @@ pub(crate) fn trailing_whitespace( if let Some(prev) = prev_line { let trimmed = prev.trim_end(); if trimmed.ends_with('\\') { - let initial_len = prev.text_len(); + // Shift the diagnostic to remove the continuation as well. diagnostic.range = range.sub_start( - // Shift by the amount of whitespace in the previous line, plus the - // newline, plus the slash, plus any remaining whitespace after it. - initial_len - trimmed.text_len() - + TextSize::from(1) - + '\\'.text_len() - + (trimmed.text_len() - trimmed.trim_end().text_len() - + TextSize::from(1)), + (prev.text_len() - trimmed.text_len()) + TextSize::from(2), ); } } @@ -118,8 +112,7 @@ pub(crate) fn trailing_whitespace( } else if settings.rules.enabled(Rule::TrailingWhitespace) { let mut diagnostic = Diagnostic::new(TrailingWhitespace, range); if settings.rules.should_fix(Rule::TrailingWhitespace) { - #[allow(deprecated)] - diagnostic.set_fix(Fix::unspecified(Edit::range_deletion(range))); + diagnostic.set_fix(Fix::automatic(Edit::range_deletion(range))); } return Some(diagnostic); } diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W291_W29.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W291_W29.py.snap index 356d7ebaf7c0f..e6e4ed42aa421 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W291_W29.py.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W291_W29.py.snap @@ -12,7 +12,7 @@ W29.py:4:6: W291 [*] Trailing whitespace | = help: Remove trailing whitespace -ℹ Suggested fix +ℹ Fix 1 1 | #: Okay 2 2 | # 情 3 3 | #: W291:1:6 @@ -33,7 +33,7 @@ W29.py:11:35: W291 [*] Trailing whitespace | = help: Remove trailing whitespace -ℹ Suggested fix +ℹ Fix 8 8 | bang = 12 9 9 | #: W291:2:35 10 10 | '''multiline @@ -54,7 +54,7 @@ W29.py:13:6: W291 [*] Trailing whitespace | = help: Remove trailing whitespace -ℹ Suggested fix +ℹ Fix 10 10 | '''multiline 11 11 | string with trailing whitespace''' 12 12 | #: W291 W292 noeol From 2abdc2540a0ea7756274cbd3c7a53713f8a5d1b2 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 19 Jun 2023 22:06:45 -0400 Subject: [PATCH 3/4] Use preceded_by_continuations --- crates/ruff/src/checkers/physical_lines.rs | 6 +--- .../pycodestyle/rules/trailing_whitespace.rs | 29 ++++++++++--------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/crates/ruff/src/checkers/physical_lines.rs b/crates/ruff/src/checkers/physical_lines.rs index 9c283e18db1a3..3bf5400cd355b 100644 --- a/crates/ruff/src/checkers/physical_lines.rs +++ b/crates/ruff/src/checkers/physical_lines.rs @@ -57,8 +57,6 @@ pub(crate) fn check_physical_lines( let mut commented_lines_iter = indexer.comment_ranges().iter().peekable(); let mut doc_lines_iter = doc_lines.iter().peekable(); - let mut prev_line: Option = None; - for (index, line) in locator.contents().universal_newlines().enumerate() { while commented_lines_iter .next_if(|comment_range| line.range().contains_range(**comment_range)) @@ -147,7 +145,7 @@ pub(crate) fn check_physical_lines( } if enforce_trailing_whitespace || enforce_blank_line_contains_whitespace { - if let Some(diagnostic) = trailing_whitespace(&line, &prev_line, settings) { + if let Some(diagnostic) = trailing_whitespace(&line, locator, indexer, settings) { diagnostics.push(diagnostic); } } @@ -157,8 +155,6 @@ pub(crate) fn check_physical_lines( diagnostics.push(diagnostic); } } - - prev_line = Some(line); } if enforce_no_newline_at_end_of_file { diff --git a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs index 1f9035870cdb8..740d43d964518 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -2,6 +2,8 @@ use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers; +use ruff_python_ast::source_code::{Indexer, Locator}; use ruff_python_whitespace::Line; use crate::registry::Rule; @@ -74,7 +76,8 @@ impl AlwaysAutofixableViolation for BlankLineWithWhitespace { /// W291, W293 pub(crate) fn trailing_whitespace( line: &Line, - prev_line: &Option, + locator: &Locator, + indexer: &Indexer, settings: &Settings, ) -> Option { let whitespace_len: TextSize = line @@ -91,20 +94,18 @@ pub(crate) fn trailing_whitespace( let mut diagnostic = Diagnostic::new(BlankLineWithWhitespace, range); if settings.rules.should_fix(Rule::BlankLineWithWhitespace) { - // If this line is blank with whitespace, we have to ensure that the previous line - // doesn't end with a backslash. If it did, the file would end with a backslash - // and therefore have an "unexpected EOF" SyntaxError, so we have to remove it. - if let Some(prev) = prev_line { - let trimmed = prev.trim_end(); - if trimmed.ends_with('\\') { - // Shift the diagnostic to remove the continuation as well. - diagnostic.range = range.sub_start( - (prev.text_len() - trimmed.text_len()) + TextSize::from(2), - ); - } + // Remove any preceding continuations, to avoid introducing a potential + // syntax error. + if let Some(continuation) = + helpers::preceded_by_continuations(line.start(), locator, indexer) + { + diagnostic.set_fix(Fix::suggested(Edit::range_deletion(TextRange::new( + continuation, + range.end(), + )))); + } else { + diagnostic.set_fix(Fix::suggested(Edit::range_deletion(range))); } - - diagnostic.set_fix(Fix::suggested(Edit::range_deletion(diagnostic.range))); } return Some(diagnostic); From 12103d711fb21a88c9e70debb5d2a6a96fbe9b8d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 19 Jun 2023 22:50:17 -0400 Subject: [PATCH 4/4] Tweaks --- crates/ruff/src/checkers/physical_lines.rs | 8 +++++--- .../pycodestyle/rules/trailing_whitespace.rs | 15 ++++----------- ...f__rules__pycodestyle__tests__W293_W29.py.snap | 2 +- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/crates/ruff/src/checkers/physical_lines.rs b/crates/ruff/src/checkers/physical_lines.rs index 3bf5400cd355b..9882bc86efef9 100644 --- a/crates/ruff/src/checkers/physical_lines.rs +++ b/crates/ruff/src/checkers/physical_lines.rs @@ -1,10 +1,11 @@ //! Lint rules based on checking physical lines. -use ruff_text_size::TextSize; use std::path::Path; +use ruff_text_size::TextSize; + use ruff_diagnostics::Diagnostic; use ruff_python_ast::source_code::{Indexer, Locator, Stylist}; -use ruff_python_whitespace::{Line, UniversalNewlines}; +use ruff_python_whitespace::UniversalNewlines; use crate::registry::Rule; use crate::rules::copyright::rules::missing_copyright_notice; @@ -184,9 +185,10 @@ pub(crate) fn check_physical_lines( #[cfg(test)] mod tests { + use std::path::Path; + use rustpython_parser::lexer::lex; use rustpython_parser::Mode; - use std::path::Path; use ruff_python_ast::source_code::{Indexer, Locator, Stylist}; diff --git a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs index 740d43d964518..79452236c4adb 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/trailing_whitespace.rs @@ -92,22 +92,15 @@ pub(crate) fn trailing_whitespace( if range == line.range() { if settings.rules.enabled(Rule::BlankLineWithWhitespace) { let mut diagnostic = Diagnostic::new(BlankLineWithWhitespace, range); - if settings.rules.should_fix(Rule::BlankLineWithWhitespace) { // Remove any preceding continuations, to avoid introducing a potential // syntax error. - if let Some(continuation) = + diagnostic.set_fix(Fix::automatic(Edit::range_deletion(TextRange::new( helpers::preceded_by_continuations(line.start(), locator, indexer) - { - diagnostic.set_fix(Fix::suggested(Edit::range_deletion(TextRange::new( - continuation, - range.end(), - )))); - } else { - diagnostic.set_fix(Fix::suggested(Edit::range_deletion(range))); - } + .unwrap_or(range.start()), + range.end(), + )))); } - return Some(diagnostic); } } else if settings.rules.enabled(Rule::TrailingWhitespace) { diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W293_W29.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W293_W29.py.snap index 55fd59bee8562..14f67ce27fdde 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W293_W29.py.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W293_W29.py.snap @@ -12,7 +12,7 @@ W29.py:7:1: W293 [*] Blank line contains whitespace | = help: Remove whitespace from blank line -ℹ Suggested fix +ℹ Fix 4 4 | print 5 5 | #: W293:2:1 6 6 | class Foo(object):