From 8bff9604585a11aea272feaee53535f09d2c0977 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sat, 15 Jul 2023 18:40:38 +0200 Subject: [PATCH] Respect magic trailing comma for set expression (#5782) ## Summary This PR uses the `join_comma_separated` builder for formatting set expressions to ensure the formatting preserves magic commas, if the setting is enabled. ## Test Plan See the fixed black tests --- .../src/expression/expr_set.rs | 15 +- ...tibility@simple_cases__collections.py.snap | 312 ------------------ 2 files changed, 8 insertions(+), 319 deletions(-) delete mode 100644 crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__collections.py.snap diff --git a/crates/ruff_python_formatter/src/expression/expr_set.rs b/crates/ruff_python_formatter/src/expression/expr_set.rs index 83ff228a83a9d7..5c7a27bf4d1052 100644 --- a/crates/ruff_python_formatter/src/expression/expr_set.rs +++ b/crates/ruff_python_formatter/src/expression/expr_set.rs @@ -1,9 +1,10 @@ +use rustpython_parser::ast::{ExprSet, Ranged}; + +use ruff_python_ast::node::AnyNodeRef; + use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; use crate::prelude::*; use crate::FormatNodeRule; -use ruff_formatter::format_args; -use ruff_python_ast::node::AnyNodeRef; -use rustpython_parser::ast::ExprSet; #[derive(Default)] pub struct FormatExprSet; @@ -14,13 +15,13 @@ impl FormatNodeRule for FormatExprSet { // That would be a dict expression assert!(!elts.is_empty()); // Avoid second mutable borrow of f - let joined = format_with(|f| { - f.join_with(format_args!(text(","), soft_line_break_or_space())) - .entries(elts.iter().formatted()) + let joined = format_with(|f: &mut PyFormatter| { + f.join_comma_separated(item.end()) + .nodes(elts.iter()) .finish() }); - parenthesized("{", &format_args![joined, if_group_breaks(&text(","))], "}").fmt(f) + parenthesized("{", &joined, "}").fmt(f) } } diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__collections.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__collections.py.snap deleted file mode 100644 index 9850888f4ea60d..00000000000000 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__collections.py.snap +++ /dev/null @@ -1,312 +0,0 @@ ---- -source: crates/ruff_python_formatter/tests/fixtures.rs -input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/collections.py ---- -## Input - -```py -import core, time, a - -from . import A, B, C - -# keeps existing trailing comma -from foo import ( - bar, -) - -# also keeps existing structure -from foo import ( - baz, - qux, -) - -# `as` works as well -from foo import ( - xyzzy as magic, -) - -a = {1,2,3,} -b = { -1,2, - 3} -c = { - 1, - 2, - 3, -} -x = 1, -y = narf(), -nested = {(1,2,3),(4,5,6),} -nested_no_trailing_comma = {(1,2,3),(4,5,6)} -nested_long_lines = ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccc", (1, 2, 3), "dddddddddddddddddddddddddddddddddddddddd"] -{"oneple": (1,),} -{"oneple": (1,)} -['ls', 'lsoneple/%s' % (foo,)] -x = {"oneple": (1,)} -y = {"oneple": (1,),} -assert False, ("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa wraps %s" % bar) - -# looping over a 1-tuple should also not get wrapped -for x in (1,): - pass -for (x,) in (1,), (2,), (3,): - pass - -[1, 2, 3,] - -division_result_tuple = (6/2,) -print("foo %r", (foo.bar,)) - -if True: - IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = ( - Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING - | {pylons.controllers.WSGIController} - ) - -if True: - ec2client.get_waiter('instance_stopped').wait( - InstanceIds=[instance.id], - WaiterConfig={ - 'Delay': 5, - }) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={"Delay": 5,}, - ) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], WaiterConfig={"Delay": 5,}, - ) -``` - -## Black Differences - -```diff ---- Black -+++ Ruff -@@ -18,23 +18,12 @@ - xyzzy as magic, - ) - --a = { -- 1, -- 2, -- 3, --} -+a = {1, 2, 3} - b = {1, 2, 3} --c = { -- 1, -- 2, -- 3, --} -+c = {1, 2, 3} - x = (1,) - y = (narf(),) --nested = { -- (1, 2, 3), -- (4, 5, 6), --} -+nested = {(1, 2, 3), (4, 5, 6)} - nested_no_trailing_comma = {(1, 2, 3), (4, 5, 6)} - nested_long_lines = [ - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", -``` - -## Ruff Output - -```py -import core, time, a - -from . import A, B, C - -# keeps existing trailing comma -from foo import ( - bar, -) - -# also keeps existing structure -from foo import ( - baz, - qux, -) - -# `as` works as well -from foo import ( - xyzzy as magic, -) - -a = {1, 2, 3} -b = {1, 2, 3} -c = {1, 2, 3} -x = (1,) -y = (narf(),) -nested = {(1, 2, 3), (4, 5, 6)} -nested_no_trailing_comma = {(1, 2, 3), (4, 5, 6)} -nested_long_lines = [ - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - "cccccccccccccccccccccccccccccccccccccccc", - (1, 2, 3), - "dddddddddddddddddddddddddddddddddddddddd", -] -{ - "oneple": (1,), -} -{"oneple": (1,)} -["ls", "lsoneple/%s" % (foo,)] -x = {"oneple": (1,)} -y = { - "oneple": (1,), -} -assert False, ( - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa wraps %s" - % bar -) - -# looping over a 1-tuple should also not get wrapped -for x in (1,): - pass -for (x,) in (1,), (2,), (3,): - pass - -[ - 1, - 2, - 3, -] - -division_result_tuple = (6 / 2,) -print("foo %r", (foo.bar,)) - -if True: - IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = ( - Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING - | {pylons.controllers.WSGIController} - ) - -if True: - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) -``` - -## Black Output - -```py -import core, time, a - -from . import A, B, C - -# keeps existing trailing comma -from foo import ( - bar, -) - -# also keeps existing structure -from foo import ( - baz, - qux, -) - -# `as` works as well -from foo import ( - xyzzy as magic, -) - -a = { - 1, - 2, - 3, -} -b = {1, 2, 3} -c = { - 1, - 2, - 3, -} -x = (1,) -y = (narf(),) -nested = { - (1, 2, 3), - (4, 5, 6), -} -nested_no_trailing_comma = {(1, 2, 3), (4, 5, 6)} -nested_long_lines = [ - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - "cccccccccccccccccccccccccccccccccccccccc", - (1, 2, 3), - "dddddddddddddddddddddddddddddddddddddddd", -] -{ - "oneple": (1,), -} -{"oneple": (1,)} -["ls", "lsoneple/%s" % (foo,)] -x = {"oneple": (1,)} -y = { - "oneple": (1,), -} -assert False, ( - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa wraps %s" - % bar -) - -# looping over a 1-tuple should also not get wrapped -for x in (1,): - pass -for (x,) in (1,), (2,), (3,): - pass - -[ - 1, - 2, - 3, -] - -division_result_tuple = (6 / 2,) -print("foo %r", (foo.bar,)) - -if True: - IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = ( - Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING - | {pylons.controllers.WSGIController} - ) - -if True: - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) - ec2client.get_waiter("instance_stopped").wait( - InstanceIds=[instance.id], - WaiterConfig={ - "Delay": 5, - }, - ) -``` - -