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

[flake8-type-checking] Avoid false positives for | in TC008 #15201

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

Conversation

Daverball
Copy link
Contributor

This is a follow-up to #15180

Summary

| is not a valid runtime operation between types pre Python 3.10, so we should avoid unquoting the type expression.

Additionally most type checkers don't care about the execution context, so while they will allow | between types in stub files for convenience, they will still disallow it in type checking blocks, since those are treated as regular runtime code.

Test Plan

cargo nextest run

@Daverball

This comment was marked as outdated.

Copy link
Contributor

github-actions bot commented Dec 30, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -2 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

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

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

- airflow/decorators/condition.py:32:35: TC008 [*] Remove quotes from type alias
- airflow/decorators/condition.py:33:35: TC008 [*] Remove quotes from type alias

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
TC008 2 0 2 0 0

@Daverball

This comment was marked as resolved.

@MichaReiser
Copy link
Member

Reviewing this PR is a bit challenging right now because it's based on #15180 and I can't change the base because your an external contributor. I suggest we wait with reviewing until the base PR is merged. Please ping us if we happen to forget to review the PR after #15180 is merged.

@Daverball Daverball force-pushed the bugfix/tc008-union-syntax-pre-py310 branch from c5ad656 to 89d35c2 Compare January 8, 2025 12:40
@Daverball
Copy link
Contributor Author

@MichaReiser @AlexWaygood You should be able to review this now

@dhruvmanila dhruvmanila added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jan 8, 2025
@Daverball
Copy link
Contributor Author

Daverball commented Jan 8, 2025

The ecosystem results make sense to me now. Since airflow targets Python 3.9+ mypy would flag those type aliases if they were unquoted. So a TC008 shouldn't trigger, despite there being no runtime effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants