Refactor with statement formatting to have explicit layouts (#10296)

## Summary

This PR refactors the with item formatting to use more explicit layouts
to make it easier to understand the different formatting cases.

The benefit of the explicit layout is that it makes it easier to reasons
about layout transition between format runs. For example, today it's
possible that `SingleWithTarget` or `ParenthesizeIfExpands` add
parentheses around the with items for `with aaaaaaaaaa + bbbbbbbbbbbb:
pass`, resulting in `with (aaaaaaaaaa + bbbbbbbbbbbb): pass`. The
problem with this is that the next formatting pass uses the
`SingleParenthesizedContextExpression` layout that uses
`maybe_parenthesize_expression` which is different from
`parenthesize_if_expands(&expr)` or `optional_parentheses(&expr)`.

## Test Plan

`cargo test`

I ran the ecosystem checks locally and there are no changes.
This commit is contained in:
Micha Reiser 2024-03-09 00:40:39 +01:00 committed by GitHub
parent 1d97f27335
commit a56d42f183
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 305 additions and 166 deletions

View file

@ -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;
}

View file

@ -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()

View file

@ -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<WithItem, PyFormatContext<'_>> for FormatWithItem {
type Options = WithItemLayout;
fn with_options(self, options: Self::Options) -> Self {
Self { layout: options }
}
}
impl FormatNodeRule<WithItem> for FormatWithItem {
fn fmt_fields(&self, item: &WithItem, f: &mut PyFormatter) -> FormatResult<()> {
@ -28,40 +86,52 @@ impl FormatNodeRule<WithItem> 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)?;
}
}
} 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
)]
)?;
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
)]
)?;
}
}
if let Some(optional_vars) = optional_vars {