-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[refurb
] Add coverage for using set(...) in single-item-membership-test
(FURB171
)
#15793
base: main
Are you sure you want to change the base?
[refurb
] Add coverage for using set(...) in single-item-membership-test
(FURB171
)
#15793
Conversation
77168c6
to
781af24
Compare
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
FURB171 | 5 | 5 | 0 | 0 | 0 |
Ah, I see, it doesn't work if the single argument to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! However, I think it would be incorrect for this rule to trigger on set(1)
, since that doesn't create a set with a single member at runtime. It raises an exception.
if 1 in set(1): | ||
print("Single-element set") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, this fails at runtime:
>>> set(1)
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
set(1)
~~~^^^
TypeError: 'int' object is not iterable
if 1 in set(1, 2): | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also fails at runtime:
>>> set(1, 2)
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
set(1, 2)
~~~^^^^^^
TypeError: set expected at most 1 argument, got 2
To clarify, @naslundx, all of these succeed at runtime, and I'd be okay with adding support for these to set([1])
set((1,))
set({1}) You'd need to check that the object being passed to the |
Additionally, that element should be literal/hashable, so that errors like this one are not masked: if a in set([{}]): ... # TypeError: unhashable type: 'dict'
if a == {}: ... # No errors |
I think we should open a separate issue about this because that behavior is already present in the current scope of the rule: 1 in {set()} # TypeError: unhashable type: 'set'
1 == set() # No errors |
refurb
] Add coverage for using set(...) in single-item-membership-test
(FURB171
)
Maybe check for frozenset([1])
frozenset((1,))
frozenset({1}) |
Summary
Adds coverage of using
set(...)
in addition to `{...} in SingleItemMembershipTest.Fixes #15792
Test Plan
Updated unit test and snapshot.
Steps to reproduce are in the issue linked above.