From 594ab51e865a894e1b54cb07d3b17251c5e52115 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Wed, 17 Jan 2024 09:35:10 +0100 Subject: [PATCH] feat: Add TC010 which detects invalid use of string literals with | --- README.md | 1 + flake8_type_checking/checker.py | 33 +++++++++-- flake8_type_checking/constants.py | 2 + tests/test_tc008.py | 2 + tests/test_tc010.py | 93 +++++++++++++++++++++++++++++++ tests/test_tc201.py | 2 + 6 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 tests/test_tc010.py diff --git a/README.md b/README.md index 9661932..6f61eeb 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ And depending on which error code range you've opted into, it will tell you | TC007 | Type alias needs to be made into a string literal | | TC008 | Type alias does not need to be a string literal | | TC009 | Move declaration out of type-checking block. Variable is used for more than type hinting. | +| TC010 | Operands for | cannot be a string literal | ## Choosing how to handle forward references diff --git a/flake8_type_checking/checker.py b/flake8_type_checking/checker.py index 06fd76d..f9a3ebe 100644 --- a/flake8_type_checking/checker.py +++ b/flake8_type_checking/checker.py @@ -20,6 +20,7 @@ ATTRIBUTE_PROPERTY, ATTRS_DECORATORS, ATTRS_IMPORTS, + BINOP_OPERAND_PROPERTY, NAME_RE, TC001, TC002, @@ -30,6 +31,7 @@ TC007, TC008, TC009, + TC010, TC100, TC101, TC200, @@ -86,6 +88,8 @@ def visit(self, node: ast.AST) -> None: if isinstance(node, ast.BinOp): if not isinstance(node.op, ast.BitOr): return + setattr(node.left, BINOP_OPERAND_PROPERTY, True) + setattr(node.right, BINOP_OPERAND_PROPERTY, True) self.visit(node.left) self.visit(node.right) elif (py38 and isinstance(node, Index)) or isinstance(node, ast.Attribute): @@ -818,6 +822,9 @@ def __init__(self) -> None: #: All type annotations in the file, with quotes around them self.wrapped_annotations: list[WrappedAnnotation] = [] + #: All the invalid uses of string literals inside ast.BinOp + self.invalid_binop_literals: list[ast.Constant] = [] + def visit( self, node: ast.AST, scope: Scope | None = None, type: Literal['annotation', 'alias', 'new-alias'] | None = None ) -> None: @@ -836,13 +843,17 @@ def visit_annotation_name(self, node: ast.Name) -> None: ) def visit_annotation_string(self, node: ast.Constant) -> None: - """Register wrapped annotation.""" + """Register wrapped annotation and invalid binop literals.""" setattr(node, ANNOTATION_PROPERTY, True) - self.wrapped_annotations.append( - WrappedAnnotation( - node.lineno, node.col_offset, node.value, set(NAME_RE.findall(node.value)), self.scope, self.type + # we don't want to register them as both so we don't emit redundant errors + if getattr(node, BINOP_OPERAND_PROPERTY, False): + self.invalid_binop_literals.append(node) + else: + self.wrapped_annotations.append( + WrappedAnnotation( + node.lineno, node.col_offset, node.value, set(NAME_RE.findall(node.value)), self.scope, self.type + ) ) - ) class ImportVisitor( @@ -958,6 +969,11 @@ def wrapped_annotations(self) -> list[WrappedAnnotation]: """All type annotations in the file, with quotes around them.""" return self.annotation_visitor.wrapped_annotations + @property + def invalid_binop_literals(self) -> list[ast.Constant]: + """All invalid uses of binop literals.""" + return self.annotation_visitor.invalid_binop_literals + @property def typing_module_name(self) -> str: """ @@ -1800,6 +1816,8 @@ def __init__(self, node: ast.Module, options: Optional[Namespace]) -> None: self.empty_type_checking_blocks, # TC006 self.unquoted_type_in_cast, + # TC010 + self.invalid_string_literal_in_binop, # TC100, TC200, TC007 self.missing_quotes_or_futures_import, # TC101 @@ -1900,6 +1918,11 @@ def unquoted_type_in_cast(self) -> Flake8Generator: for lineno, col_offset, annotation in self.visitor.unquoted_types_in_casts: yield lineno, col_offset, TC006.format(annotation=annotation), None + def invalid_string_literal_in_binop(self) -> Flake8Generator: + """TC010.""" + for node in self.visitor.invalid_binop_literals: + yield node.lineno, node.col_offset, TC010, None + def missing_quotes_or_futures_import(self) -> Flake8Generator: """TC100, TC200 and TC007.""" encountered_missing_quotes = False diff --git a/flake8_type_checking/constants.py b/flake8_type_checking/constants.py index 5de9c22..37e21a3 100644 --- a/flake8_type_checking/constants.py +++ b/flake8_type_checking/constants.py @@ -6,6 +6,7 @@ ATTRIBUTE_PROPERTY = '_flake8-type-checking__parent' ANNOTATION_PROPERTY = '_flake8-type-checking__is_annotation' +BINOP_OPERAND_PROPERTY = '_flake8-type-checking__is_binop_operand' NAME_RE = re.compile(r'(?> Operands for | cannot be a string literal + +""" + +import sys +import textwrap + +import pytest + +from flake8_type_checking.constants import TC010 +from tests.conftest import _get_error + +examples = [ + # No error + ('', set()), + ('x: int | None', set()), + # Used in type annotation at runtime + ( + 'x: "int" | None', + {'1:3 ' + TC010}, + ), + ( + 'x: int | "None"', + {'1:9 ' + TC010}, + ), + ( + 'x: "int" | "None"', + {'1:3 ' + TC010, '1:11 ' + TC010}, + ), + ( + 'def foo(x: int | "str" | None) -> None: ...', + {'1:17 ' + TC010}, + ), + ( + 'def foo(x: int) -> int | "None": ...', + {'1:25 ' + TC010}, + ), + # Used in implicit type alias at runtime (can't detect) + ( + 'x = "int" | None', + set(), + ), + # Used in explicit type alias at runtime + ( + 'x: TypeAlias = "int" | None', + {'1:15 ' + TC010}, + ), + # Used in type annotations at type checking time + # We could have chosen not to emit an error inside if TYPE_CHECKING blocks + # however it is plausible that type checkers will be able to detect this + # case at some point and then it might become an error, so it's better + # to have cleaned up those annotations by then + ( + textwrap.dedent(""" + if TYPE_CHECKING: + x: "int" | None + y: int | "None" + z: "int" | "None" + + Foo: TypeAlias = "int" | None + Bar = "int" | None + + def foo(x: int | "str" | None) -> int | "None": + pass + """), + { + '3:7 ' + TC010, + '4:13 ' + TC010, + '5:7 ' + TC010, + '5:15 ' + TC010, + '7:21 ' + TC010, + '10:21 ' + TC010, + '10:44 ' + TC010, + }, + ), +] + +if sys.version_info >= (3, 12): + # PEP695 tests + examples += [ + ( + 'type x = "int" | None', + {'1:9 ' + TC010}, + ), + ] + + +@pytest.mark.parametrize(('example', 'expected'), examples) +def test_TC010_errors(example, expected): + assert _get_error(example, error_code_filter='TC010') == expected diff --git a/tests/test_tc201.py b/tests/test_tc201.py index 2fe1ea5..ffc15d1 100644 --- a/tests/test_tc201.py +++ b/tests/test_tc201.py @@ -18,6 +18,8 @@ # this case once again we could add a whitelist of subscriptable types ("x: 'Dict[int]'", set()), ("from typing import Dict\nx: 'Dict'", {'2:3 ' + TC201.format(annotation='Dict')}), + # this should emit a TC010 instead + ("from typing import Dict\nx: 'Dict' | None", set()), ("from __future__ import annotations\nx: 'int'", {'2:3 ' + TC201.format(annotation='int')}), ("if TYPE_CHECKING:\n\tfrom typing import Dict\nx: 'Dict'", set()), ("if TYPE_CHECKING:\n\tfrom typing import Dict\nx: 'Dict[int]'", set()),