runner now skips unselected visitors, added --select pytest option, f…

…ixed code coverage & tests, renamed ARGS to ARG, generated required type stubs

CONTRIBUTING.md

@@ -36,10 +36,9 @@ Lines containing `error:` are parsed as expecting an error of the code matching
 #### `TRIOxxx:`
 You can instead of `error` specify the error code.
 
-### `# INCLUDE`
-Test files by default filter out all errors not matching the file name, but if there's a line `# INCLUDE TRIO\d\d\d TRIO\d\d\d` those additional error codes are not filtered out and will be an error if encountered.
-### `# ARGS`
-With a line `# ARGS` you can also specify command-line arguments that should be passed to the plugin when parsing a file. Can be specified multiple times for several different arguments.
+### `# ARG`
+With `# ARG` lines you can also specify command-line arguments that should be passed to the plugin when parsing a file. Can be specified multiple times for several different arguments.  
+Generated tests will by default `--select` the error code of the file, which will enable any visitors that can generate that code (and if those visitors can raise other codes they might be raised too). This can be overriden by adding an `# ARG --select=...` line.
 
 ## Style Guide
 

flake8_trio.py

@@ -21,6 +21,7 @@
 from typing import Any, NamedTuple, Union, cast
 
 from flake8.options.manager import OptionManager
+from flake8.style_guide import Decision, DecisionEngine
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
 __version__ = "22.12.4"
@@ -97,12 +98,24 @@ class Flake8TrioRunner(ast.NodeVisitor):
     def __init__(self, options: Namespace):
         super().__init__()
         self._problems: list[Error] = []
+        self.decision_engine: DecisionEngine = DecisionEngine(options)
+
         self.visitors = {
             v(options, self._problems)
             for v in Flake8TrioVisitor.__subclasses__()
-            # TODO: could here refrain from including subclasses for disabled checks
+            if self.selected(v.error_codes)
         }
 
+    # Use flake8's internal decision engine to check if codes are included or not
+    # to ease debugging and speed up plugin time
+    # This isn't officially documented or supported afaik, but should be easily
+    # removed upon any breaking changes.
+    def selected(self, error_codes: dict[str, str]) -> bool:
+        return any(
+            self.decision_engine.decision_for(code) == Decision.Selected
+            for code in error_codes
+        )
+
     @classmethod
     def run(cls, tree: ast.AST, options: Namespace) -> Iterable[Error]:
         runner = cls(options)

tests/conftest.py

@@ -7,6 +7,9 @@ def pytest_addoption(parser: pytest.Parser):
     parser.addoption(
         "--runfuzz", action="store_true", default=False, help="run fuzz tests"
     )
+    parser.addoption(
+        "--select", default="TRIO", help="select error codes whose visitors to run."
+    )
 
 
 def pytest_configure(config: pytest.Config):
@@ -23,3 +26,8 @@ def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item
     for item in items:
         if "fuzz" in item.keywords:
             item.add_marker(skip_fuzz)
+
+
+@pytest.fixture
+def select(request: pytest.FixtureRequest):
+    return request.config.getoption("--select")

tests/test_decorator.py

@@ -3,9 +3,8 @@
 import ast
 
 from flake8.main.application import Application
-from test_flake8_trio import _default_option_manager
 
-from flake8_trio import Plugin, Statement, Visitor107_108, fnmatch_qualified_name
+from flake8_trio import Statement, Visitor107_108, fnmatch_qualified_name
 
 
 def dec_list(*decorators: str) -> ast.Module:
@@ -82,22 +81,6 @@ def test_pep614():
     assert not wrap(("(any, expression, we, like)",), "no match here")
 
 
-def test_plugin():
-    tree = dec_list("app.route")
-    plugin = Plugin(tree)
-
-    om = _default_option_manager()
-    plugin.add_options(om)
-
-    plugin.parse_options(om.parse_args(args=[]))
-    assert tuple(plugin.run())
-
-    arg = "--no-checkpoint-warning-decorators=app.route"
-    plugin.parse_options(om.parse_args(args=[arg]))
-
-    assert not tuple(plugin.run())
-
-
 common_flags = ["--select=TRIO", "tests/trio_options.py"]
 
 

tests/test_flake8_trio.py

@@ -10,12 +10,14 @@
 import sys
 import tokenize
 import unittest
+from collections import deque
 from collections.abc import Iterable, Sequence
 from pathlib import Path
-from typing import DefaultDict
+from typing import Any, DefaultDict
 
 import pytest
 from flake8 import __version_info__ as flake8_version_info
+from flake8.main.options import register_default_options
 from flake8.options.manager import OptionManager
 
 # import trio  # type: ignore
@@ -42,7 +44,13 @@ def _default_option_manager():
     kwargs = {}
     if flake8_version_info[0] >= 6:
         kwargs["formatter_names"] = ["default"]
