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

fix: Avoid TC201 false positives for top-level type checking decls #170

Merged
merged 10 commits into from
Oct 17, 2023
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ And depending on which error code range you've opted into, it will tell you
| TC004 | Move import out of type-checking block. Import is used for more than type hinting. |
| TC005 | Found empty type-checking block |
| TC006 | Annotation in typing.cast() should be a string literal |
| TC007 | Type alias needs to be made into a string literal |
| TC008 | Type alias does not need to be a string literal |

## Choosing how to handle forward references

Expand Down
262 changes: 209 additions & 53 deletions flake8_type_checking/checker.py

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion flake8_type_checking/constants.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import re
import sys

import flake8

ATTRIBUTE_PROPERTY = '_flake8-type-checking__parent'
ANNOTATION_PROPERTY = '_flake8-type-checking__is_annotation'
GLOBAL_PROPERTY = '_flake8-type-checking__is_global'

NAME_RE = re.compile(r'(?<![\'"])\b[A-Za-z_]\w*(?![\'"])')

ATTRS_DECORATORS = [
'attrs.define',
Expand All @@ -27,7 +31,9 @@
TC004 = "TC004 Move import '{module}' out of type-checking block. Import is used for more than type hinting."
TC005 = 'TC005 Found empty type-checking block'
TC006 = "TC006 Annotation '{annotation}' in typing.cast() should be a string literal"
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"
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"
TC201 = "TC201 Annotation '{annotation}' does not need to be a string"
TC201 = "TC201 Annotation '{annotation}' does not need to be a string literal"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the readme for this too, if needed 👍

29 changes: 29 additions & 0 deletions tests/test_name_extraction.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import pytest

from flake8_type_checking.constants import NAME_RE

examples = [
('', []),
('int', ['int']),
('dict[str, int]', ['dict', 'str', 'int']),
# make sure literals don't add names for their contents
('Literal["a"]', ['Literal']),
("Literal['a']", ['Literal']),
('Literal[0]', ['Literal']),
('Literal[1.0]', ['Literal']),
# booleans are a special case and difficult to reject using a RegEx
# for now it seems harmless to include them in the names, but if
# we do something more sophisticated with the names we may want to
# explicitly remove True/False from the result set
('Literal[True]', ['Literal', 'True']),
# try some potentially upcoming syntax
('*Ts | _T & S', ['Ts', '_T', 'S']),
# even when it's formatted badly
('*Ts|_T&P', ['Ts', '_T', 'P']),
('Union[Dict[str, Any], Literal["Foo", "Bar"], _T]', ['Union', 'Dict', 'str', 'Any', 'Literal', '_T']),
]


@pytest.mark.parametrize(('example', 'expected'), examples)
def test_name_extraction(example, expected):
assert NAME_RE.findall(example) == expected
12 changes: 12 additions & 0 deletions tests/test_tc001_to_tc003.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from __future__ import annotations

import sys
import textwrap
from typing import List, Set, Tuple

Expand Down Expand Up @@ -101,6 +102,16 @@ def get_tc_001_to_003_tests(import_: str, ERROR: str) -> L:
(f'import {import_} as x\ndef example() -> x:\n\tpass', {'1:0 ' + ERROR.format(module='x')}),
]

used_for_type_alias_only: L = [
(f'import {import_}\nx: TypeAlias = {import_}', {f"1:0 {ERROR.format(module=f'{import_}')}"}),
]

if sys.version_info >= (3, 12):
# new style type alias
used_for_type_alias_only.append(
(f'import {import_}\ntype x = {import_}', {f"1:0 {ERROR.format(module=f'{import_}')}"})
)

