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 | |
parent | c91c2ae1398e4f567ee5e4c50a1da35a9b65919b (diff) |
add suggestion to bool_comparison
Diffstat (limited to 'lib/src')
-rw-r--r-- | lib/src/lib.rs | 42 | ||||
-rw-r--r-- | lib/src/lints/bool_comparison.rs | 88 | ||||
-rw-r--r-- | lib/src/make.rs | 20 |
3 files changed, 110 insertions, 40 deletions
diff --git a/lib/src/lib.rs b/lib/src/lib.rs index c06f02d..0f92d0d 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs | |||
@@ -1,8 +1,10 @@ | |||
1 | mod lints; | 1 | mod lints; |
2 | mod make; | ||
3 | |||
2 | pub use lints::LINTS; | 4 | pub use lints::LINTS; |
3 | 5 | ||
4 | use rnix::{SyntaxElement, SyntaxKind, TextRange}; | 6 | use rnix::{SyntaxElement, SyntaxKind, TextRange}; |
5 | use std::default::Default; | 7 | use std::{default::Default, convert::Into}; |
6 | 8 | ||
7 | pub trait Rule { | 9 | pub trait Rule { |
8 | fn validate(&self, node: &SyntaxElement) -> Option<Report>; | 10 | fn validate(&self, node: &SyntaxElement) -> Option<Report>; |
@@ -12,32 +14,60 @@ pub trait Rule { | |||
12 | pub struct Diagnostic { | 14 | pub struct Diagnostic { |
13 | pub at: TextRange, | 15 | pub at: TextRange, |
14 | pub message: String, | 16 | pub message: String, |
17 | pub suggestion: Option<Suggestion>, | ||
15 | } | 18 | } |
16 | 19 | ||
17 | impl Diagnostic { | 20 | impl Diagnostic { |
18 | pub fn new(at: TextRange, message: String) -> Self { | 21 | pub fn new(at: TextRange, message: String) -> Self { |
19 | Self { at, message } | 22 | Self { at, message, suggestion: None } |
23 | } | ||
24 | pub fn suggest(at: TextRange, message: String, suggestion: Suggestion) -> Self { | ||
25 | Self { at, message, suggestion: Some(suggestion) } | ||
26 | } | ||
27 | } | ||
28 | |||
29 | #[derive(Debug)] | ||
30 | pub struct Suggestion { | ||
31 | pub at: TextRange, | ||
32 | pub fix: SyntaxElement, | ||
33 | } | ||
34 | |||
35 | impl Suggestion { | ||
36 | pub fn new<E: Into<SyntaxElement>>(at: TextRange, fix: E) -> Self { | ||
37 | Self { | ||
38 | at, | ||
39 | fix: fix.into() | ||
40 | } | ||
20 | } | 41 | } |
21 | } | 42 | } |
22 | 43 | ||
23 | #[derive(Debug, Default)] | 44 | #[derive(Debug, Default)] |
24 | pub struct Report { | 45 | pub struct Report { |
25 | pub diagnostics: Vec<Diagnostic>, | 46 | pub diagnostics: Vec<Diagnostic>, |
47 | pub note: &'static str | ||
26 | } | 48 | } |
27 | 49 | ||
28 | impl Report { | 50 | impl Report { |
29 | pub fn new() -> Self { | 51 | pub fn new(note: &'static str) -> Self { |
30 | Self::default() | 52 | Self { |
53 | note, | ||
54 | ..Default::default() | ||
55 | } | ||
31 | } | 56 | } |
32 | pub fn diagnostic(mut self, at: TextRange, message: String) -> Self { | 57 | pub fn diagnostic(mut self, at: TextRange, message: String) -> Self { |
33 | self.diagnostics.push(Diagnostic::new(at, message)); | 58 | self.diagnostics.push(Diagnostic::new(at, message)); |
34 | self | 59 | self |
35 | } | 60 | } |
61 | pub fn suggest(mut self, at: TextRange, message: String, suggestion: Suggestion) -> Self { | ||
62 | self.diagnostics.push(Diagnostic::suggest(at, message, suggestion)); | ||
63 | self | ||
64 | } | ||
65 | |||
36 | } | 66 | } |
37 | 67 | ||
38 | pub trait Metadata { | 68 | pub trait Metadata { |
39 | fn name(&self) -> &str; | 69 | fn name() -> &'static str where Self: Sized; |
40 | fn note(&self) -> &str; | 70 | fn note() -> &'static str where Self: Sized; |
41 | fn match_with(&self, with: &SyntaxKind) -> bool; | 71 | fn match_with(&self, with: &SyntaxKind) -> bool; |
42 | fn match_kind(&self) -> SyntaxKind; | 72 | fn match_kind(&self) -> SyntaxKind; |
43 | } | 73 | } |
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 | // } | ||
diff --git a/lib/src/make.rs b/lib/src/make.rs new file mode 100644 index 0000000..0b21105 --- /dev/null +++ b/lib/src/make.rs | |||
@@ -0,0 +1,20 @@ | |||
1 | use rnix::{SyntaxNode, types::{TypedNode, self}}; | ||
2 | |||
3 | fn ast_from_text<N: TypedNode>(text: &str) -> N { | ||
4 | let parse = rnix::parse(text); | ||
5 | let node = match parse.node().descendants().find_map(N::cast) { | ||
6 | Some(it) => it, | ||
7 | None => { | ||
8 | panic!("Failed to make ast node `{}` from text {}", std::any::type_name::<N>(), text) | ||
9 | } | ||
10 | }; | ||
11 | node | ||
12 | } | ||
13 | |||
14 | pub fn parenthesize(node: &SyntaxNode) -> types::Paren { | ||
15 | ast_from_text(&format!("({})", node)) | ||
16 | } | ||
17 | |||
18 | pub fn unary_not(node: &SyntaxNode) -> types::UnaryOp { | ||
19 | ast_from_text(&format!("!{}", node)) | ||
20 | } | ||