add pre-commit config, which replaces and extends 'tox -e check' adding

several flake8 plugins
pyright is not run with the pre-commit ci, since it requires an internet
connection and the free tier for FOSS projects does not allow it. So
it's called by tox in a separate GitHub action
also fixes several minor style violations reported by those plugins, and
adding a few comments

.github/workflows/ci.yml

@@ -7,7 +7,7 @@ on:
     branches: [ main ]
 
 jobs:
-  check:
+  typing:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
@@ -18,11 +18,10 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip setuptools tox
-        python -m tox --notest --recreate -e check
-    - name: Run checks
+        python -m tox --notest --recreate -e typing
+    - name: Run type checks
       run: |
-        python -m tox -e check
-        git diff --exit-code
+        python -m tox -e typing
 
   test:
     runs-on: ubuntu-latest
@@ -45,7 +44,7 @@ jobs:
 
   release:
     runs-on: ubuntu-latest
-    needs: [check, test]
+    needs: [typing, test]
     if: github.repository == 'Zac-HD/flake8-trio' &&  github.ref == 'refs/heads/main'
     steps:
     - uses: actions/checkout@v3

.pre-commit-config.yaml

@@ -0,0 +1,67 @@
+ci:
+  skip: [pyright]
+
+repos:
+-   repo: https://github.com/Zac-HD/shed
+    rev: 0.10.8
+    hooks:
+    -   id: shed
+        args: ['--py39-plus']
+
+-   repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.286
+    hooks:
+    -   id: pyright
+        entry: env PYRIGHT_PYTHON_FORCE_VERSION=latest pyright
+        args: ['--pythonversion=3.11', '--warnings']
+        additional_dependencies:
+          # Required for pyright strict mode
+          - hypothesis
+          - hypothesmith
+          - pytest
+          - flake8
+
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
+    hooks:
+    -   id: trailing-whitespace
+        args: ['--markdown-linebreak-ext=md,markdown']
+    -   id: end-of-file-fixer
+    -   id: fix-encoding-pragma
+        args: [--remove]
+    -   id: check-yaml
+    -   id: debug-statements
+        language_version: python3
+
+-   repo: https://github.com/PyCQA/flake8
+    rev: 6.0.0
+    hooks:
+    -   id: flake8
+        args: ["--exclude", ".*,tests/trio*.py"]
+        language_version: python3
+        additional_dependencies:
+          - flake8-builtins
+          - flake8-bugbear
+          - flake8-comprehensions
+          - flake8-2020
+          - flake8-bandit
+          - flake8-builtins
+          - flake8-bugbear
+          - flake8-comprehensions
+          - flake8-datetimez
+         #- flake8-docstrings
+          - flake8-mutable
+          - flake8-noqa
+          - flake8-pie
+          - flake8-pytest-style
+          - flake8-return
+          - flake8-simplify
+
+-   repo: https://github.com/PyCQA/flake8
+    rev: 5.0.4
+    hooks:
+    -   id: flake8
+        args: ["--exclude", ".*,tests/trio*.py", "--select=E800"]
+        language_version: python3
+        additional_dependencies:
+          - flake8-eradicate

flake8_trio.py

@@ -49,7 +49,7 @@ def __eq__(self, other: Any) -> bool:
 
 # convenience function used in a lot of visitors
 def get_matching_call(
-    node: ast.AST, *names: str, base: str = "trio"
+    node: ast.AST, *names: str, base: int = "trio"
 ) -> tuple[ast.Call, str] | None:
     if (
         isinstance(node, ast.Call)
@@ -274,12 +274,11 @@ def walk(self, *body: ast.AST) -> Iterable[ast.AST]:
 # ignores module and only checks the unqualified name of the decorator
 # used in 101 and 107/108
 def has_decorator(decorator_list: list[ast.expr], *names: str):
-    for dec in decorator_list:
-        if (isinstance(dec, ast.Name) and dec.id in names) or (
-            isinstance(dec, ast.Attribute) and dec.attr in names
-        ):
-            return True
-    return False
+    return any(
+        (isinstance(dec, ast.Name) and dec.id in names)
+        or (isinstance(dec, ast.Attribute) and dec.attr in names)
+        for dec in decorator_list
+    )
 
 
 # matches the fully qualified name against fnmatch pattern
@@ -612,17 +611,18 @@ def visit_For(self, node: ast.For | ast.While):
         if not self.unraised:
             return
 
+        # the following block is duplicated in Visitor107_108
         infinite_loop = False
         if isinstance(node, ast.While):
             try:
                 infinite_loop = body_guaranteed_once = bool(ast.literal_eval(node.test))
-            except Exception:
+            except Exception:  # noqa: PIE786
                 body_guaranteed_once = False
             self.visit_nodes(node.test)
         else:
-            body_guaranteed_once = iter_guaranteed_once(node.iter)
             self.visit_nodes(node.target)
             self.visit_nodes(node.iter)
+            body_guaranteed_once = iter_guaranteed_once(node.iter)
 
         self.save_state(node, "unraised_break", "unraised_continue")
         self.unraised_break = False
@@ -667,11 +667,10 @@ def iter_guaranteed_once(iterable: ast.expr) -> bool:
             else:
                 return True
         return False
+
     if isinstance(iterable, ast.Constant):
-        try:
-            return len(iterable.value) > 0
-        except Exception:
-            return False
+        return hasattr(iterable.value, "__len__") and len(iterable.value) > 0
+
     if isinstance(iterable, ast.Dict):
         for key, val in zip(iterable.keys, iterable.values):
             # {**{...}, **{<...>}} is parsed as {None: {...}, None: {<...>}}
@@ -688,7 +687,7 @@ def iter_guaranteed_once(iterable: ast.expr) -> bool:
     ):
         try:
             return len(range(*[ast.literal_eval(a) for a in iterable.args])) > 0
