Merge pull request #184 from snok/invalid-string-literal-in-binop

feat: Add TC010 which detects invalid use of string literals with |

Author: Daverball

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
 

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

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'(?<![\'".])\b[A-Za-z_]\w*(?![\'"])')
 
@@ -43,6 +44,7 @@
 TC007 = "TC007 Type alias '{alias}' needs to be made into a string literal"
 TC008 = "TC008 Type alias '{alias}' does not need to be a string literal"
 TC009 = "TC009 Move declaration '{name}' out of type-checking block. Variable is used for more than type hinting."
+TC010 = 'TC010 Operands for | cannot be a string literal'
 TC100 = "TC100 Add 'from __future__ import annotations' import"
 TC101 = "TC101 Annotation '{annotation}' does not need to be a string literal"
 TC200 = "TC200 Annotation '{annotation}' needs to be made into a string literal"

tests/test_tc008.py

@@ -16,6 +16,8 @@
 examples = [
     ('', set()),
     ("x: TypeAlias = 'int'", {'1:15 ' + TC008.format(alias='int')}),
+    # this should emit a TC010 instead
+    ("x: TypeAlias = 'int' | None", set()),
     # this used to emit an error before fixing #164 if we wanted to handle
     # this case once again we could add a whitelist of subscriptable types
     ("x: TypeAlias = 'Dict[int]'", set()),

tests/test_tc010.py

@@ -0,0 +1,93 @@
+"""
+This file tests the TC010 error:
+
+    >> 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

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()),