Skip to content

Commit

Permalink
Merge pull request #12246 from cclauss/ruff-rules-C4-and-PERF
Browse files Browse the repository at this point in the history
  • Loading branch information
uranusjr authored Sep 6, 2023
2 parents 7c8425b + af43e13 commit 5db54f3
Show file tree
Hide file tree
Showing 21 changed files with 93 additions and 102 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ repos:

- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.0.270
rev: v0.0.286
hooks:
- id: ruff

Expand Down
1 change: 1 addition & 0 deletions news/4A0C40FF-ABE1-48C7-954C-7C3EB229135F.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ruff rules ASYNC,C4,C90,PERF,PLE,PLR for minor optimizations and to set upper limits on code complexity.
31 changes: 24 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ webencodings = "https://github.com/SimonSapin/python-webencodings/raw/master/LIC

[tool.ruff]
extend-exclude = [
"_vendor",
"./build",
".scratch",
"_vendor",
"data",
]
ignore = [
Expand All @@ -88,21 +88,38 @@ ignore = [
]
line-length = 88
select = [
"ASYNC",
"B",
"C4",
"C90",
"E",
"F",
"W",
"G",
"ISC",
"I",
"ISC",
"PERF",
"PLE",
"PLR0",
"W",
]

[tool.ruff.per-file-ignores]
"noxfile.py" = ["G"]
"tests/*" = ["B011"]

[tool.ruff.isort]
# We need to explicitly make pip "first party" as it's imported by code in
# the docs and tests directories.
known-first-party = ["pip"]
known-third-party = ["pip._vendor"]

[tool.ruff.mccabe]
max-complexity = 33 # default is 10

[tool.ruff.per-file-ignores]
"noxfile.py" = ["G"]
"src/pip/_internal/*" = ["PERF203"]
"tests/*" = ["B011"]
"tests/unit/test_finder.py" = ["C414"]

[tool.ruff.pylint]
max-args = 15 # default is 5
max-branches = 28 # default is 12
max-returns = 13 # default is 6
max-statements = 134 # default is 50
6 changes: 2 additions & 4 deletions src/pip/_internal/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,10 @@ def _get_candidates(self, link: Link, canonical_package_name: str) -> List[Any]:
if can_not_cache:
return []

candidates = []
path = self.get_path_for_link(link)
if os.path.isdir(path):
for candidate in os.listdir(path):
candidates.append((candidate, path))
return candidates
return [(candidate, path) for candidate in os.listdir(path)]
return []

def get_path_for_link(self, link: Link) -> str:
"""Return a directory to store cached items in for link."""
Expand Down
5 changes: 3 additions & 2 deletions src/pip/_internal/cli/autocompletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ def autocomplete() -> None:

for opt in subcommand.parser.option_list_all:
if opt.help != optparse.SUPPRESS_HELP:
for opt_str in opt._long_opts + opt._short_opts:
options.append((opt_str, opt.nargs))
options += [
(opt_str, opt.nargs) for opt_str in opt._long_opts + opt._short_opts
]

# filter out previously specified options from available options
prev_opts = [x.split("=")[0] for x in cwords[1 : cword - 1]]
Expand Down
12 changes: 3 additions & 9 deletions src/pip/_internal/commands/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
from optparse import Values
from typing import Any, List

import pip._internal.utils.filesystem as filesystem
from pip._internal.cli.base_command import Command
from pip._internal.cli.status_codes import ERROR, SUCCESS
from pip._internal.exceptions import CommandError, PipError
from pip._internal.utils import filesystem
from pip._internal.utils.logging import getLogger

logger = getLogger(__name__)
Expand Down Expand Up @@ -151,14 +151,8 @@ def format_for_human(self, files: List[str]) -> None:
logger.info("\n".join(sorted(results)))

def format_for_abspath(self, files: List[str]) -> None:
if not files:
return

results = []
for filename in files:
results.append(filename)

logger.info("\n".join(sorted(results)))
if files:
logger.info("\n".join(sorted(files)))

def remove_cache_items(self, options: Values, args: List[Any]) -> None:
if len(args) > 1:
Expand Down
7 changes: 3 additions & 4 deletions src/pip/_internal/commands/debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,9 @@ def show_tags(options: Values) -> None:


def ca_bundle_info(config: Configuration) -> str:
levels = set()
for key, _ in config.items():
levels.add(key.split(".")[0])

# 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"

Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/commands/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def output_package_listing_columns(

# Create and add a separator.
if len(data) > 0:
pkg_strings.insert(1, " ".join(map(lambda x: "-" * x, sizes)))
pkg_strings.insert(1, " ".join("-" * x for x in sizes))

for val in pkg_strings:
write_output(val)
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/locations/_distutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def distutils_scheme(
# finalize_options(); we only want to override here if the user
# has explicitly requested it hence going back to the config
if "install_lib" in d.get_option_dict("install"):
scheme.update(dict(purelib=i.install_lib, platlib=i.install_lib))
scheme.update({"purelib": i.install_lib, "platlib": i.install_lib})

if running_under_virtualenv():
if home:
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/models/installation_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def _install_req_to_dict(cls, ireq: InstallRequirement) -> Dict[str, Any]:
}
if ireq.user_supplied and ireq.extras:
# For top level requirements, the list of requested extras, if any.
res["requested_extras"] = list(sorted(ireq.extras))
res["requested_extras"] = sorted(ireq.extras)
return res

def to_dict(self) -> Dict[str, Any]:
Expand Down
6 changes: 3 additions & 3 deletions src/pip/_internal/operations/install/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ def get_csv_rows_for_installed(
path = _fs_to_record_path(f, lib_dir)
digest, length = rehash(f)
installed_rows.append((path, digest, length))
for installed_record_path in installed.values():
installed_rows.append((installed_record_path, "", ""))
return installed_rows
return installed_rows + [
(installed_record_path, "", "") for installed_record_path in installed.values()
]


def get_console_script_specs(console: Dict[str, str]) -> List[str]:
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/req/req_uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ def stash(self, path: str) -> str:

def commit(self) -> None:
"""Commits the uninstall by removing stashed files."""
for _, save_dir in self._save_dirs.items():
for save_dir in self._save_dirs.values():
save_dir.cleanup()
self._moves = []
self._save_dirs = {}
Expand Down
20 changes: 6 additions & 14 deletions tests/functional/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ def http_cache_files(http_cache_dir: str) -> List[str]:
return []

filenames = glob(os.path.join(destination, "*"))
files = []
for filename in filenames:
files.append(os.path.join(destination, filename))
return files
return [os.path.join(destination, filename) for filename in filenames]


@pytest.fixture
Expand All @@ -50,10 +47,7 @@ def wheel_cache_files(wheel_cache_dir: str) -> List[str]:
return []

filenames = glob(os.path.join(destination, "*.whl"))
files = []
for filename in filenames:
files.append(os.path.join(destination, filename))
return files
return [os.path.join(destination, filename) for filename in filenames]


@pytest.fixture
Expand Down Expand Up @@ -107,7 +101,7 @@ def list_matches_wheel(wheel_name: str, result: TestPipResult) -> bool:
`- foo-1.2.3-py3-none-any.whl `."""
lines = result.stdout.splitlines()
expected = f" - {wheel_name}-py3-none-any.whl "
return any(map(lambda line: line.startswith(expected), lines))
return any(line.startswith(expected) for line in lines)


def list_matches_wheel_abspath(wheel_name: str, result: TestPipResult) -> bool:
Expand All @@ -120,11 +114,9 @@ def list_matches_wheel_abspath(wheel_name: str, result: TestPipResult) -> bool:
lines = result.stdout.splitlines()
expected = f"{wheel_name}-py3-none-any.whl"
return any(
map(
lambda line: (
os.path.basename(line).startswith(expected) and os.path.exists(line)
),
lines,
(
(os.path.basename(line).startswith(expected) and os.path.exists(line))
for line in lines
)
)

Expand Down
4 changes: 2 additions & 2 deletions tests/functional/test_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ def test_help_commands_equally_functional(in_memory_pip: InMemoryPip) -> None:
results = list(map(in_memory_pip.pip, ("help", "--help")))
results.append(in_memory_pip.pip())

out = map(lambda x: x.stdout, results)
ret = map(lambda x: x.returncode, results)
out = (x.stdout for x in results)
ret = (x.returncode for x in results)

msg = '"pip --help" != "pip help" != "pip"'
assert len(set(out)) == 1, "output of: " + msg
Expand Down
32 changes: 13 additions & 19 deletions tests/functional/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,25 +273,19 @@ def test_outdated_flag(script: PipTestEnvironment, data: TestData) -> None:
"latest_version": "3.0",
"latest_filetype": "sdist",
} in json_output
assert (
dict(
name="simplewheel",
version="1.0",
latest_version="2.0",
latest_filetype="wheel",
)
in json_output
)
assert (
dict(
name="pip-test-package",
version="0.1",
latest_version="0.1.1",
latest_filetype="sdist",
editable_project_location="<location>",
)
in json_output
)
assert {
"name": "simplewheel",
"version": "1.0",
"latest_version": "2.0",
"latest_filetype": "wheel",
} in json_output
assert {
"name": "pip-test-package",
"version": "0.1",
"latest_version": "0.1.1",
"latest_filetype": "sdist",
"editable_project_location": "<location>",
} in json_output
assert "simple2" not in {p["name"] for p in json_output}


Expand Down
20 changes: 8 additions & 12 deletions tests/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,21 +740,19 @@ def easy_install(self, *args: str, **kwargs: Any) -> TestPipResult:

def assert_installed(self, **kwargs: str) -> None:
ret = self.pip("list", "--format=json")
installed = set(
installed = {
(canonicalize_name(val["name"]), val["version"])
for val in json.loads(ret.stdout)
)
expected = set((canonicalize_name(k), v) for k, v in kwargs.items())
}
expected = {(canonicalize_name(k), v) for k, v in kwargs.items()}
assert expected <= installed, "{!r} not all in {!r}".format(expected, installed)

def assert_not_installed(self, *args: str) -> None:
ret = self.pip("list", "--format=json")
installed = set(
canonicalize_name(val["name"]) for val in json.loads(ret.stdout)
)
installed = {canonicalize_name(val["name"]) for val in json.loads(ret.stdout)}
# None of the given names should be listed as installed, i.e. their
# intersection should be empty.
expected = set(canonicalize_name(k) for k in args)
expected = {canonicalize_name(k) for k in args}
assert not (expected & installed), "{!r} contained in {!r}".format(
expected, installed
)
Expand Down Expand Up @@ -797,17 +795,15 @@ def prefix_match(path: str, prefix_path: StrPath) -> bool:
prefix = prefix.rstrip(os.path.sep) + os.path.sep
return path.startswith(prefix)

start_keys = {
k for k in start.keys() if not any([prefix_match(k, i) for i in ignore])
}
end_keys = {k for k in end.keys() if not any([prefix_match(k, i) for i in ignore])}
start_keys = {k for k in start if not any(prefix_match(k, i) for i in ignore)}
end_keys = {k for k in end if not any(prefix_match(k, i) for i in ignore)}
deleted = {k: start[k] for k in start_keys.difference(end_keys)}
created = {k: end[k] for k in end_keys.difference(start_keys)}
updated = {}
for k in start_keys.intersection(end_keys):
if start[k].size != end[k].size:
updated[k] = end[k]
return dict(deleted=deleted, created=created, updated=updated)
return {"deleted": deleted, "created": created, "updated": updated}


def assert_all_changes(
Expand Down
14 changes: 7 additions & 7 deletions tests/unit/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ class TestIndentingFormatter:

def make_record(self, msg: str, level_name: str) -> logging.LogRecord:
level_number = getattr(logging, level_name)
attrs = dict(
msg=msg,
created=1547704837.040001 + time.timezone,
msecs=40,
levelname=level_name,
levelno=level_number,
)
attrs = {
"msg": msg,
"created": 1547704837.040001 + time.timezone,
"msecs": 40,
"levelname": level_name,
"levelno": level_number,
}
record = logging.makeLogRecord(attrs)

return record
Expand Down
7 changes: 3 additions & 4 deletions tests/unit/test_req_uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ def iter_declared_entries(self) -> Optional[Iterator[str]]:

def test_compressed_listing(tmpdir: Path) -> None:
def in_tmpdir(paths: List[str]) -> List[str]:
li = []
for path in paths:
li.append(str(os.path.join(tmpdir, path.replace("/", os.path.sep))))
return li
return [
str(os.path.join(tmpdir, path.replace("/", os.path.sep))) for path in paths
]

sample = in_tmpdir(
[
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_self_check_outdated.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_pip_self_version_check_calls_underlying_implementation(
) -> None:
# GIVEN
mock_session = Mock()
fake_options = Values(dict(cache_dir=str(tmpdir)))
fake_options = Values({"cache_dir": str(tmpdir)})

# WHEN
self_outdated_check.pip_self_version_check(mock_session, fake_options)
Expand Down
Loading

0 comments on commit 5db54f3

Please sign in to comment.