From 7d78fa7276a9a6103478a3304a9b8d1cd13f2538 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 31 Jan 2023 14:25:19 +0100 Subject: [PATCH] add TRIO212. move out type-tracking logic from TRIO232 to TypeTrackerVisitor200 --- CHANGELOG.md | 1 + README.md | 1 + flake8_trio/visitors/flake8triovisitor.py | 6 +- flake8_trio/visitors/visitor2xx.py | 221 ++++++++++++++-------- tests/eval_files/trio212.py | 83 ++++++++ tests/eval_files/trio232.py | 6 +- 6 files changed, 230 insertions(+), 88 deletions(-) create mode 100644 tests/eval_files/trio212.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a0c3b27..b1909356 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ *[CalVer, YY.month.patch](https://calver.org/)* ## Future - Add TRIO232: blocking sync call on file object. +- Add TRIO212: blocking sync call on httpx.Client object. ## Future - TRIO103 and TRIO104 no longer triggers when `trio.Cancelled` has been handled in previous except handlers. diff --git a/README.md b/README.md index d04d8b35..2adc6724 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,7 @@ pip install flake8-trio - **TRIO200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`trio200-blocking-calls`](#trio200-blocking-calls) for how to configure it. - **TRIO210**: Sync HTTP call in async function, use `httpx.AsyncClient`. - **TRIO211**: Likely sync HTTP call in async function, use `httpx.AsyncClient`. Looks for `urllib3` method calls on pool objects, but only matching on the method signature and not the object. +- **TRIO212**: Blocking sync HTTP call on httpx object, use httpx.AsyncClient. - **TRIO220**: Sync process call in async function, use `await nursery.start(trio.run_process, ...)`. - **TRIO221**: Sync process call in async function, use `await trio.run_process(...)`. - **TRIO230**: Sync IO call in async function, use `trio.open_file(...)`. diff --git a/flake8_trio/visitors/flake8triovisitor.py b/flake8_trio/visitors/flake8triovisitor.py index 02ba4206..ee666047 100644 --- a/flake8_trio/visitors/flake8triovisitor.py +++ b/flake8_trio/visitors/flake8triovisitor.py @@ -108,7 +108,11 @@ def set_state(self, attrs: dict[str, Any], copy: bool = False): setattr(self, attr, value) def save_state(self, node: ast.AST, *attrs: str, copy: bool = False): - self.outer[node] = self.get_state(*attrs, copy=copy) + state = self.get_state(*attrs, copy=copy) + if node in self.outer: + self.outer[node].update(state) + else: + self.outer[node] = state def walk(self, *body: ast.AST) -> Iterable[ast.AST]: for b in body: diff --git a/flake8_trio/visitors/visitor2xx.py b/flake8_trio/visitors/visitor2xx.py index 4822135e..ef75beec 100644 --- a/flake8_trio/visitors/visitor2xx.py +++ b/flake8_trio/visitors/visitor2xx.py @@ -48,6 +48,88 @@ def visit_blocking_call(self, node: ast.Call): self.error(node, key, blocking_calls[key]) +# used by Visitor212 and Visitor232 +class TypeTrackerVisitor200(Visitor200): + def __init__( + self, typed_calls: dict[str, str] | None = None, *args: Any, **kwargs: Any + ): + super().__init__(*args, **kwargs) + self.typed_calls: dict[str, str] = {} if typed_calls is None else typed_calls + self.variables: dict[str, str] = {} # name: type + + def visit_AsyncFunctionDef( + self, node: ast.AsyncFunctionDef | ast.FunctionDef | ast.Lambda + ): + super().visit_AsyncFunctionDef(node) + + def or_none(node: ast.AST | None): + if not isinstance(node, ast.BinOp) or not isinstance(node.op, ast.BitOr): + return None + if isinstance(node.left, ast.Constant) and node.left.value is None: + return node.right + if isinstance(node.right, ast.Constant) and node.right.value is None: + return node.left + return None + + self.save_state(node, "variables", copy=True) + + args = node.args + for arg in *args.args, *args.posonlyargs, *args.kwonlyargs: + annotation = arg.annotation + + if ( + isinstance(annotation, ast.Subscript) + and isinstance(annotation.value, ast.Name) + and annotation.value.id == "Optional" + ): + annotation = annotation.slice + elif res := or_none(annotation): + annotation = res + + if not isinstance(annotation, (ast.Name, ast.Attribute)): + continue + + self.variables[arg.arg] = ast.unparse(annotation) + + visit_FunctionDef = visit_AsyncFunctionDef + visit_Lambda = visit_AsyncFunctionDef + + def visit_AnnAssign(self, node: ast.AnnAssign): + if not isinstance(node.target, ast.Name): + return + target = node.target.id + typename = ast.unparse(node.annotation) + self.variables[target] = typename + + def visit_Assign(self, node: ast.Assign): + if len(node.targets) != 1 or not isinstance(node.targets[0], ast.Name): + return + + # `f = open(...)` + if isinstance(node.value, ast.Call) and ( + vartype := self.typed_calls.get(ast.unparse(node.value.func), None) + ): + self.variables[node.targets[0].id] = vartype + + # f = ff (and ff is a variable with known type) + elif isinstance(node.value, ast.Name) and ( + value := self.variables.get(node.value.id, None) + ): + self.variables[node.targets[0].id] = value + + def visit_With(self, node: ast.With): + if len(node.items) != 1: + return + item = node.items[0] + if ( + isinstance(item.context_expr, ast.Call) + and isinstance(item.context_expr.func, ast.Name) + and isinstance(item.optional_vars, ast.Name) + and (vartype := self.typed_calls.get(item.context_expr.func.id, None)) + ): + self.variables[item.optional_vars.id] = vartype + + @error_class class Visitor21X(Visitor200): error_codes = { @@ -107,6 +189,57 @@ def visit_blocking_call(self, node: ast.Call): self.error(node, func_name, error_code="TRIO211") +@error_class +class Visitor212(TypeTrackerVisitor200): + error_codes = { + "TRIO212": ( + "Blocking sync HTTP call {1} on httpx object {0}, use httpx.AsyncClient" + ) + } + + def __init__(self, *args: Any, **kwargs: Any): + self.dangerous_classes = ( + "httpx.Client", + "urllib3.HTTPConnectionPool", + "urllib3.HTTPSConnectionPool", + "urllib3.PoolManager", + "urllib3.ProxyManager", + "urllib3.connectionpool.ConnectionPool", + "urllib3.connectionpool.HTTPConnectionPool", + "urllib3.connectionpool.HTTPSConnectionPool", + "urllib3.poolmanager.PoolManager", + "urllib3.poolmanager.ProxyManager", + "urllib3.request.RequestMethods", + ) + super().__init__( + dict(zip(self.dangerous_classes, self.dangerous_classes)), *args, **kwargs + ) + + def visit_blocking_call(self, node: ast.Call): + httpx_blocking_methods = ( + "close", + "delete", + "get", + "head", + "options", + "patch", + "post", + "put", + "request", + "send", + "stream", + ) + if ( + isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id in self.variables + and (anno := self.variables.get(node.func.value.id)) + and (anno != "httpx.Client" or node.func.attr in httpx_blocking_methods) + and anno in self.dangerous_classes + ): + self.error(node, node.func.attr, node.func.value.id) + + # Process invocations 202 @error_class class Visitor22X(Visitor200): @@ -187,7 +320,7 @@ def visit_blocking_call(self, node: ast.Call): @error_class -class Visitor232(Flake8TrioVisitor): +class Visitor232(TypeTrackerVisitor200): error_codes = { "TRIO232": ( "Blocking sync call {1} on file object {0}, wrap the file object" @@ -196,47 +329,9 @@ class Visitor232(Flake8TrioVisitor): } def __init__(self, *args: Any, **kwargs: Any): - super().__init__(*args, **kwargs) - self.variables: dict[str, str] = {} # name: type - self.async_function = False - - def visit_AsyncFunctionDef( - self, node: ast.AsyncFunctionDef | ast.FunctionDef | ast.Lambda - ): - def or_none(node: ast.AST | None): - if not isinstance(node, ast.BinOp) or not isinstance(node.op, ast.BitOr): - return None - if isinstance(node.left, ast.Constant) and node.left.value is None: - return node.right - if isinstance(node.right, ast.Constant) and node.right.value is None: - return node.left - return None - - self.save_state(node, "variables", "async_function", copy=True) - self.async_function = isinstance(node, ast.AsyncFunctionDef) - - args = node.args - for arg in *args.args, *args.posonlyargs, *args.kwonlyargs: - annotation = arg.annotation - - if ( - isinstance(annotation, ast.Subscript) - and isinstance(annotation.value, ast.Name) - and annotation.value.id == "Optional" - ): - annotation = annotation.slice - elif res := or_none(annotation): - annotation = res + super().__init__({"open": "io.TextIOWrapper"}, *args, **kwargs) - if not isinstance(annotation, (ast.Name, ast.Attribute)): - continue - - self.variables[arg.arg] = ast.unparse(annotation) - - visit_FunctionDef = visit_AsyncFunctionDef - visit_Lambda = visit_AsyncFunctionDef - - def visit_Call(self, node: ast.Call): + def visit_blocking_call(self, node: ast.Call): io_file_types = ( "io.TextIOWrapper", "io.BufferedReader", @@ -244,8 +339,7 @@ def visit_Call(self, node: ast.Call): "io.BufferedRandom", ) if ( - self.async_function - and isinstance(node.func, ast.Attribute) + isinstance(node.func, ast.Attribute) and isinstance(node.func.value, ast.Name) and node.func.value.id in self.variables and (anno := self.variables.get(node.func.value.id)) @@ -253,47 +347,6 @@ def visit_Call(self, node: ast.Call): ): self.error(node, node.func.attr, node.func.value.id) - def visit_AnnAssign(self, node: ast.AnnAssign): - if not isinstance(node.target, ast.Name): - return - target = node.target.id - typename = ast.unparse(node.annotation) - self.variables[target] = typename - - def visit_Assign(self, node: ast.Assign): - if len(node.targets) != 1 or not isinstance(node.targets[0], ast.Name): - return - - # `f = open(...)` - if ( - isinstance(node.value, ast.Call) - and isinstance(node.value.func, ast.Name) - and node.value.func.id == "open" - ): - self.variables[ - node.targets[0].id - ] = "io.TextIOWrapper" # doesn't matter which - - # f = ff (and ff is a variable with known type) - elif isinstance(node.value, ast.Name) and ( - value := self.variables.get(node.value.id, None) - ): - self.variables[node.targets[0].id] = value - - def visit_With(self, node: ast.With): - if len(node.items) != 1: - return - item = node.items[0] - if ( - isinstance(item.context_expr, ast.Call) - and isinstance(item.context_expr.func, ast.Name) - and item.context_expr.func.id == "open" - and isinstance(item.optional_vars, ast.Name) - ): - self.variables[ - item.optional_vars.id - ] = "io.TextIOWrapper" # doesn't matter which - @error_class class Visitor24X(Visitor200): diff --git a/tests/eval_files/trio212.py b/tests/eval_files/trio212.py new file mode 100644 index 00000000..86465f01 --- /dev/null +++ b/tests/eval_files/trio212.py @@ -0,0 +1,83 @@ +import urllib3 + + +async def foo_call(): + client = httpx.Client() + client.close() # TRIO212: 4, "close", "client" + client.delete() # TRIO212: 4, "delete", "client" + client.get() # TRIO212: 4, "get", "client" + client.head() # TRIO212: 4, "head", "client" + client.options() # TRIO212: 4, "options", "client" + client.patch() # TRIO212: 4, "patch", "client" + client.post() # TRIO212: 4, "post", "client" + client.put() # TRIO212: 4, "put", "client" + client.request() # TRIO212: 4, "request", "client" + client.send() # TRIO212: 4, "send", "client" + client.stream() # TRIO212: 4, "stream", "client" + + client.anything() + client.build_request() + client.is_closed + + +async def foo_ann_par(client: httpx.Client): + client.send(...) # TRIO212: 4, "send", "client" + + +async def foo_ann_var(): + client: httpx.Client = ... + client.send() # TRIO212: 4, "send", "client" + + +glob_client = httpx.Client() + + +async def foo_global_client(): + glob_client.send() # TRIO212: 4, "send", "glob_client" + + +def sync_global_client(): + glob_client.send() + + +glob_client: int = 4 + + +async def foo_global_client_overriden(): + glob_client.send() + + +# not implemented +from httpx import Client + + +async def foo_alias_import(): + client: Client = Client() + client.send() + + +# urllib3 pools +async def foo_urllib3(): + c: urllib3.HTTPConnectionPool = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.HTTPSConnectionPool = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.PoolManager = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.ProxyManager = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.connectionpool.ConnectionPool = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.connectionpool.HTTPConnectionPool = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.connectionpool.HTTPSConnectionPool = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.poolmanager.PoolManager = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.poolmanager.ProxyManager = ... + c.anything() # TRIO212: 4, "anything", "c" + c: urllib3.request.RequestMethods = ... + c.anything() # TRIO212: 4, "anything", "c" + + c: urllib3.anything = ... + c.anything() diff --git a/tests/eval_files/trio232.py b/tests/eval_files/trio232.py index 0bb04ad6..1ec23790 100644 --- a/tests/eval_files/trio232.py +++ b/tests/eval_files/trio232.py @@ -158,8 +158,8 @@ async def global_vars(): g.read() # TRIO232: 4, 'read', 'g' -# If the type is explicitly overriden, it will not error -async def overriden_type(f: TextIOWrapper): +# If the type is explicitly overridden, it will not error +async def overridden_type(f: TextIOWrapper): f: int = 7 f.read() f: TextIOWrapper = ... @@ -173,7 +173,7 @@ async def overriden_type(f: TextIOWrapper): # ***** Known unhandled cases ***** # It will error on non-explicit assignments -async def implicit_overriden_type(): +async def implicit_overridden_type(): f: TextIOWrapper = ... f = arbitrary_function() f.read() # TRIO232: 4, 'read', 'f'