diff --git a/README.md b/README.md index 0317eccd..2046d75e 100644 --- a/README.md +++ b/README.md @@ -370,6 +370,8 @@ for family, grp in itertools.groupby(collected.checks.items(), key=lambda x: x[1 - [`MY104`](https://learn.scientific-python.org/development/guides/style#MY104): MyPy enables ignore-without-code - [`MY105`](https://learn.scientific-python.org/development/guides/style#MY105): MyPy enables redundant-expr - [`MY106`](https://learn.scientific-python.org/development/guides/style#MY106): MyPy enables truthy-bool +- [`MY107`](https://learn.scientific-python.org/development/guides/style#MY107): ty fails on warnings +- [`MY108`](https://learn.scientific-python.org/development/guides/style#MY108): ty enables blanket-ignore-comment ### Nox diff --git a/docs/guides/style.md b/docs/guides/style.md index f1b452a0..a317ca07 100644 --- a/docs/guides/style.md +++ b/docs/guides/style.md @@ -748,8 +748,9 @@ ignore-missing-imports = ["numpy"] Pyrefly checks all your code by default, rather than only annotated functions, so you can start from a looser `preset` and tighten as you go. Set -`preset = "strict"` (the equivalent of MyPy's `strict = true`) once you are -ready {rr}`MY101`, and toggle individual error codes in a +`preset = "strict"` (the equivalent of MyPy's `strict = true`) or +`preset = "all"` once you are ready {rr}`MY101`. The strict preset also reports +unused `# pyrefly: ignore` comments. You can toggle individual error codes in a `[tool.pyrefly.errors]` table, for example: ```toml @@ -778,12 +779,23 @@ exclude = ["**/tests"] [tool.ty.rules] # Each rule can be set to "ignore", "warn", or "error" +blanket-ignore-comment = "error" unresolved-import = "ignore" + +[tool.ty.terminal] +error-on-warning = true ``` Like Pyrefly, ty checks all your code rather than only annotated functions, and it has no single `strict` switch -- you tighten or relax individual rules in -`[tool.ty.rules]`. To silence missing imports for specific modules (the +`[tool.ty.rules]` {rr}`MY101`. Set `all = "error"` there to turn on every rule. + +ty exits successfully on warnings by default, so CI does not fail on rules such +as `unused-ignore-comment`. Set `error-on-warning` {rr}`MY107` to change this. +The `blanket-ignore-comment` rule {rr}`MY108` forces every `# ty: ignore` to +list the rules it ignores, like MyPy's `ignore-without-code`. + +To silence missing imports for specific modules (the equivalent of MyPy's per-module `ignore_missing_imports`) rather than turning the rule off everywhere, treat their imports as `Any` in the `[tool.ty.analysis]` table: diff --git a/pyproject.toml b/pyproject.toml index 7a8b1e23..6ebe3bec 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,10 +76,12 @@ noxfile = "sp_repo_review.checks.noxfile:repo_review_checks" dependabot = "sp_repo_review.checks.github:dependabot" noxfile = "sp_repo_review.checks.noxfile:noxfile" precommit = "sp_repo_review.checks.precommit:precommit" +pyrefly = "sp_repo_review.checks.mypy:pyrefly" pytest = "sp_repo_review.checks.pyproject:pytest" readthedocs = "sp_repo_review.checks.readthedocs:readthedocs" ruff = "sp_repo_review.checks.ruff:ruff" setupcfg = "sp_repo_review.checks.setupcfg:setupcfg" +ty = "sp_repo_review.checks.mypy:ty" workflows = "sp_repo_review.checks.github:workflows" [project.entry-points."repo_review.families"] diff --git a/src/sp_repo_review/checks/mypy.py b/src/sp_repo_review/checks/mypy.py index 14045515..1ea685cc 100644 --- a/src/sp_repo_review/checks/mypy.py +++ b/src/sp_repo_review/checks/mypy.py @@ -1,5 +1,8 @@ from __future__ import annotations +__lazy_modules__ = ["tomllib"] + +import tomllib from typing import TYPE_CHECKING, Any from . import mk_url @@ -7,7 +10,32 @@ if TYPE_CHECKING: from importlib.resources.abc import Traversable -MY100_CONFIG_FILES = ("ty.toml", "pyrefly.toml") + +def _load_config( + pyproject: dict[str, Any], package: Traversable, name: str +) -> dict[str, Any] | None: + path = package.joinpath(f"{name}.toml") + if path.is_file(): + with path.open("rb") as f: + return tomllib.load(f) + config: dict[str, Any] | None = pyproject.get("tool", {}).get(name) + return config + + +def ty(pyproject: dict[str, Any], package: Traversable) -> dict[str, Any] | None: + """ + Returns the ty configuration, or None if ty is not configured. A + ``ty.toml`` file takes precedence over ``pyproject.toml``. + """ + return _load_config(pyproject, package, "ty") + + +def pyrefly(pyproject: dict[str, Any], package: Traversable) -> dict[str, Any] | None: + """ + Returns the Pyrefly configuration, or None if Pyrefly is not configured. + A ``pyrefly.toml`` file takes precedence over ``pyproject.toml``. + """ + return _load_config(pyproject, package, "pyrefly") class MyPy: @@ -21,23 +49,21 @@ class MY100(MyPy): url = mk_url("style") @staticmethod - def check(pyproject: dict[str, Any], package: Traversable) -> bool: + def check( + pyproject: dict[str, Any], + ty: dict[str, Any] | None, + pyrefly: dict[str, Any] | None, + ) -> bool: """ Must have a `tool.mypy`, `tool.pyrefly`, or `tool.ty` section in - `pyproject.toml`, or a `ty.toml` or `pyrefly.toml` file. The other checks in this family only read - `pyproject.toml`. + `pyproject.toml`, or a `ty.toml` or `pyrefly.toml` file. """ - if any(package.joinpath(fn).is_file() for fn in MY100_CONFIG_FILES): - return True - match pyproject: - case { - "tool": {"mypy": object()} | {"pyrefly": object()} | {"ty": object()} - }: + case {"tool": {"mypy": object()}}: return True case _: - return False + return ty is not None or pyrefly is not None class MY101(MyPy): @@ -47,12 +73,16 @@ class MY101(MyPy): url = mk_url("style") @staticmethod - def check(pyproject: dict[str, Any]) -> bool | None: + def check( + pyproject: dict[str, Any], + ty: dict[str, Any] | None, + pyrefly: dict[str, Any] | None, + ) -> bool | None: """ Should opt into strict checking. For MyPy, set `strict` (true/false). - For Pyrefly, set a `preset` (such as `"strict"`). ty has no strict - switch -- you tighten individual rules in `[tool.ty.rules]` -- so this - check is skipped when only ty is configured. + For Pyrefly, set `preset` to `"strict"` or `"all"`. ty has no strict + switch, so set the levels of rules in `[tool.ty.rules]`. Passes if any + configured type checker is strict. ```toml [tool.mypy] @@ -60,15 +90,20 @@ def check(pyproject: dict[str, Any]) -> bool | None: ``` """ - match pyproject: - case {"tool": {"mypy": {"strict": bool()}}}: + mypy = pyproject.get("tool", {}).get("mypy") + if mypy is None and ty is None and pyrefly is None: + return None + + match mypy: + case {"strict": bool()}: return True - case {"tool": {"pyrefly": {"preset": str()}}}: + match pyrefly: + case {"preset": "strict" | "all"}: return True - case {"tool": {"mypy": object()} | {"pyrefly": object()}}: - return False - case _: - return None + match ty: + case {"rules": dict()}: + return True + return False class MY102(MyPy): @@ -207,5 +242,64 @@ def check(pyproject: dict[str, Any]) -> bool | None: return None +class MY107(MyPy): + "ty fails on warnings" + + requires = {"MY100"} + url = mk_url("style") + + @staticmethod + def check(ty: dict[str, Any] | None) -> bool | None: + """ + Must set `error-on-warning` (true/false) in `[tool.ty.terminal]`. ty + exits successfully on warnings by default, so CI does not fail on + rules such as `unused-ignore-comment`. Skipped if ty is not configured. + + ```toml + [tool.ty.terminal] + error-on-warning = true + ``` + """ + + match ty: + case None: + return None + case {"terminal": {"error-on-warning": bool()}}: + return True + case _: + return False + + +class MY108(MyPy): + "ty enables blanket-ignore-comment" + + requires = {"MY100"} + url = mk_url("style") + + @staticmethod + def check(ty: dict[str, Any] | None) -> bool | None: + """ + Must enable the `blanket-ignore-comment` rule (or set `all` to a + level other than `"ignore"`). This forces every `# ty: ignore` to list + the rules it ignores, like MyPy's `ignore-without-code`. Skipped if ty + is not configured. + + ```toml + [tool.ty.rules] + blanket-ignore-comment = "error" + ``` + """ + + match ty: + case None: + return None + case {"rules": {"blanket-ignore-comment": str(level)}}: + return level != "ignore" + case {"rules": {"all": str(level)}}: + return level != "ignore" + case _: + return False + + def repo_review_checks() -> dict[str, MyPy]: return {p.__name__: p() for p in MyPy.__subclasses__()} diff --git a/tests/test_mypy.py b/tests/test_mypy.py index 386c6c50..06b26ceb 100644 --- a/tests/test_mypy.py +++ b/tests/test_mypy.py @@ -3,33 +3,73 @@ import pytest from repo_review.testing import compute_check, toml_loads +from sp_repo_review.checks.mypy import pyrefly, ty -def test_my100_present(tmp_path: Path): + +def test_my100_present(): toml = toml_loads(""" [tool.mypy] strict = true """) - assert compute_check("MY100", pyproject=toml, package=tmp_path).result + assert compute_check("MY100", pyproject=toml, ty=None, pyrefly=None).result + + +def test_my100_pyrefly(): + assert compute_check( + "MY100", pyproject={}, ty=None, pyrefly={"project-includes": ["src"]} + ).result + +def test_my100_ty(): + assert compute_check( + "MY100", pyproject={}, ty={"src": {"include": ["src"]}}, pyrefly=None + ).result -def test_my100_pyrefly(tmp_path: Path): + +def test_my100_missing(): + assert not compute_check("MY100", pyproject={}, ty=None, pyrefly=None).result + + +def test_ty_fixture_pyproject(tmp_path: Path): + toml = toml_loads(""" + [tool.ty.rules] + all = "error" + """) + assert ty(toml, tmp_path) == {"rules": {"all": "error"}} + + +def test_ty_fixture_file(tmp_path: Path): + tmp_path.joinpath("ty.toml").write_text('[rules]\nall = "warn"\n') + toml = toml_loads(""" + [tool.ty.rules] + all = "error" + """) + assert ty(toml, tmp_path) == {"rules": {"all": "warn"}} + + +def test_ty_fixture_missing(tmp_path: Path): + assert ty({}, tmp_path) is None + + +def test_pyrefly_fixture_pyproject(tmp_path: Path): toml = toml_loads(""" [tool.pyrefly] - project-includes = ["src"] + preset = "strict" """) - assert compute_check("MY100", pyproject=toml, package=tmp_path).result + assert pyrefly(toml, tmp_path) == {"preset": "strict"} -def test_my100_ty(tmp_path: Path): +def test_pyrefly_fixture_file(tmp_path: Path): + tmp_path.joinpath("pyrefly.toml").write_text('preset = "all"\n') toml = toml_loads(""" - [tool.ty.src] - include = ["src"] + [tool.pyrefly] + preset = "strict" """) - assert compute_check("MY100", pyproject=toml, package=tmp_path).result + assert pyrefly(toml, tmp_path) == {"preset": "all"} -def test_my100_missing(tmp_path: Path): - assert not compute_check("MY100", pyproject={}, package=tmp_path).result +def test_pyrefly_fixture_missing(tmp_path: Path): + assert pyrefly({}, tmp_path) is None def test_my101_present_bool(): @@ -37,7 +77,7 @@ def test_my101_present_bool(): [tool.mypy] strict = false """) - assert compute_check("MY101", pyproject=toml).result + assert compute_check("MY101", pyproject=toml, ty=None, pyrefly=None).result def test_my101_missing(): @@ -45,31 +85,45 @@ def test_my101_missing(): [tool.mypy] warn_unreachable = true """) - assert not compute_check("MY101", pyproject=toml).result + assert not compute_check("MY101", pyproject=toml, ty=None, pyrefly=None).result -def test_my101_pyrefly_preset(): - toml = toml_loads(""" - [tool.pyrefly] - preset = "strict" - """) - assert compute_check("MY101", pyproject=toml).result +@pytest.mark.parametrize("preset", ["strict", "all"]) +def test_my101_pyrefly_preset(preset: str): + assert compute_check( + "MY101", pyproject={}, ty=None, pyrefly={"preset": preset} + ).result + + +@pytest.mark.parametrize("preset", ["basic", "legacy", "default"]) +def test_my101_pyrefly_loose_preset(preset: str): + result = compute_check("MY101", pyproject={}, ty=None, pyrefly={"preset": preset}) + assert result.result is False def test_my101_pyrefly_no_preset(): - toml = toml_loads(""" - [tool.pyrefly] - project-includes = ["src"] - """) - assert compute_check("MY101", pyproject=toml).result is False + result = compute_check( + "MY101", pyproject={}, ty=None, pyrefly={"project-includes": ["src"]} + ) + assert result.result is False -def test_my101_ty_skipped(): - toml = toml_loads(""" - [tool.ty.src] - include = ["src"] - """) - assert compute_check("MY101", pyproject=toml).result is None +def test_my101_ty_rules(): + result = compute_check( + "MY101", pyproject={}, ty={"rules": {"all": "error"}}, pyrefly=None + ) + assert result.result + + +def test_my101_ty_no_rules(): + result = compute_check( + "MY101", pyproject={}, ty={"src": {"include": ["src"]}}, pyrefly=None + ) + assert result.result is False + + +def test_my101_skipped(): + assert compute_check("MY101", pyproject={}, ty=None, pyrefly=None).result is None def test_my10x_skipped_without_mypy(): @@ -81,6 +135,45 @@ def test_my10x_skipped_without_mypy(): assert compute_check(name, pyproject=toml).result is None +def test_my107_error_on_warning(): + result = compute_check("MY107", ty={"terminal": {"error-on-warning": False}}) + assert result.result + + +def test_my107_missing(): + assert compute_check("MY107", ty={"rules": {}}).result is False + + +def test_my107_skipped(): + assert compute_check("MY107", ty=None).result is None + + +@pytest.mark.parametrize("level", ["warn", "error"]) +def test_my108_blanket_ignore(level: str): + result = compute_check("MY108", ty={"rules": {"blanket-ignore-comment": level}}) + assert result.result + + +def test_my108_all_error(): + assert compute_check("MY108", ty={"rules": {"all": "error"}}).result + + +def test_my108_ignored(): + result = compute_check( + "MY108", + ty={"rules": {"all": "error", "blanket-ignore-comment": "ignore"}}, + ) + assert result.result is False + + +def test_my108_missing(): + assert compute_check("MY108", ty={"rules": {}}).result is False + + +def test_my108_skipped(): + assert compute_check("MY108", ty=None).result is None + + def test_my102_pass_without_show_error_codes(): toml = toml_loads(""" [tool.mypy] @@ -159,9 +252,3 @@ def test_my106_missing(): enable_error_code = ["ignore-without-code", "redundant-expr"] """) assert not compute_check("MY106", pyproject=toml).result - - -@pytest.mark.parametrize("name", ["ty.toml", "pyrefly.toml"]) -def test_my100_config_file(tmp_path: Path, name: str): - tmp_path.joinpath(name).touch() - assert compute_check("MY100", pyproject={}, package=tmp_path).result