Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 28 additions & 5 deletions flake8_type_checking/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ATTRIBUTE_PROPERTY,
ATTRS_DECORATORS,
ATTRS_IMPORTS,
BINOP_OPERAND_PROPERTY,
NAME_RE,
TC001,
TC002,
Expand All @@ -30,6 +31,7 @@
TC007,
TC008,
TC009,
TC010,
TC100,
TC101,
TC200,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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:
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions flake8_type_checking/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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*(?![\'"])')

Expand Down Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions tests/test_tc008.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
93 changes: 93 additions & 0 deletions tests/test_tc010.py
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tests/test_tc201.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down