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

[pyflakes] Treat if TYPE_CHECKING blocks like stub files for F821 #11262

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented May 3, 2024

Summary

Fixes #11245.

if TYPE_CHECKING blocks, like stub files, are never executed at runtime -- so I think it makes sense to give them stub-file semantics when it comes to whether forward references are or aren't permitted. Currently we allow forward references in stub files on the r.h.s. of type alias definitions and in class bases; this PR implements the same logic for if TYPE_CHECKING blocks.

Test Plan

cargo test / cargo insta review. Two fixtures have been extended.

@AlexWaygood AlexWaygood changed the title [pyflakes] Treat if TYPE_CHECKING` blocks like stub files for F821 [pyflakes] Treat if TYPE_CHECKING blocks like stub files for F821 May 3, 2024
@AlexWaygood AlexWaygood added bug Something isn't working linter Related to the linter labels May 3, 2024
@AlexWaygood
Copy link
Member Author

Type definitions created in if TYPE_CHECKING blocks obviously won't be available at runtime. If you want to use them elsewhere in the file, you'll either need to quote the definition or (if you only need to use it in an annotation context) put from __future__ import annotations at the top of the file. Possibly we could have an additional lint checking that the user does that, but I think it's out of scope of this PR.

Copy link
Contributor

github-actions bot commented May 3, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+47 -0 violations, +0 -0 fixes in 7 projects; 37 projects unchanged)

apache/airflow (+28 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ airflow/executors/base_executor.py:53:19: UP006 Use `list` instead of `List` for type annotation
+ airflow/executors/base_executor.py:59:30: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:59:54: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:63:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:63:34: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:66:17: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:66:53: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:66:68: UP007 Use `X | Y` for type annotations
+ airflow/executors/local_executor.py:56:24: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/local_executor.py:56:30: UP007 Use `X | Y` for type annotations
+ airflow/executors/local_executor.py:56:57: UP007 Use `X | Y` for type annotations
+ airflow/models/mappedoperator.py:90:39: UP007 Use `X | Y` for type annotations
+ airflow/models/mappedoperator.py:90:76: UP006 Use `list` instead of `List` for type annotation
+ airflow/providers/celery/executors/celery_executor.py:98:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/celery/executors/celery_executor.py:98:64: UP007 Use `X | Y` for type annotations
+ airflow/providers/celery/executors/celery_executor_utils.py:62:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/celery/executors/celery_executor_utils.py:62:64: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:28:25: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:28:66: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:29: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:52: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:61: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:27: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:43: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:52: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:84: UP006 Use `dict` instead of `Dict` for type annotation
+ airflow/providers/microsoft/azure/hooks/cosmos.py:47:24: UP007 Use `X | Y` for type annotations
... 1 additional changes omitted for rule UP007
+ airflow/providers/microsoft/azure/hooks/cosmos.py:47:35: UP006 Use `list` instead of `List` for type annotation

bokeh/bokeh (+8 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ src/bokeh/core/has_props.py:98:25: UP007 Use `X | Y` for type annotations
+ src/bokeh/document/events.py:122:26: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:87:37: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:89:24: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:89:48: UP007 Use `X | Y` for type annotations
+ src/bokeh/plotting/contour.py:42:31: UP007 Use `X | Y` for type annotations
+ src/bokeh/plotting/contour.py:43:40: UP007 Use `X | Y` for type annotations
+ src/bokeh/transform.py:71:26: UP007 Use `X | Y` for type annotations

latchbio/latch (+2 -0 violations, +0 -0 fixes)

+ latch/ldata/_transfer/upload.py:31:39: UP007 Use `X | Y` for type annotations
+ latch/ldata/_transfer/upload.py:32:42: UP007 Use `X | Y` for type annotations

pypa/cibuildwheel (+1 -0 violations, +0 -0 fixes)

+ cibuildwheel/typing.py:19:17: UP007 Use `X | Y` for type annotations

python/mypy (+1 -0 violations, +0 -0 fixes)

+ mypyc/build.py:58:32: UP007 Use `X | Y` for type annotations

python-poetry/poetry (+6 -0 violations, +0 -0 fixes)

+ src/poetry/repositories/link_sources/base.py:26:17: UP006 Use `collections.defaultdict` instead of `DefaultDict` for type annotation
+ src/poetry/repositories/link_sources/base.py:26:45: UP006 Use `collections.defaultdict` instead of `DefaultDict` for type annotation
+ src/poetry/repositories/link_sources/base.py:26:66: UP006 Use `list` instead of `List` for type annotation
+ tests/inspection/test_lazy_wheel.py:31:25: UP006 Use `tuple` instead of `Tuple` for type annotation
+ tests/inspection/test_lazy_wheel.py:31:36: UP006 Use `dict` instead of `Dict` for type annotation
+ tests/inspection/test_lazy_wheel.py:33:33: UP006 Use `dict` instead of `Dict` for type annotation

scikit-build/scikit-build-core (+1 -0 violations, +0 -0 fixes)

+ src/scikit_build_core/_compat/importlib/metadata.py:20:23: UP006 Use `list` instead of `typing.List` for type annotation

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
UP007 27 27 0 0 0
UP006 20 20 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+47 -0 violations, +0 -0 fixes in 7 projects; 37 projects unchanged)

apache/airflow (+28 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/executors/base_executor.py:53:19: UP006 Use `list` instead of `List` for type annotation
+ airflow/executors/base_executor.py:59:30: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:59:54: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:63:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:63:34: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:66:17: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/base_executor.py:66:53: UP007 Use `X | Y` for type annotations
+ airflow/executors/base_executor.py:66:68: UP007 Use `X | Y` for type annotations
+ airflow/executors/local_executor.py:56:24: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/executors/local_executor.py:56:30: UP007 Use `X | Y` for type annotations
+ airflow/executors/local_executor.py:56:57: UP007 Use `X | Y` for type annotations
+ airflow/models/mappedoperator.py:90:39: UP007 Use `X | Y` for type annotations
+ airflow/models/mappedoperator.py:90:76: UP006 Use `list` instead of `List` for type annotation
+ airflow/providers/celery/executors/celery_executor.py:98:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/celery/executors/celery_executor.py:98:64: UP007 Use `X | Y` for type annotations
+ airflow/providers/celery/executors/celery_executor_utils.py:62:28: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/celery/executors/celery_executor_utils.py:62:64: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:28:25: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:28:66: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:29: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:52: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:31:61: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:27: UP006 Use `tuple` instead of `Tuple` for type annotation
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:43: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:52: UP007 Use `X | Y` for type annotations
+ airflow/providers/cncf/kubernetes/executors/kubernetes_executor_types.py:34:84: UP006 Use `dict` instead of `Dict` for type annotation
+ airflow/providers/microsoft/azure/hooks/cosmos.py:47:24: UP007 Use `X | Y` for type annotations
... 1 additional changes omitted for rule UP007
+ airflow/providers/microsoft/azure/hooks/cosmos.py:47:35: UP006 Use `list` instead of `List` for type annotation

bokeh/bokeh (+8 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ src/bokeh/core/has_props.py:98:25: UP007 Use `X | Y` for type annotations
+ src/bokeh/document/events.py:122:26: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:87:37: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:89:24: UP007 Use `X | Y` for type annotations
+ src/bokeh/models/sources.py:89:48: UP007 Use `X | Y` for type annotations
+ src/bokeh/plotting/contour.py:42:31: UP007 Use `X | Y` for type annotations
+ src/bokeh/plotting/contour.py:43:40: UP007 Use `X | Y` for type annotations
+ src/bokeh/transform.py:71:26: UP007 Use `X | Y` for type annotations

latchbio/latch (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ latch/ldata/_transfer/upload.py:31:39: UP007 Use `X | Y` for type annotations
+ latch/ldata/_transfer/upload.py:32:42: UP007 Use `X | Y` for type annotations

pypa/cibuildwheel (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ cibuildwheel/typing.py:19:17: UP007 Use `X | Y` for type annotations

python/mypy (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ mypyc/build.py:58:32: UP007 Use `X | Y` for type annotations

python-poetry/poetry (+6 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/poetry/repositories/link_sources/base.py:26:17: UP006 Use `collections.defaultdict` instead of `DefaultDict` for type annotation
+ src/poetry/repositories/link_sources/base.py:26:45: UP006 Use `collections.defaultdict` instead of `DefaultDict` for type annotation
+ src/poetry/repositories/link_sources/base.py:26:66: UP006 Use `list` instead of `List` for type annotation
+ tests/inspection/test_lazy_wheel.py:31:25: UP006 Use `tuple` instead of `Tuple` for type annotation
+ tests/inspection/test_lazy_wheel.py:31:36: UP006 Use `dict` instead of `Dict` for type annotation
+ tests/inspection/test_lazy_wheel.py:33:33: UP006 Use `dict` instead of `Dict` for type annotation

scikit-build/scikit-build-core (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/scikit_build_core/_compat/importlib/metadata.py:20:23: UP006 Use `list` instead of `typing.List` for type annotation

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
UP007 27 27 0 0 0
UP006 20 20 0 0 0

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Looks good!

crates/ruff_python_semantic/src/model.rs Outdated Show resolved Hide resolved
crates/ruff_linter/src/checkers/ast/mod.rs Show resolved Hide resolved
crates/ruff_python_semantic/src/model.rs Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood marked this pull request as draft May 3, 2024 22:14
@AlexWaygood AlexWaygood marked this pull request as ready for review May 4, 2024 17:33
Comment on lines +2065 to +2068
if self.semantic.in_annotation()
&& (self.semantic.in_typing_only_context()
|| self.semantic.future_annotations())
{
Copy link
Member Author

Choose a reason for hiding this comment

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

I think honestly PYI020 is a better fit for stub files and if TYPE_CHECKING blocks, so the logic here should probably be changed. But for now I'm just maintaining compatibility with previous versions of Ruff, which emitted this lint on annotations in stub files as well as annotations in .py files. Abruptly reducing the scope of the rule like that would maybe be slightly backwards-incompatible; it at least should be considered in a separate PR.

@AlexWaygood AlexWaygood marked this pull request as draft May 5, 2024 08:24
@@ -42,3 +42,7 @@ class MyClass:
baz: MyClass
eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file
eggs = "baz" # always okay

if TYPE_CHECKING:

Choose a reason for hiding this comment

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

Can we a test for if there is code in an else block after the TYPE_CHECKING? That shouldn't be treated a stub, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can add a test for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linter Related to the linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive for F821 on circular reference in TYPE_CHECKING block
3 participants