Skip to content

Rule proposal: in-empty-collection #16569

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

Closed
naslundx opened this issue Mar 8, 2025 · 8 comments
Closed

Rule proposal: in-empty-collection #16569

naslundx opened this issue Mar 8, 2025 · 8 comments
Labels
rule Implementing or modifying a lint rule

Comments

@naslundx
Copy link
Contributor

naslundx commented Mar 8, 2025

Summary

After discussions in #15732 and #15729 an idea was proposed to add a rule for checking unnecessary membership test in empty collections.

This would reinstate the old behavior in rule PLR6201 to explicitly check for things like if 1 in [] or if 'a' not in ''.

Should ruff include it?

Example implementation is here: #16480

@MichaReiser MichaReiser changed the title Rule proposal: InEmptyCollection Rule proposal: in-empty-collection Mar 10, 2025
@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Mar 10, 2025
@MichaReiser
Copy link
Member

Unrelated on whether we decide to add this rule. I think I'd prefer naming it in-empty-sequence to match python's semantics OR exclude str from the rule

Strings are immutable sequences of Unicode code points. String literals are written in a variety of ways:
source

@MichaReiser
Copy link
Member

I think having this as a rule does make sense. What would you suggest as proposed fix?

@MichaReiser
Copy link
Member

@dylwil3 you asked in #16480 to open this issue. Do you have any concerns with the rule or should we accept this request and land the PR?

@dylwil3
Copy link
Collaborator

dylwil3 commented Apr 28, 2025

I'm personally happy with the rule, was just checking boxes - sorry for the bureaucratic red tape! @naslundx would you like to sync with main, respond to the remaining comments on the PR and we can get this landed?

@Avasam
Copy link
Contributor

Avasam commented Apr 29, 2025

On the name semantics, in is the operator for __contains__ * (see message below for caveat), and typing.Container is the Protocol for def __contains__. So in-empty-container ?
That covers strings, lists, tuples, dicts, sets, deque, and I'm sure many other stdlib containers I'm forgetting.
Ok, these are all collections... tbh I'm having a hard time finding a stdlib container that isn't a collection XD

Btw sequences are collections with a few extra methods. So the originally proposed name in-empty-collection still works whilst including strings imo.

@Avasam
Copy link
Contributor

Avasam commented Apr 29, 2025

"" in iter(()) and "" in iter("") (ie: Iterator[Never]) seem to be in the spirit of this rule, but not a Container in the Protocol sense. But in also works for Iterables and implementers of __getitem__() https://docs.python.org/3/reference/expressions.html#in

Actually, any empty <whatever this rule checks for> passed to iter I guess.

@naslundx
Copy link
Contributor Author

naslundx commented May 4, 2025

Not a problem at all @dylwil3 , sorry for dropping this for a while.

Agree with @Avasam that collection seems like the more proper name considering it doesn't just apply to sequences.

I will polish up the PR.

@dylwil3
Copy link
Collaborator

dylwil3 commented May 6, 2025

Resolved in #16480

@dylwil3 dylwil3 closed this as completed May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants