PEP 614 support #1717

QuentinSoubeyran · 2020-09-18T16:20:38Z

Adds support for PEP 614 to fix #1711. This is still WIP

Changes

updated typed-ast to 1.4.1 so that pipenv install --dev doesn't fail within a python 3.9 venv due to compile errors
changed the definition ofdecorator in blib2to3/Grammar.txt to reflect the relaxed decorator syntax
added a PY39 target
added a RELAXED_DECORATORS feature
added an eager detection of the above feature in get_feature_used(), with helper functions
moved PY36_VERSIONS from src/black/__init__.py to tests/test_black.py
added test for the detection of RELAXED_DECORATORS feature
added test for python 3.9 decorator formatting

Note

--safe doesn't work on python 3.9 code from black running on python <=3.8.5. This might already be the case with python 3.8 code formatted from black running on python 3.7, because typed_ast doesn't support python 3.8+ syntax. So older python version cannot parse the syntax with either the builtin ast or typed_ast

QuentinSoubeyran · 2020-09-18T17:06:11Z

I'm looking for help on the space after @ bug, I have no idea where this is added in the code. My hypothesis is that all expression get a space before, I'd like to special-case that so it doesn't happens if the previous character is @ and we are in a decorator (so that the @ as __matmul__ operator is still properly spaced)

zsol · 2020-09-18T18:24:22Z

For the whitespace issue, this will need to be handled in whitespace around here: https://github.com/psf/black/blob/master/src/black/__init__.py#L2259

Please add a couple of explicit test cases for the feature detection, as well as some more complex decorator examples.

zsol · 2020-09-18T18:21:40Z

src/black/__init__.py

+
+
+def is_simple_decorator_expression(node: LN) -> bool:
+    """Return True iff `node` is the 'dotted name' in the grammar @ dotted_name [arguments] NEWLINE"""


nit

Suggested change

"""Return True iff `node` is the 'dotted name' in the grammar @ dotted_name [arguments] NEWLINE"""

"""Return True iff `node` is the 'namedexpr_test' in the grammar @ namedexpr_test [arguments] NEWLINE"""

Tried to clarify that this function does test against a dotted_name grammar. Is this better ?

zsol · 2020-09-18T18:28:09Z

Also feel free to put this PR in draft mode and ping me explicitly if you need more help until you're ready to publish it :)

QuentinSoubeyran · 2020-09-18T20:54:07Z

I should have checked here earlier, I just found the whitespace() function by using pdb. Thanks anyway :)

QuentinSoubeyran · 2020-09-19T11:46:15Z

Hi @zsol , I'm not sure how to handle the following: the --safe option doesn't work on python 3.9 code when black run on python 3.8. (tested on 3.8.2 and 3.8.5)
This is because parse_ast uses the builtin ast module, which cannot parse 3.9 code in python 3.8.5. Judging from the grammar on the ast doc, where decorator list are expr*, this might be fixed in python 3.8.6.

Still, python 3.9 code cannot be safely reformatted used python <=3.8.5. What is the best way to handle that ? Should I introduce a call to ast3.parse(..., feature_version=9) in parse_ast to fix that ?

QuentinSoubeyran · 2020-09-19T12:23:22Z

Also, I tried running black-primer, but it fails for some projects (hypothesis, poetry, pyramid, pytest, sqlalchemy). Should I just skip them as hinted in the contribution guide, or should I do something about that ?

zsol · 2020-09-19T12:40:38Z

The idea behind that parse_ast function is to use the builtin ast module on the latest supported python version. That will be 3.9 with this PR, so feel free to change the conditions so that

on 3.9 we use the builtin ast module
on 3.6 3.7 and 3.8 we use typed_ast

zsol · 2020-09-19T12:49:14Z

I'm not sure about primer. What kinds of changes are there? CI looks green

QuentinSoubeyran · 2020-09-19T13:06:09Z

I changed typed_ast as described. Curiously, this makes some unit test fail on python3.8, while everything works fine on python3.9. Perhaps typed_ast doesn't lke PEP570, PEP572 and stared returns (those are the unit test that fail due to SyntaxError).

Sorry I forgot the details about black-primer. I didn't look at all changes individually, but those I saw were arguments that weresplitted on multiline, or sometime just adding the trailing comma in already-splitted arguments.

QuentinSoubeyran · 2020-09-19T13:21:21Z

Just found out that typed_ast does not support python 3.8 syntax and won't support it. So I guess I should revert the above commit ? To parse python 3.8, we need builtin the ast, so we need python >=3.8.

I think --safe would not work for python 3.8 code formatted from python 3.7 due to that, so black run on python3.8 not formatting python 3.9 might be acceptable

This reverts commit 3073d07.

QuentinSoubeyran · 2020-09-19T13:57:17Z

I went through all changes reported by black-primer, those are all one of:

