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

Include actual conditions in E712 diagnostics #10254

Merged
merged 3 commits into from
Mar 8, 2024
Merged

Include actual conditions in E712 diagnostics #10254

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
825d561
Include actual conditions in E712 diagnostics
tjkuson Mar 6, 2024
7f29cb7
Use `SourceCodeSnippet` in diagnostic messages
tjkuson Mar 7, 2024
9006070
Tweak some
charliermarsh Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 73 additions & 25 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 @@ -102,39 +102,47 @@ impl AlwaysFixableViolation for NoneComparison {
///
/// [PEP 8]: https://peps.python.org/pep-0008/#programming-recommendations
#[violation]
pub struct TrueFalseComparison(bool, EqCmpOp);
pub struct TrueFalseComparison {
value: bool,
op: EqCmpOp,
cond: Option<String>,
}

impl AlwaysFixableViolation for TrueFalseComparison {
#[derive_message_formats]
fn message(&self) -> String {
let TrueFalseComparison(value, op) = self;
match (value, op) {
(true, EqCmpOp::Eq) => {
format!("Avoid equality comparisons to `True`; use `if cond:` for truth checks")
let TrueFalseComparison { value, op, cond } = self;
match (value, op, cond) {
(true, EqCmpOp::Eq, Some(cond)) => {
format!("Avoid equality comparisons to `True`; use `if {cond}:` for truth checks")
}
(true, EqCmpOp::NotEq) => {
(true, EqCmpOp::NotEq, Some(cond)) => {
format!(
"Avoid inequality comparisons to `True`; use `if not cond:` for false checks"
"Avoid inequality comparisons to `True`; use `if not {cond}:` for false checks"
)
}
(false, EqCmpOp::Eq) => {
(false, EqCmpOp::Eq, Some(cond)) => {
format!(
"Avoid equality comparisons to `False`; use `if not cond:` for false checks"
"Avoid equality comparisons to `False`; use `if not {cond}:` for false checks"
)
}
(false, EqCmpOp::NotEq) => {
format!("Avoid inequality comparisons to `False`; use `if cond:` for truth checks")
(false, EqCmpOp::NotEq, Some(cond)) => {
format!(
"Avoid inequality comparisons to `False`; use `if {cond}:` for truth checks"
)
}
_ => format!("Avoid equality comparisons to `True` or `False`"),
}
}

fn fix_title(&self) -> String {
let TrueFalseComparison(value, op) = self;
match (value, op) {
(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(),
let TrueFalseComparison { value, op, cond } = self;
match (value, op, cond) {
(_, _, None) => "Replace comparison".to_string(),
(true, EqCmpOp::Eq, Some(cond)) => format!("Replace with `{cond}`"),
(true, EqCmpOp::NotEq, Some(cond)) => format!("Replace with `not {cond}`"),
(false, EqCmpOp::Eq, Some(cond)) => format!("Replace with `not {cond}`"),
(false, EqCmpOp::NotEq, Some(cond)) => format!("Replace with `{cond}`"),
}
}
}
Expand Down Expand Up @@ -178,17 +186,35 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
if let Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, .. }) = comparator {
match op {
EqCmpOp::Eq => {
let cond = if compare.ops.len() < 2 {
Some(checker.locator().slice(next).into())
} else {
None
};
let diagnostic = Diagnostic::new(
TrueFalseComparison(*value, op),
comparator.range(),
TrueFalseComparison {
value: *value,
op,
cond,
},
compare.range(),
);
bad_ops.insert(0, CmpOp::Is);
diagnostics.push(diagnostic);
}
EqCmpOp::NotEq => {
let cond = if compare.ops.len() < 2 {
Some(checker.locator().slice(next).into())
} else {
None
};
let diagnostic = Diagnostic::new(
TrueFalseComparison(*value, op),
comparator.range(),
TrueFalseComparison {
value: *value,
op,
cond,
},
compare.range(),
);
bad_ops.insert(0, CmpOp::IsNot);
diagnostics.push(diagnostic);
Expand Down Expand Up @@ -231,14 +257,36 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
if let Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, .. }) = next {
match op {
EqCmpOp::Eq => {
let diagnostic =
Diagnostic::new(TrueFalseComparison(*value, op), next.range());
let cond = if compare.ops.len() < 2 {
Some(checker.locator().slice(comparator).into())
} else {
None
};
let diagnostic = Diagnostic::new(
TrueFalseComparison {
value: *value,
op,
cond,
},
compare.range(),
);
bad_ops.insert(index, CmpOp::Is);
diagnostics.push(diagnostic);
}
EqCmpOp::NotEq => {
let diagnostic =
Diagnostic::new(TrueFalseComparison(*value, op), next.range());
let cond = if compare.ops.len() < 2 {
Some(checker.locator().slice(comparator).into())
} else {
None
};
let diagnostic = Diagnostic::new(
TrueFalseComparison {
value: *value,
op,
cond,
},
compare.range(),
);
bad_ops.insert(index, CmpOp::IsNot);
diagnostics.push(diagnostic);
}
Expand Down
76 changes: 37 additions & 39 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 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
E712.py:2:4: E712 [*] Avoid equality comparisons to `True`; use `if res:` for truth checks
|
1 | #: E712
2 | if res == True:
| ^^^^ E712
| ^^^^^^^^^^^ E712
3 | pass
4 | #: E712
|
= help: Replace with `cond`
= help: Replace with `res`

ℹ Unsafe fix
1 1 | #: E712
Expand All @@ -19,16 +19,16 @@ E712.py:2:11: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
4 4 | #: E712
5 5 | if res != False:

E712.py:5:11: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` for truth checks
E712.py:5:4: E712 [*] Avoid inequality comparisons to `False`; use `if res:` for truth checks
|
3 | pass
4 | #: E712
5 | if res != False:
| ^^^^^ E712
| ^^^^^^^^^^^^ E712
6 | pass
7 | #: E712
|
= help: Replace with `cond`
= help: Replace with `res`

ℹ Unsafe fix
2 2 | if res == True:
Expand All @@ -40,16 +40,16 @@ E712.py:5:11: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` f
7 7 | #: E712
8 8 | if True != res:

E712.py:8:4: E712 [*] Avoid inequality comparisons to `True`; use `if not cond:` for false checks
E712.py:8:4: E712 [*] Avoid inequality comparisons to `True`; use `if not res:` for false checks
|
6 | pass
7 | #: E712
8 | if True != res:
| ^^^^ E712
| ^^^^^^^^^^^ E712
9 | pass
10 | #: E712
|
= help: Replace with `not cond`
= help: Replace with `not res`

ℹ Unsafe fix
5 5 | if res != False:
Expand All @@ -61,16 +61,16 @@ E712.py:8:4: E712 [*] Avoid inequality comparisons to `True`; use `if not cond:`
10 10 | #: E712
11 11 | if False == res:

E712.py:11:4: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
E712.py:11:4: E712 [*] Avoid equality comparisons to `False`; use `if not res:` for false checks
|
9 | pass
10 | #: E712
11 | if False == res:
| ^^^^^ E712
| ^^^^^^^^^^^^ E712
12 | pass
13 | #: E712
|
= help: Replace with `not cond`
= help: Replace with `not res`

ℹ Unsafe fix
8 8 | if True != res:
Expand All @@ -82,16 +82,16 @@ E712.py:11:4: E712 [*] Avoid equality comparisons to `False`; use `if not cond:`
13 13 | #: E712
14 14 | if res[1] == True:

E712.py:14:14: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
E712.py:14:4: E712 [*] Avoid equality comparisons to `True`; use `if res[1]:` for truth checks
|
12 | pass
13 | #: E712
14 | if res[1] == True:
| ^^^^ E712
| ^^^^^^^^^^^^^^ E712
15 | pass
16 | #: E712
|
= help: Replace with `cond`
= help: Replace with `res[1]`

ℹ Unsafe fix
11 11 | if False == res:
Expand All @@ -103,16 +103,16 @@ E712.py:14:14: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
16 16 | #: E712
17 17 | if res[1] != False:

E712.py:17:14: E712 [*] Avoid inequality comparisons to `False`; use `if cond:` for truth checks
E712.py:17:4: E712 [*] Avoid inequality comparisons to `False`; use `if res[1]:` for truth checks
|
15 | pass
16 | #: E712
17 | if res[1] != False:
| ^^^^^ E712
| ^^^^^^^^^^^^^^^ E712
18 | pass
19 | #: E712
|
= help: Replace with `cond`
= help: Replace with `res[1]`

ℹ Unsafe fix
14 14 | if res[1] == True:
Expand All @@ -124,12 +124,12 @@ E712.py:17:14: E712 [*] Avoid inequality comparisons to `False`; use `if cond:`
19 19 | #: E712
20 20 | var = 1 if cond == True else -1 if cond == False else cond

E712.py:20:20: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
E712.py:20:12: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
|
18 | pass
19 | #: E712
20 | var = 1 if cond == True else -1 if cond == False else cond
| ^^^^ E712
| ^^^^^^^^^^^^ E712
21 | #: E712
22 | if (True) == TrueElement or x == TrueElement:
|
Expand All @@ -145,12 +145,12 @@ E712.py:20:20: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
22 22 | if (True) == TrueElement or x == TrueElement:
23 23 | pass

E712.py:20:44: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
E712.py:20:36: E712 [*] Avoid equality comparisons to `False`; use `if not cond:` for false checks
|
18 | pass
19 | #: E712
20 | var = 1 if cond == True else -1 if cond == False else cond
| ^^^^^ E712
| ^^^^^^^^^^^^^ E712
21 | #: E712
22 | if (True) == TrueElement or x == TrueElement:
|
Expand All @@ -166,15 +166,15 @@ E712.py:20:44: E712 [*] Avoid equality comparisons to `False`; use `if not cond:
22 22 | if (True) == TrueElement or x == TrueElement:
23 23 | pass

E712.py:22:5: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for truth checks
E712.py:22:4: E712 [*] Avoid equality comparisons to `True`; use `if TrueElement:` 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
| ^^^^^^^^^^^^^^^^^^^^^ E712
23 | pass
|
= help: Replace with `cond`
= help: Replace with `TrueElement`

ℹ Unsafe fix
19 19 | #: E712
Expand All @@ -186,15 +186,15 @@ E712.py:22:5: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
24 24 |
25 25 | if res == True != False:

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

ℹ Unsafe fix
22 22 | if (True) == TrueElement or x == TrueElement:
Expand All @@ -206,15 +206,15 @@ E712.py:25:11: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
27 27 |
28 28 | if(True) == TrueElement or x == TrueElement:

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

ℹ Unsafe fix
22 22 | if (True) == TrueElement or x == TrueElement:
Expand All @@ -226,15 +226,15 @@ E712.py:25:19: E712 [*] Avoid inequality comparisons to `False`; use `if cond:`
27 27 |
28 28 | if(True) == TrueElement or x == TrueElement:

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

ℹ Unsafe fix
25 25 | if res == True != False:
Expand All @@ -246,15 +246,15 @@ E712.py:28:4: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
30 30 |
31 31 | if (yield i) == True:

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

ℹ Unsafe fix
28 28 | if(True) == TrueElement or x == TrueElement:
Expand All @@ -265,5 +265,3 @@ E712.py:31:17: E712 [*] Avoid equality comparisons to `True`; use `if cond:` for
32 32 | print("even")
33 33 |
34 34 | #: Okay


Loading
Loading