From 022fc3a4f661ec42afac5a21eb0404fe13f62380 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 1 May 2023 16:58:49 +0200 Subject: [PATCH 1/2] don't autofix noqa'd errors, add --disable-noqa, add noqa test parameter to eval_files --- flake8_trio/__init__.py | 30 +++++-- flake8_trio/base.py | 1 + flake8_trio/runner.py | 22 ++++- flake8_trio/visitors/flake8triovisitor.py | 22 ++++- flake8_trio/visitors/visitor100.py | 7 +- flake8_trio/visitors/visitor101.py | 1 + flake8_trio/visitors/visitor91x.py | 45 +++++++---- pyproject.toml | 2 +- tests/autofix_files/noqa.py | 68 ++++++++++++++++ tests/autofix_files/noqa.py.diff | 52 ++++++++++++ tests/autofix_files/noqa_testing.py | 10 +++ tests/autofix_files/noqa_testing.py.diff | 9 +++ tests/eval_files/noqa.py | 55 ++++++------- tests/eval_files/noqa_no_autofix.py | 30 +++++++ tests/eval_files/noqa_testing.py | 9 +++ tests/eval_files/trio106.py | 5 +- tests/eval_files/trio22x.py | 3 + tests/test_config_and_args.py | 98 ++++++++++++++++++----- tests/test_flake8_trio.py | 61 ++++++++++---- 19 files changed, 433 insertions(+), 97 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 04b55f53..20ba88d2 100644 --- a/flake8_trio/__init__.py +++ b/flake8_trio/__init__.py @@ -150,21 +150,24 @@ def from_source(cls, source: str) -> Plugin: return plugin def run(self) -> Iterable[Error]: - problems_ast = Flake8TrioRunner.run(self._tree, self.options) - - cst_runner = Flake8TrioRunner_cst(self.options, self.module) - problems_cst = cst_runner.run() - # 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: + self.options.disable_noqa = True + + cst_runner = Flake8TrioRunner_cst(self.options, self.module) + # any noqa'd errors are suppressed upon being generated + yield from cst_runner.run() + + 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: + # access the stored noqas in cst_runner + 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 @@ -184,6 +187,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 +339,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 b718e5c6..8b055071 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 a33f95ae..762ed31a 100644 --- a/flake8_trio/runner.py +++ b/flake8_trio/runner.py @@ -18,6 +18,7 @@ utility_visitors, utility_visitors_cst, ) +from .visitors.visitor_utility import NoqaHandler if TYPE_CHECKING: from collections.abc import Iterable @@ -113,22 +114,35 @@ class Flake8TrioRunner_cst(__CommonRunner): def __init__(self, options: Options, module: Module): super().__init__(options) self.options = options + self.noqas: dict[int, set[str]] = {} + + utility_visitors = utility_visitors_cst.copy() + if self.options.disable_noqa: + utility_visitors.remove(NoqaHandler) # Could possibly enable/disable utility visitors here, if visitors declared # dependencies self.utility_visitors: tuple[Flake8TrioVisitor_cst, ...] = tuple( - v(self.state) for v in utility_visitors_cst + v(self.state) for v in utility_visitors ) + # 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 + if not self.options.disable_noqa: + self.noqas = v.noqas # type: ignore[reportUnboundVariable] diff --git a/flake8_trio/visitors/flake8triovisitor.py b/flake8_trio/visitors/flake8triovisitor.py index 4b3c36d5..9d3fb9f0 100644 --- a/flake8_trio/visitors/flake8triovisitor.py +++ b/flake8_trio/visitors/flake8triovisitor.py @@ -197,12 +197,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 +218,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,11 +234,15 @@ 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)) + # this does not currently need to check for `noqa`s, as error() does that + # and no codes tries to autofix if there's no error. This might change if/when + # "printing an error" and "autofixing" becomes independent. See issue #192 return code in self.options.autofix_codes @property diff --git a/flake8_trio/visitors/visitor100.py b/flake8_trio/visitors/visitor100.py index d9ccecc4..9e37f1cb 100644 --- a/flake8_trio/visitors/visitor100.py +++ b/flake8_trio/visitors/visitor100.py @@ -55,10 +55,13 @@ def leave_With( self, original_node: cst.With, updated_node: cst.With ) -> cst.BaseStatement | cst.FlattenSentinel[cst.BaseStatement]: if not self.has_checkpoint_stack.pop(): + autofix = len(updated_node.items) == 1 for res in self.node_dict[original_node]: - self.error(res.node, res.base, res.function) + autofix &= self.error( + res.node, res.base, res.function + ) and self.should_autofix(res.node) - if self.should_autofix() and len(updated_node.items) == 1: + if autofix: return flatten_preserving_comments(updated_node) return updated_node diff --git a/flake8_trio/visitors/visitor101.py b/flake8_trio/visitors/visitor101.py index b771dcc2..53a819dd 100644 --- a/flake8_trio/visitors/visitor101.py +++ b/flake8_trio/visitors/visitor101.py @@ -60,6 +60,7 @@ def visit_FunctionDef(self, node: cst.FunctionDef): node, "contextmanager", "asynccontextmanager", "fixture" ) + # trigger on leaving yield so any comments are parsed for noqas def visit_Yield(self, node: cst.Yield): if self._yield_is_error: self.error(node) diff --git a/flake8_trio/visitors/visitor91x.py b/flake8_trio/visitors/visitor91x.py index 64f13199..77ffd579 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,8 +199,8 @@ def __init__( def library(self) -> tuple[str, ...]: return self.__library if self.__library else ("trio",) - def should_autofix(self) -> bool: - return True + def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool: + return not self.noautofix def leave_Yield( self, @@ -206,7 +209,9 @@ def leave_Yield( ) -> cst.Yield: # Needs to be passed *original* node, since updated node is a copy # which loses identity equality - if original_node in self.nodes_needing_checkpoint and not self.noautofix: + if original_node in self.nodes_needing_checkpoint and self.should_autofix( + original_node + ): self.add_statement = checkpoint_statement(self.library[0]) return updated_node @@ -239,8 +244,10 @@ 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 not self.noautofix and 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 +297,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 @@ -327,17 +334,20 @@ def check_function_exit( # Add this as a node potentially needing checkpoints only if it # missing checkpoints solely depends on whether the artificial statement is # "real" - if len(self.uncheckpointed_statements) == 1 and not self.noautofix: + if len(self.uncheckpointed_statements) == 1 and self.should_autofix( + original_node + ): 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 +367,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 +375,7 @@ def error_91x( else: msg = node.__class__.__name__.lower() - self.error( + return self.error( node, msg, statement, @@ -403,7 +413,9 @@ def leave_Yield( return updated_node self.has_yield = True - if self.check_function_exit(original_node) and not self.noautofix: + if self.check_function_exit(original_node) and self.should_autofix( + original_node + ): self.add_statement = self.checkpoint_statement() # mark as requiring checkpoint after @@ -596,8 +608,7 @@ def leave_While_body(self, node: cst.For | cst.While): ): if stmt == ARTIFICIAL_STATEMENT: continue - any_error = True - self.error_91x(err_node, stmt) + any_error |= self.error_91x(err_node, stmt) # if there's no errors from artificial statements, we don't need to insert # the potential checkpoints @@ -664,7 +675,7 @@ 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: transformer = InsertCheckpointsInLoopBody( self.loop_state.nodes_needing_checkpoints, self.library ) diff --git a/pyproject.toml b/pyproject.toml index c7eccf1d..a0f7ec0d 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 00000000..c411f992 --- /dev/null +++ b/tests/autofix_files/noqa.py @@ -0,0 +1,68 @@ +# 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 + +# check that noqas work after line numbers have been modified in a different visitor + +# this will remove one line +# TRIO100: 5, 'trio', 'fail_after' +... + + +async def foo_changed_lineno(): + yield # noqa: TRIO911 + await trio.lowlevel.checkpoint() + + +# this will add two lines +async def foo_changing_lineno(): # TRIO911: 0, "exit", Statement("yield", lineno+1) + await trio.lowlevel.checkpoint() + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) + await trio.lowlevel.checkpoint() + + +with trio.fail_after(5): # noqa: TRIO100 + ... diff --git a/tests/autofix_files/noqa.py.diff b/tests/autofix_files/noqa.py.diff new file mode 100644 index 00000000..2fd89e1d --- /dev/null +++ b/tests/autofix_files/noqa.py.diff @@ -0,0 +1,52 @@ +--- ++++ +@@ 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() + + +@@ x,8 x,8 @@ + # check that noqas work after line numbers have been modified in a different visitor + + # this will remove one line +-with trio.fail_after(5): # TRIO100: 5, 'trio', 'fail_after' +- ... ++# TRIO100: 5, 'trio', 'fail_after' ++... + + + async def foo_changed_lineno(): +@@ x,7 x,9 @@ + + # this will add two lines + async def foo_changing_lineno(): # TRIO911: 0, "exit", Statement("yield", lineno+1) ++ await trio.lowlevel.checkpoint() + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) ++ await trio.lowlevel.checkpoint() + + + with trio.fail_after(5): # noqa: TRIO100 diff --git a/tests/autofix_files/noqa_testing.py b/tests/autofix_files/noqa_testing.py new file mode 100644 index 00000000..4ecc03f7 --- /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 00000000..a154297c --- /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 6995ef43..59516dcd 100644 --- a/tests/eval_files/noqa.py +++ b/tests/eval_files/noqa.py @@ -1,63 +1,64 @@ -# 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 +# check that noqas work after line numbers have been modified in a different visitor -# errors from AST visitors -async def foo() -> Any: +# this will remove one line +with trio.fail_after(5): # TRIO100: 5, 'trio', 'fail_after' ... -async def foo_no_noqa_102(): - try: - pass - finally: - await foo() # TRIO102: 8, Statement("try/finally", lineno-3) +async def foo_changed_lineno(): + yield # noqa: TRIO911 + await trio.lowlevel.checkpoint() -async def foo_noqa_102(): - try: - pass - finally: - await foo() # noqa: TRIO102 +# this will add two lines +async def foo_changing_lineno(): # TRIO911: 0, "exit", Statement("yield", lineno+1) + yield # TRIO911: 4, "yield", Statement("function definition", lineno-1) -async def foo_bare_noqa_102(): - try: - pass - finally: - await foo() # noqa +with trio.fail_after(5): # noqa: TRIO100 + ... diff --git a/tests/eval_files/noqa_no_autofix.py b/tests/eval_files/noqa_no_autofix.py new file mode 100644 index 00000000..8dcc56be --- /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 00000000..7c4b230d --- /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 81f52a97..eaf85333 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/eval_files/trio22x.py b/tests/eval_files/trio22x.py index 2b90957a..46f73923 100644 --- a/tests/eval_files/trio22x.py +++ b/tests/eval_files/trio22x.py @@ -3,6 +3,9 @@ async def foo(): + await async_fun( + subprocess.getoutput() # TRIO221: 8, 'subprocess.getoutput', "trio" + ) subprocess.Popen() # TRIO220: 4, 'subprocess.Popen', "trio" os.system() # TRIO221: 4, 'os.system', "trio" diff --git a/tests/test_config_and_args.py b/tests/test_config_and_args.py index 246661f4..e8238603 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 bc370a17..d5ab0d8a 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,10 @@ def test_eval( # if substituting we're messing up columns ignore_column = True + if noqa: + # 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") @@ -242,7 +259,7 @@ def test_eval( message = error.message.format(*error.args) assert "anyio" in message or "trio" not in message - if autofix: + if autofix and not noqa: check_autofix(test, plugin, content, generate_autofix, anyio=anyio) else: # make sure content isn't modified @@ -266,7 +283,7 @@ def test_autofix(test: str): _ = assert_expected_errors(plugin, args=parsed_args) diff = diff_strings(plugin.module.code, content) - if diff: + if diff.strip(): print(diff) assert plugin.module.code == content, "autofixed file changed when autofixed again" @@ -579,7 +596,7 @@ def info_tuple(error: Error): # eval_files tests check that noqa is respected when running as standalone, but # they don't check anything when running as plugin. # When run as a plugin, flake8 will handle parsing of `noqa`. -def test_noqa_respected_depending_on_standalone(tmp_path: Path): +def test_noqa_respected_depending_on_standalone(): text = """import trio with trio.move_on_after(10): ... # noqa """ @@ -593,6 +610,22 @@ def test_noqa_respected_depending_on_standalone(tmp_path: Path): assert len(tuple(plugin.run())) == 1 +# TODO: failing test due to issue #193 +# the != in the assert should be a == +def test_line_numbers_match_end_result(): + text = """import trio +with trio.move_on_after(10): + ... + +trio.sleep(0) +""" + plugin = Plugin.from_source(text) + initialize_options(plugin, args=["--enable=TRIO100,TRIO115", "--autofix=TRIO100"]) + errors = tuple(plugin.run()) + plugin.module.code + assert errors[1].line != plugin.module.code.split("\n").index("trio.sleep(0)") + 1 + + @pytest.mark.fuzz() def test_910_permutations(): """Tests all possible permutations for TRIO910. From 0c24e4bc1ff55cdc86ba007c17c27a11d6205f07 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 May 2023 11:08:18 +0200 Subject: [PATCH 2/2] pyright: exclude eval&autofix files due to new errors from update --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index a0f7ec0d..b51de0e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ warn_unreachable = true warn_unused_ignores = false [tool.pyright] -exclude = ["**/node_modules", "**/__pycache__", "**/.*"] +exclude = ["**/node_modules", "**/__pycache__", "**/.*", "tests/eval_files/*", "tests/autofix_files/*"] # TODO: fix errors in eval/autofix files reportCallInDefaultInitializer = true reportImplicitStringConcatenation = false # black generates implicit string concats reportMissingSuperCall = true