ronantakizawa/codereview-bench huggingface dataset:
Repository: psf/black
File: tests/test_blackd.py (Python)

Code before:
import re
from unittest.mock import patch

from click.testing import CliRunner
import pytest

from tests.util import read_data, DETERMINISTIC_HEADER

try:
    import blackd
    from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop
    from aiohttp import web
except ImportError:
    has_blackd_deps = False
else:
    has_blackd_deps = True


@pytest.mark.blackd
class BlackDTestCase(AioHTTPTestCase):
    def test_blackd_main(self) -> None:
        with patch("blackd.web.run_app"):
            result = CliRunner().invoke(blackd.main, [])
            if result.exception is not None:
                raise result.exception
            self.assertEqual(result.exit_code, 0)

    async def get_application(self) -> web.Application:
        return blackd.make_app()

    @unittest_run_loop
    async def test_blackd_request_needs_formatting(self) -> None:
        response = await self.client.post("/", data=b"print('hello world')")
        self.assertEqual(response.status, 200)
        self.assertEqual(response.charset, "utf8")
        self.assertEqual(await response.read(), b'print("hello world")\n')

    @unittest_run_loop
    async def test_blackd_request_no_change(self) -> None:
        response = await self.client.post("/", data=b'print("hello world")\n')

Review comment (suggestion):
The old `has_blackd_deps` logic was doing absolutely nothing useful so instead let's error out with an informative exception.

Code after:
import re
from typing import Any
from unittest.mock import patch

from click.testing import CliRunner
import pytest

from tests.util import read_data, DETERMINISTIC_HEADER

try:
    import blackd
    from aiohttp.test_utils import AioHTTPTestCase
    from aiohttp import web
except ImportError as e:
    raise RuntimeError("Please install Black with the 'd' extra") from e

try:
    from aiohttp.test_utils import unittest_run_loop
except ImportError:
    # unittest_run_loop is unnecessary and a no-op since aiohttp 3.8, and aiohttp 4
    # removed it. To maintain compatibility we can make our own no-op decorator.
    def unittest_run_loop(func: Any, *args: Any, **kwargs: Any) -> Any:
        return func


@pytest.mark.blackd
class BlackDTestCase(AioHTTPTestCase):
    def test_blackd_main(self) -> None:
        with patch("blackd.web.run_app"):
            result = CliRunner().invoke(blackd.main, [])
            if result.exception is not None:
                raise result.exception
            self.assertEqual(result.exit_code, 0)

    async def get_application(self) -> web.Application:
        return blackd.make_app()

    @unittest_run_loop
    async def test_blackd_request_needs_formatting(self) -> None:
        response = await self.client.post("/", data=b"print('hello world')")
Repository: psf/black
File: tests/data/remove_await_parens.py (Python)

Code before:
        )))        )))
        )))         )))
        )))         )))
        )))        )))
        )))))))))))))
    )

# Keep brackets around non power operations and nested awaits
async def main():
    await (set_of_tasks | other_set)

async def main():
    await (await asyncio.sleep(1))

# It's brackets all the way down...
async def main():
    await (await (asyncio.sleep(1)))

async def main():
    await (await (await (await (await (asyncio.sleep(1))))))

# output
import asyncio

# Control example
async def main():
    await asyncio.sleep(1)


# Remove brackets for short coroutine/task
async def main():
    await asyncio.sleep(1)


async def main():
    await asyncio.sleep(1)


async def main():
    await asyncio.sleep(1)


# Check comments
async def main():
    await asyncio.sleep(1)  # Hello


async def main():
    await asyncio.sleep(1)  # Hello



Review comment (suggestion):
Can you try things like `await (yield x)` or `yield (await x)`, just in case?

(`await (yield from x)` is not allowed I believe)

Code after:
        )))        )))
        )))         )))
        )))         )))
        )))        )))
        )))))))))))))
    )

# Keep brackets around non power operations and nested awaits
async def main():
    await (set_of_tasks | other_set)

async def main():
    await (await asyncio.sleep(1))

# It's awaits all the way down...
async def main():
    await (await x)

async def main():
    await (yield x)

async def main():
    await (await (asyncio.sleep(1)))

async def main():
    await (await (await (await (await (asyncio.sleep(1))))))

# output
import asyncio

# Control example
async def main():
    await asyncio.sleep(1)


# Remove brackets for short coroutine/task
async def main():
    await asyncio.sleep(1)


async def main():
    await asyncio.sleep(1)


async def main():
    await asyncio.sleep(1)


# Check comments
async def main():
    await asyncio.sleep(1)  # Hello
Repository: psf/black
File: scripts/migrate-black.py (Python)

Code before:
#!/usr/bin/env python3
# check out every commit added by the current branch, blackify them,
# and generate diffs to reconstruct the original commits, but then
# blackified
import logging
import os
import sys
from collections import abc
from subprocess import check_output, run, Popen, PIPE


def git(*args: abc.Iterable[str]) -> str:
    return check_output(["git"] + list(args)).decode("utf8").strip()


def blackify(base_branch: str, black_command: str, logger: logging.Logger) -> int:
    current_branch = git("branch", "--show-current")

    if not current_branch or base_branch == current_branch:
        logger.error("You need to check out a feature brach to work on")
        return 1

    if not os.path.exists(".git"):
        logger.error("Run me in the root of your repo")
        return 1

    merge_base = git("merge-base", "HEAD", base_branch)
    if not merge_base:
        logger.error(
            "Could not find a common commit for current head and %s" % base_branch
        )
        return 1

    commits = git(
        "log", "--reverse", "--pretty=format:%H", "%s~1..HEAD" % merge_base
    ).split()
    for commit in commits:

Review comment (suggestion):
```suggestion
def git(*args: Sequence[str]) -> str:
```

Code after:
#!/usr/bin/env python3
# check out every commit added by the current branch, blackify them,
# and generate diffs to reconstruct the original commits, but then
# blackified
import logging
import os
import sys
from subprocess import check_output, run, Popen, PIPE


def git(*args: str) -> str:
    return check_output(["git"] + list(args)).decode("utf8").strip()


def blackify(base_branch: str, black_command: str, logger: logging.Logger) -> int:
    current_branch = git("branch", "--show-current")

    if not current_branch or base_branch == current_branch:
        logger.error("You need to check out a feature brach to work on")
        return 1

    if not os.path.exists(".git"):
        logger.error("Run me in the root of your repo")
        return 1

    merge_base = git("merge-base", "HEAD", base_branch)
    if not merge_base:
        logger.error(
            "Could not find a common commit for current head and %s" % base_branch
        )
        return 1

    commits = git(
        "log", "--reverse", "--pretty=format:%H", "%s~1..HEAD" % merge_base
    ).split()
    for commit in commits:
        git("checkout", commit, "-b%s-black" % commit)
Repository: psf/black
File: scripts/migrate-black.py (Python)

Code before:
#!/usr/bin/env python3
# check out every commit added by the current branch, blackify them,
# and generate diffs to reconstruct the original commits, but then
# blackified
import logging
import os
import sys
from collections import abc
from subprocess import check_output, run, Popen, PIPE


def git(*args: abc.Iterable[str]) -> str:
    return check_output(["git"] + list(args)).decode("utf8").strip()


def blackify(base_branch: str, black_command: str, logger: logging.Logger) -> int:
    current_branch = git("branch", "--show-current")

    if not current_branch or base_branch == current_branch:
        logger.error("You need to check out a feature brach to work on")
        return 1

    if not os.path.exists(".git"):
        logger.error("Run me in the root of your repo")
        return 1

    merge_base = git("merge-base", "HEAD", base_branch)
    if not merge_base:
        logger.error(
            "Could not find a common commit for current head and %s" % base_branch
        )
        return 1


Review comment (suggestion):
```suggestion
from subprocess import check_output, run, Popen, PIPE
from typing import Sequence
```

Code after:
#!/usr/bin/env python3
# check out every commit added by the current branch, blackify them,
# and generate diffs to reconstruct the original commits, but then
# blackified
import logging
import os
import sys
from subprocess import check_output, run, Popen, PIPE


def git(*args: str) -> str:
    return check_output(["git"] + list(args)).decode("utf8").strip()


def blackify(base_branch: str, black_command: str, logger: logging.Logger) -> int:
    current_branch = git("branch", "--show-current")

    if not current_branch or base_branch == current_branch:
        logger.error("You need to check out a feature brach to work on")
        return 1

    if not os.path.exists(".git"):
        logger.error("Run me in the root of your repo")
        return 1

    merge_base = git("merge-base", "HEAD", base_branch)
    if not merge_base:
        logger.error(
            "Could not find a common commit for current head and %s" % base_branch
        )
        return 1

    commits = git(
Repository: psf/black
File: tests/test_format.py (Python)

Code before:
    "pattern_matching_extras",
    "pattern_matching_style",
    "pattern_matching_generic",
    "parenthesized_context_managers",
]

PY311_CASES: List[str] = [
    "pep_654",
    "pep_654_style",
]

