Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tweak E712 docs #8613

Merged
merged 2 commits into from
Mar 6, 2024
Merged

Tweak E712 docs #8613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
394544c
Tweak `E712` docs
tjkuson Nov 11, 2023
a30e5aa
Update crates/ruff_linter/src/rules/pycodestyle/rules/literal_compari…
tjkuson Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 30 additions & 19 deletions crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,33 @@ impl AlwaysFixableViolation for NoneComparison {
}

/// ## What it does
/// Checks for comparisons to booleans which are not using the `is` operator.
tjkuson marked this conversation as resolved.
Show resolved Hide resolved
/// Checks for equality comparisons to boolean literals.
///
/// ## Why is this bad?
/// According to [PEP 8], "Comparisons to singletons like None should always be done with
/// is or is not, never the equality operators."
/// [PEP 8] recommends against using the equality operators `==` and `!=` to
/// compare values to `True` or `False`.
///
/// Instead, use `if cond:` or `if not cond:` to check for truth values.
///
/// If you intend to check if a value is the boolean literal `True` or `False`,
/// consider using `is` or `is not` to check for identity instead.
///
/// ## Example
/// ```python
/// if arg == True:
/// pass
/// if False == arg:
/// pass
/// if foo == True:
/// ...
///
/// if bar == False:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// if arg is True:
/// pass
/// if arg is False:
/// pass
/// if foo:
/// ...
///
/// if not bar:
/// ...
/// ```
///
/// [PEP 8]: https://peps.python.org/pep-0008/#programming-recommendations
Expand All @@ -103,27 +110,31 @@ impl AlwaysFixableViolation for TrueFalseComparison {
let TrueFalseComparison(value, op) = self;
match (value, op) {
(true, EqCmpOp::Eq) => {
format!("Comparison to `True` should be `cond is True` or `if cond:`")
format!("Avoid equality comparisons to `True`; use `if cond:` for truth checks")
}
(true, EqCmpOp::NotEq) => {
format!("Comparison to `True` should be `cond is not True` or `if not cond:`")
format!(
"Avoid inequality comparisons to `True`; use `if not cond:` for false checks"
)
}
(false, EqCmpOp::Eq) => {
format!("Comparison to `False` should be `cond is False` or `if not cond:`")
format!(
"Avoid equality comparisons to `False`; use `if not cond:` for false checks"
)
}
(false, EqCmpOp::NotEq) => {
format!("Comparison to `False` should be `cond is not False` or `if cond:`")
format!("Avoid inequality comparisons to `False`; use `if cond:` for truth checks")
}
}
}

fn fix_title(&self) -> String {
let TrueFalseComparison(value, op) = self;
match (value, op) {
(true, EqCmpOp::Eq) => "Replace with `cond is True`".to_string(),
(true, EqCmpOp::NotEq) => "Replace with `cond is not True`".to_string(),
(false, EqCmpOp::Eq) => "Replace with `cond is False`".to_string(),
(false, EqCmpOp::NotEq) => "Replace with `cond is not False`".to_string(),
(true, EqCmpOp::Eq) => "Replace with `cond`".to_string(),
(true, EqCmpOp::NotEq) => "Replace with `not cond`".to_string(),
(false, EqCmpOp::Eq) => "Replace with `not cond`".to_string(),
(false, EqCmpOp::NotEq) => "Replace with `cond`".to_string(),
}
}
}
Expand Down
52 changes: 26 additions & 26 deletions ...src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E712_E712.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E712.py:2:11: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:2:11: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
1 | #: E712
2 | if res == True:
| ^^^^ E712
3 | pass
4 | #: E712
|
= help: Replace with `cond is True`
= help: Replace with `cond`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possibly outside the scope of this PR (you can leave it for now, if you like), but this suggestion is pretty confusing. There's no variable named "cond" in scope here, and I don't think it's obvious that this means "replace if res == True with if res", especially when the carets only point to True rather than res == True.


ℹ Unsafe fix
1 1 | #: E712
Expand All @@ -19,7 +19,7 @@ E712.py:2:11: E712 [*] Comparison to `True` should be `cond is True` or `if cond
4 4 | #: E712
5 5 | if res != False:

E712.py:5:11: E712 [*] Comparison to `False` should be `cond is not False` or `if cond:`
E712.py:5:11: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` for truth checks
|
3 | pass
4 | #: E712
Expand All @@ -28,7 +28,7 @@ E712.py:5:11: E712 [*] Comparison to `False` should be `cond is not False` or `i
6 | pass
7 | #: E712
|
= help: Replace with `cond is not False`
= help: Replace with `cond`

ℹ Unsafe fix
2 2 | if res == True:
Expand All @@ -40,7 +40,7 @@ E712.py:5:11: E712 [*] Comparison to `False` should be `cond is not False` or `i
7 7 | #: E712
8 8 | if True != res:

E712.py:8:4: E712 [*] Comparison to `True` should be `cond is not True` or `if not cond:`
E712.py:8:4: E712 [*] Avoid inequality comparisons to `True`; use `if not cond:` for false checks
|
6 | pass
7 | #: E712
Expand All @@ -49,7 +49,7 @@ E712.py:8:4: E712 [*] Comparison to `True` should be `cond is not True` or `if n
9 | pass
10 | #: E712
|
= help: Replace with `cond is not True`
= help: Replace with `not cond`

ℹ Unsafe fix
5 5 | if res != False:
Expand All @@ -61,7 +61,7 @@ E712.py:8:4: E712 [*] Comparison to `True` should be `cond is not True` or `if n
10 10 | #: E712
11 11 | if False == res:

E712.py:11:4: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:`
E712.py:11:4: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
|
9 | pass
10 | #: E712
Expand All @@ -70,7 +70,7 @@ E712.py:11:4: E712 [*] Comparison to `False` should be `cond is False` or `if no
12 | pass
13 | #: E712
|
= help: Replace with `cond is False`
= help: Replace with `not cond`

ℹ Unsafe fix
8 8 | if True != res:
Expand All @@ -82,7 +82,7 @@ E712.py:11:4: E712 [*] Comparison to `False` should be `cond is False` or `if no
13 13 | #: E712
14 14 | if res[1] == True:

E712.py:14:14: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:14:14: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
12 | pass
13 | #: E712
Expand All @@ -91,7 +91,7 @@ E712.py:14:14: E712 [*] Comparison to `True` should be `cond is True` or `if con
15 | pass
16 | #: E712
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
11 11 | if False == res:
Expand All @@ -103,7 +103,7 @@ E712.py:14:14: E712 [*] Comparison to `True` should be `cond is True` or `if con
16 16 | #: E712
17 17 | if res[1] != False:

E712.py:17:14: E712 [*] Comparison to `False` should be `cond is not False` or `if cond:`
E712.py:17:14: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` for truth checks
|
15 | pass
16 | #: E712
Expand All @@ -112,7 +112,7 @@ E712.py:17:14: E712 [*] Comparison to `False` should be `cond is not False` or `
18 | pass
19 | #: E712
|
= help: Replace with `cond is not False`
= help: Replace with `cond`

ℹ Unsafe fix
14 14 | if res[1] == True:
Expand All @@ -124,7 +124,7 @@ E712.py:17:14: E712 [*] Comparison to `False` should be `cond is not False` or `
19 19 | #: E712
20 20 | var = 1 if cond == True else -1 if cond == False else cond

E712.py:20:20: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:20:20: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
18 | pass
19 | #: E712
Expand All @@ -133,7 +133,7 @@ E712.py:20:20: E712 [*] Comparison to `True` should be `cond is True` or `if con
21 | #: E712
22 | if (True) == TrueElement or x == TrueElement:
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
17 17 | if res[1] != False:
Expand All @@ -145,7 +145,7 @@ E712.py:20:20: E712 [*] Comparison to `True` should be `cond is True` or `if con
22 22 | if (True) == TrueElement or x == TrueElement:
23 23 | pass

E712.py:20:44: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:`
E712.py:20:44: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
|
18 | pass
19 | #: E712
Expand All @@ -154,7 +154,7 @@ E712.py:20:44: E712 [*] Comparison to `False` should be `cond is False` or `if n
21 | #: E712
22 | if (True) == TrueElement or x == TrueElement:
|
= help: Replace with `cond is False`
= help: Replace with `not cond`

ℹ Unsafe fix
17 17 | if res[1] != False:
Expand All @@ -166,15 +166,15 @@ E712.py:20:44: E712 [*] Comparison to `False` should be `cond is False` or `if n
22 22 | if (True) == TrueElement or x == TrueElement:
23 23 | pass

E712.py:22:5: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:22:5: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
20 | var = 1 if cond == True else -1 if cond == False else cond
21 | #: E712
22 | if (True) == TrueElement or x == TrueElement:
| ^^^^ E712
23 | pass
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
19 19 | #: E712
Expand All @@ -186,15 +186,15 @@ E712.py:22:5: E712 [*] Comparison to `True` should be `cond is True` or `if cond
24 24 |
25 25 | if res == True != False:

E712.py:25:11: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:25:11: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
23 | pass
24 |
25 | if res == True != False:
| ^^^^ E712
26 | pass
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
22 22 | if (True) == TrueElement or x == TrueElement:
Expand All @@ -206,15 +206,15 @@ E712.py:25:11: E712 [*] Comparison to `True` should be `cond is True` or `if con
27 27 |
28 28 | if(True) == TrueElement or x == TrueElement:

E712.py:25:19: E712 [*] Comparison to `False` should be `cond is not False` or `if cond:`
E712.py:25:19: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` for truth checks
|
23 | pass
24 |
25 | if res == True != False:
| ^^^^^ E712
26 | pass
|
= help: Replace with `cond is not False`
= help: Replace with `cond`

ℹ Unsafe fix
22 22 | if (True) == TrueElement or x == TrueElement:
Expand All @@ -226,15 +226,15 @@ E712.py:25:19: E712 [*] Comparison to `False` should be `cond is not False` or `
27 27 |
28 28 | if(True) == TrueElement or x == TrueElement:

E712.py:28:4: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:28:4: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
26 | pass
27 |
28 | if(True) == TrueElement or x == TrueElement:
| ^^^^ E712
29 | pass
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
25 25 | if res == True != False:
Expand All @@ -246,15 +246,15 @@ E712.py:28:4: E712 [*] Comparison to `True` should be `cond is True` or `if cond
30 30 |
31 31 | if (yield i) == True:

E712.py:31:17: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
E712.py:31:17: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
29 | pass
30 |
31 | if (yield i) == True:
| ^^^^ E712
32 | print("even")
|
= help: Replace with `cond is True`
= help: Replace with `cond`

ℹ Unsafe fix
28 28 | if(True) == TrueElement or x == TrueElement:
Expand Down
8 changes: 4 additions & 4 deletions ...ules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__constant_literals.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ constant_literals.py:12:4: F632 [*] Use `==` to compare constant literals
14 14 | if False == None: # E711, E712 (fix)
15 15 | pass

constant_literals.py:14:4: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:`
constant_literals.py:14:4: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
|
12 | if False is "abc": # F632 (fix, but leaves behind unfixable E712)
13 | pass
Expand All @@ -115,7 +115,7 @@ constant_literals.py:14:4: E712 [*] Comparison to `False` should be `cond is Fal
15 | pass
16 | if None == False: # E711, E712 (fix)
|
= help: Replace with `cond is False`
= help: Replace with `not cond`

ℹ Unsafe fix
11 11 | pass
Expand Down Expand Up @@ -168,15 +168,15 @@ constant_literals.py:16:4: E711 [*] Comparison to `None` should be `cond is None
18 18 |
19 19 | ###

constant_literals.py:16:12: E712 [*] Comparison to `False` should be `cond is False` or `if not cond:`
constant_literals.py:16:12: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
|
14 | if False == None: # E711, E712 (fix)
15 | pass
16 | if None == False: # E711, E712 (fix)
| ^^^^^ E712
17 | pass
|
= help: Replace with `cond is False`
= help: Replace with `not cond`

ℹ Unsafe fix
13 13 | pass
Expand Down