arguments split on multiline due to long line
arguments split on multiline due to magic trailing comma
a docstring reformat to remove spaces after """
in sqlalchemy, reformat due to the call chain style

All in all, every formatting I saw is due to a well-defined rule explained in the code style, so I think this PR is ready for review

JelleZijlstra

Looks good apart from one nit

JelleZijlstra · 2020-09-19T15:05:33Z

tests/data/decorators.py

+# Those before the 'output' comment are valid under the old syntax
+# Those after the 'ouput' comment require PEP614 relaxed syntax
+# Do not remove the double # separator before the first test case, it allows
+# to the comment before the test case


remove "to"?

I think I also forgot the verb. It should be fixed now. Thanks !

zsol

LGTM. Thanks for taking the time to work on this

QuentinSoubeyran · 2020-09-19T17:25:28Z

My pleasure :) Thanks to you two for your help. And thank everyone for Black, it is awesome !

https://pypi.org/project/black/ 21.5b2 was released on May 31, 2021. The current version 20.8 was released ~August 2020. `py39` was added in September 2020 with psf/black#1717.

QuentinSoubeyran added 2 commits September 18, 2020 18:10

started PEP 614 support

06f2bf6

removed spurious prints

4d55d6a

QuentinSoubeyran mentioned this pull request Sep 18, 2020

[python 3.9] black doesn't support PEP 614 #1711

Closed

zsol reviewed Sep 18, 2020

View reviewed changes

QuentinSoubeyran marked this pull request as draft September 18, 2020 20:56

QuentinSoubeyran added 3 commits September 18, 2020 23:12

fixed decorators having a space after @ symbol

61f517f

added check for previous fix

993903e

added test cases for RELAXED_DECORATORS feature

1237a69

added test cases

da558a9

updated parse_ast()

3073d07

Revert "updated parse_ast()"

5258a4e

This reverts commit 3073d07.

QuentinSoubeyran marked this pull request as ready for review September 19, 2020 14:01

QuentinSoubeyran changed the title ~~[WIP] PEP 614 support~~ PEP 614 support Sep 19, 2020

added entry in CHANGES.md added PEP 614 support

f38e4fc

JelleZijlstra approved these changes Sep 19, 2020

View reviewed changes

fixed typo

8db6d84

zsol approved these changes Sep 19, 2020

View reviewed changes

JelleZijlstra merged commit 6dddbd7 into psf:master Sep 19, 2020

CiderMan mentioned this pull request Jan 20, 2021

Inconsistent formatting of decorators #1927

Closed

sveitser mentioned this pull request Apr 26, 2021

Test failure with black version 21.04b rupert/pyls-black#36

Closed

carlosperate mentioned this pull request Apr 28, 2021

Black breaking changes in latest releases (CI will fail until fixed) mu-editor/mu#1518

Closed

noxan pushed a commit to noxan/black that referenced this pull request Jun 6, 2021

PEP 614 support (psf#1717)

b4c6274

This was referenced Jun 9, 2021

bump black to 21.5b2 gnawhleinad/mega-linter#1

Closed

bump black to 21.5b2 oxsecurity/megalinter#506

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PEP 614 support #1717

PEP 614 support #1717

QuentinSoubeyran commented Sep 18, 2020 •

edited

QuentinSoubeyran commented Sep 18, 2020

zsol commented Sep 18, 2020

zsol Sep 18, 2020

QuentinSoubeyran Sep 18, 2020

zsol commented Sep 18, 2020

QuentinSoubeyran commented Sep 18, 2020

QuentinSoubeyran commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020

zsol commented Sep 19, 2020

zsol commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020 •

edited

QuentinSoubeyran commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020

JelleZijlstra left a comment

JelleZijlstra Sep 19, 2020

QuentinSoubeyran Sep 19, 2020

zsol left a comment

QuentinSoubeyran commented Sep 19, 2020



		def is_simple_decorator_expression(node: LN) -> bool:
		"""Return True iff `node` is the 'dotted name' in the grammar @ dotted_name [arguments] NEWLINE"""

	"""Return True iff `node` is the 'dotted name' in the grammar @ dotted_name [arguments] NEWLINE"""
	"""Return True iff `node` is the 'namedexpr_test' in the grammar @ namedexpr_test [arguments] NEWLINE"""

PEP 614 support #1717

PEP 614 support #1717

Conversation

QuentinSoubeyran commented Sep 18, 2020 • edited

QuentinSoubeyran commented Sep 18, 2020

zsol commented Sep 18, 2020

zsol Sep 18, 2020

Choose a reason for hiding this comment

QuentinSoubeyran Sep 18, 2020

Choose a reason for hiding this comment

zsol commented Sep 18, 2020

QuentinSoubeyran commented Sep 18, 2020

QuentinSoubeyran commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020

zsol commented Sep 19, 2020

zsol commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020 • edited

QuentinSoubeyran commented Sep 19, 2020

QuentinSoubeyran commented Sep 19, 2020

JelleZijlstra left a comment

Choose a reason for hiding this comment

JelleZijlstra Sep 19, 2020

Choose a reason for hiding this comment

QuentinSoubeyran Sep 19, 2020

Choose a reason for hiding this comment

zsol left a comment

Choose a reason for hiding this comment

QuentinSoubeyran commented Sep 19, 2020

QuentinSoubeyran commented Sep 18, 2020 •

edited

QuentinSoubeyran commented Sep 19, 2020 •

edited