Allow is and is not for direct type comparisons
#7905
Conversation
| { | ||
| if !matches!(op, CmpOp::Is | CmpOp::IsNot | CmpOp::Eq | CmpOp::NotEq) { | ||
| continue; | ||
| if is_type(left, checker.semantic()) || is_type(right, checker.semantic()) { |
There was a problem hiding this comment.
Unlike pycodestyle, our rule is symmetric... so, e.g., we flag both type(x) == int and int == type(x).
PR Check ResultsEcosystemℹ️ ecosystem check detected changes. (+3, -1, 0 error(s)) rotki (+3, -1)
- [*] 11 fixable with the `--fix` option (225 hidden fixes can be enabled with the `--unsafe-fixes` option). + [*] 13 fixable with the `--fix` option (225 hidden fixes can be enabled with the `--unsafe-fixes` option). + rotkehlchen/tests/api/test_settings.py:146:81: RUF100 [*] Unused `noqa` directive (unused: `E721`) + rotkehlchen/tests/api/test_settings.py:150:80: RUF100 [*] Unused `noqa` directive (unused: `E721`)
|
| /// if type(obj) is type(1): | ||
| /// if type(obj) == type(1): | ||
| /// pass |
There was a problem hiding this comment.
I think that the "Use instead" section needs to contain the following:
if type(obj) is type(1):
pass| Expr::Attribute(ast::ExprAttribute { value, .. }) => { | ||
| // Ex) `type(obj) is types.NoneType` | ||
| if checker | ||
| .semantic() | ||
| .resolve_call_path(value.as_ref()) | ||
| .is_some_and(|call_path| matches!(call_path.as_slice(), ["types", ..])) | ||
| { | ||
| checker | ||
| .diagnostics | ||
| .push(Diagnostic::new(TypeComparison, compare.range())); | ||
| } | ||
| } |
There was a problem hiding this comment.
This isn't present in the refactored code, is that an expected behavior?
The following doesn't get violated then:
if type(foo) == types.LambdaType:
...There was a problem hiding this comment.
Yeah, they removed this from pycodestyle with the understanding that a user-defined types would be a lot more common than accessing the standard library's types. I'm torn on it but figured it's fine to follow suit.
|
This may need to go out under preview \cc @zanieb |
|
@zanieb - What do you think of this? |
|
I agree preview makes sense for this change. Perhaps we should clarify in the versioning policy that we will confine changes in the scope of rules to minor releases. |
charliermarsh
force-pushed
the
charlie/E721
branch
4 times, most recently
from
7fa0918
to
dfcb2f7
Compare
charliermarsh
force-pushed
the
charlie/E721
branch
from
dfcb2f7
to
3ec2ee0
Compare
Summary
This PR updates our E721 implementation and semantics to match the updated
pycodestylelogic, which I think is an improvement. Specifically, we now allowtype(obj) is intfor exact type comparisons, which were previously impossible. So now, we're largely just linting against code liketype(obj) == int.This change is gated to preview mode.
Closes #7904.
Test Plan
Updated the test fixture and ensured parity with latest Flake8.