From 034c6f039aa350148b159b709ec34def117e890f Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 08:09:52 +0300 Subject: [PATCH 1/6] add pylint configuration but not tests yet --- default.nix | 1 + pyproject.toml | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/default.nix b/default.nix index ef04011a..ccf54187 100644 --- a/default.nix +++ b/default.nix @@ -19,6 +19,7 @@ python3.pkgs.buildPythonApplication { mypy python3.pkgs.setuptools python3.pkgs.black + python3.pkgs.pylint ruff glibcLocales diff --git a/pyproject.toml b/pyproject.toml index 082fae66..e7900ee9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,3 +72,25 @@ ignore_missing_imports = true [[tool.mypy.overrides]] module = "pytest.*" ignore_missing_imports = true + +[tool.pylint.FORMAT] +output-format = "colorized" + +[tool.pylint.messages_control] +disable = [ + # Disable failure for TODO items in the codebase (code will always have TODOs). + "fixme", + + # Annoying. + "line-too-long", + + # Too many missing docstrings. + "missing-docstring", + + # Not very important, maybe fix in the future. + "invalid-name", + "too-many-instance-attributes", + "too-few-public-methods", + "too-many-arguments", + "too-many-locals", +] From 70813335b56f09bf132e018ce5218a3bef2d9ad6 Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 08:23:39 +0300 Subject: [PATCH 2/6] flake.lock: Update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/e08e100b0c6a6651c3235d1520c53280142d9d5f' (2023-08-05) → 'github:NixOS/nixpkgs/12bdeb01ff9e2d3917e6a44037ed7df6e6c3df9d' (2023-10-15) --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index b895a672..fcb773a7 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1691251374, - "narHash": "sha256-NrSns//HR4/U9L5SkmCoJwRCBMOZa89ib2x5yyNR1kM=", + "lastModified": 1697379843, + "narHash": "sha256-RcnGuJgC2K/UpTy+d32piEoBXq2M+nVFzM3ah/ZdJzg=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "e08e100b0c6a6651c3235d1520c53280142d9d5f", + "rev": "12bdeb01ff9e2d3917e6a44037ed7df6e6c3df9d", "type": "github" }, "original": { From b85b1defa71750bbda675be6c6b7c15312fec900 Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 09:16:44 +0300 Subject: [PATCH 3/6] mypy check everything move the strict enablement to the pyproject.toml so disabling checks works correctly because `--strict` takes precedence over the file Checking everything was disabled in 60205b9a due to (seemingly) a bug --- default.nix | 2 +- pyproject.toml | 3 +++ tests/conftest.py | 3 +-- tests/test_pr.py | 14 +++++++++----- tests/test_wip.py | 6 ++++-- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/default.nix b/default.nix index ccf54187..4ec5fa84 100644 --- a/default.nix +++ b/default.nix @@ -40,7 +40,7 @@ python3.pkgs.buildPythonApplication { echo -e "\x1b[32m## run ruff\x1b[0m" ruff . echo -e "\x1b[32m## run mypy\x1b[0m" - mypy --strict nixpkgs_review + mypy nixpkgs_review echo -e "\x1b[32m## run nixpkgs-review --help\x1b[0m" diff --git a/pyproject.toml b/pyproject.toml index e7900ee9..80c90a4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,10 +60,13 @@ exclude = ''' [tool.mypy] python_version = "3.10" +strict = true warn_redundant_casts = true disallow_untyped_calls = true disallow_untyped_defs = true no_implicit_optional = true +# Missing type parameters for generic type "CaptureFixture" [type-arg] +disallow_any_generics = false [[tool.mypy.overrides]] module = "setuptools.*" diff --git a/tests/conftest.py b/tests/conftest.py index 5e0fef45..8ccfee57 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -137,7 +137,6 @@ def nixpkgs() -> Iterator[Nixpkgs]: yield setup_git(nixpkgs_path) -# pytest.fixture is untyped -@pytest.fixture # type: ignore +@pytest.fixture def helpers() -> type[Helpers]: return Helpers diff --git a/tests/test_pr.py b/tests/test_pr.py index 692a6716..3a401659 100644 --- a/tests/test_pr.py +++ b/tests/test_pr.py @@ -2,7 +2,7 @@ import shutil import subprocess -from unittest.mock import MagicMock, mock_open, patch +from unittest.mock import MagicMock, Mock, mock_open, patch import pytest @@ -13,14 +13,14 @@ @patch("nixpkgs_review.utils.shutil.which", return_value=None) -def test_default_to_nix_if_nom_not_found(mock_shutil): +def test_default_to_nix_if_nom_not_found(mock_shutil: Mock) -> None: return_value = nix_nom_tool() assert return_value == "nix" mock_shutil.assert_called_once() @pytest.mark.skipif(not shutil.which("nom"), reason="`nom` not found in PATH") -def test_pr_local_eval(helpers: Helpers, capfd) -> None: +def test_pr_local_eval(helpers: Helpers, capfd: pytest.CaptureFixture) -> None: with helpers.nixpkgs() as nixpkgs: with open(nixpkgs.path.joinpath("pkg1.txt"), "w") as f: f.write("foo") @@ -47,7 +47,9 @@ def test_pr_local_eval(helpers: Helpers, capfd) -> None: @patch("nixpkgs_review.cli.nix_nom_tool", return_value="nix") -def test_pr_local_eval_missing_nom(mock_tool, helpers: Helpers, capfd) -> None: +def test_pr_local_eval_missing_nom( + mock_tool: Mock, helpers: Helpers, capfd: pytest.CaptureFixture +) -> None: with helpers.nixpkgs() as nixpkgs: with open(nixpkgs.path.joinpath("pkg1.txt"), "w") as f: f.write("foo") @@ -74,7 +76,9 @@ def test_pr_local_eval_missing_nom(mock_tool, helpers: Helpers, capfd) -> None: assert "$ nix build" in captured.out -def test_pr_local_eval_without_nom(helpers: Helpers, capfd) -> None: +def test_pr_local_eval_without_nom( + helpers: Helpers, capfd: pytest.CaptureFixture +) -> None: with helpers.nixpkgs() as nixpkgs: with open(nixpkgs.path.joinpath("pkg1.txt"), "w") as f: f.write("foo") diff --git a/tests/test_wip.py b/tests/test_wip.py index dae528cc..8160c0eb 100644 --- a/tests/test_wip.py +++ b/tests/test_wip.py @@ -10,7 +10,7 @@ @pytest.mark.skipif(not shutil.which("nom"), reason="`nom` not found in PATH") -def test_wip_command(helpers: Helpers, capfd) -> None: +def test_wip_command(helpers: Helpers, capfd: pytest.CaptureFixture) -> None: with helpers.nixpkgs() as nixpkgs: with open(nixpkgs.path.joinpath("pkg1.txt"), "w") as f: f.write("foo") @@ -24,7 +24,9 @@ def test_wip_command(helpers: Helpers, capfd) -> None: assert "$ nom build" in captured.out -def test_wip_command_without_nom(helpers: Helpers, capfd) -> None: +def test_wip_command_without_nom( + helpers: Helpers, capfd: pytest.CaptureFixture +) -> None: with helpers.nixpkgs() as nixpkgs: with open(nixpkgs.path.joinpath("pkg1.txt"), "w") as f: f.write("foo") From 21e9292b82cbf825a2c96ef632ff4dcfb653a2b7 Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 09:26:19 +0300 Subject: [PATCH 4/6] dedupe mypy overrides remove redundant enables ``` --strict Strict mode; enables the following flags: --warn-unused-configs, --disallow-any-generics, --disallow- subclassing-any, --disallow-untyped-calls, --disallow-untyped-defs, --disallow-incomplete-defs, --check- untyped-defs, --disallow-untyped-decorators, --warn-redundant-casts, --warn-unused-ignores, --warn-return-any, --no-implicit-reexport, --strict-equality, --strict-concatenate ``` --- pyproject.toml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 80c90a4c..6c9d37a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,19 +61,15 @@ exclude = ''' [tool.mypy] python_version = "3.10" strict = true -warn_redundant_casts = true -disallow_untyped_calls = true -disallow_untyped_defs = true no_implicit_optional = true # Missing type parameters for generic type "CaptureFixture" [type-arg] disallow_any_generics = false [[tool.mypy.overrides]] -module = "setuptools.*" -ignore_missing_imports = true - -[[tool.mypy.overrides]] -module = "pytest.*" +module = [ + "setuptools", + "pytest" +] ignore_missing_imports = true [tool.pylint.FORMAT] From e4d9f2bf64a66d8e31463ad0f4339dc57d8fcdf8 Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 11:08:35 +0300 Subject: [PATCH 5/6] Fix 'unspecified-encoding' lint --- nixpkgs_review/cli/__init__.py | 2 +- nixpkgs_review/cli/post_result.py | 2 +- nixpkgs_review/nix.py | 2 +- nixpkgs_review/report.py | 8 +++++--- nixpkgs_review/review.py | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/nixpkgs_review/cli/__init__.py b/nixpkgs_review/cli/__init__.py index c61b69ea..43164031 100644 --- a/nixpkgs_review/cli/__init__.py +++ b/nixpkgs_review/cli/__init__.py @@ -135,7 +135,7 @@ def read_github_token() -> str | None: if token: return token try: - with open(hub_config_path()) as f: + with open(hub_config_path(), encoding="utf-8") as f: for line in f: # Allow substring match as hub uses yaml. Example string we match: # " - oauth_token: ghp_abcdefghijklmnopqrstuvwxyzABCDEF1234\n" diff --git a/nixpkgs_review/cli/post_result.py b/nixpkgs_review/cli/post_result.py index 5e94f49b..415c927f 100644 --- a/nixpkgs_review/cli/post_result.py +++ b/nixpkgs_review/cli/post_result.py @@ -21,6 +21,6 @@ def post_result_command(args: argparse.Namespace) -> None: warn(f"Report not found in {report}. Are you in a nixpkgs-review nix-shell?") sys.exit(1) - with open(report) as f: + with open(report, encoding="utf-8") as f: report_text = f.read() github_client.comment_issue(pr, report_text) diff --git a/nixpkgs_review/nix.py b/nixpkgs_review/nix.py index 718d9678..1e5506c6 100644 --- a/nixpkgs_review/nix.py +++ b/nixpkgs_review/nix.py @@ -297,7 +297,7 @@ def nix_build( def write_shell_expression( filename: Path, attrs: list[str], system: str, nixpkgs_config: Path ) -> None: - with open(filename, "w+") as f: + with open(filename, "w+", encoding="utf-8") as f: f.write( f"""{{ pkgs ? import ./nixpkgs {{ system = \"{system}\"; config = import {nixpkgs_config}; }} }}: with pkgs; diff --git a/nixpkgs_review/report.py b/nixpkgs_review/report.py index 679d4913..3e94a852 100644 --- a/nixpkgs_review/report.py +++ b/nixpkgs_review/report.py @@ -70,7 +70,9 @@ def write_error_logs(attrs: list[Attr], directory: Path) -> None: for path in [f"{attr.drv_path}^*", attr.path]: if not path: continue - with open(logs.ensure().joinpath(attr.name + ".log"), "w+") as f: + with open( + logs.ensure().joinpath(attr.name + ".log"), "w+", encoding="utf-8" + ) as f: nix_log = subprocess.run( [ "nix", @@ -121,10 +123,10 @@ def built_packages(self) -> list[str]: return [a.name for a in self.built] def write(self, directory: Path, pr: int | None) -> None: - with open(directory.joinpath("report.md"), "w+") as f: + with open(directory.joinpath("report.md"), "w+", encoding="utf-8") as f: f.write(self.markdown(pr)) - with open(directory.joinpath("report.json"), "w+") as f: + with open(directory.joinpath("report.json"), "w+", encoding="utf-8") as f: f.write(self.json(pr)) write_error_logs(self.attrs, directory) diff --git a/nixpkgs_review/review.py b/nixpkgs_review/review.py index 1dd34002..ff2885c0 100644 --- a/nixpkgs_review/review.py +++ b/nixpkgs_review/review.py @@ -400,7 +400,7 @@ def list_packages( % res.returncode ) tmp.flush() - with open(tmp.name) as f: + with open(tmp.name, encoding="utf-8") as f: return parse_packages_xml(f) From b7b3fe716ec8dd3a67fadc3834550896e7ee2422 Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 19 Oct 2023 11:40:42 +0300 Subject: [PATCH 6/6] fix 'consider-using-f-string' lint --- nixpkgs_review/cli/wip.py | 2 +- nixpkgs_review/review.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/nixpkgs_review/cli/wip.py b/nixpkgs_review/cli/wip.py index 6377d7e5..9fba5e11 100644 --- a/nixpkgs_review/cli/wip.py +++ b/nixpkgs_review/cli/wip.py @@ -11,7 +11,7 @@ def wip_command(args: argparse.Namespace) -> Path: allow = AllowedFeatures(args.allow) with Buildenv(allow.aliases, args.extra_nixpkgs_config) as nixpkgs_config: return review_local_revision( - "rev-%s-dirty" % verify_commit_hash("HEAD"), + f"rev-{verify_commit_hash('HEAD')}-dirty", args, allow, nixpkgs_config, diff --git a/nixpkgs_review/review.py b/nixpkgs_review/review.py index ff2885c0..278b1ab9 100644 --- a/nixpkgs_review/review.py +++ b/nixpkgs_review/review.py @@ -147,7 +147,7 @@ def apply_unstaged(self, staged: bool = False) -> None: result = subprocess.run(["git", "apply"], cwd=self.worktree_dir(), input=diff) if result.returncode != 0: - warn("Failed to apply diff in %s" % self.worktree_dir()) + warn(f"Failed to apply diff in {self.worktree_dir()}") sys.exit(1) def build_commit( @@ -396,8 +396,7 @@ def list_packages( res = subprocess.run(cmd, stdout=tmp) if res.returncode != 0: raise NixpkgsReviewError( - "Failed to list packages: nix-env failed with exit code %d" - % res.returncode + f"Failed to list packages: nix-env failed with exit code {res.returncode}" ) tmp.flush() with open(tmp.name, encoding="utf-8") as f: