Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add pre-commit config #88

Merged
merged 1 commit into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
branches: [ main ]

jobs:
check:
typing:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Expand All @@ -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
Expand All @@ -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
Expand Down
67 changes: 67 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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']
Comment on lines +13 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget the exact syntax here, but we'll want this to run on all-files rather than passing changed filenames, because changing one file can create typecheck errors in another unchanged file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the CI check it will run on all files (it runs pre-commit run pyright --all-files), but I personally prefer the commit hook not to run on all files as that comes with a decently hefty time penalty (5->10 seconds on my machine).

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
Comment on lines +43 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

(makes my pin-your-deps senses tingle, but no worse than the status quo I suppose... I really want better transitive-pins support from pre-commit but it's a pretty big project)


- 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
29 changes: 15 additions & 14 deletions flake8_trio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: {<...>}}
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
12 changes: 4 additions & 8 deletions tests/test_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 = (
Expand All @@ -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, "")
75 changes: 48 additions & 27 deletions tests/test_flake8_trio.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import os
import re
import site
import subprocess
import subprocess # noqa: S404
import sys
import tokenize
import unittest
Expand Down Expand Up @@ -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 = {
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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_)
Expand Down Expand Up @@ -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 {
Expand All @@ -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`.
Expand Down Expand Up @@ -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")

Expand All @@ -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))
Expand Down Expand Up @@ -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:
Expand Down
Loading