Implement PYI030: Unnecessary literal union #5570

zanieb · 2023-07-06T19:00:46Z

Implements PYI030 as part of #848

Union expressions should never have more than one Literal member, as Literal[1] | Literal[2] is semantically identical to Literal[1, 2].

Note we differ slightly from the flake8-pyi implementation:

We detect cases where there are parentheses or nested unions
We detect cases with mixed Union and | syntax
We use the same error message for all violations; flake8-pyi has two different messages
We retain the user's quoting style when displaying string literals; flake8-pyi uses single quotes
We warn on duplicates of the same literal Literal[1] | Literal[1]
- flake8-pyi does not warn on this because it overlaps with PYI016; we can avoid a violation here if we want but then we'll need to track the hashes of all seen literals alongside the vector of literal expressions which I would prefer to keep ordered

❯ flake8 --select Y030 crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:9:9: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:12:17: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:17:16: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:22:9: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:23:9: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:24:9: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:25:9: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:28:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:35:11: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:38:15: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:41:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2, 3]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:42:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2, 3, 4]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:45:10: Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[1, 2, 3]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:48:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 'foo', True]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:60:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:64:5: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:71:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2, 3, 4]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:74:23: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:77:10: Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
(19 errors)

❯ ./target/debug/ruff --select PYI030 crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:9:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:12:17: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:17:16: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:22:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:23:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:24:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:25:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:28:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:31:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:34:9: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:35:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:38:15: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:41:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:42:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3, 4]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:45:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:48:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, "foo", True]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:51:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 1]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:60:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:63:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:71:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2, 3, 4]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:74:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:77:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:80:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi:83:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 2]`
Found 24 errors.

github-actions · 2023-07-06T19:13:31Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+3, -0, 0 error(s))

airflow (+2, -0)

+ airflow/cli/commands/task_command.py:72:21: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[False, "db", "memory"]`
+ airflow/models/mappedoperator.py:80:20: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal["expand", "partial"]`

bokeh (+1, -0)

+ src/bokeh/core/serialization.py:148:25: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal["bool", "object"]`

Rules changed: 1

Rule	Changes	Additions	Removals
PYI030	3	3	0

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      9.1±0.37ms     4.5 MB/sec    1.00      9.0±0.45ms     4.5 MB/sec
formatter/numpy/ctypeslib.py               1.00  1960.9±78.37µs     8.5 MB/sec    1.02  1996.2±111.16µs     8.3 MB/sec
formatter/numpy/globals.py                 1.00   228.9±15.92µs    12.9 MB/sec    1.02   232.9±20.43µs    12.7 MB/sec
formatter/pydantic/types.py                1.00      4.3±0.22ms     5.9 MB/sec    1.02      4.4±0.25ms     5.8 MB/sec
linter/all-rules/large/dataset.py          1.02     15.9±0.55ms     2.6 MB/sec    1.00     15.6±0.50ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.03      4.0±0.26ms     4.2 MB/sec    1.00      3.9±0.20ms     4.3 MB/sec
linter/all-rules/numpy/globals.py          1.01   507.1±34.80µs     5.8 MB/sec    1.00   500.2±27.49µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.03      7.0±0.34ms     3.6 MB/sec    1.00      6.8±0.28ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.00      7.6±0.28ms     5.4 MB/sec    1.00      7.6±0.23ms     5.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1619.5±67.59µs    10.3 MB/sec    1.02  1645.2±78.43µs    10.1 MB/sec
linter/default-rules/numpy/globals.py      1.00   193.8±12.17µs    15.2 MB/sec    1.03   200.3±14.57µs    14.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.4±0.14ms     7.5 MB/sec    1.04      3.5±0.18ms     7.2 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.9±0.38ms     3.4 MB/sec    1.00     11.9±0.48ms     3.4 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.6±0.12ms     6.4 MB/sec    1.01      2.6±0.10ms     6.4 MB/sec
formatter/numpy/globals.py                 1.00   293.6±19.61µs    10.1 MB/sec    1.01   296.5±18.84µs    10.0 MB/sec
formatter/pydantic/types.py                1.00      5.5±0.26ms     4.6 MB/sec    1.04      5.8±0.28ms     4.4 MB/sec
linter/all-rules/large/dataset.py          1.01     20.5±0.66ms  2031.6 KB/sec    1.00     20.2±0.55ms     2.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      5.4±0.29ms     3.1 MB/sec    1.00      5.3±0.18ms     3.1 MB/sec
linter/all-rules/numpy/globals.py          1.00   654.2±44.01µs     4.5 MB/sec    1.01   663.5±36.00µs     4.4 MB/sec
linter/all-rules/pydantic/types.py         1.03      9.2±0.39ms     2.8 MB/sec    1.00      8.9±0.31ms     2.9 MB/sec
linter/default-rules/large/dataset.py      1.01     10.4±0.50ms     3.9 MB/sec    1.00     10.3±0.30ms     3.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.2±0.11ms     7.6 MB/sec    1.03      2.3±0.09ms     7.4 MB/sec
linter/default-rules/numpy/globals.py      1.00   253.7±17.24µs    11.6 MB/sec    1.03   261.8±15.93µs    11.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.5±0.21ms     5.6 MB/sec    1.02      4.6±0.17ms     5.5 MB/sec

charliermarsh

Looking good!

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

...s/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap

zanieb · 2023-07-06T21:37:11Z

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

+}
+
+/// Traverse a "union" type annotation, calling `func` on each expression in the union.
+fn traverse_union<'a, F>(func: &mut F, expr: &'a Expr, semantic: &SemanticModel)


In a follow-up, we can probably use this in PYI016 (#3922) and fix support for typing.Union there

zanieb · 2023-07-06T21:38:12Z

...s/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI030_PYI030.pyi.snap

+PYI030.pyi:51:10: PYI030 Multiple literal members in a union. Use a single literal, e.g. `Literal[1, 1]`
+   |
+50 | # Shouldn't emit for duplicate field types with same value; covered by Y016
+51 | field16: Literal[1] | Literal[1]  # OK
+   |          ^^^^^^^^^^^^^^^^^^^^^^^ PYI030
+52 | 
+53 | # Shouldn't emit if in new parent type


We should probably avoid emitting this but I'm hesitant to add a hash map alongside the vector used to display items in the correct order.

zanieb · 2023-07-06T21:43:24Z

Airflow example https://github.com/apache/airflow/blob/358e6e8fa18166084fc17b23e75c6c29a37f245f/airflow/cli/commands/task_command.py#L72

Bokeh example https://github.com/bokeh/bokeh/blob/ee3f7f27fb744860763edd1d44fcac9b797e36b4/src/bokeh/core/serialization.py#L148

charliermarsh · 2023-07-06T22:27:38Z

crates/ruff/src/checkers/ast/mod.rs

@@ -3136,6 +3150,7 @@ where
                if self.is_stub {
                    if self.enabled(Rule::DuplicateUnionMember)
                        && self.semantic.in_type_definition()


We may want to add these (&& self.semantic.in_type_definition()) to the Rule::UnnecessaryLiteralUnion condition too. I can't quite remember why they're necessary here. I'm guessing it's because for |, if it's used outside of a type definition, it's not a union operator (it's a bitwise or, I think).

That's true for the DuplicateUnionMember rule where they're checking for any duplicate items in a | but here we know we are constrained to literal types where this does apply.

We test for this at

https://github.com/charliermarsh/ruff/blob/1759d914456b62dfc479518ce8717949935f6c87/crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.pyi#L28

You can see that a bitwise or of literals resolve to a Union outside of an annotation:

In [3]: Literal[1] | Literal[2] Out[3]: typing.Union[typing.Literal[1], typing.Literal[2]] In [4]: x = Literal[1] | Literal[2] In [5]: x Out[5]: typing.Union[typing.Literal[1], typing.Literal[2]]

I guess it's possible that someone has Literal[1] | bar | Literal[2] where bar implements some sort of custom bitwise or behavior and this rule no longer holds but I'm not sure that's worth handling

Interesting, that makes sense. Thanks for the thorough follow-up!

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

zanieb · 2023-07-06T22:28:29Z

Note we could implement auto-fix for this but it seems painful given the enumerated edge-cases in the test file. We can pursue it at a later time if requested.

charliermarsh

LGTM! Couple small comments, address at your discretion.

crates/ruff/src/checkers/ast/mod.rs

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

Should reduce the number of heap allocations for uses without violations

zanieb changed the title ~~Implement PYI0303: Unnecessary literal union~~ Implement PYI030: Unnecessary literal union Jul 6, 2023

zanieb added 5 commits July 6, 2023 14:35

Implement PYI0303: Unnecessary literal union

7a05d80

Fix trailing quote on comments

fe2dd78

Add test coverage for typing_extensions.Literal and typing.Union

ca29457

Remove message information from test file for readability

d707704

Update schema

56f7d09

charliermarsh reviewed Jul 6, 2023

View reviewed changes

zanieb force-pushed the rule/pyi030 branch from ed3d331 to 9e1dd4f Compare July 6, 2023 20:19

Add support for typing.Union

9c7194a

zanieb force-pushed the rule/pyi030 branch from 9e1dd4f to 9c7194a Compare July 6, 2023 21:05

zanieb added 4 commits July 6, 2023 16:08

m -> literal_expr

e088b34

Clippy

8b9d54c

Update case number to 25

7c605c3

Fix duplicate violation reports for |

40e4fe3

zanieb commented Jul 6, 2023

View reviewed changes

Fix duplicate violation reports for Union

1759d91

zanieb marked this pull request as ready for review July 6, 2023 22:22

charliermarsh reviewed Jul 6, 2023

View reviewed changes

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs Show resolved Hide resolved

charliermarsh approved these changes Jul 6, 2023

View reviewed changes

Add imports to eamples

19f9cbf

charliermarsh added the rule Implementing or modifying a lint rule label Jul 6, 2023

MichaReiser approved these changes Jul 7, 2023

View reviewed changes

crates/ruff/src/checkers/ast/mod.rs Outdated Show resolved Hide resolved

crates/ruff/src/checkers/ast/mod.rs Outdated Show resolved Hide resolved

crates/ruff/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs Outdated Show resolved Hide resolved

zanieb added 3 commits July 7, 2023 10:47

Fix tuple check expression

8b068ad

Improve match check for parent union

becf391

Use a SmallVec for storing literal expressions

80d2269

Should reduce the number of heap allocations for uses without violations

Add missing ;

47d8983

zanieb enabled auto-merge (squash) July 7, 2023 16:32

zanieb merged commit bb7303f into astral-sh:main Jul 7, 2023

zanieb mentioned this pull request Jul 7, 2023

Add support for Union declarations without | to PYI016 #5598

Merged

p-linnane mentioned this pull request Jul 12, 2023

ruff 0.0.278 Homebrew/homebrew-core#136446

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement PYI030: Unnecessary literal union #5570

Implement PYI030: Unnecessary literal union #5570

zanieb commented Jul 6, 2023 •

edited

Loading

github-actions bot commented Jul 6, 2023 •

edited

Loading

charliermarsh left a comment

zanieb Jul 6, 2023

zanieb Jul 6, 2023

zanieb commented Jul 6, 2023

charliermarsh Jul 6, 2023

zanieb Jul 6, 2023

zanieb Jul 6, 2023

charliermarsh Jul 6, 2023

zanieb commented Jul 6, 2023

charliermarsh left a comment

Implement PYI030: Unnecessary literal union #5570

Implement PYI030: Unnecessary literal union #5570

Conversation

zanieb commented Jul 6, 2023 • edited Loading

github-actions bot commented Jul 6, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

charliermarsh left a comment

Choose a reason for hiding this comment

zanieb Jul 6, 2023

Choose a reason for hiding this comment

zanieb Jul 6, 2023

Choose a reason for hiding this comment

zanieb commented Jul 6, 2023

charliermarsh Jul 6, 2023

Choose a reason for hiding this comment

zanieb Jul 6, 2023

Choose a reason for hiding this comment

zanieb Jul 6, 2023

Choose a reason for hiding this comment

charliermarsh Jul 6, 2023

Choose a reason for hiding this comment

zanieb commented Jul 6, 2023

charliermarsh left a comment

Choose a reason for hiding this comment

zanieb commented Jul 6, 2023 •

edited

Loading

github-actions bot commented Jul 6, 2023 •

edited

Loading