Skip to content

fix: avoid early return in used_type_checking_imports#123

Merged
sondrelg merged 1 commit intosnok:mainfrom
shiftinv:fix/function-scope-import-use
Aug 4, 2022
Merged

fix: avoid early return in used_type_checking_imports#123
sondrelg merged 1 commit intosnok:mainfrom
shiftinv:fix/function-scope-import-use

Conversation

@shiftinv
Copy link
Copy Markdown
Contributor

@shiftinv shiftinv commented Aug 4, 2022

This PR prevents false negatives for TC004, which were caused by used_type_checking_imports returning immediately on the first function-scoped import usage, and therefore not warning about TC004 errors that would have followed.

I don't think this can be reproduced/tested reliably, as it depends on the order in which the type_checking_block_imports set is iterated over, and AST node hashes arbitrarily change which results in different iteration orders over subsequent flake8 runs.

Reproduction

from __future__ import annotations
from typing import TYPE_CHECKING


if TYPE_CHECKING:
    from datetime import datetime
    from base64 import b64encode


# runtime usage of `b64encode`, should emit TC004 for the import above
x = b64encode("")


def f() -> datetime:
    # function scope import of `datetime`, special-cased by `used_type_checking_imports`
    from datetime import datetime
    return datetime.utcnow()
$ flake8 test.py
test.py:7:1: TC004 Move import 'b64encode' out of type-checking block. Import is used for more than type hinting.
$ flake8 test.py
test.py:7:1: TC004 Move import 'b64encode' out of type-checking block. Import is used for more than type hinting.
$ flake8 test.py
$ flake8 test.py
$ flake8 test.py
$ flake8 test.py
$ flake8 test.py
test.py:7:1: TC004 Move import 'b64encode' out of type-checking block. Import is used for more than type hinting.
$ flake8 test.py
test.py:7:1: TC004 Move import 'b64encode' out of type-checking block. Import is used for more than type hinting.
$ flake8 test.py
$ flake8 test.py
test.py:7:1: TC004 Move import 'b64encode' out of type-checking block. Import is used for more than type hinting.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 4, 2022

Codecov Report

Merging #123 (8e0d351) into main (7be3641) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #123   +/-   ##
=====================================
  Coverage   98.7%   98.7%           
=====================================
  Files          3       3           
  Lines        466     469    +3     
=====================================
+ Hits         460     463    +3     
  Misses         6       6           
Impacted Files Coverage Δ
flake8_type_checking/checker.py 98.5% <100.0%> (+<0.1%) ⬆️

Copy link
Copy Markdown
Member

@sondrelg sondrelg left a comment

Choose a reason for hiding this comment

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

Great catch 👏

@sondrelg sondrelg merged commit d6aed04 into snok:main Aug 4, 2022
@shiftinv shiftinv deleted the fix/function-scope-import-use branch August 4, 2022 15:09
@sondrelg
Copy link
Copy Markdown
Member

sondrelg commented Aug 4, 2022

Think I'll try to create a fix for #120 tonight then release a new version @shiftinv 👍

@shiftinv
Copy link
Copy Markdown
Contributor Author

shiftinv commented Aug 4, 2022

@sondrelg Thanks, sounds good! I'm in no rush to get these fixes into a release, so take your time 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants