From 520317e2628dc38ef64d170238eb841f5cbd4510 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Mon, 10 May 2021 17:08:25 +0200 Subject: [PATCH 1/7] Provide a more useful error when parsing fails during AST safety checks (#2218) --- CHANGES.md | 1 + src/black/parsing.py | 14 +++++++++++--- tests/test_black.py | 23 +++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 16d9ebc3fe5..fd1a84ed5dc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -53,6 +53,7 @@ ### _Black_ - Refactor `src/black/__init__.py` into many files (#2206) +- Provide a more useful error when parsing fails during AST safety checks (#2218) ### Documentation diff --git a/src/black/parsing.py b/src/black/parsing.py index 8e9feea9120..587cd64b701 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -108,18 +108,23 @@ def lib2to3_unparse(node: Node) -> str: def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" + error = None if sys.version_info >= (3, 8): # TODO: support Python 4+ ;) for minor_version in range(sys.version_info[1], 4, -1): try: return ast.parse(src, filename, feature_version=(3, minor_version)) - except SyntaxError: + except SyntaxError as e: + if error is None: + error = str(e) continue else: for feature_version in (7, 6): try: return ast3.parse(src, filename, feature_version=feature_version) - except SyntaxError: + except SyntaxError as e: + if error is None: + error = str(e) continue if ast27.__name__ == "ast": raise SyntaxError( @@ -127,7 +132,10 @@ def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: "If you are trying to format Python 2 files please reinstall Black" " with the 'python2' extra: `python3 -m pip install black[python2]`." ) - return ast27.parse(src) + try: + return ast27.parse(src) + except SyntaxError as e: + raise SyntaxError(error or str(e)) def stringify_ast( diff --git a/tests/test_black.py b/tests/test_black.py index 455cb33e827..9b0b2762516 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -479,6 +479,29 @@ def test_python2_should_fail_without_optional_install(self) -> None: ) self.assertIn(msg, actual) + @pytest.mark.python2 + def test_safety_check_syntax_error(self) -> None: + source = "e = 'test'\nx = f'{'" + tmp_file = Path(black.dump_to_file(source)) + try: + runner = BlackRunner() + result = runner.invoke(black.main, [str(tmp_file)]) + self.assertEqual(result.exit_code, 123) + finally: + os.unlink(tmp_file) + actual = ( + runner.stderr_bytes.decode() + .replace("\n", "") + .replace("\\n", "") + .replace("\\r", "") + .replace("\r", "") + ) + msg = ( + "cannot use --safe with this file; failed to parse source file." + " AST error message: f-string: expecting '}' (, line 2)" + ) + self.assertIn(msg, actual) + @pytest.mark.python2 @patch("black.dump_to_file", dump_to_stderr) def test_python2_print_function(self) -> None: From f31375fc324ba9fc990d88f3061924163051c947 Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Thu, 3 Jun 2021 21:39:07 +0300 Subject: [PATCH 2/7] Only raise first exception and remove test case --- src/black/parsing.py | 31 +++++++++++-------------- tests/test_black.py | 54 -------------------------------------------- 2 files changed, 13 insertions(+), 72 deletions(-) diff --git a/src/black/parsing.py b/src/black/parsing.py index 587cd64b701..3dc8ccb5854 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -108,34 +108,29 @@ def lib2to3_unparse(node: Node) -> str: def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" - error = None + first_error = "" if sys.version_info >= (3, 8): # TODO: support Python 4+ ;) for minor_version in range(sys.version_info[1], 4, -1): try: return ast.parse(src, filename, feature_version=(3, minor_version)) except SyntaxError as e: - if error is None: - error = str(e) - continue + if not first_error: + first_error = str(e) else: for feature_version in (7, 6): try: return ast3.parse(src, filename, feature_version=feature_version) - except SyntaxError as e: - if error is None: - error = str(e) - continue - if ast27.__name__ == "ast": - raise SyntaxError( - "The requested source code has invalid Python 3 syntax.\n" - "If you are trying to format Python 2 files please reinstall Black" - " with the 'python2' extra: `python3 -m pip install black[python2]`." - ) - try: - return ast27.parse(src) - except SyntaxError as e: - raise SyntaxError(error or str(e)) + except SyntaxError: + pass + + if ast27.__name__ != "ast": + try: + return ast27.parse(src) + except SyntaxError: + pass + + raise SyntaxError(first_error) def stringify_ast( diff --git a/tests/test_black.py b/tests/test_black.py index 9b0b2762516..5ef9ee39ac5 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -448,60 +448,6 @@ def test_skip_magic_trailing_comma(self) -> None: ) self.assertEqual(expected, actual, msg) - @pytest.mark.no_python2 - def test_python2_should_fail_without_optional_install(self) -> None: - if sys.version_info < (3, 8): - self.skipTest( - "Python 3.6 and 3.7 will install typed-ast to work and as such will be" - " able to parse Python 2 syntax without explicitly specifying the" - " python2 extra" - ) - - source = "x = 1234l" - tmp_file = Path(black.dump_to_file(source)) - try: - runner = BlackRunner() - result = runner.invoke(black.main, [str(tmp_file)]) - self.assertEqual(result.exit_code, 123) - finally: - os.unlink(tmp_file) - actual = ( - result.stderr_bytes.decode() - .replace("\n", "") - .replace("\\n", "") - .replace("\\r", "") - .replace("\r", "") - ) - msg = ( - "The requested source code has invalid Python 3 syntax." - "If you are trying to format Python 2 files please reinstall Black" - " with the 'python2' extra: `python3 -m pip install black[python2]`." - ) - self.assertIn(msg, actual) - - @pytest.mark.python2 - def test_safety_check_syntax_error(self) -> None: - source = "e = 'test'\nx = f'{'" - tmp_file = Path(black.dump_to_file(source)) - try: - runner = BlackRunner() - result = runner.invoke(black.main, [str(tmp_file)]) - self.assertEqual(result.exit_code, 123) - finally: - os.unlink(tmp_file) - actual = ( - runner.stderr_bytes.decode() - .replace("\n", "") - .replace("\\n", "") - .replace("\\r", "") - .replace("\r", "") - ) - msg = ( - "cannot use --safe with this file; failed to parse source file." - " AST error message: f-string: expecting '}' (, line 2)" - ) - self.assertIn(msg, actual) - @pytest.mark.python2 @patch("black.dump_to_file", dump_to_stderr) def test_python2_print_function(self) -> None: From 2140054abad7c2718dffdd490bed952279f92259 Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Thu, 3 Jun 2021 22:05:39 +0300 Subject: [PATCH 3/7] Refactor AST parsing --- src/black/parsing.py | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/black/parsing.py b/src/black/parsing.py index 3dc8ccb5854..3401d6c6614 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -3,7 +3,7 @@ """ import ast import sys -from typing import Iterable, Iterator, List, Set, Union +from typing import Iterable, Iterator, List, Set, Union, Tuple # lib2to3 fork from blib2to3.pytree import Node, Leaf @@ -106,29 +106,35 @@ def lib2to3_unparse(node: Node) -> str: return code -def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: +def parse_single_version( + src: str, version: Tuple[int, int] +) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" - first_error = "" + if (3, 8) <= version < (4, 0): + return ast.parse(src, filename, feature_version=version) + elif version >= (3, 6): + return ast3.parse(src, filename, feature_version=version[1]) + elif version == (2, 7): + return ast27.parse(src) + raise AssertionError("INTERNAL ERROR: Tried parsing unsupported Python version!") + + +def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: + versions = [(3, 6), (3, 7)] if sys.version_info >= (3, 8): # TODO: support Python 4+ ;) - for minor_version in range(sys.version_info[1], 4, -1): - try: - return ast.parse(src, filename, feature_version=(3, minor_version)) - except SyntaxError as e: - if not first_error: - first_error = str(e) - else: - for feature_version in (7, 6): - try: - return ast3.parse(src, filename, feature_version=feature_version) - except SyntaxError: - pass + versions.extend([(3, minor) for minor in range(8, sys.version_info[1] + 1)]) if ast27.__name__ != "ast": + versions.append((2, 7)) + + first_error = "" + for version in sorted(versions, reverse=True): try: - return ast27.parse(src) - except SyntaxError: - pass + return parse_single_version(src, version) + except SyntaxError as e: + if not first_error: + first_error = str(e) raise SyntaxError(first_error) From f3c640a5e5d43254baa87dd7a7116c27a3ac6f37 Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Thu, 3 Jun 2021 23:00:52 +0300 Subject: [PATCH 4/7] Move and update changelog entry --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index fd1a84ed5dc..8c6ea92c963 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,7 @@ - Fixed option usage when using the `--code` flag (#2259) - Do not call `uvloop.install()` when _Black_ is used as a library (#2303) - Added `--required-version` option to require a specific version to be running (#2300) +- Provide a more useful error when parsing fails during AST safety checks (#2304) ## 21.5b2 @@ -53,7 +54,6 @@ ### _Black_ - Refactor `src/black/__init__.py` into many files (#2206) -- Provide a more useful error when parsing fails during AST safety checks (#2218) ### Documentation From 3131386b39182410af5b4d1bc56855c708687f72 Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Wed, 9 Jun 2021 22:41:00 +0300 Subject: [PATCH 5/7] Redundantly test for version_info to satisfy mypy --- src/black/parsing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/parsing.py b/src/black/parsing.py index 3401d6c6614..916de53a8b0 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -110,7 +110,7 @@ def parse_single_version( src: str, version: Tuple[int, int] ) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" - if (3, 8) <= version < (4, 0): + if sys.version_info >= (3, 8) and (3, 8) <= version < (4, 0): return ast.parse(src, filename, feature_version=version) elif version >= (3, 6): return ast3.parse(src, filename, feature_version=version[1]) From a9333634bd80b2ac78cc490d72cafa90c5c46aeb Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Sun, 13 Jun 2021 21:45:30 +0300 Subject: [PATCH 6/7] Correct logic for determining parser --- src/black/parsing.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/black/parsing.py b/src/black/parsing.py index 916de53a8b0..84a83a43c05 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -110,9 +110,10 @@ def parse_single_version( src: str, version: Tuple[int, int] ) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" - if sys.version_info >= (3, 8) and (3, 8) <= version < (4, 0): + # typed_ast is needed because of feature version limitations in the builtin ast + if version >= (3,) and sys.version_info >= (3, 8): return ast.parse(src, filename, feature_version=version) - elif version >= (3, 6): + elif version >= (3,): return ast3.parse(src, filename, feature_version=version[1]) elif version == (2, 7): return ast27.parse(src) @@ -120,10 +121,8 @@ def parse_single_version( def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]: - versions = [(3, 6), (3, 7)] - if sys.version_info >= (3, 8): - # TODO: support Python 4+ ;) - versions.extend([(3, minor) for minor in range(8, sys.version_info[1] + 1)]) + # TODO: support Python 4+ ;) + versions = [(3, minor) for minor in range(3, sys.version_info[1] + 1)] if ast27.__name__ != "ast": versions.append((2, 7)) From c008ac7b62cd7115bbb2892926c91005c9c8a2c8 Mon Sep 17 00:00:00 2001 From: felix-hilden Date: Sun, 13 Jun 2021 21:53:54 +0300 Subject: [PATCH 7/7] What is mypy doing... --- src/black/parsing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/parsing.py b/src/black/parsing.py index 84a83a43c05..0b8d984cedd 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -111,7 +111,7 @@ def parse_single_version( ) -> Union[ast.AST, ast3.AST, ast27.AST]: filename = "" # typed_ast is needed because of feature version limitations in the builtin ast - if version >= (3,) and sys.version_info >= (3, 8): + if sys.version_info >= (3, 8) and version >= (3,): return ast.parse(src, filename, feature_version=version) elif version >= (3,): return ast3.parse(src, filename, feature_version=version[1])