INTERNAL ERROR: Black produced invalid code: invalid syntax (maye be caused by line continuation in an async with statement)

[olivierphi]

Describe the bug

Black fails on the following snippet (extracted from real code), with this message:
error: cannot format tests/app/test_black.py: INTERNAL ERROR: Black produced invalid code: invalid syntax (<unknown>, line 9)

import asyncio

import aiohttp
from _pytest.monkeypatch import MonkeyPatch

from app import test_utils


async def test_ping_1(
    aiohttp_client: aiohttp.test_utils.TestClient,
    loop: asyncio.AbstractEventLoop,
    monkeypatch: MonkeyPatch,
):
  test_cases = (
    # test_input, expected
    ({"http_method": "HEAD"}, {"status": 204}),
    ({"http_method": "GET"}, {"status": 204}),
    ({"http_method": "POST"}, {"status": 405}),
  )

  # fmt: off
  async with \
      test_utils.ctx_test_config(monkeypatch=monkeypatch), \
      test_utils.ctx_mqtt_broker(loop=loop, monkeypatch=monkeypatch), \
      test_utils.ctx_app(loop=loop) as app \
  :
    # fmt: on
    client = await aiohttp_client(app)

    for (test_input, expected) in test_cases:
      resp = await client.request(test_input["http_method"], "/ping")
      assert resp.status == expected["status"]


# N.B. exact same function than `test_ping_1`, just there to
# illustrate the fact that Black "thinks" that `test_ping_2` is inside `test_ping_1`
async def test_ping_2(
    aiohttp_client: aiohttp.test_utils.TestClient,
    loop: asyncio.AbstractEventLoop,
    monkeypatch: MonkeyPatch,
):
  test_cases = (
    # test_input, expected
    ({"http_method": "HEAD"}, {"status": 204}),
    ({"http_method": "GET"}, {"status": 204}),
    ({"http_method": "POST"}, {"status": 405}),
  )

  # fmt: off
  async with \
      test_utils.ctx_test_config(monkeypatch=monkeypatch), \
      test_utils.ctx_mqtt_broker(loop=loop, monkeypatch=monkeypatch), \
      test_utils.ctx_app(loop=loop) as app \
  :
    # fmt: on
    client = await aiohttp_client(app)

    for (test_input, expected) in test_cases:
      resp = await client.request(test_input["http_method"], "/ping")
      assert resp.status == expected["status"]

This code style with line continuations in async with statements and # fmt: off/# fmt: on annotations can be discussed of course, and people may find it is a terrible idea to do this; however it is valid Python code, and the tests run successfully :-)

It looks like Black "thinks" that test_ping_2 is inside test_ping_1, which may be the cause of the error.

To Reproduce Steps to reproduce the behavior:

1. Take this file https://gist.github.com/DrBenton/c75b4a24c66a3cf450f9aed70b93b507
2. Run Black on it with these arguments --target-version py37 test_black.py
3. See error

Complete gist:

mkdir black-bug
cd black-bug
python3.7 -m venv .venv
source .venv/bin/activate
pip install black==19.10b0
curl -o test_black.py \
  https://gist.githubusercontent.com/DrBenton/c75b4a24c66a3cf450f9aed70b93b507/raw/d5656082b5e41d5775d4d6cbc21dc0c128f518d0/test_black.py
python -m py_compile test_black.py # passes successfully
black --target-version py37 test_black.py # fails

Expected behavior

The file should be "Blackified" successfuly, as it is valid Python 3.7 code.

This code runs successfully when the dependencies are installed, and most importantly it does pass the test of python -m py_compile test_black.py. (I'm not a Python expert but it seems that this is a decent command to run to check that a Python file is valid from a syntax point of view?)

Environment (please complete the following information):

• Version: 19.10b0
• OS and Python version: Linux/Python 3.7.4

Does this bug also happen on master?

• It does not happen on master (using the online formatter) when the test file contains only 1 function (i.e. test_ping_2 is removed).
• It does happen however as soon as there is more than 1 of these functions, as the second one is embedded into the 1st one by Black.

See line 36/37 of this screenshot of the online formatter (@b7624c): the second function becomes embedded into the first one:

Additional context

Log file attached:

  File "/home/olivier/_WORK/tests/async-mqtt-python/.venv/lib/python3.7/site-packages/black.py", line 3762, in assert_equivalent
    dst_ast = parse_ast(dst)
  File "/home/olivier/_WORK/tests/async-mqtt-python/.venv/lib/python3.7/site-packages/black.py", line 3686, in parse_ast
    return ast27.parse(src)
  File "/home/olivier/_WORK/tests/async-mqtt-python/.venv/lib/python3.7/site-packages/typed_ast/ast27.py", line 50, in parse
    return _ast27.parse(source, filename, mode)
import asyncio

import aiohttp
from _pytest.monkeypatch import MonkeyPatch

from app import test_utils


async def test_ping_1(
    aiohttp_client: aiohttp.test_utils.TestClient,
    loop: asyncio.AbstractEventLoop,
    monkeypatch: MonkeyPatch,
):
    test_cases = (
        # test_input, expected
        ({"http_method": "HEAD"}, {"status": 204}),
        ({"http_method": "GET"}, {"status": 204}),
        ({"http_method": "POST"}, {"status": 405}),
    )

    # fmt: off
  async with \
      test_utils.ctx_test_config(monkeypatch=monkeypatch), \
      test_utils.ctx_mqtt_broker(loop=loop, monkeypatch=monkeypatch), \
      test_utils.ctx_app(loop=loop) as app \
  :
    # fmt: on
    client = await aiohttp_client(app)

    for (test_input, expected) in test_cases:
      resp = await client.request(test_input["http_method"], "/ping")
      assert resp.status == expected["status"]

    # N.B. exact same function than `test_ping_1`, just there to
    # illustrate the fact than Black "thinks" that `test_ping_2` is inside `test_ping_1`
    async def test_ping_2(
        aiohttp_client: aiohttp.test_utils.TestClient,
        loop: asyncio.AbstractEventLoop,
        monkeypatch: MonkeyPatch,
    ):
        test_cases = (
            # test_input, expected
            ({"http_method": "HEAD"}, {"status": 204}),
            ({"http_method": "GET"}, {"status": 204}),
            ({"http_method": "POST"}, {"status": 405}),
        )

        # fmt: off
  async with \
      test_utils.ctx_test_config(monkeypatch=monkeypatch), \
      test_utils.ctx_mqtt_broker(loop=loop, monkeypatch=monkeypatch), \
      test_utils.ctx_app(loop=loop) as app \
  :
    # fmt: on
    client = await aiohttp_client(app)

    for (test_input, expected) in test_cases:
      resp = await client.request(test_input["http_method"], "/ping")
      assert resp.status == expected["status"]

I don't know the internals of Black, so maybe seeing this ast27.py is normal, but as I understand it it may be a source of error, as I would have expected to see some Python 3 AST module rather than 2.7 ? (even though I pass the --target-version py37 option explicitly)

[ichard26]

This is definitely another case of GH-569. When a fmt: off falls off its indented scope undetermined, Black messes up the indentation somehow.

I don't know the internals of Black, so maybe seeing this ast27.py is normal, but as I understand it it may be a source of error, as I would have expected to see some Python 3 AST module rather than 2.7 ? (even though I pass the --target-version py37 option explicitly)

The problem with Black misidentifying which line has invalid syntax when doing parsing has already been reported as #1263 and #423. Black tries all Python versions / grammars when parsing the input. So when there is a parse error Black tries a lower version until Python 2.7. So when parsing with all versions fail, the parse error from Python 2.7 is the one shown to the user.

I'll close this in favour of #569, and thank you for the report!

[olivierphi]

I'll close this in favour of #569, and thank you for the report!

Thanks for your detailed explanations! 😊
(and sorry for the too verbose code in my bug report, I should have isolated the issue in just a few lines of code 😅 )