Add new rule InEmptyCollection by naslundx · Pull Request #16480 · astral-sh/ruff

naslundx · 2025-03-03T19:11:59Z

Summary

Introducing a new rule based on discussions in #15732 and #15729 that checks for unnecessary in with empty collections.

I called it in_empty_collection and gave the rule number PLR6202 but of course open for suggestions/clarifications.

Technically fixable (can be replaced by boolean or, if in an if-statement, removed entirely) and can implement that as well if it makes sense.

Rule is in preview group. (Tried following https://docs.astral.sh/ruff/contributing/#example-adding-a-new-lint-rule as best I could)

Test Plan

Tried locally against sample code. Added unit test, and created snapshot.

github-actions · 2025-03-03T19:18:49Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

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

+ tools/lib/provision.py:153:28: RUF060 Unnecessary membership test on empty collection
+ tools/lib/provision.py:153:73: RUF060 Unnecessary membership test on empty collection

pytest-dev/pytest (+3 -0 violations, +0 -0 fixes)

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

+ testing/test_assertrewrite.py:630:20: RUF060 Unnecessary membership test on empty collection
+ testing/test_assertrewrite.py:630:32: RUF060 Unnecessary membership test on empty collection
+ testing/test_assertrewrite.py:637:39: RUF060 Unnecessary membership test on empty collection

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF060	5	5	0	0	0

MichaReiser · 2025-03-04T10:38:34Z

@dylwil3 would you mind taking a look at this? I think you've most context on the change as you've been involved in both linked issues

dylwil3

Thanks @naslundx , this looks nice! One thing is that my comment in the linked issue was that we should open a separate issue to decide whether this should be a rule, but we never actually did that.

So would you be willing to propose this as a rule in a new issue, and link this PR to it? Then we can see what folks think and do some design/decision-making there.

If we do decide to go ahead with this rule, then we'll also probably have to move it to a RUF rule, otherwise there will be a clash when pylint adds an R6202.

crates/ruff_linter/src/rules/pylint/rules/in_empty_collection.rs

naslundx · 2025-03-08T19:54:06Z

Thanks @dylwil3 ! Created an issue here and to reach a decision #16569

Will let the PR rest until then.

VascoSch92 · 2025-03-09T14:15:31Z

crates/ruff_linter/resources/test/fixtures/pylint/in_empty_collection.py

@@ -0,0 +1,19 @@
+# Errors


Can you also detect something like this?

a=[] b= 7 if b in a: pass

I think is possible or?

With the current implementation, no. If there are mechanisms implemented to allow for this, please point me in the right direction!

Something like this may be possible when the symbol is defined in the same file, but it would be a bit tricky to do reliably especially when there is complicated control flow. I think we can look into extending the rule to cover this case later, but for now I think we should stick with catching literals.

dylwil3

Thanks for this! Would you mind adding a few more containers and tests for them?

dylwil3 · 2025-05-04T21:46:54Z

crates/ruff_linter/src/rules/pylint/rules/in_empty_collection.rs

+            func,
+            arguments,
+            range: _,
+        }) => semantic.match_builtin_expr(func, "set") && arguments.is_empty(),


We certainly at least want list, tuple, and dict here as well, but maybe we also want:

a match arm for strings, bytes literals, and f-strings of length zero?

the remaining builtin functions that create empty containers, which I think are: bytearray, bytes, frozenset, and str

Thanks, that makes a lot of sense! The f-string was a bit tricky to figure out but I think I got it right.

Updates are in a new commit

naslundx · 2025-05-05T14:26:20Z

we'll also probably have to move it to a RUF rule, otherwise there will be a clash when pylint adds an R6202.

Lmk if I should change the code @dylwil3

dylwil3 · 2025-05-05T17:30:30Z

Lmk if I should change the code @dylwil3

Yes thank you for the reminder! Looks like you're lucky number RUF060

dylwil3