PREVIEW_CASES: List[str] = [
    # string processing
    "cantfit",
    "comments7",
    "comments8",
    "long_strings",
    "long_strings__edge_case",
    "long_strings__regression",
    "percent_precedence",
    "remove_except_parens",
    "remove_for_brackets",
    "one_element_subscript",
    "remove_await_parens",
    "return_annotation_brackets",
    "docstring2",
]

SOURCES: List[str] = [
    "src/black/__init__.py",
    "src/black/__main__.py",
    "src/black/brackets.py",
    "src/black/cache.py",
    "src/black/comments.py",
    "src/black/concurrency.py",
    "src/black/const.py",
    "src/black/debug.py",
    "src/black/files.py",
    "src/black/linegen.py",
    "src/black/lines.py",
    "src/black/mode.py",
    "src/black/nodes.py",
    "src/black/numerics.py",
    "src/black/output.py",
    "src/black/parsing.py",
    "src/black/report.py",
    "src/black/rusty.py",
    "src/black/strings.py",
    "src/black/trans.py",
    "src/blackd/__init__.py",
    "src/blib2to3/pygram.py",

Review comment (suggestion):
```suggestion
    "docstring_preview",
```
or `docstring_quotes` or similar if we want to be more precise. Would make it more explicit why the files were split.

Code after:
    "pattern_matching_extras",
    "pattern_matching_style",
    "pattern_matching_generic",
    "parenthesized_context_managers",
]

PY311_CASES: List[str] = [
    "pep_654",
    "pep_654_style",
]

PREVIEW_CASES: List[str] = [
    # string processing
    "cantfit",
    "comments7",
    "comments8",
    "long_strings",
    "long_strings__edge_case",
    "long_strings__regression",
    "percent_precedence",
    "remove_except_parens",
    "remove_for_brackets",
    "one_element_subscript",
    "remove_await_parens",
    "return_annotation_brackets",
    "docstring_preview",
]

SOURCES: List[str] = [
    "src/black/__init__.py",
    "src/black/__main__.py",
    "src/black/brackets.py",
    "src/black/cache.py",
    "src/black/comments.py",
    "src/black/concurrency.py",
    "src/black/const.py",
    "src/black/debug.py",
    "src/black/files.py",
    "src/black/linegen.py",
    "src/black/lines.py",
    "src/black/mode.py",
    "src/black/nodes.py",
    "src/black/numerics.py",
    "src/black/output.py",
    "src/black/parsing.py",
    "src/black/report.py",
    "src/black/rusty.py",
    "src/black/strings.py",
    "src/black/trans.py",
    "src/blackd/__init__.py",
    "src/blib2to3/pygram.py",
Repository: psf/black
File: tests/data/simple_cases/fmtonoff5.py (Python)

Code before:
# Regression test for https://github.com/psf/black/issues/3129.
setup(
    entry_points={
        # fmt: off
        "console_scripts": [
            "foo-bar"
            "=foo.bar.:main",
        # fmt: on
        ]
    },
)


# Regression test for https://github.com/psf/black/issues/3129.
run(
    # fmt: off
    [
        "ls",
        "-la",
    ]
    # fmt: on
    + path
    ,
    check=True,
)


# Regression test for https://github.com/psf/black/issues/3026.
def test_func():
    # yapf: disable
    if a:
        return True
    # yapf: enable
    elif b:
        return True

    return False


Review comment (suggestion):
Hmmm, this is concerning. I looked into it and the problem is that the `+` and `path` leaves are being ignored too. Fixing that with a hacky "if `child` is a Leaf (so doesn't contain children) and contains a `# fmt: on`, then return" works, but it breaks this example:
```python
setup(
    entry_points={
        # fmt: off
        "console_scripts": [
            "foo-bar"
            "=foo.bar.:main",
        # fmt: on
        ]
    },
)
```
... since `]` should be ignored too. Unfortunately looking at the CST structure, I can't see an easy way of differentiating these two cases :/  

I'd prefer to just reject this (i.e. error out / ignore the formatting directive) as an invalid use of `# fmt: off/on` but then the problem is how do you detect invalid uses of `# fmt: off/on` as it seems non-trivial :(

Code after:
# Regression test for https://github.com/psf/black/issues/3129.
setup(
    entry_points={
        # fmt: off
        "console_scripts": [
            "foo-bar"
            "=foo.bar.:main",
        # fmt: on
            ]  # Includes an formatted indentation.
    },
)


# Regression test for https://github.com/psf/black/issues/2015.
run(
    # fmt: off
    [
        "ls",
        "-la",
    ]
    # fmt: on
    + path,
    check=True,
)


# Regression test for https://github.com/psf/black/issues/3026.
def test_func():
    # yapf: disable
    if  unformatted(  args  ):
        return True
    # yapf: enable
    elif b:
        return True

    return False
Repository: psf/black
File: src/black/nodes.py (Python)

Code before:
"""
blib2to3 Node/Leaf transformation-related utility functions.
"""

import sys
from typing import Generic, Iterator, List, Optional, Set, Tuple, TypeVar, Union

if sys.version_info >= (3, 8):
    from typing import Final
else:
    from typing_extensions import Final
if sys.version_info >= (3, 10):
    from typing import TypeGuard
else:
    from typing_extensions import TypeGuard

from mypy_extensions import mypyc_attr

from black.cache import CACHE_DIR
from black.strings import has_triple_quotes
from blib2to3 import pygram
from blib2to3.pgen2 import token

# lib2to3 fork
from blib2to3.pytree import NL, Leaf, Node, type_repr

pygram.initialize(CACHE_DIR)
syms: Final = pygram.python_symbols


# types
T = TypeVar("T")
LN = Union[Leaf, Node]
LeafID = int
NodeType = int


WHITESPACE: Final = {token.DEDENT, token.INDENT, token.NEWLINE}
STATEMENT: Final = {
    syms.if_stmt,
    syms.while_stmt,
    syms.for_stmt,
    syms.try_stmt,
    syms.except_clause,
    syms.with_stmt,
    syms.funcdef,
    syms.classdef,
    syms.match_stmt,
    syms.case_block,

Review comment (suggestion):
Did this comment cause the next line to be in it's own section? It seems like it would make more sense above all blib2to3 imports.

Note there are a couple of other occurrences of this comment in different files that may need looking at. They could possibly all just be removed though tbh.

Code after:
"""
blib2to3 Node/Leaf transformation-related utility functions.
"""

import sys
from typing import Generic, Iterator, List, Optional, Set, Tuple, TypeVar, Union

if sys.version_info >= (3, 8):
    from typing import Final
else:
    from typing_extensions import Final
if sys.version_info >= (3, 10):
    from typing import TypeGuard
else:
    from typing_extensions import TypeGuard

from mypy_extensions import mypyc_attr

from black.cache import CACHE_DIR
from black.strings import has_triple_quotes
from blib2to3 import pygram
from blib2to3.pgen2 import token
from blib2to3.pytree import NL, Leaf, Node, type_repr

pygram.initialize(CACHE_DIR)
syms: Final = pygram.python_symbols


# types
T = TypeVar("T")
LN = Union[Leaf, Node]
LeafID = int
NodeType = int


WHITESPACE: Final = {token.DEDENT, token.INDENT, token.NEWLINE}
STATEMENT: Final = {
    syms.if_stmt,
    syms.while_stmt,
    syms.for_stmt,
    syms.try_stmt,
    syms.except_clause,
    syms.with_stmt,
    syms.funcdef,
    syms.classdef,
    syms.match_stmt,
    syms.case_block,
}
STANDALONE_COMMENT: Final = 153
Repository: psf/black
File: src/black/__init__.py (Python)

Code before:
    modules: List[Any] = []
    try:
        from click import core
    except ImportError:
        pass
    else:
        modules.append(core)
    try:
        # Removed in Click 8.1.0 and newer; we keep this around for users who have
        # older versions installed.
        from click import _unicodefun  # type: ignore
    except ImportError:
        pass
    else:
        modules.append(_unicodefun)

    for module in modules:
        if hasattr(module, "_verify_python3_env"):
            module._verify_python3_env = lambda: None  # type: ignore
        if hasattr(module, "_verify_python_env"):
            module._verify_python_env = lambda: None  # type: ignore


def patched_main() -> None:
    global __BLACK_MAIN_CALLED__
    __BLACK_MAIN_CALLED__ = True

    if sys.platform == "win32" and getattr(sys, "frozen", False):
        from multiprocessing import freeze_support

        freeze_support()

    patch_click()
    main()


if __name__ == "__main__":
    patched_main()


Review comment (suggestion):
Instead of this global, what if we return early in `maybe_install_uvloop` if uvloop is already in sys.modules?

Or we could just call it unconditionally. It's only called once per formatting run and it's pretty fast:

```
In [14]: %timeit black.concurrency.maybe_install_uvloop()
1.67 µs ± 30.3 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
```

Code after:
    except ImportError:
        pass
    else:
        modules.append(core)
    try:
        # Removed in Click 8.1.0 and newer; we keep this around for users who have
        # older versions installed.
        from click import _unicodefun  # type: ignore
    except ImportError:
        pass
    else:
        modules.append(_unicodefun)

    for module in modules:
        if hasattr(module, "_verify_python3_env"):
            module._verify_python3_env = lambda: None  # type: ignore
        if hasattr(module, "_verify_python_env"):
            module._verify_python_env = lambda: None  # type: ignore


def patched_main() -> None:
    if sys.platform == "win32" and getattr(sys, "frozen", False):
        from multiprocessing import freeze_support

        freeze_support()

    patch_click()
    main()


if __name__ == "__main__":
    patched_main()
Repository: psf/black
File: src/blackd/__init__.py (Python)

Code before:
    from aiohttp import web

    from .middlewares import cors
except ImportError as ie:
    raise ImportError(
        f"aiohttp dependency is not installed: {ie}. "
        + "Please re-install black with the '[d]' extra install "
        + "to obtain aiohttp_cors: `pip install black[d]`"
    ) from None

import click

import black
from _black_version import version as __version__
from black.concurrency import maybe_install_uvloop

# This is used internally by tests to shut down the server prematurely
_stop_signal = asyncio.Event()

# Request headers
PROTOCOL_VERSION_HEADER = "X-Protocol-Version"
LINE_LENGTH_HEADER = "X-Line-Length"
PYTHON_VARIANT_HEADER = "X-Python-Variant"
SKIP_STRING_NORMALIZATION_HEADER = "X-Skip-String-Normalization"
SKIP_MAGIC_TRAILING_COMMA = "X-Skip-Magic-Trailing-Comma"
PREVIEW = "X-PREVIEW"
FAST_OR_SAFE_HEADER = "X-Fast-Or-Safe"
DIFF_HEADER = "X-Diff"

BLACK_HEADERS = [
    PROTOCOL_VERSION_HEADER,
    LINE_LENGTH_HEADER,
    PYTHON_VARIANT_HEADER,
    SKIP_STRING_NORMALIZATION_HEADER,
    SKIP_MAGIC_TRAILING_COMMA,
    PREVIEW,
    FAST_OR_SAFE_HEADER,
    DIFF_HEADER,
]

# Response headers
BLACK_VERSION_HEADER = "X-Black-Version"


class InvalidVariantHeader(Exception):
    pass


@click.command(context_settings={"help_option_names": ["-h", "--help"]})
@click.option(
    "--bind-host", type=str, help="Address to bind the server to.", default="localhost"

Review comment (security):
```suggestion
PREVIEW = "X-Preview"
```
I know HTTP headers are case-insensitive, but we might as make it consistent with the other constants :)

Code after:
    from aiohttp import web

    from .middlewares import cors
except ImportError as ie:
    raise ImportError(
        f"aiohttp dependency is not installed: {ie}. "
        + "Please re-install black with the '[d]' extra install "
        + "to obtain aiohttp_cors: `pip install black[d]`"
    ) from None

import click

import black
from _black_version import version as __version__
from black.concurrency import maybe_install_uvloop

# This is used internally by tests to shut down the server prematurely
_stop_signal = asyncio.Event()

# Request headers
PROTOCOL_VERSION_HEADER = "X-Protocol-Version"
LINE_LENGTH_HEADER = "X-Line-Length"
PYTHON_VARIANT_HEADER = "X-Python-Variant"
SKIP_STRING_NORMALIZATION_HEADER = "X-Skip-String-Normalization"
SKIP_MAGIC_TRAILING_COMMA = "X-Skip-Magic-Trailing-Comma"
PREVIEW = "X-Preview"
FAST_OR_SAFE_HEADER = "X-Fast-Or-Safe"
DIFF_HEADER = "X-Diff"

BLACK_HEADERS = [
    PROTOCOL_VERSION_HEADER,
    LINE_LENGTH_HEADER,
    PYTHON_VARIANT_HEADER,
    SKIP_STRING_NORMALIZATION_HEADER,
    SKIP_MAGIC_TRAILING_COMMA,
    PREVIEW,
    FAST_OR_SAFE_HEADER,
    DIFF_HEADER,
]

# Response headers
BLACK_VERSION_HEADER = "X-Black-Version"


class InvalidVariantHeader(Exception):
    pass


@click.command(context_settings={"help_option_names": ["-h", "--help"]})
@click.option(
    "--bind-host", type=str, help="Address to bind the server to.", default="localhost"