[python 3.9] black doesn't support PEP 614

[QuentinSoubeyran]

Describe the bug
PEP 614 relaxed the syntax for decorators in python 3.9. However, black doesn't (yet ?) have a py39 target, and doesn't support this syntax.

To Reproduce

1. Take this very simple file test.py

from dataclasses import dataclass
d = [dataclass]

@d[0]
class A:
    attr: int

print(A)

2. Run Black on it : black test.py
3. See error error: cannot format test.py: Cannot parse: 5:2: @d[0]

Expected behavior Black should accept and format code that is valid for python 3.9 (python test.py prints <class '__main__.A'> as expected).

Environment (please complete the following information):

• Version: master (commit 811decd)
• OS and Python version: Linux, Python 3.9.0rc1

Does this bug also happen on master?

1. Use the online formatter at https://black.now.sh/?version=master
   This produce the above error
2. Or run Black on your machine:
   • create a new virtualenv (make sure it's the same Python version);
   • clone this repository;
   • run pip install -e .;
   • make sure it's sane by running python -m unittest (this fails, see Test failures with Python 3.9.0b1 #1441 )
   • run black like you did last time.

This also produce the above error

Additional context Python 3.9 is in pre-release stage, so I'm not sure if this is a bug report or a feature request - I can change to the latter. PEP 614 highlights the use-case of the syntax (with examples from PyQt5), the above file is a minimalist file to illustrate the issue

[QuentinSoubeyran]

So I'm considering working on a PR for that if I can manage. Do I need to update blib2to3 to a newer version of lib2to3 (python original), or should I create a new grammar file to load in blid2to3.pygram.initialize() where decorator is changed from '@' dotted_name [ '(' [arglist] ')' ] NEWLINE to '@' namedexpr_test NEWLINE as the PEP explains ? (And perhaps replicate all changes to the grammer that leads to the python3.6+ grammar).

It seems to me I need to do the second but I'm new to Black's codebase

[JelleZijlstra]

If lib2to3 has been updated, you should feel free to reuse aspects of the update, but I'm not sure CPython is still updating it.

I don't know blib2to3 too well but it sounds like we should just add to the grammar, similar to what we've done for other new grammar features like the walrus operator in the past. We'll also likely need to add to the Feature enum in https://github.com/psf/black/blob/master/src/black/__init__.py#L189. That can be used so that Black can auto-detect Python 3.9-only code.

Thanks for working on this!

[QuentinSoubeyran]

That's right, I can draw inspiration from the walrus operator support. I think I should also add a detection of relaxed decorators in get_features_used ?

[QuentinSoubeyran]

Quick questions:

• is RELAXED_DECORATORS a suitable feature name ?
• should I add the new PY39 target to the PY36_VERSIONS constant ?

[JelleZijlstra]

Yes to all your questions.

[hugovk]

Perhaps also rename PY36_VERSIONS, whilst Black is still in pre-stable? The name is probably a remnant of the replaced and removed --py36 option.

In fact, it's only used by tests:

$ git grep PY36_VERSIONS
src/black/__init__.py:PY36_VERSIONS = {TargetVersion.PY36, TargetVersion.PY37, TargetVersion.PY38}
tests/test_black.py:    f"--target-version={version.name.lower()}" for version in black.PY36_VERSIONS
tests/test_black.py:        mode = replace(DEFAULT_MODE, target_versions=black.PY36_VERSIONS)
tests/test_black.py:        mode = replace(DEFAULT_MODE, target_versions=black.PY36_VERSIONS)
tests/test_black.py:        py36_mode = replace(DEFAULT_MODE, target_versions=black.PY36_VERSIONS)
tests/test_black.py:        py36_mode = replace(DEFAULT_MODE, target_versions=black.PY36_VERSIONS)

Does it need to be part of the public API, or be underscore prefixed?

[JelleZijlstra]

Good catch, we should just remove it from the code, and define it in test_black.py if we still need it there.

[QuentinSoubeyran]

I moved PY36_VERSIONS to tests/test_black.py and started support for PEP 614. Turns out detecting old decorator syntax while the grammar accepts the new syntax isn't that easy. I'd like to have some input from people more used to that grammar, I have surely forgotten some use-case.

Would pushing to my fork multiple commits (and perhaps rebasing at the end so that there is only a single pretty commit to PR) be appropriate ? I see that there are quite some pre-commit hooks, and since I'm not used to that I don't want to mess up the project history

[JelleZijlstra]

It's fine to prepare your PR in whatever way you want. The precommit hook are just there as a way to help people writing code, and you're free to not use them if you prefer (I personally like to commit early and fast, so I don't like the friction pre-commit hooks add). As long as CI is happy when you put up your PR, we're happy.

Are you referring to how get_features_used() should work? I haven't thought too much about it but I think you can just detect whether there's a node that contains @ followed by something previously illegal, like an index expression. It's also not as important to get that function right as it is to make sure Black can format files that use the new syntax, so we can also just punt get_features_used() to later.

[QuentinSoubeyran]

Created the above PR (#1717) to keep working on that. There are still bugs (space after the @) but the grammar has been modified.

For get_features_used() I did the reverse, since changing dotted_name to namedexpr_test changed the node type (even for the old syntax) to something that was not allowed before: I default to new-style, and if the decorator passes a check, it is considered old-style.