language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-03 18:28:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Don't add a magic trailing comma for a single entry (#5463)

Browse source 
## Summary

If a comma separated list has only one entry, black will respect the
magic trailing comma, but it will not add a new one.

The following code will remain as is:

```python
b1 = [
    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
b2 = [
    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
]
b3 = [
    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
```

## Test Plan

This was first discovered in
7eeadc82c2/django/contrib/admin/checks.py (L674-L681),
which i've minimized into a call test.

I've added tests for the three cases (one entry + no comma, one entry +
comma, more than one entry) to the list tests.

The diffs from the black tests get smaller.
This commit is contained in:
konsti 2023-07-03 21:48:44 +02:00 • committed by GitHub
parent 3992c47c00
commit a647f31600
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 119 additions and 149 deletions
Download patch file Download diff file Expand all files Collapse all files

5
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/call.py → crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py vendored
View file

@ -81,3 +81,8 @@ f(
dict()
)
# Don't add a magic trailing comma when there is only one entry
# Minimized from https://github.com/django/django/blob/7eeadc82c2f7d7a778e3bb43c34d642e6275dacf/django/contrib/admin/checks.py#L674-L681
f(
a.very_long_function_function_that_is_so_long_that_it_expands_the_parent_but_its_only_a_single_argument()
)

13
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list.py vendored
View file

@ -8,3 +8,16 @@ a2 = [ # a
a3 = [
# b
]
# Add magic trailing comma only if there is more than one entry, but respect it if it's
# already there
b1 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
b2 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
]
b3 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]

30
crates/ruff_python_formatter/src/builders.rs
View file

