[`pycodestyle`] Remove deprecated functionality from `type-comparison` (`E721`) (#11220)

## Summary

Stabilizes `E721` behavior implemented in #7905.

The functionality change in `E721` was implemented in #7905, released in
[v0.1.2](https://github.com/astral-sh/ruff/releases/tag/v0.1.2). And
seems functionally stable since #9676, without an explicit release but
would correspond to
[v0.2.0](https://github.com/astral-sh/ruff/releases/tag/v0.2.0). So the
deprecated functionally should be removable in the next minor release.

resolves: #6465
This commit is contained in:
Auguste Lalande 2024-06-25 12:21:45 -04:00 committed by Micha Reiser
parent c54bf0c734
commit c9a283a5ad
4 changed files with 36 additions and 318 deletions

View File

@ -69,7 +69,6 @@ mod tests {
} }
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))] #[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))] #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
#[test_case(Rule::RedundantBackslash, Path::new("E502.py"))] #[test_case(Rule::RedundantBackslash, Path::new("E502.py"))]
#[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_0.py"))] #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_0.py"))]

View File

@ -7,7 +7,6 @@ use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::settings::types::PreviewMode;
/// ## What it does /// ## What it does
/// Checks for object type comparisons using `==` and other comparison /// Checks for object type comparisons using `==` and other comparison
@ -37,119 +36,19 @@ use crate::settings::types::PreviewMode;
/// pass /// pass
/// ``` /// ```
#[violation] #[violation]
pub struct TypeComparison { pub struct TypeComparison;
preview: PreviewMode,
}
impl Violation for TypeComparison { impl Violation for TypeComparison {
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
match self.preview { format!(
PreviewMode::Disabled => format!("Do not compare types, use `isinstance()`"),
PreviewMode::Enabled => format!(
"Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks" "Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks"
), )
}
} }
} }
/// E721 /// E721
pub(crate) fn type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) { pub(crate) fn type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) {
match checker.settings.preview {
PreviewMode::Disabled => deprecated_type_comparison(checker, compare),
PreviewMode::Enabled => preview_type_comparison(checker, compare),
}
}
fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) {
for ((left, right), op) in std::iter::once(compare.left.as_ref())
.chain(compare.comparators.iter())
.tuple_windows()
.zip(compare.ops.iter())
{
if !matches!(op, CmpOp::Is | CmpOp::IsNot | CmpOp::Eq | CmpOp::NotEq) {
continue;
}
// Left-hand side must be, e.g., `type(obj)`.
let Expr::Call(ast::ExprCall { func, .. }) = left else {
continue;
};
let semantic = checker.semantic();
if !semantic.match_builtin_expr(func, "type") {
continue;
}
// Right-hand side must be, e.g., `type(1)` or `int`.
match right {
Expr::Call(ast::ExprCall {
func, arguments, ..
}) => {
// Ex) `type(obj) is type(1)`
if semantic.match_builtin_expr(func, "type") {
// Allow comparison for types which are not obvious.
if arguments
.args
.first()
.is_some_and(|arg| !arg.is_name_expr() && !arg.is_none_literal_expr())
{
checker.diagnostics.push(Diagnostic::new(
TypeComparison {
preview: PreviewMode::Disabled,
},
compare.range(),
));
}
}
}
Expr::Attribute(ast::ExprAttribute { value, .. }) => {
// Ex) `type(obj) is types.NoneType`
if semantic
.resolve_qualified_name(value.as_ref())
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["types", ..])
})
{
checker.diagnostics.push(Diagnostic::new(
TypeComparison {
preview: PreviewMode::Disabled,
},
compare.range(),
));
}
}
Expr::Name(ast::ExprName { id, .. }) => {
// Ex) `type(obj) is int`
if matches!(
id.as_str(),
"int"
| "str"
| "float"
| "bool"
| "complex"
| "bytes"
| "list"
| "dict"
| "set"
| "memoryview"
) && semantic.has_builtin_binding(id)
{
checker.diagnostics.push(Diagnostic::new(
TypeComparison {
preview: PreviewMode::Disabled,
},
compare.range(),
));
}
}
_ => {}
}
}
}
pub(crate) fn preview_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) {
for (left, right) in std::iter::once(compare.left.as_ref()) for (left, right) in std::iter::once(compare.left.as_ref())
.chain(compare.comparators.iter()) .chain(compare.comparators.iter())
.tuple_windows() .tuple_windows()
@ -165,12 +64,9 @@ pub(crate) fn preview_type_comparison(checker: &mut Checker, compare: &ast::Expr
} }
// Disallow the comparison. // Disallow the comparison.
checker.diagnostics.push(Diagnostic::new( checker
TypeComparison { .diagnostics
preview: PreviewMode::Enabled, .push(Diagnostic::new(TypeComparison, compare.range()));
},
compare.range(),
));
} }
} }
} }

View File

@ -1,7 +1,7 @@
--- ---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
--- ---
E721.py:2:4: E721 Do not compare types, use `isinstance()` E721.py:2:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
1 | #: E721 1 | #: E721
2 | if type(res) == type(42): 2 | if type(res) == type(42):
@ -10,7 +10,7 @@ E721.py:2:4: E721 Do not compare types, use `isinstance()`
4 | #: E721 4 | #: E721
| |
E721.py:5:4: E721 Do not compare types, use `isinstance()` E721.py:5:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
3 | pass 3 | pass
4 | #: E721 4 | #: E721
@ -20,7 +20,7 @@ E721.py:5:4: E721 Do not compare types, use `isinstance()`
7 | #: E721 7 | #: E721
| |
E721.py:8:4: E721 Do not compare types, use `isinstance()` E721.py:8:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
6 | pass 6 | pass
7 | #: E721 7 | #: E721
@ -30,17 +30,7 @@ E721.py:8:4: E721 Do not compare types, use `isinstance()`
10 | #: Okay 10 | #: Okay
| |
E721.py:18:4: E721 Do not compare types, use `isinstance()` E721.py:21:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
16 | import types
17 |
18 | if type(res) is not types.ListType:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
19 | pass
20 | #: E721
|
E721.py:21:8: E721 Do not compare types, use `isinstance()`
| |
19 | pass 19 | pass
20 | #: E721 20 | #: E721
@ -50,7 +40,7 @@ E721.py:21:8: E721 Do not compare types, use `isinstance()`
23 | assert type(res) == type([]) 23 | assert type(res) == type([])
| |
E721.py:23:8: E721 Do not compare types, use `isinstance()` E721.py:23:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
21 | assert type(res) == type(False) or type(res) == type(None) 21 | assert type(res) == type(False) or type(res) == type(None)
22 | #: E721 22 | #: E721
@ -60,7 +50,7 @@ E721.py:23:8: E721 Do not compare types, use `isinstance()`
25 | assert type(res) == type(()) 25 | assert type(res) == type(())
| |
E721.py:25:8: E721 Do not compare types, use `isinstance()` E721.py:25:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
23 | assert type(res) == type([]) 23 | assert type(res) == type([])
24 | #: E721 24 | #: E721
@ -70,7 +60,7 @@ E721.py:25:8: E721 Do not compare types, use `isinstance()`
27 | assert type(res) == type((0,)) 27 | assert type(res) == type((0,))
| |
E721.py:27:8: E721 Do not compare types, use `isinstance()` E721.py:27:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
25 | assert type(res) == type(()) 25 | assert type(res) == type(())
26 | #: E721 26 | #: E721
@ -80,7 +70,7 @@ E721.py:27:8: E721 Do not compare types, use `isinstance()`
29 | assert type(res) == type((0)) 29 | assert type(res) == type((0))
| |
E721.py:29:8: E721 Do not compare types, use `isinstance()` E721.py:29:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
27 | assert type(res) == type((0,)) 27 | assert type(res) == type((0,))
28 | #: E721 28 | #: E721
@ -90,7 +80,7 @@ E721.py:29:8: E721 Do not compare types, use `isinstance()`
31 | assert type(res) != type((1, )) 31 | assert type(res) != type((1, ))
| |
E721.py:31:8: E721 Do not compare types, use `isinstance()` E721.py:31:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
29 | assert type(res) == type((0)) 29 | assert type(res) == type((0))
30 | #: E721 30 | #: E721
@ -100,27 +90,7 @@ E721.py:31:8: E721 Do not compare types, use `isinstance()`
33 | assert type(res) is type((1, )) 33 | assert type(res) is type((1, ))
| |
E721.py:33:8: E721 Do not compare types, use `isinstance()` E721.py:37:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
31 | assert type(res) != type((1, ))
32 | #: Okay
33 | assert type(res) is type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
34 | #: Okay
35 | assert type(res) is not type((1, ))
|
E721.py:35:8: E721 Do not compare types, use `isinstance()`
|
33 | assert type(res) is type((1, ))
34 | #: Okay
35 | assert type(res) is not type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
36 | #: E211 E721
37 | assert type(res) == type ([2, ])
|
E721.py:37:8: E721 Do not compare types, use `isinstance()`
| |
35 | assert type(res) is not type((1, )) 35 | assert type(res) is not type((1, ))
36 | #: E211 E721 36 | #: E211 E721
@ -130,7 +100,7 @@ E721.py:37:8: E721 Do not compare types, use `isinstance()`
39 | assert type(res) == type( ( ) ) 39 | assert type(res) == type( ( ) )
| |
E721.py:39:8: E721 Do not compare types, use `isinstance()` E721.py:39:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
37 | assert type(res) == type ([2, ]) 37 | assert type(res) == type ([2, ])
38 | #: E201 E201 E202 E721 38 | #: E201 E201 E202 E721
@ -140,7 +110,7 @@ E721.py:39:8: E721 Do not compare types, use `isinstance()`
41 | assert type(res) == type( (0, ) ) 41 | assert type(res) == type( (0, ) )
| |
E721.py:41:8: E721 Do not compare types, use `isinstance()` E721.py:41:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
39 | assert type(res) == type( ( ) ) 39 | assert type(res) == type( ( ) )
40 | #: E201 E202 E721 40 | #: E201 E202 E721
@ -149,25 +119,26 @@ E721.py:41:8: E721 Do not compare types, use `isinstance()`
42 | #: 42 | #:
| |
E721.py:107:12: E721 Do not compare types, use `isinstance()` E721.py:59:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
105 | def asdf(self, value: str | None): 57 | pass
106 | #: E721 58 | #: E721
107 | if type(value) is str: 59 | if type(res) == type:
| ^^^^^^^^^^^^^^^^^^ E721 | ^^^^^^^^^^^^^^^^^ E721
108 | ... 60 | pass
61 | #: Okay
| |
E721.py:117:12: E721 Do not compare types, use `isinstance()` E721.py:140:1: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
115 | def asdf(self, value: str | None): 139 | #: E721
116 | #: E721 140 | dtype == float
117 | if type(value) is str: | ^^^^^^^^^^^^^^ E721
| ^^^^^^^^^^^^^^^^^^ E721 141 |
118 | ... 142 | import builtins
| |
E721.py:144:4: E721 Do not compare types, use `isinstance()` E721.py:144:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
| |
142 | import builtins 142 | import builtins
143 | 143 |

View File

@ -1,148 +0,0 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E721.py:2:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
1 | #: E721
2 | if type(res) == type(42):
| ^^^^^^^^^^^^^^^^^^^^^ E721
3 | pass
4 | #: E721
|
E721.py:5:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
3 | pass
4 | #: E721
5 | if type(res) != type(""):
| ^^^^^^^^^^^^^^^^^^^^^ E721
6 | pass
7 | #: E721
|
E721.py:8:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
6 | pass
7 | #: E721
8 | if type(res) == memoryview:
| ^^^^^^^^^^^^^^^^^^^^^^^ E721
9 | pass
10 | #: Okay
|
E721.py:21:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
19 | pass
20 | #: E721
21 | assert type(res) == type(False) or type(res) == type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
22 | #: E721
23 | assert type(res) == type([])
|
E721.py:23:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
21 | assert type(res) == type(False) or type(res) == type(None)
22 | #: E721
23 | assert type(res) == type([])
| ^^^^^^^^^^^^^^^^^^^^^ E721
24 | #: E721
25 | assert type(res) == type(())
|
E721.py:25:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
23 | assert type(res) == type([])
24 | #: E721
25 | assert type(res) == type(())
| ^^^^^^^^^^^^^^^^^^^^^ E721
26 | #: E721
27 | assert type(res) == type((0,))
|
E721.py:27:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
25 | assert type(res) == type(())
26 | #: E721
27 | assert type(res) == type((0,))
| ^^^^^^^^^^^^^^^^^^^^^^^ E721
28 | #: E721
29 | assert type(res) == type((0))
|
E721.py:29:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
27 | assert type(res) == type((0,))
28 | #: E721
29 | assert type(res) == type((0))
| ^^^^^^^^^^^^^^^^^^^^^^ E721
30 | #: E721
31 | assert type(res) != type((1, ))
|
E721.py:31:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
29 | assert type(res) == type((0))
30 | #: E721
31 | assert type(res) != type((1, ))
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
32 | #: Okay
33 | assert type(res) is type((1, ))
|
E721.py:37:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
35 | assert type(res) is not type((1, ))
36 | #: E211 E721
37 | assert type(res) == type ([2, ])
| ^^^^^^^^^^^^^^^^^^^^^^^^^ E721
38 | #: E201 E201 E202 E721
39 | assert type(res) == type( ( ) )
|
E721.py:39:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
37 | assert type(res) == type ([2, ])
38 | #: E201 E201 E202 E721
39 | assert type(res) == type( ( ) )
| ^^^^^^^^^^^^^^^^^^^^^^^^ E721
40 | #: E201 E202 E721
41 | assert type(res) == type( (0, ) )
|
E721.py:41:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
39 | assert type(res) == type( ( ) )
40 | #: E201 E202 E721
41 | assert type(res) == type( (0, ) )
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
42 | #:
|
E721.py:59:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
57 | pass
58 | #: E721
59 | if type(res) == type:
| ^^^^^^^^^^^^^^^^^ E721
60 | pass
61 | #: Okay
|
E721.py:140:1: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
139 | #: E721
140 | dtype == float
| ^^^^^^^^^^^^^^ E721
141 |
142 | import builtins
|
E721.py:144:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
142 | import builtins
143 |
144 | if builtins.type(res) == memoryview: # E721
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
145 | pass
|