diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index 295d3b91edfd8..3abdedd860099 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -219,7 +219,6 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> { if let Some(last_end) = self.entries.position() { let magic_trailing_comma = has_magic_trailing_comma( TextRange::new(last_end, self.sequence_end), - self.fmt.options(), self.fmt.context(), ); diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index 62871cdf31b2d..98a0a334be4b1 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -205,11 +205,7 @@ fn is_arguments_huggable(arguments: &Arguments, context: &PyFormatContext) -> bo // If the expression has a trailing comma, then we can't hug it. if options.magic_trailing_comma().is_respect() - && commas::has_magic_trailing_comma( - TextRange::new(arg.end(), arguments.end()), - options, - context, - ) + && commas::has_magic_trailing_comma(TextRange::new(arg.end(), arguments.end()), context) { return false; } diff --git a/crates/ruff_python_formatter/src/other/commas.rs b/crates/ruff_python_formatter/src/other/commas.rs index 326daba4255b5..a102e0d59e878 100644 --- a/crates/ruff_python_formatter/src/other/commas.rs +++ b/crates/ruff_python_formatter/src/other/commas.rs @@ -1,17 +1,14 @@ +use ruff_formatter::FormatContext; use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::TextRange; use crate::prelude::*; -use crate::{MagicTrailingComma, PyFormatOptions}; +use crate::MagicTrailingComma; /// Returns `true` if the range ends with a magic trailing comma (and the magic trailing comma /// should be respected). -pub(crate) fn has_magic_trailing_comma( - range: TextRange, - options: &PyFormatOptions, - context: &PyFormatContext, -) -> bool { - match options.magic_trailing_comma() { +pub(crate) fn has_magic_trailing_comma(range: TextRange, context: &PyFormatContext) -> bool { + match context.options().magic_trailing_comma() { MagicTrailingComma::Respect => { let first_token = SimpleTokenizer::new(context.source(), range) .skip_trivia() diff --git a/crates/ruff_python_formatter/src/other/with_item.rs b/crates/ruff_python_formatter/src/other/with_item.rs index 3ea82b8a9df69..38088e3b78cda 100644 --- a/crates/ruff_python_formatter/src/other/with_item.rs +++ b/crates/ruff_python_formatter/src/other/with_item.rs @@ -1,4 +1,4 @@ -use ruff_formatter::write; +use ruff_formatter::{write, FormatRuleWithOptions}; use ruff_python_ast::WithItem; use crate::comments::SourceComment; @@ -8,8 +8,66 @@ use crate::expression::parentheses::{ }; use crate::prelude::*; +#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] +pub enum WithItemLayout { + /// A with item that is the `with`s only context manager and its context expression is parenthesized. + /// + /// ```python + /// with ( + /// a + b + /// ) as b: + /// ... + /// ``` + /// + /// This layout is used independent of the target version. + SingleParenthesizedContextManager, + + /// This layout is used when the target python version doesn't support parenthesized context managers and + /// it's either a single, unparenthesized with item or multiple items. + /// + /// ```python + /// with a + b: + /// ... + /// + /// with a, b: + /// ... + /// ``` + Python38OrOlder, + + /// A with item where the `with` formatting adds parentheses around all context managers if necessary. + /// + /// ```python + /// with ( + /// a, + /// b, + /// ): pass + /// ``` + /// + /// This layout is generally used when the target version is Python 3.9 or newer, but it is used + /// for Python 3.8 if the with item has a leading or trailing comment. + /// + /// ```python + /// with ( + /// # leading + /// a + // ): ... + /// ``` + #[default] + ParenthesizedContextManagers, +} + #[derive(Default)] -pub struct FormatWithItem; +pub struct FormatWithItem { + layout: WithItemLayout, +} + +impl FormatRuleWithOptions> for FormatWithItem { + type Options = WithItemLayout; + + fn with_options(self, options: Self::Options) -> Self { + Self { layout: options } + } +} impl FormatNodeRule for FormatWithItem { fn fmt_fields(&self, item: &WithItem, f: &mut PyFormatter) -> FormatResult<()> { @@ -28,40 +86,52 @@ impl FormatNodeRule for FormatWithItem { f.context().source(), ); - // Remove the parentheses of the `with_items` if the with statement adds parentheses - if f.context().node_level().is_parenthesized() { - if is_parenthesized { - // ...except if the with item is parenthesized, then use this with item as a preferred breaking point - // or when it has comments, then parenthesize it to prevent comments from moving. - maybe_parenthesize_expression( - context_expr, - item, - Parenthesize::IfBreaksOrIfRequired, - ) - .fmt(f)?; - } else { - context_expr - .format() - .with_options(Parentheses::Never) + match self.layout { + // Remove the parentheses of the `with_items` if the with statement adds parentheses + WithItemLayout::ParenthesizedContextManagers => { + if is_parenthesized { + // ...except if the with item is parenthesized, then use this with item as a preferred breaking point + // or when it has comments, then parenthesize it to prevent comments from moving. + maybe_parenthesize_expression( + context_expr, + item, + Parenthesize::IfBreaksOrIfRequired, + ) .fmt(f)?; + } else { + context_expr + .format() + .with_options(Parentheses::Never) + .fmt(f)?; + } + } + + WithItemLayout::SingleParenthesizedContextManager => { + write!( + f, + [maybe_parenthesize_expression( + context_expr, + item, + Parenthesize::IfBreaks + )] + )?; + } + + WithItemLayout::Python38OrOlder => { + let parenthesize = if is_parenthesized { + Parenthesize::IfBreaks + } else { + Parenthesize::IfRequired + }; + write!( + f, + [maybe_parenthesize_expression( + context_expr, + item, + parenthesize + )] + )?; } - } else { - // Prefer keeping parentheses for already parenthesized expressions over - // parenthesizing other nodes. - let parenthesize = if is_parenthesized { - Parenthesize::IfBreaks - } else { - Parenthesize::IfRequired - }; - - write!( - f, - [maybe_parenthesize_expression( - context_expr, - item, - parenthesize - )] - )?; } if let Some(optional_vars) = optional_vars { diff --git a/crates/ruff_python_formatter/src/statement/stmt_with.rs b/crates/ruff_python_formatter/src/statement/stmt_with.rs index ce0af821f101d..696e7a0e0acbe 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_with.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_with.rs @@ -1,6 +1,6 @@ -use ruff_formatter::{format_args, write, FormatError}; -use ruff_python_ast::AstNode; +use ruff_formatter::{format_args, write, FormatContext, FormatError}; use ruff_python_ast::StmtWith; +use ruff_python_ast::{AstNode, WithItem}; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange}; @@ -11,9 +11,10 @@ use crate::expression::parentheses::{ is_expression_parenthesized, optional_parentheses, parenthesized, }; use crate::other::commas; +use crate::other::with_item::WithItemLayout; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader}; -use crate::{PyFormatOptions, PythonVersion}; +use crate::PythonVersion; #[derive(Default)] pub struct FormatStmtWith; @@ -61,58 +62,59 @@ impl FormatNodeRule for FormatStmtWith { ] )?; - if parenthesized_comments.is_empty() { - let format_items = format_with(|f| { - let mut joiner = - f.join_comma_separated(with_stmt.body.first().unwrap().start()); + let layout = WithItemsLayout::from_statement( + with_stmt, + f.context(), + parenthesized_comments, + )?; + + match layout { + WithItemsLayout::SingleWithTarget(single) => { + optional_parentheses(&single.format()).fmt(f) + } - for item in &with_stmt.items { - joiner.entry_with_line_separator( - item, - &item.format(), - soft_line_break_or_space(), + WithItemsLayout::SingleParenthesizedContextManager(single) => single + .format() + .with_options(WithItemLayout::SingleParenthesizedContextManager) + .fmt(f), + + WithItemsLayout::ParenthesizeIfExpands => { + parenthesize_if_expands(&format_with(|f| { + let mut joiner = f.join_comma_separated( + with_stmt.body.first().unwrap().start(), ); - } - joiner.finish() - }); - - match should_parenthesize(with_stmt, f.options(), f.context())? { - ParenthesizeWith::Optional => { - optional_parentheses(&format_items).fmt(f)?; - } - ParenthesizeWith::IfExpands => { - parenthesize_if_expands(&format_items).fmt(f)?; - } - ParenthesizeWith::UnlessCommented => { - if let [item] = with_stmt.items.as_slice() { - // This is similar to `maybe_parenthesize_expression`, but we're not - // dealing with an expression here, it's a `WithItem`. - if comments.has_leading(item) || comments.has_trailing(item) - { - parenthesized("(", &item.format(), ")").fmt(f)?; - } else { - item.format().fmt(f)?; - } - } else { - f.join_with(format_args![token(","), space()]) - .entries(with_stmt.items.iter().formatted()) - .finish()?; + + for item in &with_stmt.items { + joiner.entry_with_line_separator( + item, + &item.format(), + soft_line_break_or_space(), + ); } - } + joiner.finish() + })) + .fmt(f) } - } else { - let joined = format_with(|f: &mut PyFormatter| { - f.join_comma_separated(with_stmt.body.first().unwrap().start()) - .nodes(&with_stmt.items) - .finish() - }); - - parenthesized("(", &joined, ")") - .with_dangling_comments(parenthesized_comments) - .fmt(f)?; - } - Ok(()) + WithItemsLayout::Python38OrOlder => f + .join_with(format_args![token(","), space()]) + .entries(with_stmt.items.iter().map(|item| { + item.format().with_options(WithItemLayout::Python38OrOlder) + })) + .finish(), + + WithItemsLayout::Parenthesized => parenthesized( + "(", + &format_with(|f: &mut PyFormatter| { + f.join_comma_separated(with_stmt.body.first().unwrap().start()) + .nodes(&with_stmt.items) + .finish() + }), + ")", + ) + .with_dangling_comments(parenthesized_comments) + .fmt(f), + } }) ), clause_body(&with_stmt.body, colon_comments) @@ -130,92 +132,167 @@ impl FormatNodeRule for FormatStmtWith { } } -/// Determines whether the `with` items should be parenthesized (over parenthesizing each item), -/// and if so, which parenthesizing layout to use. -/// -/// Parenthesize `with` items if -/// * The last item has a trailing comma (implying that the with items were parenthesized in the source) -/// * There's more than one item and they're already parenthesized -/// * There's more than one item, the [`wrap_multiple_context_managers_in_parens`](is_wrap_multiple_context_managers_in_parens) preview style is enabled, -/// and the target python version is >= 3.9 -/// * There's a single non-parenthesized item. The function returns [`ParenthesizeWith::Optional`] -/// if the parentheses can be omitted if breaking around parenthesized sub-expressions is sufficient -/// to make the expression fit. It returns [`ParenthesizeWith::IfExpands`] otherwise. -/// * The only item is parenthesized and has comments. -fn should_parenthesize( - with: &StmtWith, - options: &PyFormatOptions, - context: &PyFormatContext, -) -> FormatResult { - if has_magic_trailing_comma(with, options, context) { - return Ok(ParenthesizeWith::IfExpands); - } +#[derive(Clone, Copy, Debug)] +enum WithItemsLayout<'a> { + /// The with statement's only item has a parenthesized context manager. + /// + /// ```python + /// with ( + /// a + b + /// ): + /// ... + /// + /// with ( + /// a + b + /// ) as b: + /// ... + /// ``` + /// + /// In this case, prefer keeping the parentheses around the context expression instead of parenthesizing the entire + /// with item. + SingleParenthesizedContextManager(&'a WithItem), - let can_parenthesize = options.target_version() >= PythonVersion::Py39 - || are_with_items_parenthesized(with, context)?; + /// It's a single with item with a target. Use the optional parentheses layout (see [`optional_parentheses`]) + /// to mimic the `maybe_parenthesize_expression` behavior. + /// + /// ```python + /// with ( + /// a + b + /// ) as b: + /// ... + /// ``` + /// + /// Only used for Python 3.9+ + SingleWithTarget(&'a WithItem), - if !can_parenthesize { - return Ok(ParenthesizeWith::UnlessCommented); - } + /// The target python version doesn't support parenthesized context managers because it is Python 3.8 or older. + /// + /// In this case, never add parentheses and join the with items with spaces. + /// + /// ```python + /// with ContextManager1( + /// aaaaaaaaaaaaaaa, b + /// ), ContextManager2(), ContextManager3(), ContextManager4(): + /// pass + /// ``` + Python38OrOlder, + + /// Wrap the with items in parentheses if they don't fit on a single line and join them by soft line breaks. + /// + /// ```python + /// with ( + /// ContextManager1(aaaaaaaaaaaaaaa, b), + /// ContextManager1(), + /// ContextManager1(), + /// ContextManager1(), + /// ): + /// pass + /// ``` + /// + /// Only used for Python 3.9+. + ParenthesizeIfExpands, + + /// Always parenthesize because the context managers open-parentheses have a trailing comment: + /// + /// ```python + /// with ( # comment + /// CtxManager() as example + /// ): + /// ... + /// ``` + /// + /// Or because it is a single item with a trailing or leading comment. + /// + /// ```python + /// with ( + /// # leading + /// CtxManager() + /// # trailing + /// ): pass + /// ``` + Parenthesized, +} + +impl<'a> WithItemsLayout<'a> { + fn from_statement( + with: &'a StmtWith, + context: &PyFormatContext, + parenthesized_comments: &[SourceComment], + ) -> FormatResult { + // The with statement already has parentheses around the entire with items. Guaranteed to be Python 3.9 or newer + // ``` + // with ( # comment + // CtxManager() as example + // ): + // pass + // ``` + if !parenthesized_comments.is_empty() { + return Ok(Self::Parenthesized); + } + + // A trailing comma at the end guarantees that the context managers are parenthesized and that it is Python 3.9 or newer syntax. + // ```python + // with ( # comment + // CtxManager() as example, + // ): + // pass + // ``` + if has_magic_trailing_comma(with, context) { + return Ok(Self::ParenthesizeIfExpands); + } - if let [single] = with.items.as_slice() { - return Ok( + if let [single] = with.items.as_slice() { // If the with item itself has comments (not the context expression), then keep the parentheses + // ```python + // with ( + // # leading + // a + // ): pass + // ``` if context.comments().has_leading(single) || context.comments().has_trailing(single) { - ParenthesizeWith::IfExpands + return Ok(Self::Parenthesized); } - // If it is the only expression and it has comments, then the with statement - // as well as the with item add parentheses - else if is_expression_parenthesized( + + // Preserve the parentheses around the context expression instead of parenthesizing the entire + // with items. + if is_expression_parenthesized( (&single.context_expr).into(), context.comments().ranges(), context.source(), ) { - // Preserve the parentheses around the context expression instead of parenthesizing the entire - // with items. - ParenthesizeWith::UnlessCommented - } else if can_omit_optional_parentheses(&single.context_expr, context) { - ParenthesizeWith::Optional - } else { - ParenthesizeWith::IfExpands - }, - ); - } - - // Always parenthesize multiple items - Ok(ParenthesizeWith::IfExpands) -} + return Ok(Self::SingleParenthesizedContextManager(single)); + } + } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ParenthesizeWith { - /// Don't wrap the with items in parentheses except if it is a single item - /// and it has leading or trailing comment. - /// - /// This is required because `are_with_items_parenthesized` cannot determine if - /// `with (expr)` is a parenthesized expression or a parenthesized with item. - UnlessCommented, + let can_parenthesize = context.options().target_version() >= PythonVersion::Py39 + || are_with_items_parenthesized(with, context)?; - /// Wrap the with items in optional parentheses - Optional, + // If the target version doesn't support parenthesized context managers and they aren't + // parenthesized by the user, bail out. + if !can_parenthesize { + return Ok(Self::Python38OrOlder); + } - /// Wrap the with items in parentheses if they expand - IfExpands, + Ok(match with.items.as_slice() { + [single] => { + if can_omit_optional_parentheses(&single.context_expr, context) { + Self::SingleWithTarget(single) + } else { + Self::ParenthesizeIfExpands + } + } + // Always parenthesize multiple items + [..] => Self::ParenthesizeIfExpands, + }) + } } -fn has_magic_trailing_comma( - with: &StmtWith, - options: &PyFormatOptions, - context: &PyFormatContext, -) -> bool { +fn has_magic_trailing_comma(with: &StmtWith, context: &PyFormatContext) -> bool { let Some(last_item) = with.items.last() else { return false; }; - commas::has_magic_trailing_comma( - TextRange::new(last_item.end(), with.end()), - options, - context, - ) + commas::has_magic_trailing_comma(TextRange::new(last_item.end(), with.end()), context) } fn are_with_items_parenthesized(with: &StmtWith, context: &PyFormatContext) -> FormatResult {