Add check for is comparison with mutable initialisers to rule F632 (#8607)

## Summary

Adds an extra check to F632 to check for any `is` comparisons to a
mutable initialisers.
Implements #8589 .

Example:
```Python
named_var = {}
if named_var is {}:  # F632 (fix)
    pass
```
The if condition will always evaluate to False because it checks on
identity and it's impossible to take the same identity as a hard coded
list/set/dict initializer.

## Test Plan

Multiple test cases were added to ensure the rule works + doesn't flag
false positives + the fix works correctly.
This commit is contained in:
Jesse Serrao 2023-11-11 00:29:23 +00:00 committed by GitHub
parent 8207d6df82
commit 39728a1198
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 594 additions and 3 deletions

View File

@ -16,6 +16,48 @@ if False == None: # E711, E712 (fix)
if None == False: # E711, E712 (fix) if None == False: # E711, E712 (fix)
pass pass
named_var = []
if [] is []: # F632 (fix)
pass
if named_var is []: # F632 (fix)
pass
if [] is named_var: # F632 (fix)
pass
if named_var is [1]: # F632 (fix)
pass
if [1] is named_var: # F632 (fix)
pass
if named_var is [i for i in [1]]: # F632 (fix)
pass
named_var = {}
if {} is {}: # F632 (fix)
pass
if named_var is {}: # F632 (fix)
pass
if {} is named_var: # F632 (fix)
pass
if named_var is {1}: # F632 (fix)
pass
if {1} is named_var: # F632 (fix)
pass
if named_var is {i for i in [1]}: # F632 (fix)
pass
named_var = {1: 1}
if {1: 1} is {1: 1}: # F632 (fix)
pass
if named_var is {1: 1}: # F632 (fix)
pass
if {1: 1} is named_var: # F632 (fix)
pass
if named_var is {1: 1}: # F632 (fix)
pass
if {1: 1} is named_var: # F632 (fix)
pass
if named_var is {i: 1 for i in [1]}: # F632 (fix)
pass
### ###
# Non-errors # Non-errors
### ###
@ -33,3 +75,45 @@ if False is None:
pass pass
if None is False: if None is False:
pass pass
named_var = []
if [] == []:
pass
if named_var == []:
pass
if [] == named_var:
pass
if named_var == [1]:
pass
if [1] == named_var:
pass
if named_var == [i for i in [1]]:
pass
named_var = {}
if {} == {}:
pass
if named_var == {}:
pass
if {} == named_var:
pass
if named_var == {1}:
pass
if {1} == named_var:
pass
if named_var == {i for i in [1]}:
pass
named_var = {1: 1}
if {1: 1} == {1: 1}:
pass
if named_var == {1: 1}:
pass
if {1: 1} == named_var:
pass
if named_var == {1: 1}:
pass
if {1: 1} == named_var:
pass
if named_var == {i: 1 for i in [1]}:
pass

View File

@ -63,6 +63,7 @@ mod tests {
Ok(()) Ok(())
} }
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
#[test_case(Rule::TypeComparison, Path::new("E721.py"))] #[test_case(Rule::TypeComparison, Path::new("E721.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!( let snapshot = format!(

View File

@ -166,7 +166,7 @@ constant_literals.py:16:4: E711 [*] Comparison to `None` should be `cond is None
16 |+if None is False: # E711, E712 (fix) 16 |+if None is False: # E711, E712 (fix)
17 17 | pass 17 17 | pass
18 18 | 18 18 |
19 19 | ### 19 19 | named_var = []
constant_literals.py:16:12: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:` constant_literals.py:16:12: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:`
| |
@ -186,6 +186,6 @@ constant_literals.py:16:12: E712 [*] Comparison to `False` should be `cond is Fa
16 |+if None is False: # E711, E712 (fix) 16 |+if None is False: # E711, E712 (fix)
17 17 | pass 17 17 | pass
18 18 | 18 18 |
19 19 | ### 19 19 | named_var = []

View File

@ -0,0 +1,481 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
constant_literals.py:4:4: F632 [*] Use `==` to compare constant literals
|
2 | # Errors
3 | ###
4 | if "abc" is "def": # F632 (fix)
| ^^^^^^^^^^^^^^ F632
5 | pass
6 | if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
|
= help: Replace `is` with `==`
Safe fix
1 1 | ###
2 2 | # Errors
3 3 | ###
4 |-if "abc" is "def": # F632 (fix)
4 |+if "abc" == "def": # F632 (fix)
5 5 | pass
6 6 | if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
7 7 | pass
constant_literals.py:6:4: F632 [*] Use `==` to compare constant literals
|
4 | if "abc" is "def": # F632 (fix)
5 | pass
6 | if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
| ^^^^^^^^^^^^^ F632
7 | pass
8 | if None is "abc": # F632 (fix, but leaves behind unfixable E711)
|
= help: Replace `is` with `==`
Safe fix
3 3 | ###
4 4 | if "abc" is "def": # F632 (fix)
5 5 | pass
6 |-if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
6 |+if "abc" == None: # F632 (fix, but leaves behind unfixable E711)
7 7 | pass
8 8 | if None is "abc": # F632 (fix, but leaves behind unfixable E711)
9 9 | pass
constant_literals.py:8:4: F632 [*] Use `==` to compare constant literals
|
6 | if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
7 | pass
8 | if None is "abc": # F632 (fix, but leaves behind unfixable E711)
| ^^^^^^^^^^^^^ F632
9 | pass
10 | if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
|
= help: Replace `is` with `==`
Safe fix
5 5 | pass
6 6 | if "abc" is None: # F632 (fix, but leaves behind unfixable E711)
7 7 | pass
8 |-if None is "abc": # F632 (fix, but leaves behind unfixable E711)
8 |+if None == "abc": # F632 (fix, but leaves behind unfixable E711)
9 9 | pass
10 10 | if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
11 11 | pass
constant_literals.py:10:4: F632 [*] Use `==` to compare constant literals
|
8 | if None is "abc": # F632 (fix, but leaves behind unfixable E711)
9 | pass
10 | if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
| ^^^^^^^^^^^^^^ F632
11 | pass
12 | if False is "abc": # F632 (fix, but leaves behind unfixable E712)
|
= help: Replace `is` with `==`
Safe fix
7 7 | pass
8 8 | if None is "abc": # F632 (fix, but leaves behind unfixable E711)
9 9 | pass
10 |-if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
10 |+if "abc" == False: # F632 (fix, but leaves behind unfixable E712)
11 11 | pass
12 12 | if False is "abc": # F632 (fix, but leaves behind unfixable E712)
13 13 | pass
constant_literals.py:12:4: F632 [*] Use `==` to compare constant literals
|
10 | if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
11 | pass
12 | if False is "abc": # F632 (fix, but leaves behind unfixable E712)
| ^^^^^^^^^^^^^^ F632
13 | pass
14 | if False == None: # E711, E712 (fix)
|
= help: Replace `is` with `==`
Safe fix
9 9 | pass
10 10 | if "abc" is False: # F632 (fix, but leaves behind unfixable E712)
11 11 | pass
12 |-if False is "abc": # F632 (fix, but leaves behind unfixable E712)
12 |+if False == "abc": # F632 (fix, but leaves behind unfixable E712)
13 13 | pass
14 14 | if False == None: # E711, E712 (fix)
15 15 | pass
constant_literals.py:20:4: F632 [*] Use `==` to compare constant literals
|
19 | named_var = []
20 | if [] is []: # F632 (fix)
| ^^^^^^^^ F632
21 | pass
22 | if named_var is []: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
17 17 | pass
18 18 |
19 19 | named_var = []
20 |-if [] is []: # F632 (fix)
20 |+if [] == []: # F632 (fix)
21 21 | pass
22 22 | if named_var is []: # F632 (fix)
23 23 | pass
constant_literals.py:22:4: F632 [*] Use `==` to compare constant literals
|
20 | if [] is []: # F632 (fix)
21 | pass
22 | if named_var is []: # F632 (fix)
| ^^^^^^^^^^^^^^^ F632
23 | pass
24 | if [] is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
19 19 | named_var = []
20 20 | if [] is []: # F632 (fix)
21 21 | pass
22 |-if named_var is []: # F632 (fix)
22 |+if named_var == []: # F632 (fix)
23 23 | pass
24 24 | if [] is named_var: # F632 (fix)
25 25 | pass
constant_literals.py:24:4: F632 [*] Use `==` to compare constant literals
|
22 | if named_var is []: # F632 (fix)
23 | pass
24 | if [] is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^ F632
25 | pass
26 | if named_var is [1]: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
21 21 | pass
22 22 | if named_var is []: # F632 (fix)
23 23 | pass
24 |-if [] is named_var: # F632 (fix)
24 |+if [] == named_var: # F632 (fix)
25 25 | pass
26 26 | if named_var is [1]: # F632 (fix)
27 27 | pass
constant_literals.py:26:4: F632 [*] Use `==` to compare constant literals
|
24 | if [] is named_var: # F632 (fix)
25 | pass
26 | if named_var is [1]: # F632 (fix)
| ^^^^^^^^^^^^^^^^ F632
27 | pass
28 | if [1] is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
23 23 | pass
24 24 | if [] is named_var: # F632 (fix)
25 25 | pass
26 |-if named_var is [1]: # F632 (fix)
26 |+if named_var == [1]: # F632 (fix)
27 27 | pass
28 28 | if [1] is named_var: # F632 (fix)
29 29 | pass
constant_literals.py:28:4: F632 [*] Use `==` to compare constant literals
|
26 | if named_var is [1]: # F632 (fix)
27 | pass
28 | if [1] is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^^ F632
29 | pass
30 | if named_var is [i for i in [1]]: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
25 25 | pass
26 26 | if named_var is [1]: # F632 (fix)
27 27 | pass
28 |-if [1] is named_var: # F632 (fix)
28 |+if [1] == named_var: # F632 (fix)
29 29 | pass
30 30 | if named_var is [i for i in [1]]: # F632 (fix)
31 31 | pass
constant_literals.py:30:4: F632 [*] Use `==` to compare constant literals
|
28 | if [1] is named_var: # F632 (fix)
29 | pass
30 | if named_var is [i for i in [1]]: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F632
31 | pass
|
= help: Replace `is` with `==`
Safe fix
27 27 | pass
28 28 | if [1] is named_var: # F632 (fix)
29 29 | pass
30 |-if named_var is [i for i in [1]]: # F632 (fix)
30 |+if named_var == [i for i in [1]]: # F632 (fix)
31 31 | pass
32 32 |
33 33 | named_var = {}
constant_literals.py:34:4: F632 [*] Use `==` to compare constant literals
|
33 | named_var = {}
34 | if {} is {}: # F632 (fix)
| ^^^^^^^^ F632
35 | pass
36 | if named_var is {}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
31 31 | pass
32 32 |
33 33 | named_var = {}
34 |-if {} is {}: # F632 (fix)
34 |+if {} == {}: # F632 (fix)
35 35 | pass
36 36 | if named_var is {}: # F632 (fix)
37 37 | pass
constant_literals.py:36:4: F632 [*] Use `==` to compare constant literals
|
34 | if {} is {}: # F632 (fix)
35 | pass
36 | if named_var is {}: # F632 (fix)
| ^^^^^^^^^^^^^^^ F632
37 | pass
38 | if {} is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
33 33 | named_var = {}
34 34 | if {} is {}: # F632 (fix)
35 35 | pass
36 |-if named_var is {}: # F632 (fix)
36 |+if named_var == {}: # F632 (fix)
37 37 | pass
38 38 | if {} is named_var: # F632 (fix)
39 39 | pass
constant_literals.py:38:4: F632 [*] Use `==` to compare constant literals
|
36 | if named_var is {}: # F632 (fix)
37 | pass
38 | if {} is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^ F632
39 | pass
40 | if named_var is {1}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
35 35 | pass
36 36 | if named_var is {}: # F632 (fix)
37 37 | pass
38 |-if {} is named_var: # F632 (fix)
38 |+if {} == named_var: # F632 (fix)
39 39 | pass
40 40 | if named_var is {1}: # F632 (fix)
41 41 | pass
constant_literals.py:40:4: F632 [*] Use `==` to compare constant literals
|
38 | if {} is named_var: # F632 (fix)
39 | pass
40 | if named_var is {1}: # F632 (fix)
| ^^^^^^^^^^^^^^^^ F632
41 | pass
42 | if {1} is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
37 37 | pass
38 38 | if {} is named_var: # F632 (fix)
39 39 | pass
40 |-if named_var is {1}: # F632 (fix)
40 |+if named_var == {1}: # F632 (fix)
41 41 | pass
42 42 | if {1} is named_var: # F632 (fix)
43 43 | pass
constant_literals.py:42:4: F632 [*] Use `==` to compare constant literals
|
40 | if named_var is {1}: # F632 (fix)
41 | pass
42 | if {1} is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^^ F632
43 | pass
44 | if named_var is {i for i in [1]}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
39 39 | pass
40 40 | if named_var is {1}: # F632 (fix)
41 41 | pass
42 |-if {1} is named_var: # F632 (fix)
42 |+if {1} == named_var: # F632 (fix)
43 43 | pass
44 44 | if named_var is {i for i in [1]}: # F632 (fix)
45 45 | pass
constant_literals.py:44:4: F632 [*] Use `==` to compare constant literals
|
42 | if {1} is named_var: # F632 (fix)
43 | pass
44 | if named_var is {i for i in [1]}: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F632
45 | pass
|
= help: Replace `is` with `==`
Safe fix
41 41 | pass
42 42 | if {1} is named_var: # F632 (fix)
43 43 | pass
44 |-if named_var is {i for i in [1]}: # F632 (fix)
44 |+if named_var == {i for i in [1]}: # F632 (fix)
45 45 | pass
46 46 |
47 47 | named_var = {1: 1}
constant_literals.py:48:4: F632 [*] Use `==` to compare constant literals
|
47 | named_var = {1: 1}
48 | if {1: 1} is {1: 1}: # F632 (fix)
| ^^^^^^^^^^^^^^^^ F632
49 | pass
50 | if named_var is {1: 1}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
45 45 | pass
46 46 |
47 47 | named_var = {1: 1}
48 |-if {1: 1} is {1: 1}: # F632 (fix)
48 |+if {1: 1} == {1: 1}: # F632 (fix)
49 49 | pass
50 50 | if named_var is {1: 1}: # F632 (fix)
51 51 | pass
constant_literals.py:50:4: F632 [*] Use `==` to compare constant literals
|
48 | if {1: 1} is {1: 1}: # F632 (fix)
49 | pass
50 | if named_var is {1: 1}: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^ F632
51 | pass
52 | if {1: 1} is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
47 47 | named_var = {1: 1}
48 48 | if {1: 1} is {1: 1}: # F632 (fix)
49 49 | pass
50 |-if named_var is {1: 1}: # F632 (fix)
50 |+if named_var == {1: 1}: # F632 (fix)
51 51 | pass
52 52 | if {1: 1} is named_var: # F632 (fix)
53 53 | pass
constant_literals.py:52:4: F632 [*] Use `==` to compare constant literals
|
50 | if named_var is {1: 1}: # F632 (fix)
51 | pass
52 | if {1: 1} is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^ F632
53 | pass
54 | if named_var is {1: 1}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
49 49 | pass
50 50 | if named_var is {1: 1}: # F632 (fix)
51 51 | pass
52 |-if {1: 1} is named_var: # F632 (fix)
52 |+if {1: 1} == named_var: # F632 (fix)
53 53 | pass
54 54 | if named_var is {1: 1}: # F632 (fix)
55 55 | pass
constant_literals.py:54:4: F632 [*] Use `==` to compare constant literals
|
52 | if {1: 1} is named_var: # F632 (fix)
53 | pass
54 | if named_var is {1: 1}: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^ F632
55 | pass
56 | if {1: 1} is named_var: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
51 51 | pass
52 52 | if {1: 1} is named_var: # F632 (fix)
53 53 | pass
54 |-if named_var is {1: 1}: # F632 (fix)
54 |+if named_var == {1: 1}: # F632 (fix)
55 55 | pass
56 56 | if {1: 1} is named_var: # F632 (fix)
57 57 | pass
constant_literals.py:56:4: F632 [*] Use `==` to compare constant literals
|
54 | if named_var is {1: 1}: # F632 (fix)
55 | pass
56 | if {1: 1} is named_var: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^ F632
57 | pass
58 | if named_var is {i: 1 for i in [1]}: # F632 (fix)
|
= help: Replace `is` with `==`
Safe fix
53 53 | pass
54 54 | if named_var is {1: 1}: # F632 (fix)
55 55 | pass
56 |-if {1: 1} is named_var: # F632 (fix)
56 |+if {1: 1} == named_var: # F632 (fix)
57 57 | pass
58 58 | if named_var is {i: 1 for i in [1]}: # F632 (fix)
59 59 | pass
constant_literals.py:58:4: F632 [*] Use `==` to compare constant literals
|
56 | if {1: 1} is named_var: # F632 (fix)
57 | pass
58 | if named_var is {i: 1 for i in [1]}: # F632 (fix)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F632
59 | pass
|
= help: Replace `is` with `==`
Safe fix
55 55 | pass
56 56 | if {1: 1} is named_var: # F632 (fix)
57 57 | pass
58 |-if named_var is {i: 1 for i in [1]}: # F632 (fix)
58 |+if named_var == {i: 1 for i in [1]}: # F632 (fix)
59 59 | pass
60 60 |
61 61 | ###

View File

@ -8,6 +8,7 @@ use ruff_python_parser::locate_cmp_ops;
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 `is` and `is not` comparisons against constant literals, like /// Checks for `is` and `is not` comparisons against constant literals, like
@ -26,6 +27,12 @@ use crate::checkers::ast::Checker;
/// Instead, use `==` and `!=` to compare constant literals, which will compare /// Instead, use `==` and `!=` to compare constant literals, which will compare
/// the values of the objects instead of their identities. /// the values of the objects instead of their identities.
/// ///
/// In [preview], this rule will also flag `is` and `is not` comparisons against
/// non-constant literals, like lists, sets, and dictionaries. While such
/// comparisons will not raise a `SyntaxWarning`, they are still likely to be
/// incorrect, as they will compare the identities of the objects instead of
/// their values, which will always evaluate to `False`.
///
/// ## Example /// ## Example
/// ```python /// ```python
/// x = 200 /// x = 200
@ -44,6 +51,8 @@ use crate::checkers::ast::Checker;
/// - [Python documentation: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not) /// - [Python documentation: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not)
/// - [Python documentation: Value comparisons](https://docs.python.org/3/reference/expressions.html#value-comparisons) /// - [Python documentation: Value comparisons](https://docs.python.org/3/reference/expressions.html#value-comparisons)
/// - [_Why does Python log a SyntaxWarning for is with literals?_ by Adam Johnson](https://adamj.eu/tech/2020/01/21/why-does-python-3-8-syntaxwarning-for-is-literal/) /// - [_Why does Python log a SyntaxWarning for is with literals?_ by Adam Johnson](https://adamj.eu/tech/2020/01/21/why-does-python-3-8-syntaxwarning-for-is-literal/)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[violation] #[violation]
pub struct IsLiteral { pub struct IsLiteral {
cmp_op: IsCmpOp, cmp_op: IsCmpOp,
@ -81,7 +90,10 @@ pub(crate) fn invalid_literal_comparison(
for (index, (op, right)) in ops.iter().zip(comparators).enumerate() { for (index, (op, right)) in ops.iter().zip(comparators).enumerate() {
if matches!(op, CmpOp::Is | CmpOp::IsNot) if matches!(op, CmpOp::Is | CmpOp::IsNot)
&& (helpers::is_constant_non_singleton(left) && (helpers::is_constant_non_singleton(left)
|| helpers::is_constant_non_singleton(right)) || helpers::is_constant_non_singleton(right)
|| (matches!(checker.settings.preview, PreviewMode::Enabled)
&& (helpers::is_mutable_iterable_initializer(left)
|| helpers::is_mutable_iterable_initializer(right))))
{ {
let mut diagnostic = Diagnostic::new(IsLiteral { cmp_op: op.into() }, expr.range()); let mut diagnostic = Diagnostic::new(IsLiteral { cmp_op: op.into() }, expr.range());
if lazy_located.is_none() { if lazy_located.is_none() {

View File

@ -607,6 +607,19 @@ pub const fn is_const_false(expr: &Expr) -> bool {
) )
} }
/// Return `true` if the [`Expr`] is a mutable iterable initializer, like `{}` or `[]`.
pub const fn is_mutable_iterable_initializer(expr: &Expr) -> bool {
matches!(
expr,
Expr::Set(_)
| Expr::SetComp(_)
| Expr::List(_)
| Expr::ListComp(_)
| Expr::Dict(_)
| Expr::DictComp(_)
)
}
/// Extract the names of all handled exceptions. /// Extract the names of all handled exceptions.
pub fn extract_handled_exceptions(handlers: &[ExceptHandler]) -> Vec<&Expr> { pub fn extract_handled_exceptions(handlers: &[ExceptHandler]) -> Vec<&Expr> {
let mut handled_exceptions = Vec::new(); let mut handled_exceptions = Vec::new();