Skip to content

Commit b64f2ea

Browse files
authored
Formatter: Improve single-with item formatting for Python 3.8 or older (#10276)
## Summary This PR changes how we format `with` statements with a single with item for Python 3.8 or older. This change is not compatible with Black. This is how we format a single-item with statement today ```python def run(data_path, model_uri): with pyspark.sql.SparkSession.builder.config( key="spark.python.worker.reuse", value=True ).config(key="spark.ui.enabled", value=False).master( "local-cluster[2, 1, 1024]" ).getOrCreate(): # ignore spark log output spark.sparkContext.setLogLevel("OFF") print(score_model(spark, data_path, model_uri)) ``` This is different than how we would format the same expression if it is inside any other clause header (`while`, `if`, ...): ```python def run(data_path, model_uri): while ( pyspark.sql.SparkSession.builder.config( key="spark.python.worker.reuse", value=True ) .config(key="spark.ui.enabled", value=False) .master("local-cluster[2, 1, 1024]") .getOrCreate() ): # ignore spark log output spark.sparkContext.setLogLevel("OFF") print(score_model(spark, data_path, model_uri)) ``` Which seems inconsistent to me. This PR changes the formatting of the single-item with Python 3.8 or older to match that of other clause headers. ```python def run(data_path, model_uri): with ( pyspark.sql.SparkSession.builder.config( key="spark.python.worker.reuse", value=True ) .config(key="spark.ui.enabled", value=False) .master("local-cluster[2, 1, 1024]") .getOrCreate() ): # ignore spark log output spark.sparkContext.setLogLevel("OFF") print(score_model(spark, data_path, model_uri)) ``` According to our versioning policy, this style change is gated behind a preview flag. ## Test Plan See added tests. Added
1 parent 4bce801 commit b64f2ea

File tree

5 files changed

+167
-4
lines changed

5 files changed

+167
-4
lines changed

crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py

+10
Original file line numberDiff line numberDiff line change
@@ -318,3 +318,13 @@
318318
)
319319
):
320320
pass
321+
322+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
323+
pass
324+
325+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b:
326+
pass
327+
328+
if True:
329+
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext() as b:
330+
pass

crates/ruff_python_formatter/src/other/with_item.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::expression::parentheses::{
77
is_expression_parenthesized, parenthesized, Parentheses, Parenthesize,
88
};
99
use crate::prelude::*;
10+
use crate::preview::is_with_single_item_pre_39_enabled;
1011

1112
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
1213
pub enum WithItemLayout {
@@ -49,7 +50,7 @@ pub enum WithItemLayout {
4950
/// with a, b:
5051
/// ...
5152
/// ```
52-
Python38OrOlder,
53+
Python38OrOlder { single: bool },
5354

5455
/// A with item where the `with` formatting adds parentheses around all context managers if necessary.
5556
///
@@ -135,8 +136,10 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
135136
)?;
136137
}
137138

138-
WithItemLayout::Python38OrOlder => {
139-
let parenthesize = if is_parenthesized {
139+
WithItemLayout::Python38OrOlder { single } => {
140+
let parenthesize = if (single && is_with_single_item_pre_39_enabled(f.context()))
141+
|| is_parenthesized
142+
{
140143
Parenthesize::IfBreaks
141144
} else {
142145
Parenthesize::IfRequired

crates/ruff_python_formatter/src/preview.rs

+4
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,7 @@ pub(crate) const fn is_hug_parens_with_braces_and_square_brackets_enabled(
1818
pub(crate) fn is_f_string_formatting_enabled(context: &PyFormatContext) -> bool {
1919
context.is_preview()
2020
}
21+
22+
pub(crate) fn is_with_single_item_pre_39_enabled(context: &PyFormatContext) -> bool {
23+
context.is_preview()
24+
}

crates/ruff_python_formatter/src/statement/stmt_with.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ impl FormatNodeRule<StmtWith> for FormatStmtWith {
104104
WithItemsLayout::Python38OrOlder => f
105105
.join_with(format_args![token(","), space()])
106106
.entries(with_stmt.items.iter().map(|item| {
107-
item.format().with_options(WithItemLayout::Python38OrOlder)
107+
item.format().with_options(WithItemLayout::Python38OrOlder {
108+
single: with_stmt.items.len() == 1,
109+
})
108110
}))
109111
.finish(),
110112

crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap

+144
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,16 @@ with (
324324
)
325325
):
326326
pass
327+
328+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
329+
pass
330+
331+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b:
332+
pass
333+
334+
if True:
335+
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext() as b:
336+
pass
327337
```
328338

329339
## Outputs
@@ -688,6 +698,121 @@ with open(
688698
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
689699
):
690700
pass
701+
702+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
703+
pass
704+
705+
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b:
706+
pass
707+
708+
if True:
709+
with anyio.CancelScope(
710+
shield=True
711+
) if get_running_loop() else contextlib.nullcontext() as b:
712+
pass
713+
```
714+
715+
716+
#### Preview changes
717+
```diff
718+
--- Stable
719+
+++ Preview
720+
@@ -49,7 +49,9 @@
721+
with a: # should remove brackets
722+
pass
723+
724+
-with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
725+
+with (
726+
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
727+
+) as c:
728+
pass
729+
730+
731+
@@ -214,7 +216,9 @@
732+
pass
733+
734+
# Breaking of with items.
735+
-with test as ( # bar # foo
736+
+with (
737+
+ test # bar
738+
+) as ( # foo
739+
# test
740+
foo
741+
):
742+
@@ -226,7 +230,9 @@
743+
):
744+
pass
745+
746+
-with test as ( # bar # foo # baz
747+
+with (
748+
+ test # bar
749+
+) as ( # foo # baz
750+
# test
751+
foo
752+
):
753+
@@ -279,7 +285,9 @@
754+
) as b:
755+
pass
756+
757+
-with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b:
758+
+with (
759+
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
760+
+) as b:
761+
pass
762+
763+
with (
764+
@@ -322,15 +330,19 @@
765+
pass
766+
767+
if True:
768+
- with anyio.CancelScope(
769+
- shield=True
770+
- ) if get_running_loop() else contextlib.nullcontext():
771+
+ with (
772+
+ anyio.CancelScope(shield=True)
773+
+ if get_running_loop()
774+
+ else contextlib.nullcontext()
775+
+ ):
776+
pass
777+
778+
if True:
779+
- with anyio.CancelScope(
780+
- shield=True
781+
- ) if get_running_loop() else contextlib.nullcontext() as c:
782+
+ with (
783+
+ anyio.CancelScope(shield=True)
784+
+ if get_running_loop()
785+
+ else contextlib.nullcontext()
786+
+ ) as c:
787+
pass
788+
789+
with Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Document(
790+
@@ -344,14 +356,20 @@
791+
):
792+
pass
793+
794+
-with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
795+
+with (
796+
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
797+
+):
798+
pass
799+
800+
-with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b:
801+
+with (
802+
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
803+
+) as b:
804+
pass
805+
806+
if True:
807+
- with anyio.CancelScope(
808+
- shield=True
809+
- ) if get_running_loop() else contextlib.nullcontext() as b:
810+
+ with (
811+
+ anyio.CancelScope(shield=True)
812+
+ if get_running_loop()
813+
+ else contextlib.nullcontext()
814+
+ ) as b:
815+
pass
691816
```
692817

693818

@@ -1093,4 +1218,23 @@ with open(
10931218
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
10941219
):
10951220
pass
1221+
1222+
with (
1223+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
1224+
):
1225+
pass
1226+
1227+
with (
1228+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1229+
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as b
1230+
):
1231+
pass
1232+
1233+
if True:
1234+
with (
1235+
anyio.CancelScope(shield=True)
1236+
if get_running_loop()
1237+
else contextlib.nullcontext() as b
1238+
):
1239+
pass
10961240
```

0 commit comments

Comments
 (0)