Ruff comprehensions and performance (#66)

Upgrade [`ruff`](https://docs.astral.sh/ruff) to the current version.

Complete the compliance with Pylint rules.

Add various list, dict, set comprehension, and performance improvements.
* https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4
* https://docs.astral.sh/ruff/rules/#perflint-perf

.pre-commit-config.yaml

@@ -16,7 +16,7 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.6.9"
+    rev: "v0.8.0"
     hooks:
       - id: ruff
         args: [--fix]

pyodide_build/build_env.py

@@ -236,7 +236,9 @@ def emscripten_version() -> str:
 
 def get_emscripten_version_info() -> str:
     """Extracted for testing purposes."""
-    return subprocess.run(["emcc", "-v"], capture_output=True, encoding="utf8").stderr
+    return subprocess.run(
+        ["emcc", "-v"], capture_output=True, encoding="utf8", check=True
+    ).stderr
 
 
 def check_emscripten_version() -> None:

pyodide_build/common.py

@@ -73,7 +73,7 @@ def parse_top_level_import_name(whlfile: Path) -> list[str] | None:
     whlzip = zipfile.Path(whlfile)
 
     def _valid_package_name(dirname: str) -> bool:
-        return all([invalid_chr not in dirname for invalid_chr in ".- "])
+        return all(invalid_chr not in dirname for invalid_chr in ".- ")
 
     def _has_python_file(subdir: zipfile.Path) -> bool:
         queue = deque([subdir])

pyodide_build/config.py

@@ -66,6 +66,7 @@ def _get_make_environment_vars(self) -> Mapping[str, str]:
             capture_output=True,
             text=True,
             env={"PYODIDE_ROOT": str(self.pyodide_root)},
+            check=False,
         )
 
         if result.returncode != 0:

pyodide_build/mkpkg.py

@@ -138,7 +138,7 @@ def _download_wheel(pypi_metadata: URLDict) -> Iterator[Path]:
 
 
 def run_prettier(meta_path: str | Path) -> None:
-    subprocess.run(["npx", "prettier", "-w", meta_path])
+    subprocess.run(["npx", "prettier", "-w", meta_path], check=True)
 
 
 def make_package(

pyodide_build/out_of_tree/venv.py

@@ -95,6 +95,7 @@ def get_pip_monkeypatch(venv_bin: Path) -> str:
         ],
         capture_output=True,
         encoding="utf8",
+        check=False,
     )
     check_result(result, "ERROR: failed to invoke Pyodide")
     platform_data = result.stdout
@@ -248,6 +249,7 @@ def install_stdlib(venv_bin: Path) -> None:
         ],
         capture_output=True,
         encoding="utf8",
+        check=False,
     )
     check_result(result, "ERROR: failed to install unvendored stdlib modules")
 

pyodide_build/pypabuild.py