Almost there! One nit and then we'll need to move the rule to RUF060. Thanks for bearing with me!

dylwil3 · 2025-05-05T17:43:14Z

crates/ruff_linter/src/rules/pylint/rules/in_empty_collection.rs

+        Expr::FString(s) => match s.as_single_part_fstring() {
+            Some(FString { elements, .. }) => elements.is_empty(),
+            _ => false,
+        },


I think this won't catch implicit concatenation like "a" in f"" "", so maybe:

Suggested change

Expr::FString(s) => match s.as_single_part_fstring() {

Some(FString { elements, .. }) => elements.is_empty(),

_ => false,

},

Expr::FString(s) => s

.value

.elements()

.all(|elt| elt.as_literal().is_some_and(|elt| elt.is_empty())),

naslundx · 2025-05-06T08:02:29Z

Change rule number to RUF060 ✔️

dylwil3

This is a great contribution, and I was really impressed with how comfortable you got with the codebase so quickly! Thank you!

## Summary Introducing a new rule based on discussions in astral-sh#15732 and astral-sh#15729 that checks for unnecessary in with empty collections. I called it in_empty_collection and gave the rule number RUF060. Rule is in preview group.

naslundx · 2025-05-07T08:31:29Z

Thanks a lot @dylwil3 !
I think there is still some room for improvement on this rule, I'll take another look when I find some time. :)

ntBre added the rule Implementing or modifying a lint rule label Mar 3, 2025

naslundx force-pushed the add-rule-in-empty-collection branch 2 times, most recently from 5c4b0a1 to b122182 Compare March 3, 2025 20:24

MichaReiser assigned dylwil3 Mar 4, 2025

dylwil3 reviewed Mar 6, 2025

View reviewed changes

crates/ruff_linter/src/rules/pylint/rules/in_empty_collection.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/pylint/rules/in_empty_collection.rs Outdated Show resolved Hide resolved

naslundx mentioned this pull request Mar 8, 2025

Rule proposal: in-empty-collection #16569

Closed

VascoSch92 reviewed Mar 9, 2025

View reviewed changes

naslundx force-pushed the add-rule-in-empty-collection branch from b122182 to 37779a5 Compare May 4, 2025 18:40

dylwil3 approved these changes May 4, 2025

View reviewed changes

naslundx force-pushed the add-rule-in-empty-collection branch from 953b74a to e4a4e8f Compare May 5, 2025 14:21

dylwil3 requested changes May 5, 2025

View reviewed changes

naslundx force-pushed the add-rule-in-empty-collection branch from e4a4e8f to b29590c Compare May 6, 2025 07:44

naslundx added 2 commits May 6, 2025 09:49

Add new rule InEmptyCollection

afe3f20

Cover more collections in InEmptyCollection

f66df9d

naslundx force-pushed the add-rule-in-empty-collection branch from b29590c to f66df9d Compare May 6, 2025 07:49

add example to documentation

0d52dbf

dylwil3 approved these changes May 6, 2025

View reviewed changes

dylwil3 enabled auto-merge (squash) May 6, 2025 13:51

dylwil3 merged commit 76b6d53 into astral-sh:main May 6, 2025
33 checks passed

BrewTestBot mentioned this pull request May 9, 2025

ruff 0.11.9 Homebrew/homebrew-core#222996

Merged

maxmynter mentioned this pull request May 10, 2025

Add Rule to check for singular yield in @contextlib.{async}contextmanager decorated functions (RUF062) #17281

Open

Comments

Conversation

naslundx commented Mar 3, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Mar 3, 2025 • 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!

MichaReiser commented Mar 4, 2025

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

naslundx commented Mar 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naslundx commented May 5, 2025

Uh oh!

dylwil3 commented May 5, 2025

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naslundx commented May 6, 2025

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

naslundx commented May 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Mar 3, 2025 •

edited

Loading

`ruff-ecosystem` results