diff --git a/crates/ruff/resources/test/fixtures/flake8_pytest_style/PT018.py b/crates/ruff/resources/test/fixtures/flake8_pytest_style/PT018.py index cedd7ba170..4b70bae72e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pytest_style/PT018.py +++ b/crates/ruff/resources/test/fixtures/flake8_pytest_style/PT018.py @@ -43,3 +43,12 @@ message assert something # OK assert something and something_else # Error assert something and something_else and something_third # Error + + +def test_multiline(): + assert something and something_else; x = 1 + + x = 1; assert something and something_else + + x = 1; \ + assert something and something_else diff --git a/crates/ruff/src/autofix/edits.rs b/crates/ruff/src/autofix/edits.rs index 0a41c8bfc2..4f02de6c32 100644 --- a/crates/ruff/src/autofix/edits.rs +++ b/crates/ruff/src/autofix/edits.rs @@ -226,25 +226,25 @@ fn trailing_semicolon(offset: TextSize, locator: &Locator) -> Option { fn next_stmt_break(semicolon: TextSize, locator: &Locator) -> TextSize { let start_location = semicolon + TextSize::from(1); - let contents = &locator.contents()[usize::from(start_location)..]; - for line in NewlineWithTrailingNewline::from(contents) { + for line in + NewlineWithTrailingNewline::with_offset(locator.after(start_location), start_location) + { let trimmed = line.trim_whitespace(); // Skip past any continuations. if trimmed.starts_with('\\') { continue; } - return start_location - + if trimmed.is_empty() { - // If the line is empty, then despite the previous statement ending in a - // semicolon, we know that it's not a multi-statement line. - line.start() - } else { - // Otherwise, find the start of the next statement. (Or, anything that isn't - // whitespace.) - let relative_offset = line.find(|c: char| !is_python_whitespace(c)).unwrap(); - line.start() + TextSize::try_from(relative_offset).unwrap() - }; + return if trimmed.is_empty() { + // If the line is empty, then despite the previous statement ending in a + // semicolon, we know that it's not a multi-statement line. + line.start() + } else { + // Otherwise, find the start of the next statement. (Or, anything that isn't + // whitespace.) + let relative_offset = line.find(|c: char| !is_python_whitespace(c)).unwrap(); + line.start() + TextSize::try_from(relative_offset).unwrap() + }; } locator.line_end(start_location) diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 8dbfa4b61a..4711149a9a 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -562,7 +562,7 @@ fn add_noqa_inner( let mut prev_end = TextSize::default(); for (offset, (rules, directive)) in matches_by_line { - output.push_str(&locator.contents()[TextRange::new(prev_end, offset)]); + output.push_str(locator.slice(TextRange::new(prev_end, offset))); let line = locator.full_line(offset); @@ -619,7 +619,7 @@ fn add_noqa_inner( prev_end = offset + line.text_len(); } - output.push_str(&locator.contents()[usize::from(prev_end)..]); + output.push_str(locator.after(prev_end)); (count, output) } diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs b/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index 0ea4f69059..f00944d1e8 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs @@ -140,7 +140,7 @@ fn move_initialization( let mut body = function_def.body.iter(); let statement = body.next()?; - if indexer.preceded_by_multi_statement_line(statement, locator) { + if indexer.in_multi_statement_line(statement, locator) { return None; } @@ -170,7 +170,7 @@ fn move_initialization( if let Some(statement) = body.next() { // If there's a second statement, insert _before_ it, but ensure this isn't a // multi-statement line. - if indexer.preceded_by_multi_statement_line(statement, locator) { + if indexer.in_multi_statement_line(statement, locator) { return None; } Edit::insertion(content, locator.line_start(statement.start())) diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs index 60d8ba496f..e516cd56e8 100644 --- a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs @@ -126,8 +126,8 @@ pub(crate) fn implicit( } fn concatenate_strings(a_range: TextRange, b_range: TextRange, locator: &Locator) -> Option { - let a_text = &locator.contents()[a_range]; - let b_text = &locator.contents()[b_range]; + let a_text = locator.slice(a_range); + let b_text = locator.slice(b_range); let a_leading_quote = leading_quote(a_text)?; let b_leading_quote = leading_quote(b_text)?; diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs index 2a1ea637aa..0201096a05 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; -use anyhow::bail; use anyhow::Result; +use anyhow::{bail, Context}; use libcst_native::{ self, Assert, BooleanOp, CompoundStatement, Expression, ParenthesizableWhitespace, ParenthesizedNode, SimpleStatementLine, SimpleWhitespace, SmallStatement, Statement, @@ -635,9 +635,8 @@ fn parenthesize<'a>(expression: Expression<'a>, parent: &Expression<'a>) -> Expr /// `assert a == "hello"` and `assert b == "world"`. fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) -> Result { // Infer the indentation of the outer block. - let Some(outer_indent) = whitespace::indentation(locator, stmt) else { - bail!("Unable to fix multiline statement"); - }; + let outer_indent = + whitespace::indentation(locator, stmt).context("Unable to fix multiline statement")?; // Extract the module text. let contents = locator.lines(stmt.range()); @@ -672,11 +671,11 @@ fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) -> &mut indented_block.body }; - let [Statement::Simple(simple_statement_line)] = &statements[..] else { + let [Statement::Simple(simple_statement_line)] = statements.as_slice() else { bail!("Expected one simple statement") }; - let [SmallStatement::Assert(assert_statement)] = &simple_statement_line.body[..] else { + let [SmallStatement::Assert(assert_statement)] = simple_statement_line.body.as_slice() else { bail!("Expected simple statement to be an assert") }; @@ -754,6 +753,9 @@ pub(crate) fn composite_condition( if matches!(composite, CompositionKind::Simple) && msg.is_none() && !checker.indexer().comment_ranges().intersects(stmt.range()) + && !checker + .indexer() + .in_multi_statement_line(stmt, checker.locator()) { diagnostic.try_set_fix(|| { fix_composite_condition(stmt, checker.locator(), checker.stylist()) diff --git a/crates/ruff/src/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT018.snap b/crates/ruff/src/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT018.snap index cbfde1160d..d9c9771a7d 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT018.snap +++ b/crates/ruff/src/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT018.snap @@ -299,6 +299,8 @@ PT018.py:44:1: PT018 [*] Assertion should be broken down into multiple parts 44 |+assert something 45 |+assert something_else 45 46 | assert something and something_else and something_third # Error +46 47 | +47 48 | PT018.py:45:1: PT018 [*] Assertion should be broken down into multiple parts | @@ -316,5 +318,37 @@ PT018.py:45:1: PT018 [*] Assertion should be broken down into multiple parts 45 |-assert something and something_else and something_third # Error 45 |+assert something and something_else 46 |+assert something_third +46 47 | +47 48 | +48 49 | def test_multiline(): + +PT018.py:49:5: PT018 Assertion should be broken down into multiple parts + | +48 | def test_multiline(): +49 | assert something and something_else; x = 1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018 +50 | +51 | x = 1; assert something and something_else + | + = help: Break down assertion into multiple parts + +PT018.py:51:12: PT018 Assertion should be broken down into multiple parts + | +49 | assert something and something_else; x = 1 +50 | +51 | x = 1; assert something and something_else + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018 +52 | +53 | x = 1; \ + | + = help: Break down assertion into multiple parts + +PT018.py:54:9: PT018 Assertion should be broken down into multiple parts + | +53 | x = 1; \ +54 | assert something and something_else + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018 + | + = help: Break down assertion into multiple parts diff --git a/crates/ruff/src/rules/isort/rules/organize_imports.rs b/crates/ruff/src/rules/isort/rules/organize_imports.rs index 8b72f149ab..62c7ba47d7 100644 --- a/crates/ruff/src/rules/isort/rules/organize_imports.rs +++ b/crates/ruff/src/rules/isort/rules/organize_imports.rs @@ -1,16 +1,16 @@ use std::path::Path; use itertools::{EitherOrBoth, Itertools}; -use ruff_python_ast::{PySourceType, Ranged, Stmt}; -use ruff_text_size::TextRange; use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::whitespace::{followed_by_multi_statement_line, trailing_lines_end}; +use ruff_python_ast::whitespace::trailing_lines_end; +use ruff_python_ast::{PySourceType, Ranged, Stmt}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_python_trivia::{leading_indentation, textwrap::indent, PythonWhitespace}; use ruff_source_file::{Locator, UniversalNewlines}; +use ruff_text_size::TextRange; use crate::line_width::LineWidth; use crate::registry::AsRule; @@ -97,7 +97,7 @@ pub(crate) fn organize_imports( // Special-cases: there's leading or trailing content in the import block. These // are too hard to get right, and relatively rare, so flag but don't fix. if indexer.preceded_by_multi_statement_line(block.imports.first().unwrap(), locator) - || followed_by_multi_statement_line(block.imports.last().unwrap(), locator) + || indexer.followed_by_multi_statement_line(block.imports.last().unwrap(), locator) { return Some(Diagnostic::new(UnsortedImports, range)); } diff --git a/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs b/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs index fd2982c11a..5606cb74c3 100644 --- a/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs +++ b/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs @@ -62,8 +62,7 @@ fn find_useless_f_strings<'a>( kind: StringKind::FString | StringKind::RawFString, .. } => { - let first_char = - &locator.contents()[TextRange::at(range.start(), TextSize::from(1))]; + let first_char = locator.slice(TextRange::at(range.start(), TextSize::from(1))); // f"..." => f_position = 0 // fr"..." => f_position = 0 // rf"..." => f_position = 1 diff --git a/crates/ruff_diagnostics/src/diagnostic.rs b/crates/ruff_diagnostics/src/diagnostic.rs index 5d505877bc..19a5d3ec45 100644 --- a/crates/ruff_diagnostics/src/diagnostic.rs +++ b/crates/ruff_diagnostics/src/diagnostic.rs @@ -60,6 +60,17 @@ impl Diagnostic { } } + /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`. + /// Otherwise, log the error. + #[inline] + pub fn try_set_optional_fix(&mut self, func: impl FnOnce() -> Result>) { + match func() { + Ok(None) => {} + Ok(Some(fix)) => self.fix = Some(fix), + Err(err) => error!("Failed to create fix for {}: {}", self.kind.name, err), + } + } + pub const fn range(&self) -> TextRange { self.range } diff --git a/crates/ruff_python_ast/src/whitespace.rs b/crates/ruff_python_ast/src/whitespace.rs index 282076ee29..6f89365efb 100644 --- a/crates/ruff_python_ast/src/whitespace.rs +++ b/crates/ruff_python_ast/src/whitespace.rs @@ -1,10 +1,8 @@ -use crate::{Ranged, Stmt}; +use ruff_python_trivia::{indentation_at_offset, is_python_whitespace, PythonWhitespace}; +use ruff_source_file::{newlines::UniversalNewlineIterator, Locator}; use ruff_text_size::{TextRange, TextSize}; -use ruff_python_trivia::{ - has_trailing_content, indentation_at_offset, is_python_whitespace, PythonWhitespace, -}; -use ruff_source_file::{newlines::UniversalNewlineIterator, Locator}; +use crate::{Ranged, Stmt}; /// Extract the leading indentation from a line. #[inline] @@ -18,20 +16,12 @@ where /// Return the end offset at which the empty lines following a statement. pub fn trailing_lines_end(stmt: &Stmt, locator: &Locator) -> TextSize { let line_end = locator.full_line_end(stmt.end()); - let rest = &locator.contents()[usize::from(line_end)..]; - - UniversalNewlineIterator::with_offset(rest, line_end) + UniversalNewlineIterator::with_offset(locator.after(line_end), line_end) .take_while(|line| line.trim_whitespace().is_empty()) .last() .map_or(line_end, |line| line.full_end()) } -/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with -/// other statements following it. -pub fn followed_by_multi_statement_line(stmt: &Stmt, locator: &Locator) -> bool { - has_trailing_content(stmt.end(), locator) -} - /// If a [`Ranged`] has a trailing comment, return the index of the hash. pub fn trailing_comment_start_offset(located: &T, locator: &Locator) -> Option where @@ -39,7 +29,7 @@ where { let line_end = locator.line_end(located.end()); - let trailing = &locator.contents()[TextRange::new(located.end(), line_end)]; + let trailing = locator.slice(TextRange::new(located.end(), line_end)); for (index, char) in trailing.char_indices() { if char == '#' { diff --git a/crates/ruff_python_index/src/indexer.rs b/crates/ruff_python_index/src/indexer.rs index f6cc333423..536661d04d 100644 --- a/crates/ruff_python_index/src/indexer.rs +++ b/crates/ruff_python_index/src/indexer.rs @@ -39,7 +39,7 @@ impl Indexer { let mut line_start = TextSize::default(); for (tok, range) in tokens.iter().flatten() { - let trivia = &locator.contents()[TextRange::new(prev_end, range.start())]; + let trivia = locator.slice(TextRange::new(prev_end, range.start())); // Get the trivia between the previous and the current token and detect any newlines. // This is necessary because `RustPython` doesn't emit `[Tok::Newline]` tokens @@ -250,14 +250,26 @@ impl Indexer { Some(continuation) } - /// Return `true` if a `Stmt` appears to be part of a multi-statement line, with - /// other statements preceding it. + /// Return `true` if a [`Stmt`] appears to be preceded by other statements in a multi-statement + /// line. pub fn preceded_by_multi_statement_line(&self, stmt: &Stmt, locator: &Locator) -> bool { has_leading_content(stmt.start(), locator) || self .preceded_by_continuations(stmt.start(), locator) .is_some() } + + /// Return `true` if a [`Stmt`] appears to be followed by other statements in a multi-statement + /// line. + pub fn followed_by_multi_statement_line(&self, stmt: &Stmt, locator: &Locator) -> bool { + has_trailing_content(stmt.end(), locator) + } + + /// Return `true` if a [`Stmt`] appears to be part of a multi-statement line. + pub fn in_multi_statement_line(&self, stmt: &Stmt, locator: &Locator) -> bool { + self.followed_by_multi_statement_line(stmt, locator) + || self.preceded_by_multi_statement_line(stmt, locator) + } } #[cfg(test)] diff --git a/crates/ruff_python_trivia/src/whitespace.rs b/crates/ruff_python_trivia/src/whitespace.rs index 7e23c4f3e7..5209164870 100644 --- a/crates/ruff_python_trivia/src/whitespace.rs +++ b/crates/ruff_python_trivia/src/whitespace.rs @@ -4,7 +4,7 @@ use ruff_text_size::{TextRange, TextSize}; /// Extract the leading indentation from a line. pub fn indentation_at_offset<'a>(offset: TextSize, locator: &'a Locator) -> Option<&'a str> { let line_start = locator.line_start(offset); - let indentation = &locator.contents()[TextRange::new(line_start, offset)]; + let indentation = locator.slice(TextRange::new(line_start, offset)); if indentation.chars().all(is_python_whitespace) { Some(indentation) @@ -16,14 +16,14 @@ pub fn indentation_at_offset<'a>(offset: TextSize, locator: &'a Locator) -> Opti /// Return `true` if the node starting the given [`TextSize`] has leading content. pub fn has_leading_content(offset: TextSize, locator: &Locator) -> bool { let line_start = locator.line_start(offset); - let leading = &locator.contents()[TextRange::new(line_start, offset)]; + let leading = locator.slice(TextRange::new(line_start, offset)); leading.chars().any(|char| !is_python_whitespace(char)) } /// Return `true` if the node ending at the given [`TextSize`] has trailing content. pub fn has_trailing_content(offset: TextSize, locator: &Locator) -> bool { let line_end = locator.line_end(offset); - let trailing = &locator.contents()[TextRange::new(offset, line_end)]; + let trailing = locator.slice(TextRange::new(offset, line_end)); for char in trailing.chars() { if char == '#' {