Repository: psf/black
Pull request: Support files with type comment syntax errors
### Description

Resolves #3593 

### Checklist - did you ...

- [x] Add an entry in `CHANGES.md` if necessary?
- [x] Add / update tests if necessary?
- [x] Add new / update outdated documentation?


Problem statement:
Let black run on files even if `ast.parse` with `type_comments=True` fails
**Is your feature request related to a problem? Please describe.**

Right now, if a file parsing because of invalid type comments (even if the code runs just fine), black won't format the file.

For example:

```python
import typing

def foo(
    # type: typing.List
    bar,
):
    print(bar)

foo(10)
```

While the code still runs:

```console
$ python3 asd.py            
10
```

Black no longer formats it:

```console
$ black asd.py        
error: cannot format asd.py: cannot use --safe with this file; failed to parse source file AST: invalid syntax (<unknown>, line 4)
This could be caused by running Black with an older Python version that does not support new syntax used in your source file.

Oh no! 💥 💔 💥
1 file failed to reformat.
```

This is because `ast.parse` on that code fails if you pass `type_comments=True`:

```pycon
>>> import ast
>>> ast.parse(open('asd.py').read(), type_comments=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/homebrew/Cellar/python@3.10/3.10.9/Frameworks/Python.framework/Versions/3.10/lib/python3.10/ast.py", line 50, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 4
    # type: typing.List
            ^^^^^^^^^^^
SyntaxError: invalid syntax
```

(Black does work if you do `black asd.py --fast`, as it no longer does the AST based check.)

**Describe the solution you'd like**

If `ast.parse(..., type_comments=True)` fails for the before/after check, maybe doing it with `type_comments=False` is fine, just for that file?

**Describe alternatives you've considered**

None so far.

**Additional context**

I don't think this can be considered an upstream bug, as this is indeed invalid syntax for a type checker's perspective. But black shouldn't care if the types are correct, in any way shape or form.

Hints from issue discussion:
If the change is approved, I can work on the PR as well.
Makes sense to me. We should try `ast.parse(type_comments=False)` only as a fallback, so performance won't be impacted for the case where `type_comments=True` parses correctly.

Commit message: Fix typed_ast usage in <=3.7

Patch to review:
diff --git a/CHANGES.md b/CHANGES.md
index a8b556cb7e8..bdc1459d6e2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -29,6 +29,8 @@
 
 <!-- Changes to the parser or to version autodetection -->
 
+- Added support for formatting files with invalid type comments (#3594)
+
 ### Performance
 
 <!-- Changes that improve Black's performance. -->
diff --git a/src/black/parsing.py b/src/black/parsing.py
index ba474c5e047..07c7a4f8148 100644
--- a/src/black/parsing.py
+++ b/src/black/parsing.py
@@ -148,24 +148,29 @@ def lib2to3_unparse(node: Node) -> str:
 
 
 def parse_single_version(
-    src: str, version: Tuple[int, int]
+    src: str, version: Tuple[int, int], type_comments: bool
 ) -> Union[ast.AST, ast3.AST]:
     filename = "<unknown>"
     # typed-ast is needed because of feature version limitations in the builtin ast 3.8>
     if sys.version_info >= (3, 8) and version >= (3,):
-        return ast.parse(src, filename, feature_version=version, type_comments=True)
+        return ast.parse(
+            src, filename, feature_version=version, type_comments=type_comments
+        )
 
     if _IS_PYPY:
         # PyPy 3.7 doesn't support type comment tracking which is not ideal, but there's
         # not much we can do as typed-ast won't work either.
         if sys.version_info >= (3, 8):
-            return ast3.parse(src, filename, type_comments=True)
+            return ast3.parse(src, filename, type_comments=type_comments)
         else:
             return ast3.parse(src, filename)
     else:
-        # Typed-ast is guaranteed to be used here and automatically tracks type
-        # comments separately.
-        return ast3.parse(src, filename, feature_version=version[1])
+        if type_comments:
+            # Typed-ast is guaranteed to be used here and automatically tracks type
+            # comments separately.
+            return ast3.parse(src, filename, feature_version=version[1])
+        else:
+            return ast.parse(src, filename)
 
 
 def parse_ast(src: str) -> Union[ast.AST, ast3.AST]:
@@ -175,11 +180,17 @@ def parse_ast(src: str) -> Union[ast.AST, ast3.AST]:
     first_error = ""
     for version in sorted(versions, reverse=True):
         try:
-            return parse_single_version(src, version)
+            return parse_single_version(src, version, type_comments=True)
         except SyntaxError as e:
             if not first_error:
                 first_error = str(e)
 
+            # Try to parse without type comments
+            try:
+                return parse_single_version(src, version, type_comments=False)
+            except SyntaxError:
+                pass
+
     raise SyntaxError(first_error)
 
 
diff --git a/tests/data/type_comments/type_comment_syntax_error.py b/tests/data/type_comments/type_comment_syntax_error.py
new file mode 100644
index 00000000000..2e5ca2ede8c
--- /dev/null
+++ b/tests/data/type_comments/type_comment_syntax_error.py
@@ -0,0 +1,11 @@
+def foo(
+    # type: Foo
+    x): pass
+
+# output
+
+def foo(
+    # type: Foo
+    x,
+):
+    pass
diff --git a/tests/test_black.py b/tests/test_black.py
index e5e17777715..d07a84d477c 100644
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -1938,6 +1938,14 @@ def test_equivalency_ast_parse_failure_includes_error(self) -> None:
         err.match("invalid character")
         err.match(r"\(<unknown>, line 1\)")
 
+    def test_type_comment_syntax_error(self) -> None:
+        """
+        Test that black is able to format python code with type comment syntax errors.
+        """
+        source, expected = read_data("type_comments", "type_comment_syntax_error")
+        assert_format(source, expected)
+        black.assert_equivalent(source, expected)
+
 
 class TestCaching:
     def test_get_cache_dir(


Code review comments:
diff_hunk: @@ -175,11 +180,17 @@ def parse_ast(src: str) -> Union[ast.AST, ast3.AST]:
     first_error = ""
     for version in sorted(versions, reverse=True):
         try:
-            return parse_single_version(src, version)
+            return parse_single_version(src, version, type_comments=True)
         except SyntaxError as e:
             if not first_error:
                 first_error = str(e)
 
+            # Try to parse without type comments
path: src/black/parsing.py
text: @user1:
Need to think more about whether we should have a separate loop instead. I *think* that should never affect behavior, just performance.

@author:
I'll move it to an outer for loop anyway. Just for guaranteed backwards compatibility.

diff_hunk: @@ -148,24 +148,29 @@ def lib2to3_unparse(node: Node) -> str:
 
 
 def parse_single_version(
-    src: str, version: Tuple[int, int]
+    src: str, version: Tuple[int, int], type_comments: bool
path: src/black/parsing.py
text: @user1:
```suggestion
    src: str, version: Tuple[int, int], *, type_comments: bool
```
Let's make it keyword-only