@ -182,7 +182,10 @@ impl<'fmt, 'ast, 'buf> JoinNodesBuilder<'fmt, 'ast, 'buf> {
pub(crate) struct JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
result: FormatResult<()>,
fmt: &'fmt mut PyFormatter<'ast, 'buf>,
last_end: Option<TextSize>,
end_of_last_entry: Option<TextSize>,
/// We need to track whether we have more than one entry since a sole entry doesn't get a
/// magic trailing comma even when expanded
len: usize,
}
impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
@ -190,7 +193,8 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
Self {
fmt: f,
result: Ok(()),
last_end: None,
end_of_last_entry: None,
len: 0,
}
}
@ -203,11 +207,12 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
T: Ranged,
{
self.result = self.result.and_then(|_| {
if self.last_end.is_some() {
if self.end_of_last_entry.is_some() {
write!(self.fmt, [text(","), soft_line_break_or_space()])?;
}
self.last_end = Some(node.end());
self.end_of_last_entry = Some(node.end());
self.len += 1;
content.fmt(self.fmt)
});
@ -243,18 +248,23 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
pub(crate) fn finish(&mut self) -> FormatResult<()> {
self.result.and_then(|_| {
if let Some(last_end) = self.last_end.take() {
if_group_breaks(&text(",")).fmt(self.fmt)?;
if self.fmt.options().magic_trailing_comma().is_respect()
if let Some(last_end) = self.end_of_last_entry.take() {
let magic_trailing_comma = self.fmt.options().magic_trailing_comma().is_respect()
&& matches!(
first_non_trivia_token(last_end, self.fmt.context().contents()),
Some(Token {
kind: TokenKind::Comma,
..
})
)
{
);
// If there is a single entry, only keep the magic trailing comma, don't add it if
// it wasn't there. If there is more than one entry, always add it.
if magic_trailing_comma || self.len > 1 {
if_group_breaks(&text(",")).fmt(self.fmt)?;
}
if magic_trailing_comma {
expand_parent().fmt(self.fmt)?;
}
}

29
crates/ruff_python_formatter/tests/snapshots/black_compatibility@miscellaneous__long_strings_flag_disabled.py.snap
View file

@ -312,15 +312,6 @@ long_unmergable_string_with_pragma = (
y = "Short string"
@@ -12,7 +12,7 @@
)
print(
- "This is a really long string inside of a print statement with no extra arguments attached at the end of it."
+ "This is a really long string inside of a print statement with no extra arguments attached at the end of it.",
)
D1 = {
@@ -70,8 +70,8 @@
bad_split3 = (
"What if we have inline comments on " # First Comment
@ -367,7 +358,7 @@ long_unmergable_string_with_pragma = (
comment_string = "Long lines with inline comments should have their comments appended to the reformatted string's enclosing right parentheses." # This comment gets thrown to the top.
@@ -165,30 +163,18 @@
@@ -165,25 +163,13 @@
triple_quote_string = """This is a really really really long triple quote string assignment and it should not be touched."""
@ -397,12 +388,6 @@ long_unmergable_string_with_pragma = (
some_function_call(
"With a reallly generic name and with a really really long string that is, at some point down the line, "
+ added
- + " to a variable and then added to another string."
+ + " to a variable and then added to another string.",
)
some_function_call(
@@ -212,29 +198,25 @@
)
@ -412,7 +397,7 @@ long_unmergable_string_with_pragma = (
- " which should NOT be there."
- ),
+ "This is a really long string argument to a function that has a trailing comma"
+ " which should NOT be there.",
+ " which should NOT be there."
)
func_with_bad_comma(
@ -421,7 +406,7 @@ long_unmergable_string_with_pragma = (
- " which should NOT be there."
- ), # comment after comma
+ "This is a really long string argument to a function that has a trailing comma"
+ " which should NOT be there.", # comment after comma
+ " which should NOT be there." # comment after comma
)
func_with_bad_parens_that_wont_fit_in_one_line(
@ -498,7 +483,7 @@ print(
)
print(
"This is a really long string inside of a print statement with no extra arguments attached at the end of it.",
"This is a really long string inside of a print statement with no extra arguments attached at the end of it."
)
D1 = {
@ -660,7 +645,7 @@ NOT_YET_IMPLEMENTED_StmtAssert
some_function_call(
"With a reallly generic name and with a really really long string that is, at some point down the line, "
+ added
+ " to a variable and then added to another string.",
+ " to a variable and then added to another string."
)
some_function_call(
@ -685,12 +670,12 @@ func_with_bad_comma(
func_with_bad_comma(
"This is a really long string argument to a function that has a trailing comma"
" which should NOT be there.",
" which should NOT be there."
)
func_with_bad_comma(
"This is a really long string argument to a function that has a trailing comma"
" which should NOT be there.", # comment after comma
" which should NOT be there." # comment after comma
)
func_with_bad_parens_that_wont_fit_in_one_line(

12
crates/ruff_python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_style.py.snap
View file

@ -84,7 +84,7 @@ match match(
-match(arg) # comment
+match(
+ arg, # comment
+ arg # comment
+)
match()
@ -93,7 +93,7 @@ match match(
-case(arg) # comment
+case(
+ arg, # comment
+ arg # comment
+)
case()
@ -103,7 +103,7 @@ match match(
-re.match(something) # fast
+re.match(
+ something, # fast
+ something # fast
+)
re.match()
-match match():
@ -120,7 +120,7 @@ match match(
NOT_YET_IMPLEMENTED_StmtMatch
match(
arg, # comment
arg # comment
)
match()
@ -128,7 +128,7 @@ match()
match()
case(
arg, # comment
arg # comment
)
case()
@ -137,7 +137,7 @@ case()
re.match(
something, # fast
something # fast
)
re.match()
NOT_YET_IMPLEMENTED_StmtMatch

11
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__comments3.py.snap
View file

@ -76,15 +76,6 @@ def func():
# Capture each of the exceptions in the MultiError along with each of their causes and contexts
if isinstance(exc_value, MultiError):
embedded = []
@@ -29,7 +22,7 @@
# copy the set of _seen exceptions so that duplicates
# shared between sub-exceptions are not omitted
_seen=set(_seen),
- )
+ ),
# This should be left alone (after)
)
```
## Ruff Output
@ -114,7 +105,7 @@ def func():
# copy the set of _seen exceptions so that duplicates
# shared between sub-exceptions are not omitted
_seen=set(_seen),
),
)
# This should be left alone (after)
)

11
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap
View file

@ -203,15 +203,6 @@ class C:
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
@@ -37,7 +37,7 @@
batch_size=Yyyy2YyyyYyyyyYyyy.FULL_SIZE,
).push(
# Only send the first n items.
- items=items[:num_items]
+ items=items[:num_items],
)
return (
'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s'
@@ -47,113 +47,46 @@
def omitting_trailers(self) -> None:
get_collection(
@ -418,7 +409,7 @@ class C:
batch_size=Yyyy2YyyyYyyyyYyyy.FULL_SIZE,
).push(
# Only send the first n items.
items=items[:num_items],
items=items[:num_items]
)
return (
'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s'

11
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap
View file

@ -203,15 +203,6 @@ class C:
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
@@ -37,7 +37,7 @@
batch_size=Yyyy2YyyyYyyyyYyyy.FULL_SIZE,
).push(
# Only send the first n items.
- items=items[:num_items]
+ items=items[:num_items],
)
return (
'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s'
@@ -47,113 +47,46 @@
def omitting_trailers(self) -> None:
get_collection(
@ -418,7 +409,7 @@ class C:
batch_size=Yyyy2YyyyYyyyyYyyy.FULL_SIZE,
).push(
# Only send the first n items.
items=items[:num_items],
items=items[:num_items]
)
return (
'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s'

10
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff.py.snap
View file

@ -395,12 +395,8 @@ d={'a':1,
# fmt: on
# fmt: off
# ...but comments still get reformatted even though they should not be
@@ -150,12 +172,10 @@
ast_args.kw_defaults,
parameters,
implicit_default=True,
- )
+ ),
@@ -153,9 +175,7 @@
)
)
# fmt: off
- a = (
@ -610,7 +606,7 @@ def long_lines():
ast_args.kw_defaults,
parameters,
implicit_default=True,
),
)
)
# fmt: off
a = unnecessary_bracket()

15
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff4.py.snap
View file

@ -37,20 +37,11 @@ def f(): pass
+ 2,
+ 3,
+ 4,
+ ],
+ ]
+)
# fmt: on
def f():
pass
@@ -14,7 +18,7 @@
2,
3,
4,
- ]
+ ],
)
def f():
pass
```
## Ruff Output
@ -63,7 +54,7 @@ def f(): pass
2,
3,
4,
],
]
)
# fmt: on
def f():
@ -76,7 +67,7 @@ def f():
2,
3,
4,
],
]
)
def f():
pass

4
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff5.py.snap
View file

@ -103,7 +103,7 @@ elif unformatted:
- # fmt: on
- ] # Includes an formatted indentation.
+ # fmt: on
+ ], # Includes an formatted indentation.
+ ] # Includes an formatted indentation.
},
)
@ -186,7 +186,7 @@ setup(
"foo-bar"
"=foo.bar.:main",
# fmt: on
], # Includes an formatted indentation.
] # Includes an formatted indentation.
},
)

28
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function.py.snap
View file

@ -111,14 +111,14 @@ def __await__(): return (yield)
#!/usr/bin/env python3
-import asyncio
-import sys
-
-from third_party import X, Y, Z
+NOT_YET_IMPLEMENTED_StmtImport
+NOT_YET_IMPLEMENTED_StmtImport
-from library import some_connection, some_decorator
-from third_party import X, Y, Z
+NOT_YET_IMPLEMENTED_StmtImportFrom
-from library import some_connection, some_decorator
-
-f"trigger 3.6 mode"
+NOT_YET_IMPLEMENTED_StmtImportFrom
+NOT_YET_IMPLEMENTED_ExprJoinedStr
@ -198,24 +198,6 @@ def __await__(): return (yield)
def long_lines():
@@ -87,7 +94,7 @@
ast_args.kw_defaults,
parameters,
implicit_default=True,
- )
+ ),
)
typedargslist.extend(
gen_annotated_params(
@@ -96,7 +103,7 @@
parameters,
implicit_default=True,
# trailing standalone comment
- )
+ ),
)
_type_comment_re = re.compile(
r"""
@@ -135,14 +142,8 @@
a,
**kwargs,
@ -334,7 +316,7 @@ def long_lines():
ast_args.kw_defaults,
parameters,
implicit_default=True,
),
)
)
typedargslist.extend(
gen_annotated_params(
@ -343,7 +325,7 @@ def long_lines():
parameters,
implicit_default=True,
# trailing standalone comment
),
)
)
_type_comment_re = re.compile(
r"""

30
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function_trailing_comma.py.snap
View file

@ -73,15 +73,6 @@ some_module.some_function(
```diff
--- Black
+++ Ruff
@@ -27,7 +27,7 @@
call(
arg={
"explode": "this",
- }
+ },
)
call2(
arg=[1, 2, 3],
@@ -35,7 +35,9 @@
x = {
"a": 1,
@ -93,7 +84,7 @@ some_module.some_function(
if (
a
== {
@@ -47,22 +49,24 @@
@@ -47,14 +49,16 @@
"f": 6,
"g": 7,
"h": 8,
@ -114,17 +105,6 @@ some_module.some_function(
json = {
"k": {
"k2": {
"k3": [
1,
- ]
- }
- }
+ ],
+ },
+ },
}
@@ -80,18 +84,14 @@
pass
@ -182,7 +162,7 @@ def f(
call(
arg={
"explode": "this",
},
}
)
call2(
arg=[1, 2, 3],
@ -219,9 +199,9 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
"k2": {
"k3": [
1,
],
},
},
]
}
}
}

4
crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap
View file

@ -299,9 +299,9 @@ not (aaaaaaaaaaaaaa + {NOT_IMPLEMENTED_set_value for value in NOT_IMPLEMENTED_se
[
a
+ [
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
]
in c,
in c
]

17
crates/ruff_python_formatter/tests/snapshots/format@statement__call.py.snap → crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap
View file

@ -1,6 +1,6 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/call.py
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py
---
## Input
```py
@ -87,6 +87,11 @@ f(
dict()
)
# Don't add a magic trailing comma when there is only one entry
# Minimized from https://github.com/django/django/blob/7eeadc82c2f7d7a778e3bb43c34d642e6275dacf/django/contrib/admin/checks.py#L674-L681
f(
a.very_long_function_function_that_is_so_long_that_it_expands_the_parent_but_its_only_a_single_argument()
)
```
## Output
@ -111,10 +116,10 @@ f(x=2)
f(1, x=2)
f(
this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd,
this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd
)
f(
this_is_a_very_long_keyword_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd=1,
this_is_a_very_long_keyword_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd=1
)
f(
@ -168,6 +173,12 @@ f(
**dict(),
# oddly placed own line comment
)
# Don't add a magic trailing comma when there is only one entry
# Minimized from https://github.com/django/django/blob/7eeadc82c2f7d7a778e3bb43c34d642e6275dacf/django/contrib/admin/checks.py#L674-L681
f(
a.very_long_function_function_that_is_so_long_that_it_expands_the_parent_but_its_only_a_single_argument()
)
```

4
crates/ruff_python_formatter/tests/snapshots/format@expression__compare.py.snap
View file

@ -180,10 +180,10 @@ return (
(
a
+ [
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
]
>= c
),
)
]
```

4
crates/ruff_python_formatter/tests/snapshots/format@expression__dict.py.snap
View file

@ -69,7 +69,7 @@ a = {
# before
{
# open
key: value, # key # colon # value
key: value # key # colon # value
} # close
# after
@ -82,7 +82,7 @@ a = {
}
{
**b, # middle with single item
**b # middle with single item
}
{

26
crates/ruff_python_formatter/tests/snapshots/format@expression__list.py.snap
View file

@ -14,6 +14,19 @@ a2 = [ # a
a3 = [
# b
]
# Add magic trailing comma only if there is more than one entry, but respect it if it's
# already there
b1 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
b2 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
]
b3 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
```
## Output
@ -28,6 +41,19 @@ a2 = [ # a
a3 = [
# b
]
# Add magic trailing comma only if there is more than one entry, but respect it if it's
# already there
b1 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
]
b2 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
]
b3 = [
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa,
]
```

4
crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap
View file

@ -113,9 +113,7 @@ with a: # should remove brackets
# if we do want to wrap, do we prefer to wrap the entire WithItem or to let the
# WithItem allow the `aa + bb` content expression to be wrapped
with (
(
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
) as c,
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c
):
...