-    return OptionManager(version="", plugin_versions="", parents=[], **kwargs)
+    option_manager = OptionManager(version="", plugin_versions="", parents=[], **kwargs)
+
+    # register `--select`,`--ignore`,etc needed for the DecisionManager
+    # and to be able to selectively enable/disable visitors
+    register_default_options(option_manager)
+
+    return option_manager
 
 
 # check for presence of _pyXX, skip if version is later, and prune parameter
@@ -66,14 +74,19 @@ def check_version(test: str) -> str:
 
 
 @pytest.mark.parametrize("test, path", test_files)
-def test_eval(test: str, path: str):
+def test_eval(test: str, path: str, select: str):
     # version check
     test = check_version(test)
 
     assert test in ERROR_CODES, "error code not defined in flake8_trio.py"
 
-    include = [test]
-    parsed_args = [""]
+    parsed_args = []
+
+    if select in test or test in select:
+        # `select` this error code to enable the visitor for it, if the test file
+        # specifies `# ARG --select=...` that will take precedence.
+        parsed_args = [f"--select={test}"]
+
     expected: list[Error] = []
 
     with open(os.path.join("tests", path), encoding="utf-8") as file:
@@ -82,14 +95,8 @@ def test_eval(test: str, path: str):
     for lineno, line in enumerate(lines, start=1):
         line = line.strip()
 
-        # add other error codes to check if #INCLUDE is specified
-        if reg_match := re.search(r"(?<=INCLUDE).*", line):
-            for other_code in reg_match.group().split(" "):
-                if other_code.strip():
-                    include.append(other_code.strip())
-
-        # add command-line args if specified with #ARGS
-        elif reg_match := re.search(r"(?<=ARGS).*", line):
+        # add command-line args if specified with #ARG
+        if reg_match := re.search(r"(?<=ARG ).*", line):
             for arg in reg_match.group().split(" "):
                 if arg.strip():
                     parsed_args.append(arg.strip())
@@ -144,9 +151,11 @@ def test_eval(test: str, path: str):
                 raise ParseError(msg) from e
 
     assert expected, f"failed to parse any errors in file {path}"
+    if not any("--select" in arg for arg in parsed_args):
+        pytest.skip("no `--select`ed visitors")
 
     plugin = read_file(path)
-    assert_expected_errors(plugin, include, *expected, args=parsed_args)
+    assert_expected_errors(plugin, *expected, args=parsed_args)
 
 
 # Codes that are supposed to also raise errors when run on sync code, and should
@@ -189,17 +198,19 @@ def visit_AsyncFor(self, node: ast.AST):
 
 
 @pytest.mark.parametrize("test, path", test_files)
-def test_noerror_on_sync_code(test: str, path: str):
+def test_noerror_on_sync_code(test: str, path: str, select: str):
     if any(e in test for e in error_codes_ignored_when_checking_transformed_sync_code):
         return
     with tokenize.open(f"tests/{path}") as f:
         source = f.read()
     tree = SyncTransformer().visit(ast.parse(source))
 
+    ignored_codes_string = ",".join(
+        error_codes_ignored_when_checking_transformed_sync_code
+    )
     assert_expected_errors(
         Plugin(tree),
-        include=error_codes_ignored_when_checking_transformed_sync_code,
-        invert_include=True,
+        args=[f"--select={select}", f"--ignore={ignored_codes_string}"],
     )
 
 
@@ -208,23 +219,26 @@ def read_file(test_file: str):
     return Plugin.from_filename(str(filename))
 
 
+def initialize_options(plugin: Plugin, args: list[str] | None = None):
+    om = _default_option_manager()
+    plugin.add_options(om)
+    plugin.parse_options(om.parse_args(args=(args if args else [])))
+
+    # these are not registered like flake8's normal options so we need
+    # to manually initialize them for DecisionEngine to work.
+    plugin.options.extended_default_select = []
+    plugin.options.extended_default_ignore = []
+
+
 def assert_expected_errors(
     plugin: Plugin,
-    include: Iterable[str],
     *expected: Error,
     args: list[str] | None = None,
-    invert_include: bool = False,
 ):
     # initialize default option values
-    om = _default_option_manager()
-    plugin.add_options(om)
-    plugin.parse_options(om.parse_args(args=(args if args else [""])))
+    initialize_options(plugin, args)
 
-    errors = sorted(
-        e
-        for e in plugin.run()
-        if (e.code in include if not invert_include else e.code not in include)
-    )
+    errors = sorted(e for e in plugin.run())
     expected_ = sorted(expected)
 
     print_first_diff(errors, expected_)
@@ -368,6 +382,9 @@ def test_107_permutations():
     # since each test is so fast, and there's so many permutations, manually doing
     # the permutations in a single test is much faster than the permutations from using
     # pytest parametrization - and does not clutter up the output massively.
+    plugin = Plugin(ast.AST())
+    initialize_options(plugin, args=["--select=TRIO107"])
+
     check = "await foo()"
     for try_, exc1, exc2, bare_exc, else_, finally_ in itertools.product(
         (check, "..."),
@@ -400,7 +417,10 @@ def test_107_permutations():
             )
             return
 
