[flake8-pyi] Implement autofix and handle nested unions with single element (PYI041 , PYI055)
#14214
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbrugman
commented
Nov 8, 2024
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let (subtype, supertype) = match self.redundancy { | ||
| Redundancy::IntFloatComplex => ("int | float", "complex"), |
There was a problem hiding this comment.
Emitting multiple messages for the same union, such as "Use complex instead of int | complex" and "Use complex instead of float | complex" gets confusing when there is a fix presented:
- def bad4(self, arg: Union[float | complex, int]) -> None: ... # PYI041
+ def bad4(self, arg: complex) -> None: ... # PYI041With IntFloatComplex we can assign an unique violation for each combination.
sbrugman
commented
Nov 8, 2024
| let mut has_complex = false; | ||
| let mut has_int = false; | ||
| fn check_annotation<'a>(checker: &mut Checker, annotation: &'a Expr) { | ||
| let mut numeric_flags = NumericFlags::empty(); | ||
|
|
There was a problem hiding this comment.
Instead of has_*, I've used bitflags (which are used in some other parts of the code already).
(They correspond naturally to the order of Complex > Float > Int)
sbrugman
commented
Nov 8, 2024
| // Traverse the union a second time to construct the fix. | ||
| let mut flat_nodes: Vec<&Expr> = Vec::new(); | ||
|
|
||
| let mut union_type = UnionLike::TypingUnion; |
There was a problem hiding this comment.
I've chosen to do a second loop only when there is a violation, instead of collecting all expressions for any Union and filtering them later to avoid overhead in those cases.
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PYI041 | 67 | 0 | 1 | 66 | 0 |
| PYI016 | 8 | 0 | 0 | 8 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+0 -5 violations, +74 -0 fixes in 4 projects; 50 projects unchanged)
apache/airflow (+0 -0 violations, +58 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- airflow/decorators/__init__.pyi:117:25: PYI041 Use `float` instead of `int | float` + airflow/decorators/__init__.pyi:117:25: PYI041 [*] Use `float` instead of `int | float` - airflow/decorators/__init__.pyi:256:25: PYI041 Use `float` instead of `int | float` + airflow/decorators/__init__.pyi:256:25: PYI041 [*] Use `float` instead of `int | float` - airflow/jobs/job.py:308:39: PYI041 Use `float` instead of `int | float` + airflow/jobs/job.py:308:39: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/base_stats_logger.py:39:15: PYI041 Use `float` instead of `int | float` + airflow/metrics/base_stats_logger.py:39:15: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/base_stats_logger.py:50:15: PYI041 Use `float` instead of `int | float` + airflow/metrics/base_stats_logger.py:50:15: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/base_stats_logger.py:61:15: PYI041 Use `float` instead of `int | float` + airflow/metrics/base_stats_logger.py:61:15: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/datadog_logger.py:113:16: PYI041 Use `float` instead of `int | float` + airflow/metrics/datadog_logger.py:113:16: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/otel_logger.py:246:16: PYI041 Use `float` instead of `int | float` + airflow/metrics/otel_logger.py:246:16: PYI041 [*] Use `float` instead of `int | float` - airflow/metrics/statsd_logger.py:117:16: PYI041 Use `float` instead of `int | float` + airflow/metrics/statsd_logger.py:117:16: PYI041 [*] Use `float` instead of `int | float` - airflow/utils/timezone.py:240:26: PYI041 Use `float` instead of `int | float` + airflow/utils/timezone.py:240:26: PYI041 [*] Use `float` instead of `int | float` - airflow/utils/timezone.py:305:16: PYI041 Use `float` instead of `int | float` + airflow/utils/timezone.py:305:16: PYI041 [*] Use `float` instead of `int | float` - performance/src/performance_dags/performance_dag/performance_dag_utils.py:175:27: PYI041 Use `float` instead of `int | float` + performance/src/performance_dags/performance_dag/performance_dag_utils.py:175:27: PYI041 [*] Use `float` instead of `int | float` - performance/src/performance_dags/performance_dag/performance_dag_utils.py:337:31: PYI041 Use `float` instead of `int | float` + performance/src/performance_dags/performance_dag/performance_dag_utils.py:337:31: PYI041 [*] Use `float` instead of `int | float` - providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:273:16: PYI041 Use `float` instead of `int | float` + providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:273:16: PYI041 [*] Use `float` instead of `int | float` - providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:302:56: PYI041 Use `float` instead of `int | float` + providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:302:56: PYI041 [*] Use `float` instead of `int | float` - providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:325:57: PYI041 Use `float` instead of `int | float` + providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:325:57: PYI041 [*] Use `float` instead of `int | float` - providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:516:27: PYI041 Use `float` instead of `int | float` + providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:516:27: PYI041 [*] Use `float` instead of `int | float` - providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:516:47: PYI041 Use `float` instead of `int | float` + providers/src/airflow/providers/amazon/aws/hooks/batch_client.py:516:47: PYI041 [*] Use `float` instead of `int | float` ... 22 additional changes omitted for project
apache/superset (+0 -0 violations, +10 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- superset/daos/query.py:68:52: PYI041 Use `float` instead of `int | float` + superset/daos/query.py:68:52: PYI041 [*] Use `float` instead of `int | float` - superset/models/helpers.py:900:39: PYI016 Duplicate union member `str` + superset/models/helpers.py:900:39: PYI016 [*] Duplicate union member `str` - superset/utils/cache.py:150:14: PYI041 Use `float` instead of `int | float` + superset/utils/cache.py:150:14: PYI041 [*] Use `float` instead of `int | float` - superset/utils/core.py:349:24: PYI041 Use `float` instead of `int | float` + superset/utils/core.py:349:24: PYI041 [*] Use `float` instead of `int | float` ... 3 additional changes omitted for rule PYI041
bokeh/bokeh (+0 -0 violations, +6 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- src/bokeh/core/property/factors.py:51:56: PYI016 Duplicate union member `tuple[str, str]` + src/bokeh/core/property/factors.py:51:56: PYI016 [*] Duplicate union member `tuple[str, str]` - src/bokeh/core/property/factors.py:52:85: PYI016 Duplicate union member `tp.Sequence[tuple[str, str]]` + src/bokeh/core/property/factors.py:52:85: PYI016 [*] Duplicate union member `tp.Sequence[tuple[str, str]]` - src/bokeh/plotting/contour.py:43:88: PYI016 Duplicate union member `ContourColor` + src/bokeh/plotting/contour.py:43:88: PYI016 [*] Duplicate union member `ContourColor`
python/typeshed (+0 -5 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W
- stdlib/random.pyi:45:31: PYI041 Use `float` instead of `int | float` - stdlib/random.pyi:52:27: PYI041 Use `float` instead of `int | float` - stdlib/statistics.pyi:160:15: PYI041 Use `float` instead of `int | float` - stdlib/turtle.pyi:443:16: PYI041 Use `float` instead of `int | float` - stdlib/turtle.pyi:444:17: PYI041 Use `float` instead of `int | float`
Changes by rule (2 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PYI041 | 71 | 0 | 5 | 66 | 0 |
| PYI016 | 8 | 0 | 0 | 8 | 0 |
sbrugman
commented
Nov 8, 2024
| impl Redundancy { | ||
| pub(super) fn from_numeric_flags(numeric_flags: NumericFlags) -> Option<Self> { | ||
| match numeric_flags.bits() { | ||
| 0b0110 => Some(Self::FloatComplex), |
There was a problem hiding this comment.
There must be a more elegant way of doing this, but I haven't found it yet.
sbrugman
commented
Nov 8, 2024
|
|
||
| // Generate a [`Fix`] for two or more type expressions, e.g. `int | float | complex`. | ||
| fn generate_bit_or_fix(checker: &Checker, nodes: Vec<&Expr>, annotation: &Expr) -> Fix { | ||
| debug_assert!(nodes.len() >= 2, "At least two nodes required"); |
There was a problem hiding this comment.
The code above handles nodes.len() < 2 separately and thus this function should never encounter these cases. Is debug_assert! ok to denote this?
sbrugman
commented
Nov 8, 2024
| type_exprs.push(slice); | ||
| } else { | ||
| other_exprs.push(expr); | ||
| let mut has_pep604_union = false; |
There was a problem hiding this comment.
Rather than using the same Union syntax as the top-level union, I check if any PEP604-style union is encountered. If so, then we use that. (a | b or typing.Union[a, b])
sbrugman
force-pushed
the
pyi-nested-union-fix
branch
from
a55d7c3
to
ebecc10
Compare
…sary-type-union` (`PYI055`)
…nions for `redundant-numeric-union` (`PYI041`)
…ions contains comments.
sbrugman
force-pushed
the
pyi-nested-union-fix
branch
from
caf9102
to
622d119
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
PR to implement autofix for
redundant-numeric-union(PYI041) andduplication-union-member(PYI016)#14185
There appears to be an issue with the nested union detection when the union is unary:
This issue also affects at least
PYI055.Edit: the PR grew a bit bigger after finding issues with fix safety. I'll break up the PR into multiple for easier review.
Test Plan
Extension of the test cases and
cargo test