Skip to content

Commit

Permalink
Merge pull request #182 from jakkdl/noqa
Browse files Browse the repository at this point in the history
  • Loading branch information
Zac-HD authored Apr 30, 2023
2 parents a29608c + a0b40ad commit 5aaaaad
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 11 deletions.
3 changes: 2 additions & 1 deletion flake8_trio/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ def __iter__(self):
yield None

def cmp(self):
return self.line, self.col, self.code, self.args
# column may be ignored/modified when autofixing, so sort on that last
return self.line, self.code, self.args, self.col

# for sorting in tests
def __lt__(self, other: Any) -> bool:
Expand Down
9 changes: 8 additions & 1 deletion flake8_trio/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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)
Expand Down Expand Up @@ -129,4 +130,10 @@ def run(self) -> Iterable[Error]:
return
for v in (*self.utility_visitors, *self.visitors):
self.module = cst.MetadataWrapper(self.module).visit(v)
yield from self.state.problems

for problem in self.state.problems:
noqa = self.state.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
5 changes: 3 additions & 2 deletions flake8_trio/visitors/flake8triovisitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

class Flake8TrioVisitor(ast.NodeVisitor, ABC):
# abstract attribute by not providing a value
error_codes: dict[str, str] # pyright: reportUninitializedInstanceVariable=false
error_codes: dict[str, str] # pyright: ignore[reportUninitializedInstanceVariable]

def __init__(self, shared_state: SharedState):
super().__init__()
Expand Down Expand Up @@ -158,7 +158,7 @@ def add_library(self, name: str) -> None:

class Flake8TrioVisitor_cst(cst.CSTTransformer, ABC):
# abstract attribute by not providing a value
error_codes: dict[str, str] # pyright: reportUninitializedInstanceVariable=false
error_codes: dict[str, str] # pyright: ignore[reportUninitializedInstanceVariable]
METADATA_DEPENDENCIES = (PositionProvider,)

def __init__(self, shared_state: SharedState):
Expand All @@ -167,6 +167,7 @@ def __init__(self, shared_state: SharedState):
self.__state = shared_state

self.options = self.__state.options
self.noqas = self.__state.noqas

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
Expand Down
64 changes: 63 additions & 1 deletion flake8_trio/visitors/visitor_utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@
from __future__ import annotations

import ast
from typing import Any
import functools
import re
from typing import TYPE_CHECKING, 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):
Expand Down Expand Up @@ -134,3 +140,59 @@ 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<codes>([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
36 changes: 36 additions & 0 deletions tests/eval_files/noqa.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# NOAUTOFIX - TODO TODO TODO
# NOANYIO
# ARG --enable=TRIO100,TRIO911
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)
await trio.lowlevel.checkpoint()


async def foo_noqa_bare():
with trio.fail_after(5): yield # noqa
await trio.lowlevel.checkpoint()


async def foo_noqa_101():
with trio.fail_after(5): yield # noqa: TRIO100 # TRIO911: 29, "yield", Statement("function definition", lineno-1)
await trio.lowlevel.checkpoint()


async def foo_noqa_911():
with trio.fail_after(5): yield # noqa: TRIO911 # TRIO100: 9, 'trio', 'fail_after'
await trio.lowlevel.checkpoint()


async def foo_noqa_101_911():
with trio.fail_after(5): yield # noqa: TRIO100, TRIO911
await trio.lowlevel.checkpoint()


async def foo_noqa_101_911_500():
with trio.fail_after(5): yield # noqa: TRIO100, TRIO911 , TRIO500,,,
await trio.lowlevel.checkpoint()
# fmt: on
24 changes: 18 additions & 6 deletions tests/test_flake8_trio.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,13 @@ def check_autofix(
visited_code = plugin.module.code

if "# AUTOFIX" not in unfixed_code:
assert unfixed_code == visited_code
# if the file is specifically marked with NOAUTOFIX, that means it has visitors
# that will autofix with --autofix, but the file explicitly doesn't want to check
# the result of doing that. THIS IS DANGEROUS
if "# NOAUTOFIX" in unfixed_code:
print(f"eval file {test} marked with dangerous marker NOAUTOFIX")
else:
assert unfixed_code == visited_code
return

# the full generated source code, saved from a previous run
Expand Down Expand Up @@ -185,9 +191,9 @@ def find_magic_markers(

# This could be optimized not to reopen+reread+reparse the same file over and over
# when testing the same file
@pytest.mark.parametrize(("test", "path"), test_files)
@pytest.mark.parametrize("autofix", [False, True])
@pytest.mark.parametrize("anyio", [False, True])
@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"])
def test_eval(
test: str, path: Path, autofix: bool, anyio: bool, generate_autofix: bool
):
Expand All @@ -196,7 +202,9 @@ def test_eval(
if anyio and magic_markers.NOANYIO:
pytest.skip("file marked with NOANYIO")

ignore_column = False
# if autofixing, columns may get messed up
ignore_column = autofix

if magic_markers.NOTRIO:
if not anyio:
pytest.skip("file marked with NOTRIO")
Expand Down Expand Up @@ -241,6 +249,8 @@ def test_eval(
@pytest.mark.parametrize("test", autofix_files)
def test_autofix(test: str):
content = autofix_files[test].read_text()
assert content, "empty file"

if "# NOTRIO" in content:
pytest.skip("file marked with NOTRIO")

Expand Down Expand Up @@ -334,7 +344,9 @@ def _parse_eval_file(test: str, content: str) -> tuple[list[Error], list[str], s

enabled_codes_list = enabled_codes.split(",")
for code in enabled_codes_list:
assert re.fullmatch(r"TRIO\d\d\d", code)
assert re.fullmatch(
r"TRIO\d\d\d", code
), f"invalid code {code} in list {enabled_codes_list}"

for error in expected:
for code in enabled_codes.split(","):
Expand Down
3 changes: 3 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +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
# parsed as noqa comments
flake8_trio/visitors/visitor_utility.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

0 comments on commit 5aaaaad

Please sign in to comment.