-        errors = [e for e in Plugin(tree).run() if e.code == "TRIO107"]
+        # not a type error per se, but it's pyright warning about assigning to a
+        # protected class member - hence we silence it with a `type: ignore`.
+        plugin._tree = tree  # type: ignore
+        errors = [e for e in plugin.run() if e.code == "TRIO107"]
 
         if (
             # return in exception
@@ -421,28 +441,20 @@ def test_107_permutations():
 
 def test_114_raises_on_invalid_parameter(capsys: pytest.CaptureFixture[str]):
     plugin = Plugin(ast.AST())
-    om = _default_option_manager()
-    plugin.add_options(om)
     # flake8 will reraise ArgumentError as SystemExit
     for arg in "blah.foo", "foo*", "*":
         with pytest.raises(SystemExit):
-            plugin.parse_options(
-                om.parse_args(args=[f"--startable-in-context-manager={arg}"])
-            )
+            initialize_options(plugin, args=[f"--startable-in-context-manager={arg}"])
         out, err = capsys.readouterr()
         assert not out
         assert f"{arg!r} is not a valid method identifier" in err
 
 
 def test_200_options(capsys: pytest.CaptureFixture[str]):
     plugin = Plugin(ast.AST())
-    om = _default_option_manager()
-    plugin.add_options(om)
     for i, arg in (0, "foo"), (2, "foo->->bar"), (2, "foo->bar->fee"):
         with pytest.raises(SystemExit):
-            plugin.parse_options(
-                om.parse_args(args=[f"--trio200-blocking-calls={arg}"])
-            )
+            initialize_options(plugin, args=[f"--trio200-blocking-calls={arg}"])
         out, err = capsys.readouterr()
         assert not out
         assert all(word in err for word in (str(i), arg, "->"))
@@ -473,30 +485,48 @@ async def foo():
     )
 
 
+# from https://docs.python.org/3/library/itertools.html#itertools-recipes
+def consume(iterator: Iterable[Any]):
+    deque(iterator, maxlen=0)
+
+
 @pytest.mark.fuzz
 class TestFuzz(unittest.TestCase):
     @settings(max_examples=1_000, suppress_health_check=[HealthCheck.too_slow])
     @given((from_grammar() | from_node()).map(ast.parse))
     def test_does_not_crash_on_any_valid_code(self, syntax_tree: ast.AST):
+        # TODO: figure out how to get unittest to play along with pytest options
+        # so `--select` can be passed through
+        # though I barely notice a difference manually changing this value, or even
+        # not running the plugin at all, so overhead looks to be vast majority of runtime
+        select = "TRIO"
+
         # Given any syntatically-valid source code, the checker should
         # not crash.  This tests doesn't check that we do the *right* thing,
         # just that we don't crash on valid-if-poorly-styled code!
-        Plugin(syntax_tree).run()
-
-    @staticmethod
-    def _iter_python_files():
-        # Because the generator isn't perfect, we'll also test on all the code
-        # we can easily find in our current Python environment - this includes
-        # the standard library, and all installed packages.
-        for base in sorted(set(site.PREFIXES)):
-            for dirname, _, files in os.walk(base):
-                for f in files:
-                    if f.endswith(".py"):
-                        yield Path(dirname) / f
-
-    def test_does_not_crash_on_site_code(self):
-        for path in self._iter_python_files():
-            try:
-                Plugin.from_filename(str(path)).run()
-            except Exception as err:
-                raise AssertionError(f"Failed on {path}") from err
+        plugin = Plugin(syntax_tree)
+        initialize_options(plugin, [f"--select={select}"])
+
+        consume(plugin.run())
+
+
+def _iter_python_files():
+    # Because the generator isn't perfect, we'll also test on all the code
+    # we can easily find in our current Python environment - this includes
+    # the standard library, and all installed packages.
+    for base in sorted(set(site.PREFIXES)):
+        for dirname, _, files in os.walk(base):
+            for f in files:
+                if f.endswith(".py"):
+                    yield Path(dirname) / f
+
+
+@pytest.mark.fuzz
+def test_does_not_crash_on_site_code(select: str):
+    for path in _iter_python_files():
+        try:
+            plugin = Plugin.from_filename(str(path))
+            initialize_options(plugin, [f"--select={select}"])
+            consume(plugin.run())
+        except Exception as err:
+            raise AssertionError(f"Failed on {path}") from err

tests/trio103.py

@@ -1,3 +1,5 @@
+# ARG --select=TRIO103,TRIO104
+
 from typing import Any
 
 import trio
@@ -91,7 +93,7 @@ def foo() -> Any:
     except ValueError:
         raise e
     except:
-        raise e  # though sometimes okay, TRIO104
+        raise e  # TRIO104: 8
 except BaseException:  # safe
     try:
         pass
@@ -116,7 +118,7 @@ def foo() -> Any:
     try:
         pass
     except ValueError as g:
-        raise g  # TRIO104
+        raise g  # TRIO104: 8
     except BaseException as h:
         raise h  # error? currently treated as safe
     raise e