Left hand-side trailing comma shouldn't be treated as a magic trailing comma

[pfalcon]

Describe the bug

Black 20.8b1 (well, apparently 20.8b0) changed behavior regarding formatting code fragments with trailing comma (destructuring assignments, func arg list, etc.). This makes the code which linted well with 19.10b0 to contain spurious changes when run thru 20.8b1. And the changes it introduces are arguably inconsistent and don't make much sense on their own, comparing to the corresponding syntaxes without trailing comma. (Unless Black chooses to treat trailing-comma and non-trailing-comma syntaxes very differently. But then again, that's a rather drastic and breaking change between the versions.)

To Reproduce

The simplest way to reproduce the issue:

1. Take this file:

a, b = foo
a, b, = foo

2. Run Black on it as: black --diff trailing.py

3. See the spurious changes:

$ black --version
black, version 20.8b1
$ black --diff trailing.py 
--- trailing.py	2020-08-28 06:53:51.404271 +0000
+++ trailing.py	2020-08-28 07:05:11.073338 +0000
@@ -1,2 +1,5 @@
 a, b = foo
-a, b, = foo
+(
+    a,
+    b,
+) = foo
would reformat trailing.py
All done! ✨ 🍰 ✨
1 file would be reformatted.

Expected behavior A clear and concise description of what you expected to happen.

With Black 19.10b0:

$ black --version
black, version 19.10b0
$ black --diff trailing.py 
All done! ✨ 🍰 ✨
1 file left unchanged.

The expectation is that upgrade to 20.8 release wouldn't lead to spurious changes.

Environment (please complete the following information):

• Version: 20.8b1
• OS and Python version: Ubuntu Linux 18.04/Python 3.6.9 (from system package repo)

Does this bug also happen on master?

To answer this, you have two options:

1. Use the online formatter at https://black.now.sh/?version=master, which will use the
   latest master branch.

Yes, it also happens on master:
https://black.now.sh/?version=master&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ABXAD5dAD2IimZxl1N_WlOoPbl0YzjUvhymrTQxdvJIsCnivaICEWiMvJjjn4HSMUAqYvYElO5770YCyC4fhQgqbmEAAAAASlERy4bruBsAAVpYM9liBR-2830BAAAAAARZWg==

Additional context Add any other context about the problem here.

For an example of this happening in real world, consider this Travis build log for MicroPython (https://github.com/micropython/micropython/): https://travis-ci.org/github/pfalcon/pycopy/jobs/721922300 . It would be another question whether people who put trailing comma where it's not really needed are guided or misguided. The topic of this issue is that adding a single trailing comma shouldn't lead to drastic change of Black output, between consecutive Black releases.

[ambv]

Thanks for your report. You are using pre-release software and the formatting will change between versions. That's why we insist on keeping the beta marker in the versions. Appealing to consistency with previous versions is therefore not actionable for us.

In general 19.10b0 had a feature (magic trailing commas) which was only partially functional. This changed with 20.8b0. In general this is a deliberate change. You did find a bug though: it now over-eagerly explodes also left-hand side trailing commas during tuple unpacking. We'll fix that.

Workaround for now: manually delete the unnecessary trailing comma on the left hand-side of the assignment and Black will do the right thing.

[pfalcon]

You are using pre-release software and the formatting will change between versions. That's why we insist on keeping the beta marker in the versions. Appealing to consistency with previous versions is therefore not actionable for us.

Being a developer myself (and thus liking to make changes behind users' backs ;-) ), I can appreciate that statement.

At the same time, one of the appeals of the Black was "opinionated" and "(almost) no knobs to turn". This kinda translates to "We know how to do Python code formatting right". Then suddenly, turns out, Black doesn't really have strong opinion about code formatting, as it applies swinging changes like that [randomly].

And even that can happen, but then, as a user, a thought creeps in: "Can we get our knobs to turn around?"

Workaround for now: manually delete the unnecessary trailing comma on the left hand-side of the assignment and Black will do the right thing.

Works for my code, might not work that well when dealing with 3rd-party code, e.g. a monorepo-like setup with a bunch of 3rd-party code which linted well, and now doesn't.

That said, this this report is along the lines of "people notice". I don't pledge for any changes (the obvious workaround is simply to use pre-coronavirus Black), and its importance would depend on how many people are burnt by it (I was surprised to be first to report it, now I see there're 2 vote-ups, let's see how it goes).

[altendky]

I noticed. I'm mostly satisfied with 'this half was intentional' and 'that half is a bug we will fix'. Though I do vaguely think maybe Black managing the trailing , itself could be good. Otherwise the trailing , becomes a knob. I personally tend to prefer one line per 'element' when it won't all fit on one line and also a trailing , to go with it. I admittedly haven't thought through any details of this.

[dtpc]

This bug also affects trailing commas inside function call. E.g. this file:

fn(1,)

Black 19.10b

$ black --version            
black, version 19.10b0
$ black --diff ./trailing2.py
All done! ✨ 🍰 ✨
1 file left unchanged.

Black 20.8b1

$ black --version             
black, version 20.8b1
$ black --diff ./trailing2.py
--- trailing2.py	2020-09-01 01:02:19.438550 +0000
+++ trailing2.py	2020-09-01 01:09:40.311716 +0000
@@ -1 +1,3 @@
-fn(1,)
+fn(
+    1,
+)
would reformat trailing2.py
All done! ✨ 🍰 ✨
1 file would be reformatted.

[ambv]

@dtpc this is deliberate as you can read in my first comment on this very issue. It's documented as well: https://black.readthedocs.io/en/stable/the_black_code_style.html#the-magic-trailing-comma

[dtpc]

Thanks, @ambv!

I misinterpreted the magic comma documentation saying that it is "taking existing formatting into account" to mean that it would only apply if the collection or function call is already formatted as multi-line.

In what situations does this line from the code style apply: "Unnecessary trailing commas are removed if an expression fits in one line."?

[ambv]

In what situations does this line from the code style apply: "Unnecessary trailing commas are removed if an expression fits in one line."?

I guess you're right that this no longer applies. It hasn't since 19.10b0. I clarified this in 6b935a3.

[JelleZijlstra]

The actionable part of the original complaint is that we shouldn't treat the magic trailing comma as magic in a specific context (LHS of an assignment). That would be inconsistent: why should this particular context be special? I'm closing this issue as a rejected formatting change.

Note that as of recent releases you can use --skip-magic-trailing-comma to disable this behavior.