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 cedd7ba170c710..4b70bae72e36ad 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 @@ def test_error(): 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/rules/flake8_bugbear/rules/mutable_argument_default.rs b/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index 0ea4f69059c733..f00944d1e813d3 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_pytest_style/rules/assertion.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs index 2a1ea637aa4cac..0201096a05b587 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 cbfde1160d9240..d9c9771a7d5d8f 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 8b72f149abac94..62c7ba47d7e240 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_diagnostics/src/diagnostic.rs b/crates/ruff_diagnostics/src/diagnostic.rs index 5d505877bc9247..19a5d3ec45468c 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 282076ee297813..ab2fff12bbd414 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] @@ -26,12 +24,6 @@ pub fn trailing_lines_end(stmt: &Stmt, locator: &Locator) -> TextSize { .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 diff --git a/crates/ruff_python_index/src/indexer.rs b/crates/ruff_python_index/src/indexer.rs index f6cc3334237c0f..185e6e8c41a581 100644 --- a/crates/ruff_python_index/src/indexer.rs +++ b/crates/ruff_python_index/src/indexer.rs @@ -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 7e23c4f3e73837..d47633a5687435 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)