diff options
author | Akshay <[email protected]> | 2021-09-21 12:11:24 +0100 |
---|---|---|
committer | Akshay <[email protected]> | 2021-09-21 12:11:24 +0100 |
commit | 40867728c023a9f95f346671b51be68002cc7552 (patch) | |
tree | e856327b0d42d9a4740c460a5efb633f8a73f3d9 /lib/src/lints/bool_comparison.rs | |
parent | c91c2ae1398e4f567ee5e4c50a1da35a9b65919b (diff) |
add suggestion to bool_comparison
Diffstat (limited to 'lib/src/lints/bool_comparison.rs')
-rw-r--r-- | lib/src/lints/bool_comparison.rs | 88 |
1 files changed, 54 insertions, 34 deletions
diff --git a/lib/src/lints/bool_comparison.rs b/lib/src/lints/bool_comparison.rs index 918126f..f869e17 100644 --- a/lib/src/lints/bool_comparison.rs +++ b/lib/src/lints/bool_comparison.rs | |||
@@ -1,4 +1,4 @@ | |||
1 | use crate::{Lint, Metadata, Report, Rule}; | 1 | use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; |
2 | 2 | ||
3 | use if_chain::if_chain; | 3 | use if_chain::if_chain; |
4 | use macros::lint; | 4 | use macros::lint; |
@@ -22,22 +22,58 @@ impl Rule for BoolComparison { | |||
22 | if let Some(lhs) = bin_expr.lhs(); | 22 | if let Some(lhs) = bin_expr.lhs(); |
23 | if let Some(rhs) = bin_expr.rhs(); | 23 | if let Some(rhs) = bin_expr.rhs(); |
24 | 24 | ||
25 | if let BinOpKind::Equal | BinOpKind::NotEqual = bin_expr.operator(); | 25 | if let op@(BinOpKind::Equal | BinOpKind::NotEqual) = bin_expr.operator(); |
26 | let (non_bool_side, bool_side) = if is_boolean_ident(&lhs) { | 26 | let (non_bool_side, bool_side) = if boolean_ident(&lhs).is_some() { |
27 | (rhs, lhs) | 27 | (rhs, lhs) |
28 | } else if is_boolean_ident(&rhs) { | 28 | } else if boolean_ident(&rhs).is_some() { |
29 | (lhs, rhs) | 29 | (lhs, rhs) |
30 | } else { | 30 | } else { |
31 | return None | 31 | return None |
32 | }; | 32 | }; |
33 | then { | 33 | then { |
34 | let at = node.text_range(); | 34 | let at = node.text_range(); |
35 | let replacement = { | ||
36 | match (boolean_ident(&bool_side).unwrap(), op == BinOpKind::Equal) { | ||
37 | (NixBoolean::True, true) | (NixBoolean::False, false) => { | ||
38 | // `a == true`, `a != false` replace with just `a` | ||
39 | non_bool_side.clone() | ||
40 | }, | ||
41 | (NixBoolean::True, false) | (NixBoolean::False, true) => { | ||
42 | // `a != true`, `a == false` replace with `!a` | ||
43 | match non_bool_side.kind() { | ||
44 | SyntaxKind::NODE_APPLY | ||
45 | | SyntaxKind::NODE_PAREN | ||
46 | | SyntaxKind::NODE_IDENT => { | ||
47 | // do not parenthsize the replacement | ||
48 | make::unary_not(&non_bool_side).node().clone() | ||
49 | }, | ||
50 | SyntaxKind::NODE_BIN_OP => { | ||
51 | let inner = BinOp::cast(non_bool_side.clone()).unwrap(); | ||
52 | // `!a ? b`, no paren required | ||
53 | if inner.operator() == BinOpKind::IsSet { | ||
54 | make::unary_not(&non_bool_side).node().clone() | ||
55 | } else { | ||
56 | let parens = make::parenthesize(&non_bool_side); | ||
57 | make::unary_not(parens.node()).node().clone() | ||
58 | } | ||
59 | }, | ||
60 | _ => { | ||
61 | let parens = make::parenthesize(&non_bool_side); | ||
62 | make::unary_not(parens.node()).node().clone() | ||
63 | } | ||
64 | } | ||
65 | }, | ||
66 | } | ||
67 | }; | ||
35 | let message = format!( | 68 | let message = format!( |
36 | "Comparing `{}` with boolean literal `{}`", | 69 | "Comparing `{}` with boolean literal `{}`", |
37 | non_bool_side, | 70 | non_bool_side, |
38 | bool_side | 71 | bool_side |
39 | ); | 72 | ); |
40 | Some(Report::new().diagnostic(at, message)) | 73 | Some( |
74 | Report::new(Self::note()) | ||
75 | .suggest(at, message, Suggestion::new(at, replacement)) | ||
76 | ) | ||
41 | } else { | 77 | } else { |
42 | None | 78 | None |
43 | } | 79 | } |
@@ -45,34 +81,18 @@ impl Rule for BoolComparison { | |||
45 | } | 81 | } |
46 | } | 82 | } |
47 | 83 | ||
48 | // not entirely accurate, underhanded nix programmers might write `true = false` | 84 | enum NixBoolean { |
49 | fn is_boolean_ident(node: &SyntaxNode) -> bool { | 85 | True, |
50 | if let Some(ident_expr) = Ident::cast(node.clone()) { | 86 | False, |
51 | ident_expr.as_str() == "true" || ident_expr.as_str() == "false" | ||
52 | } else { | ||
53 | false | ||
54 | } | ||
55 | } | 87 | } |
56 | 88 | ||
57 | // #[cfg(test)] | 89 | // not entirely accurate, underhanded nix programmers might write `true = false` |
58 | // mod tests { | 90 | fn boolean_ident(node: &SyntaxNode) -> Option<NixBoolean> { |
59 | // use super::*; | 91 | Ident::cast(node.clone()) |
60 | // use rnix::{parser, WalkEvent}; | 92 | .map(|ident_expr| match ident_expr.as_str() { |
61 | // | 93 | "true" => Some(NixBoolean::True), |
62 | // #[test] | 94 | "false" => Some(NixBoolean::False), |
63 | // fn trivial() { | 95 | _ => None, |
64 | // let src = r#" | 96 | }) |
65 | // a == true | 97 | .flatten() |
66 | // "#; | 98 | } |
67 | // let parsed = rnix::parse(src).as_result().ok().unwrap(); | ||
68 | // let _ = parsed | ||
69 | // .node() | ||
70 | // .preorder_with_tokens() | ||
71 | // .filter_map(|event| match event { | ||
72 | // WalkEvent::Enter(t) => Some(t), | ||
73 | // _ => None, | ||
74 | // }) | ||
75 | // .map(|node| BoolComparison.validate(&node)) | ||
76 | // .collect::<Vec<_>>(); | ||
77 | // } | ||
78 | // } | ||