Skip to content

Commit

Permalink
[E721] Flag comparisons to memoryview (#8485)
Browse files Browse the repository at this point in the history 
## Summary

Adds `memoryview` to the list of typeclasses that `fn is_type()` uses
for type comparison checks so that it raises a violation if `is`, `is
not` or `isinstance()` are not used.

## Test Plan

Added examples to existing fixture

## Issue Link

Closes: #8483
  • Loading branch information
@qdegraaf
qdegraaf authored Nov 4, 2023
1 parent c488919 commit 75c9be0
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 140 deletions.
5 changes: 5 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
#: E721
if type(res) != type(""):
pass
#: E721
if type(res) == memoryview:
pass
#: Okay
import types

Expand Down Expand Up @@ -47,6 +50,8 @@
pass
if isinstance(res, types.MethodType):
pass
if isinstance(res, memoryview):
pass
#: Okay
def func_histype(a, b, c):
pass
Expand Down
12 changes: 11 additions & 1 deletion crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
| "list"
| "dict"
| "set"
| "memoryview"
) && checker.semantic().is_builtin(id)
{
checker.diagnostics.push(Diagnostic::new(
Expand Down Expand Up @@ -197,7 +198,16 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool {
// Ex) `type(obj) == int`
matches!(
id.as_str(),
"int" | "str" | "float" | "bool" | "complex" | "bytes" | "list" | "dict" | "set"
"int"
| "str"
| "float"
| "bool"
| "complex"
| "bytes"
| "list"
| "dict"
| "set"
| "memoryview"
) && semantic.is_builtin(id)
}
_ => false,
Expand Down
180 changes: 95 additions & 85 deletions ...src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E721_E721.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,144 +17,154 @@ E721.py:5:4: E721 Do not compare types, use `isinstance()`
5 | if type(res) != type(""):
| ^^^^^^^^^^^^^^^^^^^^^ E721
6 | pass
7 | #: Okay
7 | #: E721
|

E721.py:15:4: E721 Do not compare types, use `isinstance()`
E721.py:8:4: E721 Do not compare types, use `isinstance()`
|
13 | import types
14 |
15 | if type(res) is not types.ListType:
6 | pass
7 | #: E721
8 | if type(res) == memoryview:
| ^^^^^^^^^^^^^^^^^^^^^^^ E721
9 | pass
10 | #: Okay
|

E721.py:18:4: E721 Do not compare types, use `isinstance()`
|
16 | import types
17 |
18 | if type(res) is not types.ListType:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
16 | pass
17 | #: E721
19 | pass
20 | #: E721
|

E721.py:18:8: E721 Do not compare types, use `isinstance()`
E721.py:21:8: E721 Do not compare types, use `isinstance()`
|
16 | pass
17 | #: E721
18 | assert type(res) == type(False) or type(res) == type(None)
19 | pass
20 | #: E721
21 | assert type(res) == type(False) or type(res) == type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
19 | #: E721
20 | assert type(res) == type([])
22 | #: E721
23 | assert type(res) == type([])
|

E721.py:20:8: E721 Do not compare types, use `isinstance()`
E721.py:23:8: E721 Do not compare types, use `isinstance()`
|
18 | assert type(res) == type(False) or type(res) == type(None)
19 | #: E721
20 | assert type(res) == type([])
21 | assert type(res) == type(False) or type(res) == type(None)
22 | #: E721
23 | assert type(res) == type([])
| ^^^^^^^^^^^^^^^^^^^^^ E721
21 | #: E721
22 | assert type(res) == type(())
24 | #: E721
25 | assert type(res) == type(())
|

E721.py:22:8: E721 Do not compare types, use `isinstance()`
E721.py:25:8: E721 Do not compare types, use `isinstance()`
|
20 | assert type(res) == type([])
21 | #: E721
22 | assert type(res) == type(())
23 | assert type(res) == type([])
24 | #: E721
25 | assert type(res) == type(())
| ^^^^^^^^^^^^^^^^^^^^^ E721
23 | #: E721
24 | assert type(res) == type((0,))
26 | #: E721
27 | assert type(res) == type((0,))
|

E721.py:24:8: E721 Do not compare types, use `isinstance()`
E721.py:27:8: E721 Do not compare types, use `isinstance()`
|
22 | assert type(res) == type(())
23 | #: E721
24 | assert type(res) == type((0,))
25 | assert type(res) == type(())
26 | #: E721
27 | assert type(res) == type((0,))
| ^^^^^^^^^^^^^^^^^^^^^^^ E721
25 | #: E721
26 | assert type(res) == type((0))
28 | #: E721
29 | assert type(res) == type((0))
|

E721.py:26:8: E721 Do not compare types, use `isinstance()`
E721.py:29:8: E721 Do not compare types, use `isinstance()`
|
24 | assert type(res) == type((0,))
25 | #: E721
26 | assert type(res) == type((0))
27 | assert type(res) == type((0,))
28 | #: E721
29 | assert type(res) == type((0))
| ^^^^^^^^^^^^^^^^^^^^^^ E721
27 | #: E721
28 | assert type(res) != type((1, ))
30 | #: E721
31 | assert type(res) != type((1, ))
|

E721.py:28:8: E721 Do not compare types, use `isinstance()`
E721.py:31:8: E721 Do not compare types, use `isinstance()`
|
26 | assert type(res) == type((0))
27 | #: E721
28 | assert type(res) != type((1, ))
29 | assert type(res) == type((0))
30 | #: E721
31 | assert type(res) != type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
29 | #: Okay
30 | assert type(res) is type((1, ))
32 | #: Okay
33 | assert type(res) is type((1, ))
|

E721.py:30:8: E721 Do not compare types, use `isinstance()`
E721.py:33:8: E721 Do not compare types, use `isinstance()`
|
28 | assert type(res) != type((1, ))
29 | #: Okay
30 | assert type(res) is type((1, ))
31 | assert type(res) != type((1, ))
32 | #: Okay
33 | assert type(res) is type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
31 | #: Okay
32 | assert type(res) is not type((1, ))
34 | #: Okay
35 | assert type(res) is not type((1, ))
|

E721.py:32:8: E721 Do not compare types, use `isinstance()`
E721.py:35:8: E721 Do not compare types, use `isinstance()`
|
30 | assert type(res) is type((1, ))
31 | #: Okay
32 | assert type(res) is not type((1, ))
33 | assert type(res) is type((1, ))
34 | #: Okay
35 | assert type(res) is not type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
33 | #: E211 E721
34 | assert type(res) == type ([2, ])
36 | #: E211 E721
37 | assert type(res) == type ([2, ])
|

E721.py:34:8: E721 Do not compare types, use `isinstance()`
E721.py:37:8: E721 Do not compare types, use `isinstance()`
|
32 | assert type(res) is not type((1, ))
33 | #: E211 E721
34 | assert type(res) == type ([2, ])
35 | assert type(res) is not type((1, ))
36 | #: E211 E721
37 | assert type(res) == type ([2, ])
| ^^^^^^^^^^^^^^^^^^^^^^^^^ E721
35 | #: E201 E201 E202 E721
36 | assert type(res) == type( ( ) )
38 | #: E201 E201 E202 E721
39 | assert type(res) == type( ( ) )
|

E721.py:36:8: E721 Do not compare types, use `isinstance()`
E721.py:39:8: E721 Do not compare types, use `isinstance()`
|
34 | assert type(res) == type ([2, ])
35 | #: E201 E201 E202 E721
36 | assert type(res) == type( ( ) )
37 | assert type(res) == type ([2, ])
38 | #: E201 E201 E202 E721
39 | assert type(res) == type( ( ) )
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
37 | #: E201 E202 E721
38 | assert type(res) == type( (0, ) )
40 | #: E201 E202 E721
41 | assert type(res) == type( (0, ) )
|

E721.py:38:8: E721 Do not compare types, use `isinstance()`
E721.py:41:8: E721 Do not compare types, use `isinstance()`
|
36 | assert type(res) == type( ( ) )
37 | #: E201 E202 E721
38 | assert type(res) == type( (0, ) )
39 | assert type(res) == type( ( ) )
40 | #: E201 E202 E721
41 | assert type(res) == type( (0, ) )
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
39 | #:
42 | #:
|

E721.py:96:12: E721 Do not compare types, use `isinstance()`
|
94 | def asdf(self, value: str | None):
95 | #: E721
96 | if type(value) is str:
| ^^^^^^^^^^^^^^^^^^ E721
97 | ...
|
E721.py:101:12: E721 Do not compare types, use `isinstance()`
|
99 | def asdf(self, value: str | None):
100 | #: E721
101 | if type(value) is str:
| ^^^^^^^^^^^^^^^^^^ E721
102 | ...
|

E721.py:106:12: E721 Do not compare types, use `isinstance()`
E721.py:111:12: E721 Do not compare types, use `isinstance()`
|
104 | def asdf(self, value: str | None):
105 | #: E721
106 | if type(value) is str:
109 | def asdf(self, value: str | None):
110 | #: E721
111 | if type(value) is str:
| ^^^^^^^^^^^^^^^^^^ E721
107 | ...
112 | ...
|


Loading

0 comments on commit 75c9be0

Please sign in to comment.