Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a more useful error when parsing fails during AST safety checks (#2218) #2218

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -5,6 +5,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

Expand Down
14 changes: 11 additions & 3 deletions src/black/parsing.py
Expand Up @@ -108,26 +108,34 @@ def lib2to3_unparse(node: Node) -> str:

def parse_ast(src: str) -> Union[ast.AST, ast3.AST, ast27.AST]:
filename = "<unknown>"
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(
"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]`."
)
return ast27.parse(src)
try:
return ast27.parse(src)
except SyntaxError as e:
raise SyntaxError(error or str(e))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, thanks for the PR!

If I'm not mistaken, this means that 2.7 errors are never passed onwards. To get to this try some other try above must fail, then error always has a value and the or short circuits. Indeed this only ever prints the error for Python 3.6, because it's the last to fail before 2.7. So this moves the problem from 2.7 to 3.6, but I'm pretty sure this is not quite the optimal solution.

The linked issue spoke of reparsing with the latest Python version available, and that idea had some agreement. That could be possible using sys.version_info and iffing the parser versions again.



def stringify_ast(
Expand Down
23 changes: 23 additions & 0 deletions tests/test_black.py
Expand Up @@ -496,6 +496,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 '}' (<unknown>, line 2)"
)
self.assertIn(msg, actual)

@pytest.mark.python2
@patch("black.dump_to_file", dump_to_stderr)
def test_python2_print_function(self) -> None:
Expand Down