From 7bb803ddf84e2b39152f03ee008acfa633c85b13 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 1 May 2023 16:58:49 +0200 Subject: [PATCH] don't autofix noqa'd errors, add --disable-noqa, add non-passing test that noqa always works --- flake8_trio/__init__.py | 32 +++--- flake8_trio/base.py | 1 + flake8_trio/runner.py | 15 ++- flake8_trio/visitors/flake8triovisitor.py | 123 +++++++++++++++++++++- flake8_trio/visitors/visitor100.py | 2 +- flake8_trio/visitors/visitor91x.py | 28 +++-- flake8_trio/visitors/visitor_utility.py | 64 +---------- pyproject.toml | 2 +- tests/autofix_files/noqa.py | 46 ++++++++ tests/autofix_files/noqa.py.diff | 29 +++++ tests/autofix_files/noqa_testing.py | 10 ++ tests/autofix_files/noqa_testing.py.diff | 9 ++ tests/eval_files/noqa.py | 53 +++------- tests/eval_files/noqa_no_autofix.py | 30 ++++++ tests/eval_files/noqa_testing.py | 9 ++ tests/eval_files/trio106.py | 5 +- tests/test_config_and_args.py | 98 +++++++++++++---- tests/test_flake8_trio.py | 41 ++++++-- tox.ini | 4 +- 19 files changed, 435 insertions(+), 166 deletions(-) create mode 100644 tests/autofix_files/noqa.py create mode 100644 tests/autofix_files/noqa.py.diff create mode 100644 tests/autofix_files/noqa_testing.py create mode 100644 tests/autofix_files/noqa_testing.py.diff create mode 100644 tests/eval_files/noqa_no_autofix.py create mode 100644 tests/eval_files/noqa_testing.py diff --git a/flake8_trio/__init__.py b/flake8_trio/__init__.py index 04b55f5..517c74e 100644 --- a/flake8_trio/__init__.py +++ b/flake8_trio/__init__.py @@ -150,29 +150,26 @@ def from_source(cls, source: str) -> Plugin: return plugin def run(self) -> Iterable[Error]: - problems_ast = Flake8TrioRunner.run(self._tree, self.options) + if not self.standalone: + self.options.disable_noqa = True cst_runner = Flake8TrioRunner_cst(self.options, self.module) - problems_cst = cst_runner.run() + yield from cst_runner.run() + # update saved module so modified source code can be accessed when autofixing + self.module = cst_runner.module - # when run as a flake8 plugin, flake8 handles suppressing errors from `noqa`. - # it's therefore important we don't suppress any errors for compatibility with - # flake8-noqa - if not self.standalone: + problems_ast = Flake8TrioRunner.run(self._tree, self.options) + if self.options.disable_noqa: yield from problems_ast - yield from problems_cst return - for problem in (*problems_ast, *problems_cst): - noqa = cst_runner.state.noqas.get(problem.line) + for problem in problems_ast: + noqa = cst_runner.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 - # update saved module so modified source code can be accessed when autofixing - self.module = cst_runner.module - @staticmethod def add_options(option_manager: OptionManager | ArgumentParser): if isinstance(option_manager, ArgumentParser): @@ -184,6 +181,16 @@ def add_options(option_manager: OptionManager | ArgumentParser): dest="files", help="Files(s) to format, instead of autodetection.", ) + add_argument( + "--disable-noqa", + required=False, + default=False, + action="store_true", + help=( + 'Disable the effect of "# noqa". This will report errors on ' + 'lines with "# noqa" at the end.' + ), + ) else: # if run as a flake8 plugin Plugin.standalone = False # Disable TRIO9xx calls by default @@ -326,6 +333,7 @@ def get_matching_codes( startable_in_context_manager=options.startable_in_context_manager, trio200_blocking_calls=options.trio200_blocking_calls, anyio=options.anyio, + disable_noqa=options.disable_noqa, ) diff --git a/flake8_trio/base.py b/flake8_trio/base.py index b718e5c..8b05507 100644 --- a/flake8_trio/base.py +++ b/flake8_trio/base.py @@ -21,6 +21,7 @@ class Options: startable_in_context_manager: Collection[str] trio200_blocking_calls: dict[str, str] anyio: bool + disable_noqa: bool class Statement(NamedTuple): diff --git a/flake8_trio/runner.py b/flake8_trio/runner.py index a33f95a..ab4cbdf 100644 --- a/flake8_trio/runner.py +++ b/flake8_trio/runner.py @@ -32,7 +32,6 @@ 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) @@ -113,6 +112,7 @@ class Flake8TrioRunner_cst(__CommonRunner): def __init__(self, options: Options, module: Module): super().__init__(options) self.options = options + self.noqas: dict[int, set[str]] = {} # Could possibly enable/disable utility visitors here, if visitors declared # dependencies @@ -120,15 +120,22 @@ def __init__(self, options: Options, module: Module): v(self.state) for v in utility_visitors_cst ) + # sort the error classes to get predictable behaviour when multiple autofixers + # are enabled + sorted_error_classes_cst = sorted(ERROR_CLASSES_CST, key=lambda x: x.__name__) self.visitors: tuple[Flake8TrioVisitor_cst, ...] = tuple( - v(self.state) for v in ERROR_CLASSES_CST if self.selected(v.error_codes) + v(self.state) + for v in sorted_error_classes_cst + if self.selected(v.error_codes) ) self.module = module def run(self) -> Iterable[Error]: - if not self.visitors: - return for v in (*self.utility_visitors, *self.visitors): self.module = cst.MetadataWrapper(self.module).visit(v) yield from self.state.problems + + # expose the noqa's parsed by the last visitor, so they can be used to filter + # ast problems + self.noqas = v.noqas # type: ignore[reportUnboundVariable] diff --git a/flake8_trio/visitors/flake8triovisitor.py b/flake8_trio/visitors/flake8triovisitor.py index 4b3c36d..c7c507e 100644 --- a/flake8_trio/visitors/flake8triovisitor.py +++ b/flake8_trio/visitors/flake8triovisitor.py @@ -3,6 +3,8 @@ from __future__ import annotations import ast +import functools +import re from abc import ABC from typing import TYPE_CHECKING, Any, Union @@ -13,6 +15,7 @@ if TYPE_CHECKING: from collections.abc import Iterable + from re import Match from ..runner import SharedState @@ -167,7 +170,7 @@ def __init__(self, shared_state: SharedState): self.__state = shared_state self.options = self.__state.options - self.noqas = self.__state.noqas + self.noqas: dict[int, set[str]] = {} 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 @@ -197,12 +200,19 @@ def save_state(self, node: cst.CSTNode, *attrs: str, copy: bool = False): def restore_state(self, node: cst.CSTNode): self.set_state(self.outer.pop(node, {})) + def is_noqa(self, node: cst.CSTNode, code: str): + if self.options.disable_noqa: + return False + pos = self.get_metadata(PositionProvider, node).start + noqas = self.noqas.get(pos.line) + return noqas is not None and (noqas == set() or code in noqas) + def error( self, node: cst.CSTNode, *args: str | Statement | int, error_code: str | None = None, - ): + ) -> bool: if error_code is None: assert ( len(self.error_codes) == 1 @@ -211,9 +221,12 @@ def error( # don't emit an error if this code is disabled in a multi-code visitor # TODO: write test for only one of 910/911 enabled/autofixed elif error_code[:7] not in self.options.enabled_codes: - return # pragma: no cover - pos = self.get_metadata(PositionProvider, node).start + return False # pragma: no cover + if self.is_noqa(node, error_code): + return False + + pos = self.get_metadata(PositionProvider, node).start self.__state.problems.append( Error( # 7 == len('TRIO...'), so alt messages raise the original code @@ -224,13 +237,60 @@ def error( *args, ) ) + return True - def should_autofix(self, code: str | None = None): + def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: if code is None: assert len(self.error_codes) == 1 code = next(iter(self.error_codes)) + if self.is_noqa(node, code): + return False return code in self.options.autofix_codes + # These must not be overridden without calling super() on them. + # TODO: find a good way of enforcing that statically, or add a test param that + # all errors work with noqa, or do fancy metaclass stuff + # https://stackoverflow.com/questions/75033760/require-overriding-method-to-call-super + # or just #YOLO + + def visit_SimpleStatementLine(self, node: cst.SimpleStatementLine): + super().visit_SimpleStatementLine(node) + self.save_state(node, "noqas") + self.parse_noqa(node.trailing_whitespace.comment) + + def leave_SimpleStatementLine( + self, + original_node: cst.SimpleStatementLine, + updated_node: cst.SimpleStatementLine, + ): + super_updated_node = super().leave_SimpleStatementLine( + original_node, updated_node + ) + self.restore_state(original_node) + return super_updated_node # noqa: R504 + + def visit_SimpleStatementSuite(self, node: cst.SimpleStatementSuite): + self.save_state(node, "noqas") + self.parse_noqa(node.trailing_whitespace.comment) + + def leave_SimpleStatementSuite( + self, + original_node: cst.SimpleStatementSuite, + updated_node: cst.SimpleStatementSuite, + ): + self.restore_state(original_node) + return updated_node + + def visit_IndentedBlock(self, node: cst.IndentedBlock): + self.save_state(node, "noqas") + self.parse_noqa(node.header.comment) + + def leave_IndentedBlock( + self, original_node: cst.IndentedBlock, updated_node: cst.IndentedBlock + ): + self.restore_state(original_node) + return updated_node + @property def library(self) -> tuple[str, ...]: return self.__state.library if self.__state.library else ("trio",) @@ -240,3 +300,56 @@ def library(self) -> tuple[str, ...]: def add_library(self, name: str) -> None: if name not in self.__state.library: self.__state.library = self.__state.library + (name,) + + def parse_noqa(self, node: cst.Comment | None): + if not node: + return + noqa_match = _find_noqa(node.value) + if noqa_match is None: + return + + codes_str = noqa_match.groupdict()["codes"] + + line = self.get_metadata(PositionProvider, node).start.line + + assert ( + line not in self.noqas + ), "it should not be possible to have two [noqa] comments on the same line" + + # 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 `:` + self.noqas[line] = set() + return + # split string on ",", strip of whitespace, and save in set if non-empty + # TODO: Check that code exists + self.noqas[line] = { + item_strip for item in codes_str.split(",") if (item_strip := item.strip()) + } + + +# 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) diff --git a/flake8_trio/visitors/visitor100.py b/flake8_trio/visitors/visitor100.py index d9ccecc..8501d6d 100644 --- a/flake8_trio/visitors/visitor100.py +++ b/flake8_trio/visitors/visitor100.py @@ -58,7 +58,7 @@ def leave_With( for res in self.node_dict[original_node]: self.error(res.node, res.base, res.function) - if self.should_autofix() and len(updated_node.items) == 1: + if self.should_autofix(original_node) and len(updated_node.items) == 1: return flatten_preserving_comments(updated_node) return updated_node diff --git a/flake8_trio/visitors/visitor91x.py b/flake8_trio/visitors/visitor91x.py index 64f1319..f246719 100644 --- a/flake8_trio/visitors/visitor91x.py +++ b/flake8_trio/visitors/visitor91x.py @@ -118,7 +118,7 @@ def library(self) -> tuple[str, ...]: ... @abstractmethod - def should_autofix(self) -> bool: + def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: ... # instead of trying to exclude yields found in all the weird places from @@ -126,6 +126,7 @@ def should_autofix(self) -> bool: # Several of them *could* be handled, e.g. `if ...: yield`, but # that's uncommon enough we don't care about it. def visit_SimpleStatementLine(self, node: cst.SimpleStatementLine): + super().visit_SimpleStatementLine(node) self.add_statement = None # insert checkpoint before yield with a flattensentinel, if indicated @@ -134,10 +135,12 @@ def leave_SimpleStatementLine( original_node: cst.SimpleStatementLine, updated_node: cst.SimpleStatementLine, ) -> cst.SimpleStatementLine | cst.FlattenSentinel[cst.SimpleStatementLine]: + _ = super().leave_SimpleStatementLine(original_node, updated_node) + # possible TODO: generate an error if transforming+visiting is done in a # single pass and emit-error-on-transform can be enabled/disabled. The error can't # be generated in the yield/return since it doesn't know if it will be autofixed. - if self.add_statement is None or not self.should_autofix(): + if self.add_statement is None or not self.should_autofix(original_node): return updated_node curr_add_statement = self.add_statement self.add_statement = None @@ -196,7 +199,7 @@ def __init__( def library(self) -> tuple[str, ...]: return self.__library if self.__library else ("trio",) - def should_autofix(self) -> bool: + def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: return True def leave_Yield( @@ -239,8 +242,8 @@ def __init__(self, *args: Any, **kwargs: Any): self.loop_state = LoopState() self.try_state = TryState() - def should_autofix(self, code: str | None = None) -> bool: - return super().should_autofix("TRIO911" if self.has_yield else "TRIO910") + def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: + return super().should_autofix(node, "TRIO911" if self.has_yield else "TRIO910") def checkpoint_statement(self) -> cst.SimpleStatementLine: return checkpoint_statement(self.library[0]) @@ -290,7 +293,7 @@ def leave_FunctionDef( self.async_function # updated_node does not have a position, so we must send original_node and self.check_function_exit(original_node) - and self.should_autofix() + and self.should_autofix(original_node) and isinstance(updated_node.body, cst.IndentedBlock) ): # insert checkpoint at the end of body @@ -331,13 +334,14 @@ def check_function_exit( self.loop_state.nodes_needing_checkpoints.append(original_node) return False + any_errors = False # raise the actual errors for statement in self.uncheckpointed_statements: if statement == ARTIFICIAL_STATEMENT: continue - self.error_91x(original_node, statement) + any_errors |= self.error_91x(original_node, statement) - return True + return any_errors def leave_Return( self, original_node: cst.Return, updated_node: cst.Return @@ -357,7 +361,7 @@ def error_91x( self, node: cst.Return | cst.FunctionDef | cst.Yield, statement: Statement, - ): + ) -> bool: assert statement != ARTIFICIAL_STATEMENT if isinstance(node, cst.FunctionDef): @@ -365,7 +369,7 @@ def error_91x( else: msg = node.__class__.__name__.lower() - self.error( + return self.error( node, msg, statement, @@ -664,7 +668,9 @@ def leave_While( | cst.FlattenSentinel[cst.For | cst.While] | cst.RemovalSentinel ): - if self.loop_state.nodes_needing_checkpoints and self.should_autofix(): + if self.loop_state.nodes_needing_checkpoints and self.should_autofix( + original_node + ): # TODO transformer = InsertCheckpointsInLoopBody( self.loop_state.nodes_needing_checkpoints, self.library ) diff --git a/flake8_trio/visitors/visitor_utility.py b/flake8_trio/visitors/visitor_utility.py index 0f6df09..78ff928 100644 --- a/flake8_trio/visitors/visitor_utility.py +++ b/flake8_trio/visitors/visitor_utility.py @@ -3,20 +3,14 @@ from __future__ import annotations import ast -import functools -import re -from typing import TYPE_CHECKING, Any +from typing import 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): @@ -140,59 +134,3 @@ 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/pyproject.toml b/pyproject.toml index c7eccf1..a0f7ec0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ warn_unused_ignores = false [tool.pyright] exclude = ["**/node_modules", "**/__pycache__", "**/.*"] reportCallInDefaultInitializer = true -reportImplicitStringConcatenation = true +reportImplicitStringConcatenation = false # black generates implicit string concats reportMissingSuperCall = true reportPropertyTypeMismatch = true reportShadowedImports = true diff --git a/tests/autofix_files/noqa.py b/tests/autofix_files/noqa.py new file mode 100644 index 0000000..4ffb02e --- /dev/null +++ b/tests/autofix_files/noqa.py @@ -0,0 +1,46 @@ +# AUTOFIX +# NOANYIO # TODO +# ARG --enable=TRIO100,TRIO911 +from typing import Any + +import trio + + +# fmt: off +async def foo_no_noqa(): + await trio.lowlevel.checkpoint() + # TRIO100: 9, 'trio', 'fail_after' + yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) + await trio.lowlevel.checkpoint() + + +async def foo_noqa_bare(): + with trio.fail_after(5): # noqa + yield # noqa + await trio.lowlevel.checkpoint() + + +async def foo_noqa_100(): + with trio.fail_after(5): # noqa: TRIO100 + await trio.lowlevel.checkpoint() + yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) + await trio.lowlevel.checkpoint() + + +async def foo_noqa_911(): + # TRIO100: 9, 'trio', 'fail_after' + yield # noqa: TRIO911 + await trio.lowlevel.checkpoint() + + +async def foo_noqa_100_911(): + with trio.fail_after(5): # noqa: TRIO100, TRIO911 + yield # noqa: TRIO911 + await trio.lowlevel.checkpoint() + + +async def foo_noqa_100_911_500(): + with trio.fail_after(5): # noqa: TRIO100, TRIO911 , TRIO500,,, + yield # noqa: TRIO100, TRIO911 , TRIO500,,, + await trio.lowlevel.checkpoint() +# fmt: on diff --git a/tests/autofix_files/noqa.py.diff b/tests/autofix_files/noqa.py.diff new file mode 100644 index 0000000..8e545b7 --- /dev/null +++ b/tests/autofix_files/noqa.py.diff @@ -0,0 +1,29 @@ +--- ++++ +@@ x,8 x,9 @@ + + # fmt: off + async def foo_no_noqa(): +- with trio.fail_after(5): # TRIO100: 9, 'trio', 'fail_after' +- yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) ++ await trio.lowlevel.checkpoint() ++ # TRIO100: 9, 'trio', 'fail_after' ++ yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) + await trio.lowlevel.checkpoint() + + +@@ x,13 x,14 @@ + + async def foo_noqa_100(): + with trio.fail_after(5): # noqa: TRIO100 ++ await trio.lowlevel.checkpoint() + yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) + await trio.lowlevel.checkpoint() + + + async def foo_noqa_911(): +- with trio.fail_after(5): # TRIO100: 9, 'trio', 'fail_after' +- yield # noqa: TRIO911 ++ # TRIO100: 9, 'trio', 'fail_after' ++ yield # noqa: TRIO911 + await trio.lowlevel.checkpoint() diff --git a/tests/autofix_files/noqa_testing.py b/tests/autofix_files/noqa_testing.py new file mode 100644 index 0000000..4ecc03f --- /dev/null +++ b/tests/autofix_files/noqa_testing.py @@ -0,0 +1,10 @@ +# AUTOFIX +# NOANYIO # TODO +# ARG --enable=TRIO911 +import trio + + +async def foo_0(): + await trio.lowlevel.checkpoint() + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() diff --git a/tests/autofix_files/noqa_testing.py.diff b/tests/autofix_files/noqa_testing.py.diff new file mode 100644 index 0000000..a154297 --- /dev/null +++ b/tests/autofix_files/noqa_testing.py.diff @@ -0,0 +1,9 @@ +--- ++++ +@@ x,5 x,6 @@ + + + async def foo_0(): ++ await trio.lowlevel.checkpoint() + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() diff --git a/tests/eval_files/noqa.py b/tests/eval_files/noqa.py index 6995ef4..0015b97 100644 --- a/tests/eval_files/noqa.py +++ b/tests/eval_files/noqa.py @@ -1,63 +1,44 @@ -# NOAUTOFIX - TODO TODO TODO -# NOANYIO -# ARG --enable=TRIO100,TRIO102,TRIO911 -import trio +# AUTOFIX +# NOANYIO # TODO +# ARG --enable=TRIO100,TRIO911 from typing import Any +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) + with trio.fail_after(5): # TRIO100: 9, 'trio', 'fail_after' + yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) await trio.lowlevel.checkpoint() async def foo_noqa_bare(): - with trio.fail_after(5): yield # noqa + with trio.fail_after(5): # noqa + yield # noqa await trio.lowlevel.checkpoint() async def foo_noqa_100(): - with trio.fail_after(5): yield # noqa: TRIO100 # TRIO911: 29, "yield", Statement("function definition", lineno-1) + with trio.fail_after(5): # noqa: TRIO100 + yield # TRIO911: 8, "yield", Statement("function definition", lineno-2) await trio.lowlevel.checkpoint() async def foo_noqa_911(): - with trio.fail_after(5): yield # noqa: TRIO911 # TRIO100: 9, 'trio', 'fail_after' + with trio.fail_after(5): # TRIO100: 9, 'trio', 'fail_after' + yield # noqa: TRIO911 await trio.lowlevel.checkpoint() async def foo_noqa_100_911(): - with trio.fail_after(5): yield # noqa: TRIO100, TRIO911 + with trio.fail_after(5): # noqa: TRIO100, TRIO911 + yield # noqa: TRIO911 await trio.lowlevel.checkpoint() async def foo_noqa_100_911_500(): - with trio.fail_after(5): yield # noqa: TRIO100, TRIO911 , TRIO500,,, + with trio.fail_after(5): # noqa: TRIO100, TRIO911 , TRIO500,,, + yield # noqa: TRIO100, TRIO911 , TRIO500,,, await trio.lowlevel.checkpoint() # fmt: on - - -# errors from AST visitors -async def foo() -> Any: - ... - - -async def foo_no_noqa_102(): - try: - pass - finally: - await foo() # TRIO102: 8, Statement("try/finally", lineno-3) - - -async def foo_noqa_102(): - try: - pass - finally: - await foo() # noqa: TRIO102 - - -async def foo_bare_noqa_102(): - try: - pass - finally: - await foo() # noqa diff --git a/tests/eval_files/noqa_no_autofix.py b/tests/eval_files/noqa_no_autofix.py new file mode 100644 index 0000000..8dcc56b --- /dev/null +++ b/tests/eval_files/noqa_no_autofix.py @@ -0,0 +1,30 @@ +# ARG --enable=TRIO102 + +import trio +from typing import Any + + +# errors from AST visitors +async def foo() -> Any: + ... + + +async def foo_no_noqa_102(): + try: + pass + finally: + await foo() # TRIO102: 8, Statement("try/finally", lineno-3) + + +async def foo_noqa_102(): + try: + pass + finally: + await foo() # noqa: TRIO102 + + +async def foo_bare_noqa_102(): + try: + pass + finally: + await foo() # noqa diff --git a/tests/eval_files/noqa_testing.py b/tests/eval_files/noqa_testing.py new file mode 100644 index 0000000..7c4b230 --- /dev/null +++ b/tests/eval_files/noqa_testing.py @@ -0,0 +1,9 @@ +# AUTOFIX +# NOANYIO # TODO +# ARG --enable=TRIO911 +import trio + + +async def foo_0(): + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() diff --git a/tests/eval_files/trio106.py b/tests/eval_files/trio106.py index 81f52a9..eaf8533 100644 --- a/tests/eval_files/trio106.py +++ b/tests/eval_files/trio106.py @@ -1,11 +1,12 @@ # type: ignore +# isort: skip import importlib import trio import trio as foo # error: 0, "trio" from foo import blah -from trio import * # type: ignore # noqa # error: 0, "trio" -from trio import blah, open_file as foo # noqa # error: 0, "trio" +from trio import * # error: 0, "trio" +from trio import blah, open_file as foo # error: 0, "trio" # Note that our tests exercise the Visitor classes, without going through the noqa filter later in flake8 - so these suppressions are picked up by our project-wide linter stack but not the tests. diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index 246661f..e823860 100644 --- a/tests/test_config_and_args.py +++ b/tests/test_config_and_args.py @@ -23,16 +23,27 @@ ) -def write_examplepy(tmp_path: Path) -> None: - assert tmp_path.joinpath("example.py").write_text(EXAMPLE_PY_TEXT) +def write_examplepy(tmp_path: Path, text: str = EXAMPLE_PY_TEXT) -> None: + assert tmp_path.joinpath("example.py").write_text(text) -def assert_autofixed(tmp_path: Path) -> None: - assert tmp_path.joinpath("example.py").read_text() == EXAMPLE_PY_AUTOFIXED_TEXT +def assert_autofixed(tmp_path: Path, text: str = EXAMPLE_PY_AUTOFIXED_TEXT) -> None: + assert tmp_path.joinpath("example.py").read_text() == text -def assert_unchanged(tmp_path: Path) -> None: - assert tmp_path.joinpath("example.py").read_text() == EXAMPLE_PY_TEXT +def assert_unchanged(tmp_path: Path, text: str = EXAMPLE_PY_TEXT) -> None: + assert tmp_path.joinpath("example.py").read_text() == text + + +def monkeypatch_argv( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + argv: list[Path | str] | None = None, +) -> None: + if argv is None: + argv = [tmp_path / "flake8-trio", "./example.py"] + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(sys, "argv", argv) def test_run_flake8_trio(tmp_path: Path): @@ -53,8 +64,7 @@ def test_run_flake8_trio(tmp_path: Path): def test_systemexit_0( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): - monkeypatch.chdir(tmp_path) - monkeypatch.setattr(sys, "argv", [tmp_path / "flake8-trio", "./example.py"]) + monkeypatch_argv(monkeypatch, tmp_path) tmp_path.joinpath("example.py").write_text("") @@ -71,8 +81,7 @@ def test_systemexit_1( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): write_examplepy(tmp_path) - monkeypatch.chdir(tmp_path) - monkeypatch.setattr(sys, "argv", [tmp_path / "flake8-trio", "./example.py"]) + monkeypatch_argv(monkeypatch, tmp_path) with pytest.raises(SystemExit) as exc_info: from flake8_trio import __main__ # noqa @@ -102,8 +111,7 @@ def test_run_in_git_repo(tmp_path: Path): def test_run_no_git_repo( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): - monkeypatch.chdir(tmp_path) - monkeypatch.setattr(sys, "argv", [tmp_path / "flake8-trio"]) + monkeypatch_argv(monkeypatch, tmp_path, [tmp_path / "flake8-trio"]) assert main() == 1 out, err = capsys.readouterr() assert err == "Doesn't seem to be a git repo; pass filenames to format.\n" @@ -114,9 +122,10 @@ def test_run_100_autofix( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): write_examplepy(tmp_path) - monkeypatch.chdir(tmp_path) - monkeypatch.setattr( - sys, "argv", [tmp_path / "flake8-trio", "--autofix=TRIO", "./example.py"] + monkeypatch_argv( + monkeypatch, + tmp_path, + [tmp_path / "flake8-trio", "--autofix=TRIO", "./example.py"], ) assert main() == 1 @@ -255,9 +264,9 @@ def test_enable( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): write_examplepy(tmp_path) - monkeypatch.chdir(tmp_path) - argv = [tmp_path / "flake8-trio", "./example.py"] - monkeypatch.setattr(sys, "argv", argv) + + argv: list[Path | str] = [tmp_path / "flake8-trio", "./example.py"] + monkeypatch_argv(monkeypatch, tmp_path, argv) def _helper(*args: str, error: bool = False, autofix: bool = False) -> None: for arg in args: @@ -327,3 +336,56 @@ def test_flake8_plugin_with_autofix_fails(tmp_path: Path): ) assert not res.stdout assert res.stderr == b"Cannot autofix when run as a flake8 plugin.\n" + + +NOQA_TEXT = """import trio +with trio.move_on_after(10): ... # noqa +""" +NOQA_TEXT_AST = """import trio as foo # noqa""" + + +def test_noqa( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + write_examplepy(tmp_path, text=NOQA_TEXT) + monkeypatch_argv(monkeypatch, tmp_path) + assert main() == 0 + + +def test_disable_noqa_cst( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + write_examplepy(tmp_path, text=NOQA_TEXT) + monkeypatch_argv( + monkeypatch, + tmp_path, + [tmp_path / "flake8-trio", "./example.py", "--disable-noqa"], + ) + assert main() == 1 + out, err = capsys.readouterr() + assert not err + assert ( + out + == "./example.py:2:6: TRIO100 trio.move_on_after context contains no" + " checkpoints, remove the context or add `await" + " trio.lowlevel.checkpoint()`.\n" + ) + + +def test_disable_noqa_ast( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + write_examplepy(tmp_path, text=NOQA_TEXT_AST) + monkeypatch_argv( + monkeypatch, + tmp_path, + [tmp_path / "flake8-trio", "./example.py", "--disable-noqa"], + ) + assert main() == 1 + out, err = capsys.readouterr() + assert not err + assert ( + out + == "./example.py:1:1: TRIO106 trio must be imported with `import trio` for the" + " linter to work.\n" + ) diff --git a/tests/test_flake8_trio.py b/tests/test_flake8_trio.py index bc370a1..96b52f2 100644 --- a/tests/test_flake8_trio.py +++ b/tests/test_flake8_trio.py @@ -41,7 +41,10 @@ f.stem.upper(): f for f in AUTOFIX_DIR.iterdir() if f.suffix == ".py" } # check that there's an eval file for each autofix file -assert set(autofix_files.keys()) - {f[0] for f in test_files} == set() +extra_autofix_files = set(autofix_files.keys()) - {f[0] for f in test_files} +assert ( + extra_autofix_files == set() +), f"no eval file for autofix file[s] {extra_autofix_files}" class ParseError(Exception): @@ -80,15 +83,19 @@ def format_difflib_line(s: str) -> str: def diff_strings(first: str, second: str, /) -> str: - return "".join( - map( - format_difflib_line, - difflib.unified_diff( - first.splitlines(keepends=True), - second.splitlines(keepends=True), - ), - ) + return ( + "".join( + map( + format_difflib_line, + difflib.unified_diff( + first.splitlines(keepends=True), + second.splitlines(keepends=True), + ), + ) + ).rstrip("\n") + + "\n" ) + # make sure only single newline at end of file # replaces all instances of `original` with `new` in string @@ -148,7 +155,7 @@ def check_autofix( added_autofix_diff = diff_strings(unfixed_code, visited_code) # print diff, mainly helpful during development - if diff: + if diff.strip(): print("\n", diff) # if --generate-autofix is specified, which it may be during development, @@ -198,8 +205,14 @@ def find_magic_markers( @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"]) +@pytest.mark.parametrize("noqa", [False, True], ids=["normal", "noqa"]) def test_eval( - test: str, path: Path, autofix: bool, anyio: bool, generate_autofix: bool + test: str, + path: Path, + autofix: bool, + anyio: bool, + noqa: bool, + generate_autofix: bool, ): content = path.read_text() magic_markers = find_magic_markers(content) @@ -220,6 +233,12 @@ def test_eval( # if substituting we're messing up columns ignore_column = True + if noqa: + if autofix: + pytest.skip("noqa+autofix test not implemented") # TODO + # replace all instances of some error with noqa + content = re.sub(r"#[\s]*(error|TRIO\d\d\d):.*", "# noqa", content) + expected, parsed_args, enable = _parse_eval_file(test, content) if anyio: parsed_args.insert(0, "--anyio") diff --git a/tox.ini b/tox.ini index e48d20c..7ceb5bc 100644 --- a/tox.ini +++ b/tox.ini @@ -53,9 +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 +# flake8triovisitor contains comments specifying how it parses noqa comments, which get # parsed as noqa comments - flake8_trio/visitors/visitor_utility.py: NQA101, NQA102 + flake8_trio/visitors/flake8triovisitor.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