[`flake8-pyi`] Implement `PYI062` (`duplicate-literal-member`) by tusharsadhwani · Pull Request #11269 · astral-sh/ruff

tusharsadhwani · 2024-05-03T19:27:49Z

Summary

Implements Y062 from flake8-pyi, mostly adopted from Rule::DuplicateUnionMember (without the | BinOp parts).

Currently DuplicateUnionMember also doesn't have an autofix for Union[] cases, so implementing both together seems like a good idea.

Test Plan

cargo test / cargo insta review

crates/ruff_linter/src/codes.rs

zanieb · 2024-05-03T19:48:57Z

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py

+
+x: Literal[True, False, True, False]  # PYI062 twice here
+
+y: Literal[1, print("hello"), 3, Literal[4, 1]]  # PYI062 on the last 1


Can you add test cases where the inner literal is entirely redundant? e.g.

Literal[1, Literal[1]]
Literal[1, 2, Literal[1, 2]]
Literal[1, Literal[1], Literal[1]]
Literal[1, Literal[2], Literal[2]]
Literal[1, Literal[2, Literal[1]]]

These will be more important for when we add a fix, I think.

It's also good to have coverage for cases like Literal[1, 1, 1] where we should raise the diagnostic twice.

github-actions · 2024-05-03T20:03:35Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

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

+ src/bokeh/events.py:569:36: PYI062 Duplicate literal member `-1`

python/typeshed (+14 -0 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

+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:10239: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:10244: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:10249: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:10254: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:3424: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:5833: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:5838: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:7915: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:7920: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:8172: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/compiler/xla/xla_pb2.pyi:1051:8177: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/core/protobuf/rewriter_config_pb2.pyi:496:918: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/core/protobuf/rewriter_config_pb2.pyi:496:923: PYI062 Duplicate literal member `...`
+ stubs/tensorflow/tensorflow/core/protobuf/rewriter_config_pb2.pyi:496:928: PYI062 Duplicate literal member `...`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PYI062	15	15	0	0	0

zanieb · 2024-05-03T21:08:32Z

Hm the typeshed ecosystem checks look concerning, do you know what's going on there?

tusharsadhwani · 2024-05-03T21:17:37Z

@zanieb Doesn't seem to reproduce locally at least with copypasting the concerned files :/

dhruvmanila · 2024-05-06T09:04:26Z

@tusharsadhwani can you try rebasing your PR on the latest main? Maybe it's out of sync.

AlexWaygood · 2024-05-06T09:06:26Z

@tusharsadhwani can you try rebasing your PR on the latest main? Maybe it's out of sync.

I think something might be up with the ecosystem checks generally when it comes to typeshed. I've had several odd results on some of my other PRs that have touched PYI checks, e.g. #11128. I want to try to look into this today.

tusharsadhwani · 2024-05-06T09:09:31Z

Can confirm, merging main doesn't fix the issue.

dhruvmanila · 2024-05-06T09:11:41Z

I think something might be up with the ecosystem checks generally when it comes to typeshed. I've had several odd results on some of my other PRs that have touched PYI checks, e.g. #11128. I want to try to look into this today.

Oh, I see. @AlexWaygood Can you open a new issue to keep track of it? No need to go into detail there but if you're unable to look into it we're still aware of it.

AlexWaygood · 2024-05-06T09:16:01Z

I think something might be up with the ecosystem checks generally when it comes to typeshed. I've had several odd results on some of my other PRs that have touched PYI checks, e.g. #11128. I want to try to look into this today.

Oh, I see. @AlexWaygood Can you open a new issue to keep track of it? No need to go into detail there but if you're unable to look into it we're still aware of it.

Done:

Odd ecosystem results on typeshed #11305

AlexWaygood

Nice!

crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs

crates/ruff_python_semantic/src/analyze/typing.rs

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

ruff.schema.json

AlexWaygood

LGTM, thanks!

AlexWaygood · 2024-05-07T18:26:11Z

I double-checked again, and I cannot reproduce the ecosystem results locally on my clone of typeshed. I checked out this PR branch locally in my ruff clone, and then ran this command from the directory of my typeshed clone, with the following results

% cargo run --manifest-path ../ruff/Cargo.toml -- check stubs stdlib --isolated --select=PYI062 --preview --no-cache
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `/Users/alexw/dev/ruff/target/debug/ruff check stubs stdlib --isolated --select=PYI062 --preview --no-cache`
All checks passed!

I also confirmed that the same command did highlight duplicate literal members if I purposefully introduced some to a random .pyi file in typeshed.

[flake8-pyi] Implement PYI062

54ddb9f

tusharsadhwani requested a review from AlexWaygood as a code owner May 3, 2024 19:27

update schema

85297ae

zanieb reviewed May 3, 2024

View reviewed changes

crates/ruff_linter/src/codes.rs Outdated Show resolved Hide resolved

move rule to preview group

a6e6336

zanieb reviewed May 3, 2024

View reviewed changes

Add test for nested literals

c8d3150

tusharsadhwani added 2 commits May 4, 2024 01:44

add a bunch more test cases

ecff8c5

clippy

60bcb4e

Merge branch 'main' into pyi-062

07ae2ac

AlexWaygood mentioned this pull request May 6, 2024

Odd ecosystem results on typeshed #11305

Open

AlexWaygood reviewed May 7, 2024

View reviewed changes

tusharsadhwani and others added 2 commits May 7, 2024 21:17

Apply suggestions from code review

40c75fb

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

update examples

784eb52

tusharsadhwani requested review from MichaReiser and dhruvmanila as code owners May 7, 2024 17:44

tusharsadhwani force-pushed the pyi-062 branch from 98e1a85 to 784eb52 Compare May 7, 2024 17:44

Merge branch 'main' into pyi-062

8061bb8

AlexWaygood removed request for MichaReiser and dhruvmanila May 7, 2024 17:47

remove visit in binop

f2797de

AlexWaygood reviewed May 7, 2024

View reviewed changes

ruff.schema.json Show resolved Hide resolved

undo unnecessary schema changes

8ccf569

AlexWaygood approved these changes May 7, 2024

View reviewed changes

AlexWaygood changed the title ~~[flake8-pyi] Implement PYI062~~ [flake8-pyi] Implement PYI062 (duplicate-literal-member) May 7, 2024

AlexWaygood merged commit 56b4c47 into astral-sh:main May 7, 2024

tusharsadhwani deleted the pyi-062 branch May 7, 2024 18:30

BrewTestBot mentioned this pull request May 9, 2024

ruff 0.4.4 Homebrew/homebrew-core#171299

Merged


		x: Literal[True, False, True, False] # PYI062 twice here

		y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1

Conversation

tusharsadhwani commented May 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

Uh oh!

zanieb May 3, 2024

Choose a reason for hiding this comment

Uh oh!

zanieb May 3, 2024

Choose a reason for hiding this comment

Uh oh!

zanieb May 3, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

zanieb commented May 3, 2024

Uh oh!

tusharsadhwani commented May 3, 2024

Uh oh!

dhruvmanila commented May 6, 2024

Uh oh!

AlexWaygood commented May 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tusharsadhwani commented May 6, 2024

Uh oh!

dhruvmanila commented May 6, 2024

Uh oh!

AlexWaygood commented May 6, 2024

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented May 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tusharsadhwani commented May 3, 2024 •

edited

Loading

github-actions bot commented May 3, 2024 •

edited

Loading

`ruff-ecosystem` results

AlexWaygood commented May 6, 2024 •

edited

Loading