@@ -258,13 +258,13 @@ def get_build_env(
     a package with pypa/build.
     """
 
-    kwargs = dict(
-        pkgname=pkgname,
-        cflags=cflags,
-        cxxflags=cxxflags,
-        ldflags=ldflags,
-        target_install_dir=target_install_dir,
-    )
+    kwargs = {
+        "pkgname": pkgname,
+        "cflags": cflags,
+        "cxxflags": cxxflags,
+        "ldflags": ldflags,
+        "target_install_dir": target_install_dir,
+    }
 
     args = common.environment_substitute_args(kwargs, env)
     args["builddir"] = str(Path(".").absolute())

pyodide_build/pywasmcross.py

@@ -351,7 +351,11 @@ def calculate_object_exports_readobj(objects: list[str]) -> list[str] | None:
         "-st",
     ] + objects
     completedprocess = subprocess.run(
-        args, encoding="utf8", capture_output=True, env={"PATH": os.environ["PATH"]}
+        args,
+        encoding="utf8",
+        capture_output=True,
+        env={"PATH": os.environ["PATH"]},
+        check=False,
     )
     if completedprocess.returncode:
         print(f"Command '{' '.join(args)}' failed. Output to stderr was:")
@@ -367,7 +371,11 @@ def calculate_object_exports_readobj(objects: list[str]) -> list[str] | None:
 def calculate_object_exports_nm(objects: list[str]) -> list[str]:
     args = ["emnm", "-j", "--export-symbols"] + objects
     result = subprocess.run(
-        args, encoding="utf8", capture_output=True, env={"PATH": os.environ["PATH"]}
+        args,
+        encoding="utf8",
+        capture_output=True,
+        env={"PATH": os.environ["PATH"]},
+        check=False,
     )
     if result.returncode:
         print(f"Command '{' '.join(args)}' failed. Output to stderr was:")
@@ -475,9 +483,9 @@ def handle_command_generate_args(  # noqa: C901
         return ["emcc", "-v"]
 
     cmd = line[0]
-    if cmd == "c++" or cmd == "g++":
+    if cmd in {"c++", "g++"}:
         new_args = ["em++"]
-    elif cmd in ("cc", "gcc", "ld", "lld"):
+    elif cmd in {"cc", "gcc", "ld", "lld"}:
         new_args = ["emcc"]
         # distutils doesn't use the c++ compiler when compiling c++ <sigh>
         if any(arg.endswith((".cpp", ".cc")) for arg in line):
@@ -514,7 +522,7 @@ def handle_command_generate_args(  # noqa: C901
             ]
 
         return line
-    elif cmd in ("install_name_tool", "otool"):
+    elif cmd in {"install_name_tool", "otool"}:
         # In MacOS, meson tries to run install_name_tool to fix the rpath of the shared library
         # assuming that it is a ELF file. We need to skip this step.
         # See: https://github.com/mesonbuild/meson/issues/8027
@@ -597,7 +605,7 @@ def handle_command(
         if tmp is None:
             # No source file, it's a query for information about the compiler. Pretend we're
             # gfortran by letting gfortran handle it
-            return subprocess.run(line).returncode
+            return subprocess.run(line, check=False).returncode
 
         line = tmp
 
@@ -608,7 +616,7 @@ def handle_command(
 
         scipy_fixes(new_args)
 
-    result = subprocess.run(new_args)
+    result = subprocess.run(new_args, check=False)
     return result.returncode
 
 

pyodide_build/tests/test_build_env.py

@@ -66,7 +66,7 @@ def test_get_build_environment_vars(
         build_vars = build_env.get_build_environment_vars(manager.pyodide_root)
 
         # extra variables that does not come from config files.
-        extra_vars = set(["PYODIDE", "PYODIDE_PACKAGE_ABI", "PYTHONPATH"])
+        extra_vars = {"PYODIDE", "PYODIDE_PACKAGE_ABI", "PYTHONPATH"}
 
         all_keys = set(BUILD_KEY_TO_VAR.values()) | extra_vars
         for var in build_vars:

pyodide_build/tests/test_buildpkg.py

@@ -143,7 +143,7 @@ def touch(path: Path) -> None:
 
     def rlist(input_dir):
         """Recursively list files in input_dir"""
-        paths = list(sorted(input_dir.rglob("*")))
+        paths = sorted(input_dir.rglob("*"))
         res = []
 
         for el in paths:

pyodide_build/tests/test_io.py

@@ -61,9 +61,9 @@ def test_is_rust_package_1(exe):
 @pytest.mark.parametrize(
     "reqs",
     [
-        dict(),
-        dict(requirements={"host": ["rustc"]}),
-        dict(requirements={"executable": ["something_else"]}),
+        {},
+        {"requirements": {"host": ["rustc"]}},
+        {"requirements": {"executable": ["something_else"]}},
     ],
 )
 def test_is_rust_package_2(reqs):

pyodide_build/tests/test_py_compile.py

@@ -87,7 +87,7 @@ def test_py_compile_zip(tmp_path, keep):
     else:
         expected = {"test1.zip"}
 
-    assert set(el.name for el in tmp_path.glob("*")) == expected
+    assert {el.name for el in tmp_path.glob("*")} == expected
 
     with zipfile.ZipFile(archive_path) as fh_zip:
         assert fh_zip.namelist() == ["packageA/c/a.pyc", "packageA/d.c"]
@@ -215,7 +215,7 @@ def test_py_compile_archive_dir(tmp_path, with_lockfile):
         expected_in.add("pyodide-lock.json")
         expected_out.add("pyodide-lock.json")
 
-    assert set(el.name for el in tmp_path.glob("*")) == expected_in
+    assert {el.name for el in tmp_path.glob("*")} == expected_in
 
     mapping = _py_compile_archive_dir(tmp_path, keep=False)
 
@@ -224,7 +224,7 @@ def test_py_compile_archive_dir(tmp_path, with_lockfile):
         "test1.zip": "test1.zip",
     }
 
-    assert set(el.name for el in tmp_path.glob("*")) == expected_out
+    assert {el.name for el in tmp_path.glob("*")} == expected_out
 
     if not with_lockfile:
         return

pyodide_build/tests/test_pypi.py

@@ -46,9 +46,9 @@ def _make_fake_package(
                 else:
                     requirements.append(requirement)
             extras_requirements_text = ""
-            for e in extras_requirements.keys():
-                extras_requirements_text += f"{e} = [\n"
-                for r in extras_requirements[e]:
+            for key, value in extras_requirements.items():
+                extras_requirements_text += f"{key} = [\n"
+                for r in value:
                     extras_requirements_text += f"'{r}',\n"
                 extras_requirements_text += "]\n"
             template = dedent(
@@ -100,7 +100,8 @@ def _make_fake_package(
                     build_path,
                     "--outdir",
                     packageDir,
-                ]
+                ],
+                check=True,
             )
         else:
             # make cython sdist
@@ -128,7 +129,8 @@ def _make_fake_package(
                     build_path,
                     "--outdir",
                     packageDir,
-                ]
+                ],
+                check=True,
             )
 
 

pyodide_build/tests/test_pywasmcross.py

@@ -208,8 +208,12 @@ def test_exports_node(tmp_path):
         """
     (tmp_path / "f1.c").write_text(template % (1, 1, 1))
     (tmp_path / "f2.c").write_text(template % (2, 2, 2))
-    subprocess.run(["emcc", "-c", tmp_path / "f1.c", "-o", tmp_path / "f1.o", "-fPIC"])
-    subprocess.run(["emcc", "-c", tmp_path / "f2.c", "-o", tmp_path / "f2.o", "-fPIC"])
+    subprocess.run(
+        ["emcc", "-c", tmp_path / "f1.c", "-o", tmp_path / "f1.o", "-fPIC"], check=True
+    )
+    subprocess.run(
+        ["emcc", "-c", tmp_path / "f2.c", "-o", tmp_path / "f2.o", "-fPIC"], check=True
+    )
     assert set(calculate_exports([str(tmp_path / "f1.o")], True)) == {"g1", "h1"}
     assert set(
         calculate_exports([str(tmp_path / "f1.o"), str(tmp_path / "f2.o")], True)
@@ -222,7 +226,8 @@ def test_exports_node(tmp_path):
     # Currently if the object file contains bitcode we can't tell what the
     # symbol visibility is.
     subprocess.run(
-        ["emcc", "-c", tmp_path / "f1.c", "-o", tmp_path / "f1.o", "-fPIC", "-flto"]
+        ["emcc", "-c", tmp_path / "f1.c", "-o", tmp_path / "f1.o", "-fPIC", "-flto"],
+        check=True,
     )
     assert set(calculate_exports([str(tmp_path / "f1.o")], True)) == {"f1", "g1", "h1"}
 

pyodide_build/xbuildenv.py

@@ -310,6 +310,7 @@ def _install_cross_build_packages(
             ],
             capture_output=True,
             encoding="utf8",
+            check=False,
         )
 
         if result.returncode != 0:

pyproject.toml

@@ -112,27 +112,30 @@ ignore_missing_imports = true
 
 
 [tool.ruff]
+lint.exclude = ["pyodide_build/vendor/*"]
 lint.select = [
+  "ASYNC",  # flake8-async
   "B0",     # bugbear (all B0* checks enabled by default)
   "B904",   # bugbear (Within an except clause, raise exceptions with raise ... from err)
   "B905",   # bugbear (zip() without an explicit strict= parameter set.)
+  "C4",     # flake8-comprehensions
   "C9",     # mccabe complexity
   "E",      # pycodestyles
-  "W",      # pycodestyles
   "F",      # pyflakes
+  "G004",   # f-string logging should be avoided
   "I",      # isort
+  "PERF",   # Perflint
   "PGH",    # pygrep-hooks
-  "PLC",    # pylint conventions
-  "PLE",    # pylint errors
+  "PL",     # Pylint
   "UP",     # pyupgrade
-  "G004",   # f-string logging should be avoided
+  "W",      # pycodestyles
 ]
 
 lint.logger-objects = ["pyodide_build.logger.logger"]
 
-lint.ignore = ["E402", "E501", "E731", "E741", "UP038"]
+lint.ignore = ["E402", "E501", "E731", "E741", "PERF401", "PLW2901", "UP038"]
 # line-length = 219  # E501: Recommended goal is 88 to match black
-target-version = "py311"
+target-version = "py312"
 
 [tool.ruff.lint.per-file-ignores]
 "src/py/_pyodide/_base.py" = [
@@ -144,9 +147,6 @@ target-version = "py311"
 "src/tests/test_typeconversions.py" = [
   "PGH001",  # No builtin `eval()` allowed
 ]
-"tools/*" = [
-  "B008",  # Do not perform function call `typer.Optional` in argument defaults
-]
 
 [tool.ruff.lint.flake8-bugbear]
 extend-immutable-calls = ["typer.Argument", "typer.Option"]
@@ -163,5 +163,12 @@ known-third-party = [
 [tool.ruff.lint.mccabe]
 max-complexity = 31  # C901: Recommended goal is 10
 
+[tool.ruff.lint.pylint]
+allow-magic-value-types = ["bytes", "int", "str"]
+max-args = 16        # Default is 5
+max-branches = 27    # Default is 12
+max-returns = 12     # Default is 6
+max-statements = 58  # Default is 50
+
 [tool.ruff.lint.flake8-tidy-imports]
 ban-relative-imports = "all"