From 305b55552db1e70d9d52caebd1ec2d6bfb16ba1c Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 15 Apr 2024 14:38:25 -0700 Subject: [PATCH] Strict types ghstack-source-id: 4303ab1db6871595b27652120d74f8dc5f0ae627 Pull Request resolved: https://github.com/Instagram/Fixit/pull/438 --- pyproject.toml | 37 +++++++++++-------- src/fixit/cli.py | 12 +++--- src/fixit/config.py | 9 +++-- src/fixit/engine.py | 2 +- src/fixit/ftypes.py | 13 +++---- src/fixit/rule.py | 2 +- .../compare_singleton_primitives_by_is.py | 10 +++-- src/fixit/rules/rewrite_to_comprehension.py | 4 +- .../rules/use_async_sleep_in_async_def.py | 2 +- src/fixit/testing.py | 8 ++-- src/fixit/tests/__init__.py | 4 +- src/fixit/tests/config.py | 29 ++++++++------- src/fixit/tests/engine.py | 2 +- src/fixit/tests/smoke.py | 4 +- 14 files changed, 73 insertions(+), 65 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c36ed194..138b9aad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,8 +63,24 @@ Home = "https://fixit.rtfd.io" Github = "https://github.com/Instagram/Fixit" Changelog = "https://github.com/Instagram/Fixit/blob/main/CHANGELOG.md" +[tool.attribution] +name = "Fixit" +package = "fixit" +signed_tags = true +version_file = false +ignored_authors = ["dependabot"] + [tool.black] -target-version = ["py37"] +target-version = ["py38"] + +[tool.fixit] +enable = ["fixit.rules"] +python-version = "3.10" +formatter = "ufmt" + +[[tool.fixit.overrides]] +path = "examples" +enable = [".examples.noop"] [tool.hatch.version] source = "vcs" @@ -109,18 +125,7 @@ build = [ "sphinx-build -a -b html docs html", ] -[tool.fixit] -enable = ["fixit.rules"] -python-version = "3.10" -formatter = "ufmt" - -[[tool.fixit.overrides]] -path = "examples" -enable = [".examples.noop"] - -[tool.attribution] -name = "Fixit" -package = "fixit" -signed_tags = true -version_file = false -ignored_authors = ["dependabot"] +[tool.mypy] +strict = true +python_version = "3.8" +ignore_missing_imports = true diff --git a/src/fixit/cli.py b/src/fixit/cli.py index 9d4fa85a..80522f4c 100644 --- a/src/fixit/cli.py +++ b/src/fixit/cli.py @@ -78,7 +78,7 @@ def main( config_file: Optional[Path], tags: str, rules: str, -): +) -> None: level = logging.WARNING if debug is not None: level = logging.DEBUG if debug else logging.ERROR @@ -106,7 +106,7 @@ def lint( ctx: click.Context, diff: bool, paths: Sequence[Path], -): +) -> None: """ lint one or more paths and return suggestions @@ -153,7 +153,7 @@ def fix( interactive: bool, diff: bool, paths: Sequence[Path], -): +) -> None: """ lint and autofix one or more files and return results @@ -206,7 +206,7 @@ def fix( @main.command() @click.pass_context @click.argument("rules", nargs=-1, required=True, type=str) -def test(ctx: click.Context, rules: Sequence[str]): +def test(ctx: click.Context, rules: Sequence[str]) -> None: """ test lint rules and their VALID/INVALID cases """ @@ -230,7 +230,7 @@ def test(ctx: click.Context, rules: Sequence[str]): @main.command() @click.pass_context @click.argument("paths", nargs=-1, type=click.Path(path_type=Path)) -def upgrade(ctx: click.Context, paths: Sequence[Path]): +def upgrade(ctx: click.Context, paths: Sequence[Path]) -> None: """ upgrade lint rules and apply deprecation fixes @@ -245,7 +245,7 @@ def upgrade(ctx: click.Context, paths: Sequence[Path]): @main.command() @click.pass_context @click.argument("paths", nargs=-1, type=click.Path(exists=True, path_type=Path)) -def debug(ctx: click.Context, paths: Sequence[Path]): +def debug(ctx: click.Context, paths: Sequence[Path]) -> None: """ print materialized configuration for paths """ diff --git a/src/fixit/config.py b/src/fixit/config.py index 79b3b8be..af1b7e60 100644 --- a/src/fixit/config.py +++ b/src/fixit/config.py @@ -23,12 +23,12 @@ Optional, Sequence, Set, + Tuple, Type, Union, ) from packaging.specifiers import SpecifierSet - from packaging.version import InvalidVersion, Version from .format import FORMAT_STYLES @@ -69,7 +69,7 @@ def __init__(self, msg: str, rule: QualifiedRule): super().__init__(msg) self.rule = rule - def __reduce__(self): + def __reduce__(self) -> Tuple[Type[RuntimeError], Any]: return type(self), (*self.args, self.rule) @@ -230,7 +230,7 @@ def collect_rules( if config.tags: disabled_rules.update( - {R: "tags" for R in all_rules if R.TAGS not in config.tags} + {R: "tags" for R in all_rules if R.TAGS not in config.tags} # type: ignore[comparison-overlap] ) all_rules -= set(disabled_rules) @@ -314,6 +314,7 @@ def read_configs(paths: List[Path]) -> List[RawConfig]: def get_sequence( config: RawConfig, key: str, *, data: Optional[Dict[str, Any]] = None ) -> Sequence[str]: + value: Sequence[str] if data: value = data.pop(key, ()) else: @@ -400,7 +401,7 @@ def process_subpath( options: Optional[RuleOptionsTable] = None, python_version: Any = None, formatter: Optional[str] = None, - ): + ) -> None: nonlocal target_python_version nonlocal target_formatter diff --git a/src/fixit/engine.py b/src/fixit/engine.py index 6608cef3..2e56dda5 100644 --- a/src/fixit/engine.py +++ b/src/fixit/engine.py @@ -127,7 +127,7 @@ def on_visit(self, node: CSTNode) -> bool: # don't visit children if we're going to replace the parent anyways return node not in replacements - def on_leave(self, node: CSTNode, updated: CSTNode) -> NodeReplacement: + def on_leave(self, node: CSTNode, updated: CSTNode) -> NodeReplacement: # type: ignore[type-arg] if node in replacements: new = replacements[node] return new diff --git a/src/fixit/ftypes.py b/src/fixit/ftypes.py index 3fa9ef26..0c1c03f7 100644 --- a/src/fixit/ftypes.py +++ b/src/fixit/ftypes.py @@ -27,9 +27,10 @@ from libcst import CSTNode, CSTNodeT, FlattenSentinel, RemovalSentinel from libcst._add_slots import add_slots from libcst.metadata import CodePosition as CodePosition, CodeRange as CodeRange - from packaging.version import Version +__all__ = ("Version",) + T = TypeVar("T") STDIN = Path("-") @@ -48,9 +49,7 @@ TimingsHook = Callable[[Timings], None] VisitorMethod = Callable[[CSTNode], None] -VisitHook = Callable[[str], ContextManager] - -Version +VisitHook = Callable[[str], ContextManager[None]] @dataclass(frozen=True) @@ -212,7 +211,7 @@ class Config: # post-run processing formatter: Optional[str] = None - def __post_init__(self): + def __post_init__(self) -> None: self.path = self.path.resolve() self.root = self.root.resolve() @@ -222,7 +221,7 @@ class RawConfig: path: Path data: Dict[str, Any] - def __post_init__(self): + def __post_init__(self) -> None: self.path = self.path.resolve() @@ -237,7 +236,7 @@ class LintViolation: range: CodeRange message: str node: CSTNode - replacement: Optional[NodeReplacement] + replacement: Optional[NodeReplacement[CSTNode]] diff: str = "" @property diff --git a/src/fixit/rule.py b/src/fixit/rule.py index a68a5c02..e704f921 100644 --- a/src/fixit/rule.py +++ b/src/fixit/rule.py @@ -190,7 +190,7 @@ def report( message: Optional[str] = None, *, position: Optional[Union[CodePosition, CodeRange]] = None, - replacement: Optional[NodeReplacement] = None, + replacement: Optional[NodeReplacement[CSTNode]] = None, ) -> None: """ Report a lint rule violation. diff --git a/src/fixit/rules/compare_singleton_primitives_by_is.py b/src/fixit/rules/compare_singleton_primitives_by_is.py index c782bd1e..2f43f947 100644 --- a/src/fixit/rules/compare_singleton_primitives_by_is.py +++ b/src/fixit/rules/compare_singleton_primitives_by_is.py @@ -3,7 +3,7 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -from typing import cast, FrozenSet, Union +from typing import cast, FrozenSet, Set, Union import libcst as cst from libcst.metadata import QualifiedName, QualifiedNameProvider, QualifiedNameSource @@ -78,9 +78,11 @@ class CompareSingletonPrimitivesByIs(LintRule): } ) - def is_singleton(self, node: cst.BaseExpression): - qual_name = cast(set, self.get_metadata(QualifiedNameProvider, node, set())) - return ( + def is_singleton(self, node: cst.BaseExpression) -> bool: + qual_name = cast( + Set[QualifiedName], self.get_metadata(QualifiedNameProvider, node, set()) + ) + return bool( isinstance(node, cst.Name) and qual_name and qual_name < self.QUALIFIED_SINGLETON_PRIMITIVES diff --git a/src/fixit/rules/rewrite_to_comprehension.py b/src/fixit/rules/rewrite_to_comprehension.py index 9c6d8056..68e0953c 100644 --- a/src/fixit/rules/rewrite_to_comprehension.py +++ b/src/fixit/rules/rewrite_to_comprehension.py @@ -141,9 +141,7 @@ def visit_Call(self, node: cst.Call) -> None: replacement = node.deep_replace( node, - # pyre-fixme[6]: Expected `BaseAssignTargetExpression` for 1st - # param but got `BaseExpression`. - cst.DictComp(key=key, value=value, for_in=exp.for_in), # type: ignore + cst.DictComp(key=key, value=value, for_in=exp.for_in), ) self.report( diff --git a/src/fixit/rules/use_async_sleep_in_async_def.py b/src/fixit/rules/use_async_sleep_in_async_def.py index 778a45f1..87b3dedb 100644 --- a/src/fixit/rules/use_async_sleep_in_async_def.py +++ b/src/fixit/rules/use_async_sleep_in_async_def.py @@ -112,7 +112,7 @@ async def func(): ), ] - def __init__(self): + def __init__(self) -> None: super().__init__() # is async func self.async_func = False diff --git a/src/fixit/testing.py b/src/fixit/testing.py index bab29fb0..e14a8117 100644 --- a/src/fixit/testing.py +++ b/src/fixit/testing.py @@ -11,8 +11,8 @@ from typing import Any, Callable, Collection, Dict, List, Mapping, Sequence, Type, Union from .engine import diff_violation, LintRunner -from .ftypes import Config -from .rule import Invalid, LintRule, Valid +from .ftypes import Config, Invalid, Valid +from .rule import LintRule def _dedent(src: str) -> str: @@ -30,7 +30,7 @@ def get_fixture_path( def validate_patch(report: Any, test_case: Invalid) -> None: patch = report.patch - expected_replacement = test_case.expected_replacement # type: ignore + expected_replacement = test_case.expected_replacement if patch is None: if expected_replacement is not None: @@ -168,7 +168,7 @@ def generate_lint_rule_test_cases( test_case_classes: List[Type[unittest.TestCase]] = [] for test_case in gen_all_test_methods(rules): rule_name = type(test_case.rule).__name__ - test_methods_to_add: Dict[str, Callable] = {} + test_methods_to_add: Dict[str, Callable[..., Any]] = {} for test_method_name, test_method_data in test_case.test_methods.items(): diff --git a/src/fixit/tests/__init__.py b/src/fixit/tests/__init__.py index 4021fdec..ef740dcb 100644 --- a/src/fixit/tests/__init__.py +++ b/src/fixit/tests/__init__.py @@ -3,8 +3,8 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -from fixit.config import collect_rules, Config -from fixit.ftypes import QualifiedRule +from fixit.config import collect_rules +from fixit.ftypes import Config, QualifiedRule from fixit.testing import add_lint_rule_tests_to_module from .config import ConfigTest diff --git a/src/fixit/tests/config.py b/src/fixit/tests/config.py index 9f69dddd..c741375a 100644 --- a/src/fixit/tests/config.py +++ b/src/fixit/tests/config.py @@ -7,16 +7,18 @@ from pathlib import Path from tempfile import TemporaryDirectory from textwrap import dedent +from typing import List, Sequence, Tuple, Type from unittest import TestCase from .. import config from ..ftypes import Config, QualifiedRule, RawConfig, Tags, Version +from ..rule import LintRule class ConfigTest(TestCase): maxDiff = None - def setUp(self): + def setUp(self) -> None: self.td = TemporaryDirectory() self.tdp = Path(self.td.name).resolve() @@ -72,10 +74,10 @@ def setUp(self): ) ) - def tearDown(self): + def tearDown(self) -> None: self.td.cleanup() - def test_locate_configs(self): + def test_locate_configs(self) -> None: for name, path, root, expected in ( ("top", self.tdp, None, [self.tdp / "pyproject.toml"]), ("top file", self.tdp / "hello.py", None, [self.tdp / "pyproject.toml"]), @@ -135,7 +137,7 @@ def test_locate_configs(self): actual = config.locate_configs(path, root) self.assertListEqual(expected, actual) - def test_read_configs(self): + def test_read_configs(self) -> None: # in-out priority order innerA = self.inner / "fixit.toml" innerB = self.inner / "pyproject.toml" @@ -250,11 +252,11 @@ def test_read_configs(self): actual = config.read_configs(paths) self.assertListEqual(expected, actual) - def test_merge_configs(self): + def test_merge_configs(self) -> None: root = self.tdp target = root / "a" / "b" / "c" / "foo.py" - for name, raw_configs, expected in ( + params: Sequence[Tuple[str, List[RawConfig], Config]] = ( ( "empty", [], @@ -331,12 +333,13 @@ def test_merge_configs(self): enable=[QualifiedRule("fixit.rules"), QualifiedRule("foo")], ), ), - ): + ) + for name, raw_configs, expected in params: with self.subTest(name): actual = config.merge_configs(target, raw_configs) self.assertEqual(expected, actual) - def test_generate_config(self): + def test_generate_config(self) -> None: for name, path, root, expected in ( ( "inner", @@ -422,7 +425,7 @@ def test_generate_config(self): actual = config.generate_config(path, root) self.assertDictEqual(asdict(expected), asdict(actual)) - def test_invalid_config(self): + def test_invalid_config(self) -> None: with self.subTest("inner enable-root-import"): (self.tdp / "pyproject.toml").write_text("[tool.fixit]\nroot = true\n") (self.tdp / "outer" / "pyproject.toml").write_text( @@ -432,7 +435,7 @@ def test_invalid_config(self): with self.assertRaisesRegex(config.ConfigError, "enable-root-import"): config.generate_config(self.tdp / "outer" / "foo.py") - def test_collect_rules(self): + def test_collect_rules(self) -> None: from fixit.rules.avoid_or_in_except import AvoidOrInExcept from fixit.rules.cls_in_classmethod import UseClsInClassmethod from fixit.rules.no_namedtuple import NoNamedTuple @@ -442,7 +445,7 @@ def test_collect_rules(self): UseTypesFromTyping.TAGS = {"typing"} NoNamedTuple.TAGS = {"typing", "tuples"} - def collect_types(cfg): + def collect_types(cfg: Config) -> List[Type[LintRule]]: return sorted([type(rule) for rule in config.collect_rules(cfg)], key=str) with self.subTest("everything"): @@ -476,7 +479,7 @@ def collect_types(cfg): with self.subTest("version match"): rules = collect_types( Config( - python_version="3.7", + python_version=Version("3.7"), ) ) self.assertIn(UseTypesFromTyping, rules) @@ -484,7 +487,7 @@ def collect_types(cfg): with self.subTest("version mismatch"): rules = collect_types( Config( - python_version="3.10", + python_version=Version("3.10"), ) ) self.assertNotIn(UseTypesFromTyping, rules) diff --git a/src/fixit/tests/engine.py b/src/fixit/tests/engine.py index fe5478b2..31e6ce09 100644 --- a/src/fixit/tests/engine.py +++ b/src/fixit/tests/engine.py @@ -22,7 +22,7 @@ class EngineTest(TestCase): - def test_diff_violation(self): + def test_diff_violation(self) -> None: src = dedent( """\ import sys diff --git a/src/fixit/tests/smoke.py b/src/fixit/tests/smoke.py index 53d9aba6..a917a389 100644 --- a/src/fixit/tests/smoke.py +++ b/src/fixit/tests/smoke.py @@ -16,10 +16,10 @@ class SmokeTest(TestCase): - def setUp(self): + def setUp(self) -> None: self.runner = CliRunner(mix_stderr=False) - def test_cli_version(self): + def test_cli_version(self) -> None: result = self.runner.invoke(main, ["--version"]) expected = rf"fixit, version {__version__}" self.assertIn(expected, result.stdout)