ronantakizawa/codereview-bench huggingface dataset:
Repository: pypa/pip
File: src/pip/_internal/commands/debug.py (Python)

Code before:
    suffix = ""
    if formatted_target:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    levels = {key.split(".")[0] for key, _ in config.items()}  # noqa: PERF102
    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True

    def add_options(self) -> None:

Review comment (suggestion):
```suggestion
    # Ruff misidentifies config as a dict.
    # Configuration does not have support the mapping interface.
    levels = {key.split(".")[0] for key, _ in config.items()}  # noqa: PERF102
```

(Something like this, so future readers have an easier time understanding the noqa marker.

Code after:
    suffix = ""
    if formatted_target:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    # Ruff misidentifies config as a dict.
    # Configuration does not have support the mapping interface.
    levels = {key.split(".", 1)[0] for key, _ in config.items()}  # noqa: PERF102
    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True
Repository: pypa/pip
File: src/pip/_internal/commands/debug.py (Python)

Code before:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    # Ruff misidentifies config as a dict.
    # Configuration does not have support the mapping interface.
    levels = {key.split(".")[0] for key, _ in config.items()}  # noqa: PERF102
    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True

    def add_options(self) -> None:

Review comment (performance):
```suggestion
    levels = {key.split(".", 1)[0] for key, _ in config.items()}  # noqa: PERF102
```

While we’re at it, this should _actually_ improve the performance somewhat

Code after:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    # Ruff misidentifies config as a dict.
    # Configuration does not have support the mapping interface.
    levels = {key.split(".", 1)[0] for key, _ in config.items()}  # noqa: PERF102
    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True

    def add_options(self) -> None:
Repository: pypa/pip
File: src/pip/_internal/models/direct_url.py (Python)

Code before:
        raise DirectUrlValidationError(f"{key} must have a value")
    return value


def _exactly_one_of(infos: Iterable[Optional["InfoType"]]) -> "InfoType":
    infos = [info for info in infos if info is not None]
    if not infos:
        raise DirectUrlValidationError(
            "missing one of archive_info, dir_info, vcs_info"
        )
    if len(infos) > 1:
        raise DirectUrlValidationError(
            "more than one of archive_info, dir_info, vcs_info"
        )
    assert infos[0] is not None
    return infos[0]


def _filter_none(**kwargs: Any) -> Dict[str, Any]:
    """Make dict excluding None values."""
    return {k: v for k, v in kwargs.items() if v is not None}


@dataclass
class VcsInfo:
    name: ClassVar = "vcs_info"

    vcs: str
    commit_id: str
    requested_revision: Optional[str] = None

    @classmethod
    def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["VcsInfo"]:
        if d is None:
            return None
        return cls(
            vcs=_get_required(d, str, "vcs"),
            commit_id=_get_required(d, str, "commit_id"),
            requested_revision=_get(d, str, "requested_revision"),
        )

    def _to_dict(self) -> Dict[str, Any]:
        return _filter_none(
            vcs=self.vcs,
            requested_revision=self.requested_revision,
            commit_id=self.commit_id,
        )


class ArchiveInfo:
    name = "archive_info"

Review comment (suggestion):
I opted to leave the direct_url classes non-frozen as some tests broke and it would be more consistent with `ArchiveInfo` which I did not convert (due to non-trivial logic I didn't feel like groking).

Code after:
    if value is None:
        raise DirectUrlValidationError(f"{key} must have a value")
    return value


def _exactly_one_of(infos: Iterable[Optional["InfoType"]]) -> "InfoType":
    infos = [info for info in infos if info is not None]
    if not infos:
        raise DirectUrlValidationError(
            "missing one of archive_info, dir_info, vcs_info"
        )
    if len(infos) > 1:
        raise DirectUrlValidationError(
            "more than one of archive_info, dir_info, vcs_info"
        )
    assert infos[0] is not None
    return infos[0]


def _filter_none(**kwargs: Any) -> Dict[str, Any]:
    """Make dict excluding None values."""
    return {k: v for k, v in kwargs.items() if v is not None}


@dataclass
class VcsInfo:
    name: ClassVar = "vcs_info"

    vcs: str
    commit_id: str
    requested_revision: Optional[str] = None

    @classmethod
    def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["VcsInfo"]:
        if d is None:
            return None
        return cls(
            vcs=_get_required(d, str, "vcs"),
            commit_id=_get_required(d, str, "commit_id"),
            requested_revision=_get(d, str, "requested_revision"),
        )

    def _to_dict(self) -> Dict[str, Any]:
        return _filter_none(
            vcs=self.vcs,
            requested_revision=self.requested_revision,
            commit_id=self.commit_id,
        )


class ArchiveInfo:
Repository: pypa/pip
File: src/pip/_internal/metadata/_json.py (Python)

Code before:
                        encoding = "latin1"
                chunks.append((bytes, encoding))
            return str(make_header(chunks))
        return str(h)

    result = {}
    for field, multi in METADATA_FIELDS:
        if field not in msg:
            continue
        key = json_name(field)
        if multi:
            value: Union[str, List[str]] = [
                sanitise_header(v) for v in msg.get_all(field)  # type: ignore
            ]
        else:
            value = sanitise_header(msg.get(field))  # type: ignore
            if key == "keywords":
                # Accept both comma-separated and space-separated
                # forms, for better compatibility with old data.
                if "," in value:
                    value = [v.strip() for v in value.split(",")]
                else:
                    value = value.split()
        result[key] = value

    payload: Union[str, None] = msg.get_payload()  # type: ignore
    if payload:
        result["description"] = payload

    return result


Review comment (suggestion):
```suggestion
    payload = cast(str, msg.get_payload())
```

Since `msg` is not multipart, we have a guarantee that the payload will be a string.


Code after:
                        encoding = "latin1"
                chunks.append((bytes, encoding))
            return str(make_header(chunks))
        return str(h)

    result = {}
    for field, multi in METADATA_FIELDS:
        if field not in msg:
            continue
        key = json_name(field)
        if multi:
            value: Union[str, List[str]] = [
                sanitise_header(v) for v in msg.get_all(field)  # type: ignore
            ]
        else:
            value = sanitise_header(msg.get(field))  # type: ignore
            if key == "keywords":
                # Accept both comma-separated and space-separated
                # forms, for better compatibility with old data.
                if "," in value:
                    value = [v.strip() for v in value.split(",")]
                else:
                    value = value.split()
        result[key] = value

    payload = cast(str, msg.get_payload())
    if payload:
        result["description"] = payload

    return result
Repository: pypa/pip
File: src/pip/_internal/models/selection_prefs.py (Python)

Code before:
from dataclasses import dataclass
from typing import Optional

from pip._internal.models.format_control import FormatControl


@dataclass(frozen=True)
class SelectionPreferences:
    """
    Encapsulates the candidate selection preferences for downloading
    and installing files.

    :param allow_yanked: Whether files marked as yanked (in the sense
        of PEP 592) are permitted to be candidates for install.
    :param format_control: A FormatControl object or None. Used to control
        the selection of source packages / binary packages when consulting
        the index and links.
    :param prefer_binary: Whether to prefer an old, but valid, binary
        dist over a new source dist.
    :param ignore_requires_python: Whether to ignore incompatible
        "Requires-Python" values in links. Defaults to False.
    """

    # Don't include an allow_yanked default value to make sure each call
    # site considers whether yanked releases are allowed. This also causes
    # that decision to be made explicit in the calling code, which helps
    # people when reading the code.
    allow_yanked: bool
    allow_all_prereleases: bool = False
    format_control: Optional[FormatControl] = None
    prefer_binary: bool = False
    ignore_requires_python: Optional[bool] = False

Review comment (suggestion):
I think it would be better to keep this as slots, since this is in the hot loop for the resolver and slots lookups tend to be ~10% quicker than non-slots lookups (based on my memory + rudimentary tests on Apple's arm64 + Python 3.12).


Code after:
from typing import Optional

from pip._internal.models.format_control import FormatControl


# TODO: This needs Python 3.10's improved slots support for dataclasses
# to be converted into a dataclass.
class SelectionPreferences:
    """
    Encapsulates the candidate selection preferences for downloading
    and installing files.
    """

    __slots__ = [
        "allow_yanked",
        "allow_all_prereleases",
        "format_control",
        "prefer_binary",
        "ignore_requires_python",
    ]

    # Don't include an allow_yanked default value to make sure each call
    # site considers whether yanked releases are allowed. This also causes
    # that decision to be made explicit in the calling code, which helps
    # people when reading the code.
    def __init__(
        self,
        allow_yanked: bool,
        allow_all_prereleases: bool = False,
        format_control: Optional[FormatControl] = None,
        prefer_binary: bool = False,
        ignore_requires_python: Optional[bool] = None,
Repository: pypa/pip
File: src/pip/_internal/req/req_file.py (Python)

Code before:
]

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class ParsedRequirement:
    requirement: str
    is_editable: bool
    comes_from: str
    constraint: bool
    options: Optional[Dict[str, Any]] = None
    line_source: Optional[str] = None


@dataclass(frozen=True)
class ParsedLine:
    filename: str
    lineno: int
    args: str
    opts: Values
    constraint: bool

    @property
    def is_editable(self) -> bool:
        return bool(self.opts.editables)

    @property
    def requirement(self) -> Optional[str]:
        if self.args:
            return self.args
        elif self.is_editable:
            # We don't support multiple -e on one line
            return self.opts.editables[0]
        return None


def parse_requirements(
    filename: str,
    session: "PipSession",
    finder: Optional["PackageFinder"] = None,
    options: Optional[optparse.Values] = None,
    constraint: bool = False,
) -> Generator[ParsedRequirement, None, None]:
    """Parse a requirements file and yield ParsedRequirement instances.

    :param filename:    Path or url of requirements file.
    :param session:     PipSession instance.
    :param finder:      Instance of pip.index.PackageFinder.
    :param options:     cli options.
    :param constraint:  If true, parsing a constraint file rather than

Review comment (suggestion):
```suggestion
@dataclass(frozen=True, slots=True)
```

Code after:
]

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class ParsedRequirement:
    # TODO: replace this with slots=True when dropping Python 3.9 support.
    __slots__ = (
        "requirement",
        "is_editable",
        "comes_from",
        "constraint",
        "options",
        "line_source",
    )

    requirement: str
    is_editable: bool
    comes_from: str
    constraint: bool
    options: Optional[Dict[str, Any]]
    line_source: Optional[str]


@dataclass(frozen=True)
class ParsedLine:
    __slots__ = ("filename", "lineno", "args", "opts", "constraint")

    filename: str
    lineno: int
    args: str
    opts: Values
    constraint: bool

    @property
    def is_editable(self) -> bool:
        return bool(self.opts.editables)

    @property
    def requirement(self) -> Optional[str]:
        if self.args:
            return self.args
        elif self.is_editable:
            # We don't support multiple -e on one line
            return self.opts.editables[0]
        return None


def parse_requirements(
    filename: str,
Repository: pypa/pip
File: src/pip/_internal/req/req_file.py (Python)

Code before:
    cmdoptions.trusted_host,
    cmdoptions.use_new_feature,
]

# options to be passed to requirements
SUPPORTED_OPTIONS_REQ: List[Callable[..., optparse.Option]] = [
    cmdoptions.global_options,
    cmdoptions.hash,
    cmdoptions.config_settings,
]

SUPPORTED_OPTIONS_EDITABLE_REQ: List[Callable[..., optparse.Option]] = [
    cmdoptions.config_settings,
]


# the 'dest' string values
SUPPORTED_OPTIONS_REQ_DEST = [str(o().dest) for o in SUPPORTED_OPTIONS_REQ]
SUPPORTED_OPTIONS_EDITABLE_REQ_DEST = [
    str(o().dest) for o in SUPPORTED_OPTIONS_EDITABLE_REQ
]

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class ParsedRequirement:
    requirement: str
    is_editable: bool
    comes_from: str
    constraint: bool
    options: Optional[Dict[str, Any]] = None
    line_source: Optional[str] = None


@dataclass(frozen=True)
class ParsedLine:
    filename: str
    lineno: int
    args: str
    opts: Values
    constraint: bool

    @property
    def is_editable(self) -> bool:
        return bool(self.opts.editables)

    @property
    def requirement(self) -> Optional[str]:
        if self.args:
            return self.args

Review comment (suggestion):
```suggestion
@dataclass(frozen=True, slots=True)
```

Code after:
    cmdoptions.trusted_host,
    cmdoptions.use_new_feature,
]

# options to be passed to requirements
SUPPORTED_OPTIONS_REQ: List[Callable[..., optparse.Option]] = [
    cmdoptions.global_options,
    cmdoptions.hash,
    cmdoptions.config_settings,
]

SUPPORTED_OPTIONS_EDITABLE_REQ: List[Callable[..., optparse.Option]] = [
    cmdoptions.config_settings,
]


# the 'dest' string values
SUPPORTED_OPTIONS_REQ_DEST = [str(o().dest) for o in SUPPORTED_OPTIONS_REQ]
SUPPORTED_OPTIONS_EDITABLE_REQ_DEST = [
    str(o().dest) for o in SUPPORTED_OPTIONS_EDITABLE_REQ
]

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class ParsedRequirement:
    # TODO: replace this with slots=True when dropping Python 3.9 support.
    __slots__ = (
        "requirement",
        "is_editable",
        "comes_from",
        "constraint",
        "options",
        "line_source",
    )

    requirement: str
    is_editable: bool
    comes_from: str
    constraint: bool
    options: Optional[Dict[str, Any]]
    line_source: Optional[str]


@dataclass(frozen=True)
class ParsedLine:
    __slots__ = ("filename", "lineno", "args", "opts", "constraint")

    filename: str
    lineno: int
Repository: pypa/pip
File: src/pip/_internal/req/req_file.py (Python)

Code before:
        raise_for_status(resp)
        return resp.url, resp.text

    # Assume this is a bare path.
    try:
        with open(url, "rb") as f:
            raw_content = f.read()
    except OSError as exc:
        raise InstallationError(f"Could not open requirements file: {exc}")

    try:
        content = auto_decode(raw_content)
    except UnicodeDecodeError as exc:
        fallback_encoding = "utf-8"
        # don't try an decode again if we know it will fail
        if exc.encoding == fallback_encoding:
            raise

        logging.warning(
            "unable to decode data from %s with encoding %s, "
            "falling back to encoding %s",
            url,
            exc.encoding,
            fallback_encoding,
        )
        content = raw_content.decode(fallback_encoding)

    return url, content


Review comment (suggestion):
It may be a good idea to use `error="backslashreplace"` here. Most of the time, the offending bytes would just be a part of a comment anyway and would not make a difference.

Code after:

    The only allowed format for environment variables defined in the
    requirement file is `${MY_VARIABLE_1}` to ensure two things:

    1. Strings that contain a `$` aren't accidentally (partially) expanded.
    2. Ensure consistency across platforms for requirement files.

    These points are the result of a discussion on the `github pull
    request #3514 <https://github.com/pypa/pip/pull/3514>`_.

    Valid characters in variable names follow the `POSIX standard
    <http://pubs.opengroup.org/onlinepubs/9699919799/>`_ and are limited
    to uppercase letter, digits and the `_` (underscore).
    """
    for line_number, line in lines_enum:
        for env_var, var_name in ENV_VAR_RE.findall(line):
            value = os.getenv(var_name)
            if not value:
                continue

            line = line.replace(env_var, value)

        yield line_number, line


def get_file_content(url: str, session: "PipSession") -> Tuple[str, str]:
    """Gets the content of a file; it may be a filename, file: URL, or
    http: URL.  Returns (location, content).  Content is unicode.
    Respects # -*- coding: declarations on the retrieved files.

    :param url:         File path or url.
    :param session:     PipSession instance.
    """
    scheme = urllib.parse.urlsplit(url).scheme
    # Pip has special support for file:// URLs (LocalFSAdapter).
    if scheme in ["http", "https", "file"]:
        # Delay importing heavy network modules until absolutely necessary.
        from pip._internal.network.utils import raise_for_status

        resp = session.get(url)
        raise_for_status(resp)
        return resp.url, resp.text

    # Assume this is a bare path.
    try:
        with open(url, "rb") as f:
            raw_content = f.read()
    except OSError as exc:
        raise InstallationError(f"Could not open requirements file: {exc}")

    content = _decode_req_file(raw_content, url)
Repository: pypa/pip
File: tests/functional/test_bad_url.py (Python)

Code before:
# test the error message returned by pip when
# a bad "file:" URL is passed to it.

from typing import Any


def test_filenotfound_error_message(script: Any) -> None:
    # Test the error message returned when using a bad 'file:' URL.
    # make pip to fail and get an error message
    # by running "pip install -r file:nonexistent_file"
    proc = script.pip("install", "-r", "file:///unexistent_file", expect_error=True)
    assert proc.returncode == 1
    expect = (
        "ERROR: 404 Client Error: FileNotFoundError for url: file:///unexistent_file"
    )
    assert proc.stderr.rstrip() == expect


Review comment (bug):
This change has been submitted separately as #12964; it's included here to prove CI isn't failing because of *this* patch.

Code after:
# test the error message returned by pip when
# a bad "file:" URL is passed to it.

from typing import Any


def test_filenotfound_error_message(script: Any) -> None:
    # Test the error message returned when using a bad 'file:' URL.
    # make pip to fail and get an error message
    # by running "pip install -r file:///nonexistent_file"
    proc = script.pip("install", "-r", "file:///unexistent_file", expect_error=True)
    assert proc.returncode == 1
    expect = (
        "ERROR: 404 Client Error: FileNotFoundError for url: file:///unexistent_file"
    )
    assert proc.stderr.rstrip() == expect