10000 add tests for autofix files, fix 91x autofix, and rejig test files to… · python-trio/flake8-async@bce4027 · GitHub
[go: up one dir, main page]

Skip to content

Commit bce4027

Browse files
committed
add tests for autofix files, fix 91x autofix, and rejig test files to accomodate the new tests
1 parent 220c405 commit bce4027

18 files changed

+398
-360
lines changed

flake8_trio/visitors/visitor91x.py

Lines changed: 113 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from __future__ import annotations
99

10+
from abc import ABC, abstractmethod
1011
from dataclasses import dataclass, field
1112
from typing import TYPE_CHECKING, Any
1213

@@ -90,9 +91,114 @@ def checkpoint_statement(library: str) -> cst.SimpleStatementLine:
9091
)
9192

9293

94+
class CommonVisitors(cst.CSTTransformer, ABC):
95+
def __init__(self):
96+
super().__init__()
97+
self.noautofix: bool = False
98+
# this one is not save-stated, but I fail to come up with any scenario
99+
# where that matters
100+
self.add_statement: cst.SimpleStatementLine | None = None
101+
102+
# these are file-wide, so intentionally not save-stated upon entry/exit
103+
# of functions/loops/etc
104+
self.explicitly_imported_library: dict[str, bool] = {
105+
"trio": False,
106+
"anyio": False,
107+
}
108+
self.add_import: set[str] = set()
109+
110+
self.booldepth = 0
111+
112+
@property
113+
@abstractmethod
114+
def library(self) -> tuple[str, ...]:
115+
...
116+
117+
# TODO: generate an error in these two if transforming+visiting is done in a single
118+
# pass and emit-error-on-transform can be enabled/disabled. The error can't be
119+
# generated in the yield/return since it doesn't know if it will be autofixed.
120+
121+
# instead of trying to exclude yields found in all the weird places from
122+
# setting self.add_statement, we instead clear it upon each new line
123+
def visit_SimpleStatementLine(self, node: cst.SimpleStatementLine):
124+
self.add_statement = None
125+
126+
# insert checkpoint before yield with a flattensentinel, if indicated
127+
def leave_SimpleStatementLine(
128+
self,
129+
original_node: cst.SimpleStatementLine,
130+
updated_node: cst.SimpleStatementLine,
131+
) -> cst.SimpleStatementLine | cst.FlattenSentinel[cst.SimpleStatementLine]:
132+
if self.add_statement is None:
133+
return updated_node
134+
curr_add_statement = self.add_statement
135+
self.add_statement = None
136+
137+
# multiple statements on a single line is not handled
138+
# yields in boolops are also not caught by any of the other scenarios
139+
if len(updated_node.body) > 1:
140+
return updated_node
141+
142+
self.ensure_imported_library()
143+
return cst.FlattenSentinel([curr_add_statement, updated_node])
144+
145+
def visit_BooleanOperation(self, node: cst.BooleanOperation):
146+
self.booldepth += 1
147+
self.noautofix = True
148+
149+
def leave_BooleanOperation(
150+
self, original_node: cst.BooleanOperation, updated_node: cst.BooleanOperation
151+
):
152+
assert self.booldepth
153+
self.booldepth -= 1
154+
if not self.booldepth:
155+
self.noautofix = False
156+
return updated_node
157+
158+
def ensure_imported_library(self) -> None:
159+
"""Mark library for import.
160+
161+
Check that the library we'd use to insert checkpoints
162+
is imported - if not, mark it to be inserted later.
163+
"""
164+
assert self.library
165+
if not self.explicitly_imported_library[self.library[0]]:
166+
self.add_import.add(self.library[0])
167+
168+
169+
# necessary as we don't know whether to insert checkpoints on the first pass of a loop
170+
# so we transform the loop body afterwards
171+
class InsertCheckpointsInLoopBody(CommonVisitors):
172+
def __init__(
173+
self,
174+
nodes_needing_checkpoint: Sequence[cst.Yield | cst.Return],
175+
library: tuple[str, ...],
176+
):
177+
super().__init__()
178+
self.nodes_needing_checkpoint = nodes_needing_checkpoint
179+
self.__library = library
180+
181+
@property
182+
def library(self) -> tuple[str, ...]:
183+
return self.__library if self.__library else ("trio",)
184+
185+
def leave_Yield(
186+
self,
187+
original_node: cst.Yield,
188+
updated_node: cst.Yield,
189+
) -> cst.Yield:
190+
# we need to check *original* node here, since updated node is a copy
191+
# which loses identity equality
192+
if original_node in self.nodes_needing_checkpoint and not self.noautofix:
193+
self.add_statement = checkpoint_statement(self.library[0])
194+
return updated_node
195+
196+
leave_Return = leave_Yield # type: ignore
197+
198+
93199
@error_class_cst
94200
@disabled_by_default
95-
class Visitor91X(Flake8TrioVisitor_cst):
201+
class Visitor91X(Flake8TrioVisitor_cst, CommonVisitors):
96202
error_codes = {
97203
"TRIO910": (
98204
"{0} from async function with no guaranteed checkpoint or exception "
@@ -112,18 +218,6 @@ def __init__(self, *args: Any, **kwargs: Any):
112218
self.uncheckpointed_statements: set[Statement] = set()
113219
self.comp_unknown = False
114220

115-
# these are file-wide, so intentionally not save-stated upon entry/exit
116-
# of functions/loops/etc
117-
self.explicitly_imported_library: dict[str, bool] = {
118-
"trio": False,
119-
"anyio": False,
120-
}
121-
self.add_import: set[str] = set()
122-
123-
# this one is not save-stated, but I fail to come up with any scenario
124-
# where that matters
125-
self.add_statement: cst.SimpleStatementLine | None = None
126-
127221
self.loop_state = LoopState()
128222
self.try_state = TryState()
129223

@@ -168,7 +262,6 @@ def uncheckpointed_before_break(self, value: set[Statement]):
168262
self.loop_state.uncheckpointed_before_break = value
169263

170264
def checkpoint_statement(self) -> cst.SimpleStatementLine:
171-
self.ensure_imported_library()
172265
return checkpoint_statement(self.library[0])
173266

174267
def visit_FunctionDef(self, node: cst.FunctionDef) -> None:
@@ -248,7 +341,7 @@ def check_function_exit(
248341
# Add this as a node potentially needing checkpoints only if it
249342
# missing checkpoints solely depends on whether the artificial statement is
250343
# "real"
251-
if len(self.uncheckpointed_statements) == 1:
344+
if len(self.uncheckpointed_statements) == 1 and not self.noautofix:
252345
self.loop_state.nodes_needing_checkpoints.append(original_node)
253346
F987 return False
254347

@@ -274,38 +367,6 @@ def leave_Return(
274367
assert original_node.deep_equals(updated_node)
275368
return original_node
276369

277-
# TODO: generate an error in these two if transforming+visiting is done in a single
278-
# pass and emit-error-on-transform can be enabled/disabled. The error can't be
279-
# generated in the yield/return since it doesn't know if it will be autofixed.
280-
281-
# SimpleStatementSuite and multi-statement lines can probably be autofixed, but
282-
# for now just don't insert checkpoints in the wrong place.
283-
def leave_SimpleStatementSuite(
284-
self,
285-
original_node: cst.SimpleStatementSuite,
286-
updated_node: cst.SimpleStatementSuite,
287-
) -> cst.SimpleStatementSuite:
288-
self.add_statement = None
289-
return updated_node
290-
291-
# insert checkpoint before yield with a flattensentinel, if indicated
292-
def leave_SimpleStatementLine(
293-
self,
294-
original_node: cst.SimpleStatementLine,
295-
updated_node: cst.SimpleStatementLine,
296-
) -> cst.SimpleStatementLine | cst.FlattenSentinel[cst.SimpleStatementLine]:
297-
if self.add_statement is None:
298-
return updated_node
299-
300-
# multiple statements on a single line is not handled
301-
if len(updated_node.body) > 1:
302-
self.add_statement = None
303-
return updated_node
304-
305-
res = cst.FlattenSentinel([self.add_statement, updated_node])
306-
self.add_statement = None
307-
return res # noqa: R504
308-
309370
def error_91x(
310371
self,
311372
node: cst.Return | cst.FunctionDef | cst.Yield,
@@ -356,7 +417,7 @@ def leave_Yield(
356417
return updated_node
357418
self.has_yield = True
358419

359-
if self.check_function_exit(original_node):
420+
if self.check_function_exit(original_node) and not self.noautofix:
360421
self.add_statement = self.checkpoint_statement()
361422

362423
# mark as requiring checkpoint after
@@ -616,9 +677,8 @@ def leave_While(
616677
| cst.RemovalSentinel
617678
):
618679
if self.loop_state.nodes_needing_checkpoints:
619-
self.ensure_imported_library()
620680
transformer = InsertCheckpointsInLoopBody(
621-
self.loop_state.nodes_needing_checkpoints, self.library[0]
681+
self.loop_state.nodes_needing_checkpoints, self.library
622682
)
623683
# type of updated_node expanded to the return type
624684
updated_node = updated_node.visit(transformer) # type: ignore
@@ -651,9 +711,10 @@ def visit_BooleanOperation_right(self, node: cst.BooleanOperation):
651711
def leave_BooleanOperation_right(self, node: cst.BooleanOperation):
652712
if not self.async_function:
653713
return
654-
self.uncheckpointed_statements.update(
655-
self.outer[node]["uncheckpointed_statements"]
714+
self.outer[node]["uncheckpointed_statements"].update(
715+
self.uncheckpointed_statements
656716
)
717+
self.restore_state(node)
657718

658719
# comprehensions are simpler than loops, since they cannot contain yields
659720
# or many other complicated statements, but their subfields are not in the order
@@ -735,15 +796,6 @@ def visit_Import(self, node: cst.Import):
735796
assert isinstance(alias.name.value, str)
736797
self.explicitly_imported_library[alias.name.value] = True
737798

738-
def ensure_imported_library(self) -> None:
739-
"""Mark library for import.
740-
741-
Check that the library we'd use to insert checkpoints
742-
is imported - if not, mark it to be inserted later.
743-
"""
744-
if not self.explicitly_imported_library[self.library[0]]:
745-
self.add_import.add(self.library[0])
746-
747799
def leave_Module(self, original_node: cst.Module, updated_node: cst.Module):
748800
"""Add needed library import, if any, to the module."""
749801
if not self.add_import:
@@ -765,33 +817,3 @@ def leave_Module(self, original_node: cst.Module, updated_node: cst.Module):
765817
assert len(self.add_import) == 1
766818
new_body.insert(index, cst.parse_statement(f"import {self.library[0]}"))
767819
return updated_node.with_changes(body=new_body)
768-
769-
770-
# necessary as we don't know whether to insert checkpoints on the first pass of a loop
771-
# so we transform the loop body afterwards
772-
class InsertCheckpointsInLoopBody(cst.CSTTransformer):
773-
def __init__(
774-
self, nodes_needing_checkpoint: Sequence[cst.Yield | cst.Return], library: str
775-
):
776-
super().__init__()
777-
self.nodes_needing_checkpoint = nodes_needing_checkpoint
778-
self.add_statement: cst.SimpleStatementLine | None = None
779-
self.library = library
780-
781-
# insert checkpoint before yield with a flattensentinel, if indicated
782-
# type checkers don't like going across classes, esp as the method accesses
783-
# and modifies self.add_statement, but #YOLO
784-
leave_SimpleStatementLine = Visitor91X.leave_SimpleStatementLine # type: ignore
785-
786-
def leave_Yield(
787-
self,
788-
original_node: cst.Yield,
789-
updated_node: cst.Yield,
790-
) -> cst.Yield:
791-
# we need to check *original* node here, since updated node is a copy
792-
# which loses identity equality
793-
if original_node in self.nodes_needing_checkpoint:
794-
self.add_statement = checkpoint_statement(self.library)
795-
return updated_node
796-
797-
leave_Return = leave_Yield # type: ignore

tests/autofix_files/trio100.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,3 @@ async def foo():
6767

6868
async with random_ignored_library.fail_after(10):
6969
...
70-
71-
72-
async def function_name2():
73-
with (
74-
open("") as _,
75-
trio.fail_after(10), # error: 8, "trio", "fail_after"
76-
):
77-
...
78-
79-
with (
80-
trio.fail_after(5), # error: 8, "trio", "fail_after"
81-
open("") as _,
82-
trio.move_on_after(5), # error: 8, "trio", "move_on_after"
83-
):
84-
...
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# AUTOFIX_NOFIX
2+
import trio
3+
4+
5+
# Doesn't autofix With's with multiple withitems
6+
async def function_name2():
7+
with (
8+
open("") as _,
9+
trio.fail_after(10), # error: 8, "trio", "fail_after"
10+
):
11+
...
12+
13+
with (
14+
trio.fail_after(5), # error: 8, "trio", "fail_after"
15+
open("") as _,
16+
trio.move_on_after(5), # error: 8, "trio", "move_on_after"
17+
):
18+
...
19+
20+
21+
with (
22+
trio.move_on_after(10), # error: 4, "trio", "move_on_after"
23+
open("") as f,
24+
):
25+
...

tests/autofix_files/trio100_noautofix.py.diff

Whitespace-only changes.

tests/autofix_files/trio100_simple_autofix.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@
2929
# c
3030
# d
3131

32-
# Doesn't autofix With's with multiple withitems
33-
with (
34-
trio.move_on_after(10), # error: 4, "trio", "move_on_after"
35-
open("") as f,
36-
):
37-
...
38-
3932

4033
# multiline with, despite only being one statement
4134
# a

tests/autofix_files/trio100_simple_autofix.py.diff

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
+++
3-
@@ -2,28 +2,29 @@
3+
@@ -2,50 +2,51 @@
44

55
# a
66
# b
@@ -45,7 +45,6 @@
4545
# fmt: on
4646
# c
4747
# d
48-
@@ -37,22 +38,22 @@
4948

5049

5150
# multiline with, despite only being one statement

0 commit comments

Comments
 (0)
0