Parse error on type-hints

[pkcakeout]

Describe the bug

Black fails to parse a valid python3.6 file due to a type-hint comment. The produced error is:

(pp) paul@amd6300fx:/tmp/pp$ black --target-version py36 test.py 
error: cannot format test.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 8)
Oh no! 💥 💔 💥
1 file failed to reformat.

There seem to be actually two problems observable here.

1. The issue seems to be caused by line 15 # type: (Arg) -> None AND having empty lines at the beginning of the file. Letting the file start with class SomeClass: immediately works.

2. The error is reported at line 8. The line 8 vs 15 confusion can be solved by removing the python3.6-style type annotation from line 8. Then the error is correctly reported on line 15.

To Reproduce

Take this python file:

# File must start with empty lines or comments, 
# otherwise the error does not manifest!

from typing import Dict

class SomeClass:
    @property
    def distraction(self) -> Dict[str, "Something"]:
        return "distracting type annotation"

    def a_method(self):
        """
        Some comment required for problem to manifest
        """
        # type: (Arg) -> None
        pass

Run black on it: black --target-version py36 test.py

Expected behavior

No errors, maybe a reformat. But it should be working - the file can be parsed by python afterall.

Environment:

• Version: 19.10b0 and master (19.10b1.dev111+g9171535)
• OS and Python version:

Python 3.6.9 (default, Apr 18 2020, 01:56:04) 
[GCC 8.4.0] on linux

Does this bug also happen on master?

Yes, I just downloaded master to confirm. However the online formatter with the same file works. Maybe it does something to the initial lines, which this bug seems sensitive to?

Note that the test_suite did not run all the way through for me, not sure whether this is expected:

