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-bugbear] Add fix for duplicate-value (B033) #9510

Merged
merged 6 commits into from
Jan 14, 2024

Conversation

diceroll123
Copy link
Contributor

Summary

Adds autofix for B033

Test Plan

cargo test

Copy link

codspeed-hq bot commented Jan 14, 2024

CodSpeed Performance Report

Merging #9510 will not alter performance

Comparing diceroll123:add-B033-autofix (a88fd83) with main (953d48b)

Summary

✅ 30 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jan 14, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

ruff check --no-cache --exit-zero --preview --select ALL

- tests/providers/weaviate/hooks/test_weaviate.py:734:39: B033 Sets should not contain duplicate item `"uuid2"`
+ tests/providers/weaviate/hooks/test_weaviate.py:734:39: B033 [*] Sets should not contain duplicate item `"uuid2"`

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

ruff check --no-cache --exit-zero --preview

+ pandas/core/groupby/groupby.py:1226:1: PLR0904 Too many public methods (41 > 20)
+ pandas/core/groupby/groupby.py:1298:9: PLR0917 Too many positional arguments (13/5)
+ pandas/core/groupby/groupby.py:1449:9: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:1635:9: F841 Local variable `ids` is assigned to but never used
+ pandas/core/groupby/groupby.py:1852:9: PLR0917 Too many positional arguments (6/5)
+ pandas/core/groupby/groupby.py:1949:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ pandas/core/groupby/groupby.py:1988:27: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:1990:44: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:2001:19: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:2044:28: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:2444:13: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:2630:13: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:2739:13: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:2757:9: PLR0914 Too many local variables (27/15)
+ pandas/core/groupby/groupby.py:2757:9: PLR0917 Too many positional arguments (6/5)
+ pandas/core/groupby/groupby.py:3134:13: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:3253:13: PLC0415 `import` should be at the top-level of a file
+ pandas/core/groupby/groupby.py:3321:13: PLC0415 `import` should be at the top-level of a file
... 8 additional changes omitted for rule PLC0415
+ pandas/core/groupby/groupby.py:4280:26: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:4692:9: PLR0917 Too many positional arguments (6/5)
+ pandas/core/groupby/groupby.py:5071:9: PLR0917 Too many positional arguments (6/5)
+ pandas/core/groupby/groupby.py:5332:9: PLR0917 Too many positional arguments (6/5)
+ pandas/core/groupby/groupby.py:5387:31: PLR6201 Use a `set` literal when testing for membership
+ pandas/core/groupby/groupby.py:5648:9: PLR0917 Too many positional arguments (6/5)
... 2 additional changes omitted for rule PLR0917

Changes by rule (8 rules affected)

code total + violation - violation + fix - fix
PLC0415 14 14 0 0 0
PLR0917 8 8 0 0 0
PLR6201 6 6 0 0 0
B033 2 0 0 2 0
PLR0904 1 1 0 0 0
F841 1 1 0 0 0
E721 1 1 0 0 0
PLR0914 1 1 0 0 0

@diceroll123 diceroll123 force-pushed the add-B033-autofix branch 5 times, most recently from ac694f5 to ec40f0a Compare January 14, 2024 05:59
range: TextRange::default(),
})),
range,
));
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to do something similar to remove_argument, if possible, since the generator-based approach will lose comments. Are you open to trying that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like remove_argument is strictly for ExprCalls and associated Arguments, unsure if it'd work to cast everything over.

All usages of remove_argument come from actual ExprCalls, and we're working with a vector of Expr here from a set.

Please advise! 🙏🏽

Copy link
Member

Choose a reason for hiding this comment

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

Oh sorry, I meant, like, writing a new method that removes an element from a comma-separated collections, but based on the token stream, similar to remove_argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh. I'll see what I can do!

@diceroll123
Copy link
Contributor Author

diceroll123 commented Jan 14, 2024

Tweaked accordingly. The new function is not special to this set implementation, and could be placed into the fix::edits crate for reuse, but that's up to you!

I feel like this rule could also be expanded to work on Name.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks!

@charliermarsh charliermarsh enabled auto-merge (squash) January 14, 2024 23:09
@charliermarsh charliermarsh changed the title add autofix for B033 [flake8-bugbear] Add fix for duplicate-value (B033) Jan 14, 2024
@diceroll123
Copy link
Contributor Author

Woop, left dead code in there facepalm

Thanks Charlie!

@charliermarsh charliermarsh added preview Related to preview mode features fixes Related to suggested fixes for violations labels Jan 14, 2024
@charliermarsh
Copy link
Member

No it's all good, I just realized that with the revised fix implementation, we could revert back to the previous structure!

@charliermarsh
Copy link
Member

Had to move this under preview since we're not supposed to add safe fixes outside of preview (this is gonna change in the next minor release though, and we'll elevate this out of preview).

@charliermarsh charliermarsh enabled auto-merge (squash) January 14, 2024 23:14
@charliermarsh charliermarsh merged commit 0c0d3db into astral-sh:main Jan 14, 2024
16 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.

2 participants