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-pyi] Extend fix to Python <= 3.9 for redundant-none-literal (PYI061) #16044

Merged
merged 10 commits into from
Feb 9, 2025

Conversation

dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Feb 8, 2025

This PR extends the fix offered for redundant-none-literal (PYI061) to include Python versions <= 3.9 by using typing.Optional instead of the operator |. We also offer the fix with | for any target version on stub files.

Closes #15795


For review:

  • I recommend reading the extended commit message for the second commit (where the fix logic is implemented), which hopefully explains the perhaps larger than expected diff.
  • Note that the snapshots can still have bit-or operators | since we will not remove them if they are already present, e.g. Literal[None] | int simplifies to None | int regardless of Python version or source type.

In case the version is <=3.9 and we are _not_ in a stub file,
the rule behavior changes as follows:

- The diagnostic messages reference `typing.Union` instead of `|`
- The offered fix first imports `typing.Union` and then uses it

In order to achieve this we had to make the following changes:
- `create_fix_edit` is now `create_fix` and returns a `Fix` instead of
an edit. That's because importing `Union` and using it is _two_ edits,
so we have to combine them into a `Fix` before returning.
- We use the enum `UnionKind` to determine one of three possible diagnostic
messages and fix behavior, rather than the previous `bool` of `other_literal_elements_seen`.

Finally, in order to uniformly handle building union expressions throughout
the codebase, we use the helpers `pep_604_union` and `typing_union` rather
to create the appropriate union expression, rather than manually building
this AST node.
@dylwil3 dylwil3 added fixes Related to suggested fixes for violations preview Related to preview mode features labels Feb 8, 2025
@dylwil3 dylwil3 requested a review from AlexWaygood as a code owner February 8, 2025 21:20
Copy link
Contributor

github-actions bot commented Feb 8, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+15 -15 violations, +0 -0 fixes in 4 projects; 51 projects unchanged)

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

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

+ airflow/models/dag.py:1016:36: PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]` 
- airflow/models/dag.py:1016:36: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`

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

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

+ src/latch/types/metadata.py:500:45: PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]` 
- src/latch/types/metadata.py:500:45: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`

pandas-dev/pandas (+4 -4 violations, +0 -0 fixes)

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

+ pandas/core/groupby/groupby.py:4181:39: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- pandas/core/groupby/groupby.py:4181:39: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ pandas/core/groupby/indexing.py:299:39: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- pandas/core/groupby/indexing.py:299:39: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ pandas/io/html.py:1045:28: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- pandas/io/html.py:1045:28: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ pandas/io/html.py:223:32: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- pandas/io/html.py:223:32: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`

zulip/zulip (+9 -9 violations, +0 -0 fixes)

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

+ corporate/views/billing_page.py:328:24: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/billing_page.py:328:24: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:158:26: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:158:26: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:159:42: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:159:42: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:160:48: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:160:48: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:679:26: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:679:26: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:680:42: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:680:42: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:681:48: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:681:48: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ corporate/views/remote_billing_page.py:70:5: PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]` 
- corporate/views/remote_billing_page.py:70:5: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
+ zerver/lib/event_types.py:785:67: PYI061 [*] Use `None` rather than `Literal[None]`
- zerver/lib/event_types.py:785:67: PYI061 [*] `Literal[None]` can be replaced with `None`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI061 30 15 15 0 0

@dylwil3
Copy link
Collaborator Author

dylwil3 commented Feb 8, 2025

Ecosystem changes as expected - both repos have target-version set to py39.

@AlexWaygood
Copy link
Member

Hmmm... wouldn't it be better to suggest Optional[Literal[...]] here rather than Union[Literal[...], None]? I think most people would consider that more idiomatic on Python <=3.9

@dylwil3
Copy link
Collaborator Author

dylwil3 commented Feb 9, 2025

Hmmm... wouldn't it be better to suggest Optional[Literal[...]]

🤦 of course! much better

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Excellent, thank you!

@dylwil3 dylwil3 merged commit f178ecc into astral-sh:main Feb 9, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

redundant-none-literal (PYI061): Unsafe or different autofix for Python 3.9
2 participants