Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trailing comma not removed in type annotation #1756

Closed
fuglede opened this issue Oct 9, 2020 · 6 comments
Closed

Trailing comma not removed in type annotation #1756

fuglede opened this issue Oct 9, 2020 · 6 comments
Labels
C: configuration CLI and configuration F: trailing comma Full of magic T: bug Something isn't working

Comments

@fuglede
Copy link

fuglede commented Oct 9, 2020

Describe the bug

By running black with different line lengths, I can produce inconsistent results that ultimately fail in flake8 (and look strange).

To Reproduce

Consider this relatively silly example (let's call it test.py), with a line of length 81:

from typing import Tuple


def f(a: Tuple[int, int, int, int, int, int, int, int, int, int, int, int, int]):
    return a

Both black and flake8 are happy with this:

$ flake8 test.py
test.py:4:80: E501 line too long (81 > 79 characters)
$ flake8 test.py --max-line-length=88

All is good. Now, run black with -l 70, then without a line length

$ black -l 70 test.py
reformatted test.py
All done! ✨ 🍰 ✨
1 file reformatted.
$ black test.py
reformatted test.py
All done! ✨ 🍰 ✨
1 file reformatted.

Now the file looks different from the starting point:

from typing import Tuple


def f(
    a: Tuple[int, int, int, int, int, int, int, int, int, int, int, int, int,]
):
    return a

Moreover, flake8 doesn't like the trailing comma:

$ flake8 test.py
test.py:5:77: E231 missing whitespace after ','

By ignoring the change of line length in the example above, you can also choose to think of this of an example where black (with no line length specification) causes issues with flake8; I'm not sure if this is a flake8 bug (a la this one) or a black issue. Even then, it seems like black test.py should be equivalent to black -l 70 test.py && black -l test.py.

Environment (please complete the following information):

  • Version: black, version 19.10b0
  • OS and Python version: Linux/Python 3.7.6

Does this bug also happen on master?

I tried https://black.now.sh/?version=master which fails to convert the intermediate result back to the one-liner when given a line-length of 88. I'm not sure if this is just a separate bug.

@fuglede fuglede added the T: bug Something isn't working label Oct 9, 2020
@JelleZijlstra
Copy link
Collaborator

This no longer reproduces exactly because of the magic trailing comma, but you still get the ,] behavior with black --skip-magic-trailing-comma. Instead, Black should remove the comma.

@ichard26 ichard26 added C: configuration CLI and configuration F: trailing comma Full of magic labels May 29, 2021
@JelleZijlstra
Copy link
Collaborator

Still reproduces on current main with --skip-magic-trailing-comma.

@JelleZijlstra JelleZijlstra changed the title Changing line lengths cause inconsistent type hint formatting Trailing comma not removed in type annotation Dec 19, 2022
@vors
Copy link

vors commented May 11, 2023

This would be great to fix: we are doing the black --line-length=30 && <some-mechanical-patching> && black -C --line-length=99 and this messes up the diff a great deal.

The mechanical patching part for context is adding suppressions for pyright errors.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented May 12, 2023

@vors I think I fixed this in #3209 . My guess is Jelle only reproduced the issue because he didn't use --preview, but this is now in the stable style when using --skip-magic-trailing-comma.

https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ADqAH9dAD2IimZxl1N_WlkPinBFoZStZNLAcBn-vRxIYUS7MGdv-aZUpwMHHXlN6ADR5YhOvkX521Cx0wTY5xOdWgxHqd3EFilPK3gBt5WDqzisLFGMbfK6RerlnIRjoO1IXFuQwpYRRMHoEA97YqjmYJ4D6By7mlaQg6zirnODBZfaxwAAADWFLlqSj6CBAAGbAesBAADjtyiJscRn-wIAAAAABFla

Unless you have a different repro?

@vors
Copy link

vors commented May 12, 2023

got it, yes that's kind of an old bug lol. I will check and report back.

@vors
Copy link

vors commented May 12, 2023

Yes, all fixed. I think we should close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: configuration CLI and configuration F: trailing comma Full of magic T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants