From 394544c896593a575fda87ce8ef8b228b94be725 Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Sat, 11 Nov 2023 08:40:18 +0000 Subject: [PATCH 1/2] Tweak `E712` docs Recommend `if predicate:` over `if predicate is True:` in the diagnostic message. --- .../pycodestyle/rules/literal_comparisons.rs | 49 ++++++++++------- ...les__pycodestyle__tests__E712_E712.py.snap | 52 +++++++++---------- ...pycodestyle__tests__constant_literals.snap | 8 +-- 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs index 8d650d4a25ae7..d7bcd4e75d148 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs @@ -71,26 +71,33 @@ impl AlwaysFixableViolation for NoneComparison { } /// ## What it does -/// Checks for comparisons to booleans which are not using the `is` operator. +/// Checks for equality comparisons to booleans. /// /// ## 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 @@ -103,16 +110,20 @@ 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") } } } @@ -120,10 +131,10 @@ impl AlwaysFixableViolation for TrueFalseComparison { 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(), } } } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E712_E712.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E712_E712.py.snap index ba9a0861ce91f..7aba8288f2888 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E712_E712.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E712_E712.py.snap @@ -1,7 +1,7 @@ --- 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: @@ -9,7 +9,7 @@ E712.py:2:11: E712 [*] Comparison to `True` should be `cond is True` or `if cond 3 | pass 4 | #: E712 | - = help: Replace with `cond is True` + = help: Replace with `cond` ℹ Unsafe fix 1 1 | #: E712 @@ -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 @@ -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: @@ -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 @@ -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: @@ -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 @@ -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: @@ -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 @@ -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: @@ -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 @@ -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: @@ -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 @@ -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: @@ -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 @@ -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: @@ -166,7 +166,7 @@ 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 @@ -174,7 +174,7 @@ E712.py:22:5: E712 [*] Comparison to `True` should be `cond is True` or `if cond | ^^^^ E712 23 | pass | - = help: Replace with `cond is True` + = help: Replace with `cond` ℹ Unsafe fix 19 19 | #: E712 @@ -186,7 +186,7 @@ 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 | @@ -194,7 +194,7 @@ E712.py:25:11: E712 [*] Comparison to `True` should be `cond is True` or `if con | ^^^^ E712 26 | pass | - = help: Replace with `cond is True` + = help: Replace with `cond` ℹ Unsafe fix 22 22 | if (True) == TrueElement or x == TrueElement: @@ -206,7 +206,7 @@ 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 | @@ -214,7 +214,7 @@ E712.py:25:19: E712 [*] Comparison to `False` should be `cond is not False` or ` | ^^^^^ E712 26 | pass | - = help: Replace with `cond is not False` + = help: Replace with `cond` ℹ Unsafe fix 22 22 | if (True) == TrueElement or x == TrueElement: @@ -226,7 +226,7 @@ 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 | @@ -234,7 +234,7 @@ E712.py:28:4: E712 [*] Comparison to `True` should be `cond is True` or `if cond | ^^^^ E712 29 | pass | - = help: Replace with `cond is True` + = help: Replace with `cond` ℹ Unsafe fix 25 25 | if res == True != False: @@ -246,7 +246,7 @@ 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 | @@ -254,7 +254,7 @@ E712.py:31:17: E712 [*] Comparison to `True` should be `cond is True` or `if con | ^^^^ E712 32 | print("even") | - = help: Replace with `cond is True` + = help: Replace with `cond` ℹ Unsafe fix 28 28 | if(True) == TrueElement or x == TrueElement: diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__constant_literals.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__constant_literals.snap index cf0ce6195f518..29a726e2184b4 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__constant_literals.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__constant_literals.snap @@ -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 @@ -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 @@ -168,7 +168,7 @@ 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 @@ -176,7 +176,7 @@ constant_literals.py:16:12: E712 [*] Comparison to `False` should be `cond is Fa | ^^^^^ E712 17 | pass | - = help: Replace with `cond is False` + = help: Replace with `not cond` ℹ Unsafe fix 13 13 | pass From a30e5aac80b2b2caca6f3f6d85c45b3fdb60e223 Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Wed, 6 Mar 2024 17:47:14 +0000 Subject: [PATCH 2/2] Update crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs Co-authored-by: Alex Waygood --- .../src/rules/pycodestyle/rules/literal_comparisons.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs index d7bcd4e75d148..6575f8f6002cb 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs @@ -71,7 +71,7 @@ impl AlwaysFixableViolation for NoneComparison { } /// ## What it does -/// Checks for equality comparisons to booleans. +/// Checks for equality comparisons to boolean literals. /// /// ## Why is this bad? /// [PEP 8] recommends against using the equality operators `==` and `!=` to