(pp) paul@amd6300fx:/tmp/pp/black$ python -m unittest
E....[2020-05-26 14:26:51,131] DEBUG: Using selector: EpollSelector (selector_events.py:54)
.[2020-05-26 14:26:51,132] DEBUG: Using selector: EpollSelector (selector_events.py:54)
.[2020-05-26 14:26:51,458] DEBUG: Using selector: EpollSelector (selector_events.py:54)
.[2020-05-26 14:26:51,459] DEBUG: Using selector: EpollSelector (selector_events.py:54)
[2020-05-26 14:26:51,460] INFO: 16 projects to run Black over (lib.py:311)
[2020-05-26 14:26:51,460] DEBUG: Using 2 parallel workers to run Black (lib.py:316)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on aioexabgp (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on attrs (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on bandersnatch (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on channels (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on django (lib.py:247)
[2020-05-26 14:26:51,460] INFO: Skipping django as it's disabled via config (lib.py:254)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on flake8-bugbear (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on hypothesis (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on pandas (lib.py:247)
[2020-05-26 14:26:51,460] INFO: Skipping pandas as it's disabled via config (lib.py:254)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on poetry (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on ptr (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on pyramid (lib.py:247)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on pytest (lib.py:247)
[2020-05-26 14:26:51,460] INFO: Skipping pytest as it's disabled via config (lib.py:254)
[2020-05-26 14:26:51,460] DEBUG: worker 1 working on sqlalchemy (lib.py:247)
[2020-05-26 14:26:51,461] DEBUG: worker 1 working on tox (lib.py:247)
[2020-05-26 14:26:51,461] DEBUG: worker 1 working on virtualenv (lib.py:247)
[2020-05-26 14:26:51,461] DEBUG: worker 1 working on warehouse (lib.py:247)
[2020-05-26 14:26:51,461] DEBUG: project_runner 1 exiting (lib.py:245)
[2020-05-26 14:26:51,461] DEBUG: project_runner 0 exiting (lib.py:245)
[2020-05-26 14:26:51,461] INFO: Analyzing results (lib.py:327)
.
======================================================================
ERROR: tests.test_black (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: tests.test_black
Traceback (most recent call last):
  File "/usr/lib/python3.6/unittest/loader.py", line 428, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.6/unittest/loader.py", line 369, in _get_module_from_name
    __import__(name)
  File "/tmp/pp/black/tests/test_black.py", line 1766, in <module>
    class BlackDTestCase(AioHTTPTestCase):
NameError: name 'AioHTTPTestCase' is not defined


----------------------------------------------------------------------
Ran 9 tests in 0.333s

FAILED (errors=1)
~~~~~~~

[ichard26]

In general, this problem isn't that Black can't format the file, it's that it can't do its sanity checks which verify if Black didn't make dangerous changes to the file. That's why the error message says cannot use --safe with this file. If you use --fast with Black it should run okay, but that isn't ideal since there is a chance that Black might make an unsafe changes because of bugs, like this one.

The problem in this case is that the third party AST3 parser from typed_ast doesn't like the misplaced type annotation:

(I did modify the Black source code to skip the build-in ast.parse parser and to print out all parsing attempts during sanity checks)

(black-rHKUX7ap) R:\Programming\black>black test.py
Python3.7: misplaced type annotation (<unknown>, line 15)
Python3.6: misplaced type annotation (<unknown>, line 15)
error: cannot format test.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 8)
Oh no! 💥 💔 💥
1 file failed to reformat

The second issue of Black erroring on the wrong line when trying to do its sanity checks is already tracked here: #1263. The problem is that when Black tries to parse the source code the Python3.7 grammar fails to parse with misplaced type annotation (<unknown>, line 15), so Black tries Python3.6 grammar which also fails with the same error. Finally, it tries Python2.7 grammar, which errors on the py36 style type annotation, and since that was the last tried parse attempt, that is the error message that is outputted to the user.

What's interesting about this is that it only fails when running Black on Python37 or Python36 because the built-in ast parser is only used when running on Python38. Which probably means that that online black.now.sh is running Python38 since it can format the code without error.

Environment:

• Python version: Python 3.8.1
• OS version: Windows 10 Home Edition Build 18363
• Black version: black 19.10b1.dev100+gb0f3798

Unrelated, but the unittests are failing because they need extra dependencies to test Black and blackd. You need to install with the blackd "extra" dependencies to run the test suite via something like pip install -e .[d]

[pkcakeout]

Hi ichard26. Thanks for having a look into this!

The --safe vs --fast solution was clear to me, but I did not feel comfortable reformatting the way bigger file that I compiled the overview from without safety net. Since we anyway are on python3.6 already, I was able to replace the offending with python3.6 annotations and move on that way.

Good to have the wrong-line issue a confirmed as a duplicate of #1263. It was very much unrelated then to the original problem.

For the type-hint parser issue: I guess this still remains an "issue" with black's syntax-tree checker then, and it should try to parse the file without type-hints a second time, where applicable, maybe?

[ichard26]

Regarding the source code parser issue, since the built-in parser provided by the standard library can parse your bug-causing code without error, it might be a bug with the AST3 parser. That parser is from a third party and is developed under the typed_ast project. I say it might be a bug since I have no knowledge of how type comments work, so maybe the error was right.

The only problem with Black trying to parse the file without type-hints is that we give special treatment to them because they shouldn't be moved or else Black will annoy mypy or other static type checkers. Pulled from this #379 (comment)

Note to self: type comments have special handling now to make them more sticky. graingert suggests we use this logic for all comments.

We really don't want Black to suddenly start formatting files that fail pre-commit hooks because Black moved a few type-comments to the wrong place for example. And while the formatting part of Black uses a completely different parser from what I know, the parser used for sanity checks needs to have type-hints so Black can check if it wrongly moved or deleted type-hints.

Although, I don't know why we only need the third-party parser when running on Python36 and Python37. 🤷

[ichard26]

The typed_ast parser enforces that type comments are located where they are expected. For functions annotations in your specific case, the type comment MUST be placed immediately after the function signature. This is spelled out in PEP 484 "Type Hints":

[...] where function annotations are placed in a # type: comment. Such a comment must be placed immediately following the function header (before the docstring).

Black uses typed_ast when running on CPython 3.6 and CPython 3.7 because the builtin ast module doesn't support parsing Python 2.7 code until CPython 3.8 and higher.

(yeah I know I took forever to respond with the right answer, but that's because I was more naive and dumber when I first replied so had no idea what was the actual issue. now I'm more experienced around the Black project :D )

[pkcakeout]

Hey there! Thanks for revisiting this bug again.

IMO this might just be a case of a wont-fix even, maybe. At this point we left behind python2.7 for good (for the most part, I guess) and interoperability between different python versions is not so important anymore. That then only leaves the question if this is relevant for projects before python3.5, however, for those projects I would not see why they cannot upgrade to a newer python version.

For me/us it is certainly not important anymore. I will leave it to the black-admins to determine whether this is worth fixing. Personally, I would expect this problem to just not be relevant anymore.

[ichard26]

Yeah I agree and even if it was decided this should be fixed, it would be a task for the typed_ast people to deal with anyway as Black just delegates code-to-AST parsing to it (CPython <= 3.7).

[jlisee]

I think issue lies with the underlying typed_ast library and how it supports type hints in comments. I hit this failure because of python/typed_ast#62, any stray type: foo comment will cause a failure. In this case it was an inline YAML schema as you can see in this minimal example:

# Example YAML schema:
# SomeObject:
#   type: "something"

class SomeClass:
    pass

I was able to make the error go away by double commenting the line so it become ## type: "something".

For completeness The error (Black version 21.5b1, Python version 3.6)

error: cannot format -: cannot use --safe with this file; failed to parse source file.  AST error message: misplaced type annotation (<unknown>, line 3)

All the more reason to update to CPython > 3.7.

[ichard26]

Closing since there's nothing we can reasonably do due to this being an issue with the typed_ast library. A workaround is running Black on Python 3.8+

Thanks for the bug report tho, a bit unfortunate we can't do anything about :(