From 9425ed72a07ea28f4d0dab71e345abdba2832613 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 2 Aug 2023 15:55:00 -0400 Subject: [PATCH] Break global and nonlocal statements over continuation lines (#6172) ## Summary Builds on #6170 to break `global` and `nonlocal` statements, such that we get: ```python def f(): global \ analyze_featuremap_layer, \ analyze_featuremapcompression_layer, \ analyze_latencies_post, \ analyze_motions_layer, \ analyze_size_model ``` Instead of: ```python def f(): global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model ``` Notably, we avoid applying this formatting if the statement ends in a comment. Otherwise, the comment would _need_ to be placed after the last item, like: ```python def f(): global \ analyze_featuremap_layer, \ analyze_featuremapcompression_layer, \ analyze_latencies_post, \ analyze_motions_layer, \ analyze_size_model # noqa ``` To me, this seems wrong (and would break the `# noqa` comment). Ideally, the items would be parenthesized, and the comment would be on the inner parenthesis, like: ```python def f(): global ( # noqa analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model ) ``` But that's not valid syntax. --- .../test/fixtures/ruff/statement/global.py | 4 ++ .../test/fixtures/ruff/statement/nonlocal.py | 4 ++ .../src/statement/stmt_global.rs | 43 +++++++++++++++++-- .../src/statement/stmt_nonlocal.rs | 43 +++++++++++++++++-- .../format@statement__global.py.snap | 15 ++++++- .../format@statement__nonlocal.py.snap | 15 ++++++- 6 files changed, 114 insertions(+), 10 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/global.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/global.py index faec9b9e81..1048e99223 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/global.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/global.py @@ -10,3 +10,7 @@ def f(): def f(): global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + + +def f(): + global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/nonlocal.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/nonlocal.py index 2699c513bf..98df4df832 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/nonlocal.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/nonlocal.py @@ -10,3 +10,7 @@ def f(): def f(): nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + + +def f(): + nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment diff --git a/crates/ruff_python_formatter/src/statement/stmt_global.rs b/crates/ruff_python_formatter/src/statement/stmt_global.rs index abdaf7c710..e3dd6bca41 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_global.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_global.rs @@ -1,4 +1,5 @@ use ruff_formatter::{format_args, write}; +use ruff_python_ast::node::AstNode; use ruff_python_ast::StmtGlobal; use crate::prelude::*; @@ -9,10 +10,44 @@ pub struct FormatStmtGlobal; impl FormatNodeRule for FormatStmtGlobal { fn fmt_fields(&self, item: &StmtGlobal, f: &mut PyFormatter) -> FormatResult<()> { - write!(f, [text("global"), space()])?; + // Join the `global` names, breaking across continuation lines if necessary, unless the + // `global` statement has a trailing comment, in which case, breaking the names would + // move the comment "off" of the `global` statement. + if f.context() + .comments() + .has_trailing_comments(item.as_any_node_ref()) + { + let joined = format_with(|f| { + f.join_with(format_args![text(","), space()]) + .entries(item.names.iter().formatted()) + .finish() + }); - f.join_with(format_args![text(","), space()]) - .entries(item.names.iter().formatted()) - .finish() + write!(f, [text("global"), space(), &joined]) + } else { + let joined = format_with(|f| { + f.join_with(&format_args![ + text(","), + space(), + if_group_breaks(&text("\\")), + soft_line_break(), + ]) + .entries(item.names.iter().formatted()) + .finish() + }); + + write!( + f, + [ + text("global"), + space(), + &group(&format_args!( + if_group_breaks(&text("\\")), + soft_line_break(), + &soft_block_indent(&joined) + )) + ] + ) + } } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs b/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs index f3dd3b1966..777a7ac0d1 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs @@ -1,4 +1,5 @@ use ruff_formatter::{format_args, write}; +use ruff_python_ast::node::AstNode; use ruff_python_ast::StmtNonlocal; use crate::prelude::*; @@ -9,10 +10,44 @@ pub struct FormatStmtNonlocal; impl FormatNodeRule for FormatStmtNonlocal { fn fmt_fields(&self, item: &StmtNonlocal, f: &mut PyFormatter) -> FormatResult<()> { - write!(f, [text("nonlocal"), space()])?; + // Join the `nonlocal` names, breaking across continuation lines if necessary, unless the + // `nonlocal` statement has a trailing comment, in which case, breaking the names would + // move the comment "off" of the `nonlocal` statement. + if f.context() + .comments() + .has_trailing_comments(item.as_any_node_ref()) + { + let joined = format_with(|f| { + f.join_with(format_args![text(","), space()]) + .entries(item.names.iter().formatted()) + .finish() + }); - f.join_with(format_args![text(","), space()]) - .entries(item.names.iter().formatted()) - .finish() + write!(f, [text("nonlocal"), space(), &joined]) + } else { + let joined = format_with(|f| { + f.join_with(&format_args![ + text(","), + space(), + if_group_breaks(&text("\\")), + soft_line_break(), + ]) + .entries(item.names.iter().formatted()) + .finish() + }); + + write!( + f, + [ + text("nonlocal"), + space(), + &group(&format_args!( + if_group_breaks(&text("\\")), + soft_line_break(), + &soft_block_indent(&joined) + )) + ] + ) + } } } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__global.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__global.py.snap index 1181cdba30..1209b7c3f5 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__global.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__global.py.snap @@ -16,6 +16,10 @@ def f(): def f(): global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + + +def f(): + global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment ``` ## Output @@ -31,7 +35,16 @@ def f(): def f(): - global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + global \ + analyze_featuremap_layer, \ + analyze_featuremapcompression_layer, \ + analyze_latencies_post, \ + analyze_motions_layer, \ + analyze_size_model + + +def f(): + global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__nonlocal.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__nonlocal.py.snap index fbffb2ede9..bbdcb3046d 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__nonlocal.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__nonlocal.py.snap @@ -16,6 +16,10 @@ def f(): def f(): nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + + +def f(): + nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment ``` ## Output @@ -31,7 +35,16 @@ def f(): def f(): - nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model + nonlocal \ + analyze_featuremap_layer, \ + analyze_featuremapcompression_layer, \ + analyze_latencies_post, \ + analyze_motions_layer, \ + analyze_size_model + + +def f(): + nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment ```