From a0b40ade66ac65f82bf42d8ad8753c557a983c1e Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 28 Apr 2023 13:58:16 +0200 Subject: [PATCH] Add basic support for respecting noqa comments. --- flake8_trio/base.py | 3 +- flake8_trio/runner.py | 9 +++- flake8_trio/visitors/flake8triovisitor.py | 5 +- flake8_trio/visitors/visitor_utility.py | 64 ++++++++++++++++++++++- tests/eval_files/noqa.py | 36 +++++++++++++ tests/test_flake8_trio.py | 24 ++++++--- tox.ini | 3 ++ 7 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 tests/eval_files/noqa.py diff --git a/flake8_trio/base.py b/flake8_trio/base.py index 8d904ec..b718e5c 100644 --- a/flake8_trio/base.py +++ b/flake8_trio/base.py @@ -63,7 +63,8 @@ def __iter__(self): yield None def cmp(self): - return self.line, self.col, self.code, self.args + # column may be ignored/modified when autofixing, so sort on that last + return self.line, self.code, self.args, self.col # for sorting in tests def __lt__(self, other: Any) -> bool: diff --git a/flake8_trio/runner.py b/flake8_trio/runner.py index 1892929..4ae5dee 100644 --- a/flake8_trio/runner.py +++ b/flake8_trio/runner.py @@ -32,6 +32,7 @@ class SharedState: options: Options problems: list[Error] = field(default_factory=list) + noqas: dict[int, set[str]] = field(default_factory=dict) library: tuple[str, ...] = () typed_calls: dict[str, str] = field(default_factory=dict) variables: dict[str, str] = field(default_factory=dict) @@ -129,4 +130,10 @@ def run(self) -> Iterable[Error]: return for v in (*self.utility_visitors, *self.visitors): self.module = cst.MetadataWrapper(self.module).visit(v) - yield from self.state.problems + + for problem in self.state.problems: + noqa = self.state.noqas.get(problem.line) + # if there's a noqa comment, and it's bare or this code is listed in it + if noqa is not None and (noqa == set() or problem.code in noqa): + continue + yield problem diff --git a/flake8_trio/visitors/flake8triovisitor.py b/flake8_trio/visitors/flake8triovisitor.py index 3594b73..4b3c36d 100644 --- a/flake8_trio/visitors/flake8triovisitor.py +++ b/flake8_trio/visitors/flake8triovisitor.py @@ -23,7 +23,7 @@ class Flake8TrioVisitor(ast.NodeVisitor, ABC): # abstract attribute by not providing a value - error_codes: dict[str, str] # pyright: reportUninitializedInstanceVariable=false + error_codes: dict[str, str] # pyright: ignore[reportUninitializedInstanceVariable] def __init__(self, shared_state: SharedState): super().__init__() @@ -158,7 +158,7 @@ def add_library(self, name: str) -> None: class Flake8TrioVisitor_cst(cst.CSTTransformer, ABC): # abstract attribute by not providing a value - error_codes: dict[str, str] # pyright: reportUninitializedInstanceVariable=false + error_codes: dict[str, str] # pyright: ignore[reportUninitializedInstanceVariable] METADATA_DEPENDENCIES = (PositionProvider,) def __init__(self, shared_state: SharedState): @@ -167,6 +167,7 @@ def __init__(self, shared_state: SharedState): self.__state = shared_state self.options = self.__state.options + self.noqas = self.__state.noqas def get_state(self, *attrs: str, copy: bool = False) -> dict[str, Any]: # require attrs, since we inherit a *ton* of stuff which we don't want to copy diff --git a/flake8_trio/visitors/visitor_utility.py b/flake8_trio/visitors/visitor_utility.py index 78ff928..0f6df09 100644 --- a/flake8_trio/visitors/visitor_utility.py +++ b/flake8_trio/visitors/visitor_utility.py @@ -3,14 +3,20 @@ from __future__ import annotations import ast -from typing import Any +import functools +import re +from typing import TYPE_CHECKING, Any import libcst as cst import libcst.matchers as m +from libcst.metadata import PositionProvider from .flake8triovisitor import Flake8TrioVisitor, Flake8TrioVisitor_cst from .helpers import utility_visitor, utility_visitor_cst +if TYPE_CHECKING: + from re import Match + @utility_visitor class VisitorTypeTracker(Flake8TrioVisitor): @@ -134,3 +140,59 @@ def visit_Import(self, node: cst.Import): ): assert isinstance(alias.name.value, str) self.add_library(alias.name.value) + + +# taken from +# https://github.com/PyCQA/flake8/blob/d016204366a22d382b5b56dc14b6cbff28ce929e/src/flake8/defaults.py#L27 +NOQA_INLINE_REGEXP = re.compile( + # We're looking for items that look like this: + # ``# noqa`` + # ``# noqa: E123`` + # ``# noqa: E123,W451,F921`` + # ``# noqa:E123,W451,F921`` + # ``# NoQA: E123,W451,F921`` + # ``# NOQA: E123,W451,F921`` + # ``# NOQA:E123,W451,F921`` + # We do not want to capture the ``: `` that follows ``noqa`` + # We do not care about the casing of ``noqa`` + # We want a comma-separated list of errors + # upstream links to an old version on regex101 + # https://regex101.com/r/4XUuax/5 full explanation of the regex + r"# noqa(?::[\s]?(?P([A-Z]+[0-9]+(?:[,\s]+)?)+))?", + re.IGNORECASE, +) + + +@functools.lru_cache(maxsize=512) +def _find_noqa(physical_line: str) -> Match[str] | None: + return NOQA_INLINE_REGEXP.search(physical_line) + + +@utility_visitor_cst +class NoqaHandler(Flake8TrioVisitor_cst): + def visit_Comment(self, node: cst.Comment): + noqa_match = _find_noqa(node.value) + if noqa_match is None: + return False + + codes_str = noqa_match.groupdict()["codes"] + pos = self.get_metadata(PositionProvider, node).start + + codes: set[str] + + # blanket noqa + if codes_str is None: + # this also includes a non-blanket noqa with a list of invalid codes + # so one should maybe instead specifically look for no `:` + codes = set() + else: + # split string on ",", strip of whitespace, and save in set if non-empty + codes = { + item_strip + for item in codes_str.split(",") + if (item_strip := item.strip()) + } + + # TODO: Check that code exists + self.noqas[pos.line] = codes + return False diff --git a/tests/eval_files/noqa.py b/tests/eval_files/noqa.py new file mode 100644 index 0000000..853c03b --- /dev/null +++ b/tests/eval_files/noqa.py @@ -0,0 +1,36 @@ +# NOAUTOFIX - TODO TODO TODO +# NOANYIO +# ARG --enable=TRIO100,TRIO911 +import trio + + +# fmt: off +async def foo_no_noqa(): + with trio.fail_after(5): yield # TRIO100: 9, 'trio', 'fail_after' # TRIO911: 29, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() + + +async def foo_noqa_bare(): + with trio.fail_after(5): yield # noqa + await trio.lowlevel.checkpoint() + + +async def foo_noqa_101(): + with trio.fail_after(5): yield # noqa: TRIO100 # TRIO911: 29, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() + + +async def foo_noqa_911(): + with trio.fail_after(5): yield # noqa: TRIO911 # TRIO100: 9, 'trio', 'fail_after' + await trio.lowlevel.checkpoint() + + +async def foo_noqa_101_911(): + with trio.fail_after(5): yield # noqa: TRIO100, TRIO911 + await trio.lowlevel.checkpoint() + + +async def foo_noqa_101_911_500(): + with trio.fail_after(5): yield # noqa: TRIO100, TRIO911 , TRIO500,,, + await trio.lowlevel.checkpoint() +# fmt: on diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index 4015957..959c13b 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -108,7 +108,13 @@ def check_autofix( visited_code = plugin.module.code if "# AUTOFIX" not in unfixed_code: - assert unfixed_code == visited_code + # if the file is specifically marked with NOAUTOFIX, that means it has visitors + # that will autofix with --autofix, but the file explicitly doesn't want to check + # the result of doing that. THIS IS DANGEROUS + if "# NOAUTOFIX" in unfixed_code: + print(f"eval file {test} marked with dangerous marker NOAUTOFIX") + else: + assert unfixed_code == visited_code return # the full generated source code, saved from a previous run @@ -185,9 +191,9 @@ def find_magic_markers( # This could be optimized not to reopen+reread+reparse the same file over and over # when testing the same file -@pytest.mark.parametrize(("test", "path"), test_files) -@pytest.mark.parametrize("autofix", [False, True]) -@pytest.mark.parametrize("anyio", [False, True]) +@pytest.mark.parametrize(("test", "path"), test_files, ids=[f[0] for f in test_files]) +@pytest.mark.parametrize("autofix", [False, True], ids=["noautofix", "autofix"]) +@pytest.mark.parametrize("anyio", [False, True], ids=["trio", "anyio"]) def test_eval( test: str, path: Path, autofix: bool, anyio: bool, generate_autofix: bool ): @@ -196,7 +202,9 @@ def test_eval( if anyio and magic_markers.NOANYIO: pytest.skip("file marked with NOANYIO") - ignore_column = False + # if autofixing, columns may get messed up + ignore_column = autofix + if magic_markers.NOTRIO: if not anyio: pytest.skip("file marked with NOTRIO") @@ -241,6 +249,8 @@ def test_eval( @pytest.mark.parametrize("test", autofix_files) def test_autofix(test: str): content = autofix_files[test].read_text() + assert content, "empty file" + if "# NOTRIO" in content: pytest.skip("file marked with NOTRIO") @@ -334,7 +344,9 @@ def _parse_eval_file(test: str, content: str) -> tuple[list[Error], list[str], s enabled_codes_list = enabled_codes.split(",") for code in enabled_codes_list: - assert re.fullmatch(r"TRIO\d\d\d", code) + assert re.fullmatch( + r"TRIO\d\d\d", code + ), f"invalid code {code} in list {enabled_codes_list}" for error in expected: for code in enabled_codes.split(","): diff --git a/tox.ini b/tox.ini index f2737d2..e48d20c 100644 --- a/tox.ini +++ b/tox.ini @@ -53,6 +53,9 @@ extend-enable = TC10 exclude = .*, tests/eval_files/*, tests/autofix_files/* per-file-ignores = flake8_trio/visitors/__init__.py: F401, E402 +# visitor_utility contains comments specifying how it parses noqa comments, which get +# parsed as noqa comments + flake8_trio/visitors/visitor_utility.py: NQA101, NQA102 # (E301, E302) black formats stub files without excessive blank lines # (D) we don't care about docstrings in stub files *.pyi: D, E301, E302