-        except Exception:
+        except Exception:  # noqa: PIE786
             return False
     return False
 
@@ -975,11 +974,13 @@ def visit_loop(self, node: ast.While | ast.For | ast.AsyncFor):
         if not self.async_function:
             return
         # visit condition
+
+        # the following block is duplicated in Visitor103_104
         infinite_loop = False
         if isinstance(node, ast.While):
             try:
                 infinite_loop = body_guaranteed_once = bool(ast.literal_eval(node.test))
-            except Exception:
+            except Exception:  # noqa: PIE786
                 body_guaranteed_once = False
             self.visit_nodes(node.test)
         else:

tests/conftest.py

@@ -30,6 +30,6 @@ def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item
             item.add_marker(skip_fuzz)
 
 
-@pytest.fixture
+@pytest.fixture()
 def enable_visitor_codes_regex(request: pytest.FixtureRequest):
     return request.config.getoption("--enable-visitor-codes-regex")

tests/test_decorator.py

@@ -12,8 +12,7 @@ def dec_list(*decorators: str) -> ast.Module:
     for dec in decorators:
         source += f"@{dec}\n"
     source += "async def f():\n  bar()"
-    tree = ast.parse(source)
-    return tree
+    return ast.parse(source)
 
 
 def wrap(decorators: tuple[str, ...], decs2: str) -> str | None:
@@ -86,8 +85,7 @@ def test_pep614():
 
 def test_command_line_1(capfd):
     Application().run(common_flags + ["--no-checkpoint-warning-decorators=app.route"])