other_useful_test_cases: L = [
(
textwrap.dedent(f'''
Expand Down Expand Up @@ -193,6 +204,7 @@ class Migration:
*used_for_annotations_only,
*used_for_arg_annotations_only,
*used_for_return_annotations_only,
*used_for_type_alias_only,
*other_useful_test_cases,
]

Expand Down
69 changes: 69 additions & 0 deletions tests/test_tc007.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
"""
File tests TC007:
Type alias should be wrapped in quotes
"""
import sys
import textwrap

import pytest

from flake8_type_checking.constants import TC007
from tests.conftest import _get_error

examples = [
# No error
('', set()),
('x: TypeAlias = "int"', set()),
('from typing import Dict\nx: TypeAlias = Dict[int]', set()),
('if TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = Dict', {'3:15 ' + TC007.format(alias='Dict')}),
("if TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = 'Dict'", set()),
("if TYPE_CHECKING:\n\tfrom typing import Dict as d\nx: TypeAlias = 'd[int]'", set()),
('if TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = Dict[int]', {'3:15 ' + TC007.format(alias='Dict')}),
# Regression test for issue #163
(
textwrap.dedent('''
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from collections.abc import Sequence
from typing_extensions import TypeAlias

Foo: TypeAlias = Sequence[int]
'''),
set(),
),
# Inverse regression test for issue #163
(
textwrap.dedent('''
from typing import TYPE_CHECKING
from typing_extensions import TypeAlias

if TYPE_CHECKING:
from collections.abc import Sequence

Foo: TypeAlias = Sequence[int]
'''),
{
'8:17 ' + TC007.format(alias='Sequence'),
},
),
]

if sys.version_info >= (3, 12):
# RHS on an explicit TypeAlias with 3.12 syntax should not emit a TC007
examples.append(
(
textwrap.dedent('''
if TYPE_CHECKING:
from collections.abc import Sequence

type Foo = Sequence[int]
'''),
set(),
)
)


@pytest.mark.parametrize(('example', 'expected'), examples)
def test_TC007_errors(example, expected):
assert _get_error(example, error_code_filter='TC007') == expected
113 changes: 113 additions & 0 deletions tests/test_tc008.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
"""
File tests TC008:
Type alias is wrapped in unnecessary quotes
"""
from __future__ import annotations

import sys
import textwrap

import pytest

from flake8_type_checking.constants import TC008
from tests.conftest import _get_error

examples = [
('', set()),
("x: TypeAlias = 'int'", {'1:15 ' + TC008.format(alias='int')}),
# 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()),
("from __future__ import annotations\nx: TypeAlias = 'int'", {'2:15 ' + TC008.format(alias='int')}),
("if TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = 'Dict'", set()),
("if TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = 'Dict[int]'", set()),
(
"from __future__ import annotations\nfrom typing import Dict\nx: TypeAlias = Dict['int']",
{'3:20 ' + TC008.format(alias='int')},
),
(
"from __future__ import annotations\nif TYPE_CHECKING:\n\tfrom typing import Dict\nx: TypeAlias = Dict['int']",
{'4:20 ' + TC008.format(alias='int')},
),
(
textwrap.dedent('''
from __future__ import annotations

if TYPE_CHECKING:
import something

x: TypeAlias = "something"
'''),
set(),
),
(
# Regression test for Issue #164
textwrap.dedent('''
from wtforms import Field
from wtforms.fields.core import UnboundField

foo: TypeAlias = 'UnboundField[Field]'
'''),
set(),
),
(
# avoid false positive for annotations that make
# use of a newly defined class
textwrap.dedent('''
class Foo(Protocol):
pass

x: TypeAlias = 'Foo | None'
'''),
set(),
),
(
# Regression test for Issue #168
textwrap.dedent('''
if TYPE_CHECKING:
Foo: TypeAlias = str | int

Bar: TypeAlias = 'Foo'
'''),
set(),
),
(
# Inverse regression test for Issue #168
# The declarations are inside a Protocol so they should not
# count towards declarations inside a type checking block
textwrap.dedent('''
if TYPE_CHECKING:
class X(Protocol):
Foo: str | int

Bar: TypeAlias = 'Foo'
'''),
{
'6:17 ' + TC008.format(alias='Foo'),
},
),
]

if sys.version_info >= (3, 12):
examples.extend(
[
(
# new style type alias should never be wrapped
textwrap.dedent('''
if TYPE_CHECKING:
type Foo = 'str'

type Bar = 'Foo'
'''),
{
'3:15 ' + TC008.format(alias='str'),
'5:11 ' + TC008.format(alias='Foo'),
},
)
]
)


@pytest.mark.parametrize(('example', 'expected'), examples)
def test_TC008_errors(example, expected):
assert _get_error(example, error_code_filter='TC008') == expected
31 changes: 31 additions & 0 deletions tests/test_tc101.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
File tests TC101:
Annotation is wrapped in unnecessary quotes
"""
import sys
import textwrap

import pytest
Expand Down Expand Up @@ -113,8 +114,38 @@ def foo(self) -> 'X':
'''),
set(),
),
# Make sure we didn't introduce any regressions while solving #167
# since we started to treat the RHS sort of like an annotation for
# some of the use-cases
(
textwrap.dedent('''
from __future__ import annotations
if TYPE_CHECKING:
from foo import Foo

x: TypeAlias = 'Foo'
'''),
set(),
),
]

if sys.version_info >= (3, 12):
examples.append(
(
# Make sure we didn't introduce any regressions while solving #167
# using new type alias syntax
# TODO: Make sure we actually need to wrap Foo if we use future
Daverball marked this conversation as resolved.
Show resolved Hide resolved
textwrap.dedent('''
from __future__ import annotations
if TYPE_CHECKING:
from foo import Foo

type x = 'Foo'
'''),
set(),
)
)


@pytest.mark.parametrize(('example', 'expected'), examples)
def test_TC101_errors(example, expected):
Expand Down
8 changes: 7 additions & 1 deletion tests/test_tc200.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class FooDict(TypedDict):
from typing import NamedTuple, Protocol
from typing_extensions import TypeAlias, TypedDict

Foo: TypeAlias = Sequence[int]
Foo: TypeAlias = 'Sequence[int]'

class FooTuple(NamedTuple):
seq: Sequence[int]
Expand All @@ -99,6 +99,12 @@ class FooProtocol(Protocol):

class FooDict(TypedDict):
seq: Sequence[int]

if TYPE_CHECKING:
# this should not count as a type checking global
Bar: int

x: Bar
'''),
{
'9:5 ' + TC200.format(annotation='TypeAlias'),
Expand Down
Loading