-    out, err = capfd.readouterr()
-    assert not out and not err
+    assert capfd.readouterr() == ("", "")
 
 
 expected_out = (
@@ -101,11 +99,9 @@ def test_command_line_1(capfd):
 
 def test_command_line_2(capfd):
     Application().run(common_flags + ["--no-checkpoint-warning-decorators=app"])
-    out, err = capfd.readouterr()
-    assert out == expected_out and not err
+    assert capfd.readouterr() == (expected_out, "")
 
 
 def test_command_line_3(capfd):
     Application().run(common_flags)
-    out, err = capfd.readouterr()
-    assert out == expected_out and not err
+    assert capfd.readouterr() == (expected_out, "")

tests/test_flake8_trio.py

@@ -6,7 +6,7 @@
 import os
 import re
 import site
-import subprocess
+import subprocess  # noqa: S404
 import sys
 import tokenize
 import unittest
@@ -54,7 +54,7 @@ def check_version(test: str):
         major, minor = version_str[0], version_str[1:]
         v_i = sys.version_info
         if (v_i.major, v_i.minor) < (int(major), int(minor)):
-            raise unittest.SkipTest("v_i, major, minor")
+            pytest.skip(f"python version {v_i} smaller than {major}, {minor}")
 
 
 ERROR_CODES = {
@@ -64,7 +64,7 @@ def check_version(test: str):
 }
 
 
-@pytest.mark.parametrize("test, path", test_files)
+@pytest.mark.parametrize(("test", "path"), test_files)
 def test_eval(test: str, path: str):
     # version check
     check_version(test)
@@ -104,7 +104,7 @@ def test_eval(test: str, path: str):
                 # Append a bunch of empty strings so string formatting gives garbage
                 # instead of throwing an exception
                 try:
-                    args = eval(
+                    args = eval(  # noqa: S307
                         f"[{err_args}]",
                         {
                             "lineno": lineno,
@@ -167,8 +167,7 @@ def test_eval(test: str, path: str):
 
 class SyncTransformer(ast.NodeTransformer):
     def visit_Await(self, node: ast.Await):
-        newnode = self.generic_visit(node.value)
-        return newnode
+        return self.generic_visit(node.value)
 
     def replace_async(self, node: ast.AST, target: type[ast.AST]) -> ast.AST:
         node = self.generic_visit(node)
@@ -186,7 +185,7 @@ def visit_AsyncFor(self, node: ast.AST):
         return self.replace_async(node, ast.For)
 
 
-@pytest.mark.parametrize("test, path", test_files)
+@pytest.mark.parametrize(("test", "path"), test_files)
 def test_noerror_on_sync_code(test: str, path: str):
     if any(e in test for e in error_codes_ignored_when_checking_transformed_sync_code):
         return
@@ -233,7 +232,7 @@ def assert_expected_errors(
     assert_correct_args(errors, expected_)
 
     # full check
-    unittest.TestCase().assertEqual(errors, expected_)
+    assert errors == expected_
 
     # test tuple conversion and iter types
     assert_tuple_and_types(errors, expected_)
@@ -365,24 +364,50 @@ def info_tuple(error: Error):
 
 
 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.
+    """
+    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.
+
+    generates code that looks like this, where a block content of `None` means the
+    block is excluded:
+
+    async def foo():
+        try:
+            await foo() | ...
+        except ValueError:
+            await foo() | ... | raise | return | None
+        except SyntaxError:
+            await foo() | ... | raise | return | None
+        except:
+            await foo() | ... | raise | return | None
+        else:
+            await foo() | ... | return | None
+        finally:
+            await foo() | ... | return | None
+    """
     plugin = Plugin(ast.AST())
     initialize_options(plugin, args=["--enable-visitor-codes-regex=TRIO107"])
 
     check = "await foo()"
+
+    # loop over all the possible content of the different blocks
     for try_, exc1, exc2, bare_exc, else_, finally_ in itertools.product(
-        (check, "..."),
-        (check, "...", "raise", "return", None),
-        (check, "...", "raise", "return", None),
-        (check, "...", "raise", "return", None),
-        (check, "...", "return", None),
-        (check, "...", "return", None),
+        (check, "..."),  # try_
+        (check, "...", "raise", "return", None),  # exc1
+        (check, "...", "raise", "return", None),  # exc2
+        (check, "...", "raise", "return", None),  # bare_exc
+        (check, "...", "return", None),  # else_
+        (check, "...", "return", None),  # finally_
     ):
+        # exclude duplicate tests where there's a second exception block but no first
         if exc1 is None and exc2 is not None:
             continue
 
+        # syntax error if there's no exception block but there's finally and/or else
+        if exc1 is exc2 is bare_exc is None and (finally_ is None or else_ is not None):
+            continue
+
         function_str = f"async def foo():\n  try:\n    {try_}\n"
 
         for arg, val in {
@@ -395,13 +420,7 @@ def test_107_permutations():
             if val is not None:
                 function_str += f"  {arg}:\n    {val}\n"
 
-        try:
-            tree = ast.parse(function_str)
-        except Exception:
-            assert exc1 is exc2 is bare_exc is None and (
-                finally_ is None or else_ is not None
-            )
-            return
+        tree = ast.parse(function_str)
 
         # 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`.
@@ -497,7 +516,9 @@ def test_200_from_config_flake8_internals(
 
 def test_200_from_config_subprocess(tmp_path: Path):
     err_msg = _test_trio200_from_config_common(tmp_path)
-    res = subprocess.run(["flake8"], cwd=tmp_path, capture_output=True)
+    res = subprocess.run(  # noqa: S603,S607
+        ["flake8"], cwd=tmp_path, capture_output=True
+    )
     assert not res.stderr
     assert res.stdout == err_msg.encode("ascii")
 
@@ -507,7 +528,7 @@ def consume(iterator: Iterable[Any]):
     deque(iterator, maxlen=0)
 
 
-@pytest.mark.fuzz
+@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))
@@ -540,7 +561,7 @@ def _iter_python_files():
                     yield Path(dirname) / f
 
 
-@pytest.mark.fuzz
+@pytest.mark.fuzz()
 def test_does_not_crash_on_site_code(enable_visitor_codes_regex: str):
     for path in _iter_python_files():
         try: