add suggestion to bool_comparison

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 @@
 mod lints;
+mod make;
+
 pub use lints::LINTS;
 
 use rnix::{SyntaxElement, SyntaxKind, TextRange};
-use std::default::Default;
+use std::{default::Default, convert::Into};
 
 pub trait Rule {
     fn validate(&self, node: &SyntaxElement) -> Option<Report>;
@@ -12,32 +14,60 @@ pub trait Rule {
 pub struct Diagnostic {
     pub at: TextRange,
     pub message: String,
+    pub suggestion: Option<Suggestion>,
 }
 
 impl Diagnostic {
     pub fn new(at: TextRange, message: String) -> Self {
-        Self { at, message }
+        Self { at, message, suggestion: None }
+    }
+    pub fn suggest(at: TextRange, message: String, suggestion: Suggestion) -> Self {
+        Self { at, message, suggestion: Some(suggestion) }
+    }
+}
+
+#[derive(Debug)]
+pub struct Suggestion {
+    pub at: TextRange,
+    pub fix: SyntaxElement,
+}
+
+impl Suggestion {
+    pub fn new<E: Into<SyntaxElement>>(at: TextRange, fix: E) -> Self {
+        Self {
+            at,
+            fix: fix.into()
+        }
     }
 }
 
 #[derive(Debug, Default)]
 pub struct Report {
     pub diagnostics: Vec<Diagnostic>,
+    pub note: &'static str
 }
 
 impl Report {
-    pub fn new() -> Self {
-        Self::default()
+    pub fn new(note: &'static str) -> Self {
+        Self {
+            note,
+            ..Default::default()
+        }
     }
     pub fn diagnostic(mut self, at: TextRange, message: String) -> Self {
         self.diagnostics.push(Diagnostic::new(at, message));
         self
     }
+    pub fn suggest(mut self, at: TextRange, message: String, suggestion: Suggestion) -> Self {
+        self.diagnostics.push(Diagnostic::suggest(at, message, suggestion));
+        self
+    }
+
 }
 
 pub trait Metadata {
-    fn name(&self) -> &str;
-    fn note(&self) -> &str;
+    fn name() -> &'static str where Self: Sized;
+    fn note() -> &'static str where Self: Sized;
     fn match_with(&self, with: &SyntaxKind) -> bool;
     fn match_kind(&self) -> SyntaxKind;
 }
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 @@
-use crate::{Lint, Metadata, Report, Rule};
+use crate::{make, Lint, Metadata, Report, Rule, Suggestion};
 
 use if_chain::if_chain;
 use macros::lint;
@@ -22,22 +22,58 @@ impl Rule for BoolComparison {
             if let Some(lhs) = bin_expr.lhs();
             if let Some(rhs) = bin_expr.rhs();
 
-            if let BinOpKind::Equal | BinOpKind::NotEqual = bin_expr.operator();
-            let (non_bool_side, bool_side) = if is_boolean_ident(&lhs) {
+            if let op@(BinOpKind::Equal | BinOpKind::NotEqual) = bin_expr.operator();
+            let (non_bool_side, bool_side) = if boolean_ident(&lhs).is_some() {
                 (rhs, lhs)
-            } else if is_boolean_ident(&rhs) {
+            } else if boolean_ident(&rhs).is_some() {
                 (lhs, rhs)
             } else {
                 return None
             };
             then {
                 let at = node.text_range();
+                let replacement = {
+                    match (boolean_ident(&bool_side).unwrap(), op == BinOpKind::Equal) {
+                        (NixBoolean::True, true) | (NixBoolean::False, false) => {
+                            // `a == true`, `a != false` replace with just `a`
+                            non_bool_side.clone()
+                        },
+                        (NixBoolean::True, false) | (NixBoolean::False, true) => {
+                            // `a != true`, `a == false` replace with `!a`
+                            match non_bool_side.kind() {
+                                SyntaxKind::NODE_APPLY
+                                    | SyntaxKind::NODE_PAREN
+                                    | SyntaxKind::NODE_IDENT => {
+                                    // do not parenthsize the replacement
+                                    make::unary_not(&non_bool_side).node().clone()
+                                },
+                                SyntaxKind::NODE_BIN_OP => {
+                                    let inner = BinOp::cast(non_bool_side.clone()).unwrap();
+                                    // `!a ? b`, no paren required
+                                    if inner.operator() == BinOpKind::IsSet {
+                                        make::unary_not(&non_bool_side).node().clone()
+                                    } else {
+                                        let parens = make::parenthesize(&non_bool_side);
+                                        make::unary_not(parens.node()).node().clone()
+                                    }
+                                },
+                                _ => {
+                                    let parens = make::parenthesize(&non_bool_side);
+                                    make::unary_not(parens.node()).node().clone()
+                                }
+                            }
+                        },
+                    }
+                };
                 let message = format!(
                     "Comparing `{}` with boolean literal `{}`",
                     non_bool_side,
                     bool_side
                 );
-                Some(Report::new().diagnostic(at, message))
+                Some(
+                    Report::new(Self::note())
+                      .suggest(at, message, Suggestion::new(at, replacement))
+                )
             } else {
                 None
             }
@@ -45,34 +81,18 @@ impl Rule for BoolComparison {
     }
 }
 
-// not entirely accurate, underhanded nix programmers might write `true = false`
-fn is_boolean_ident(node: &SyntaxNode) -> bool {
-    if let Some(ident_expr) = Ident::cast(node.clone()) {
-        ident_expr.as_str() == "true" || ident_expr.as_str() == "false"
-    } else {
-        false
-    }
+enum NixBoolean {
+    True,
+    False,
 }
 
-// #[cfg(test)]
-// mod tests {
-//     use super::*;
-//     use rnix::{parser, WalkEvent};
-//
-//     #[test]
-//     fn trivial() {
-//         let src = r#"
-//         a == true
-//         "#;
-//         let parsed = rnix::parse(src).as_result().ok().unwrap();
-//         let _ = parsed
-//             .node()
-//             .preorder_with_tokens()
-//             .filter_map(|event| match event {
-//                 WalkEvent::Enter(t) => Some(t),
-//                 _ => None,
-//             })
-//             .map(|node| BoolComparison.validate(&node))
-//             .collect::<Vec<_>>();
-//     }
-// }
+// not entirely accurate, underhanded nix programmers might write `true = false`
+fn boolean_ident(node: &SyntaxNode) -> Option<NixBoolean> {
+    Ident::cast(node.clone())
+        .map(|ident_expr| match ident_expr.as_str() {
+            "true" => Some(NixBoolean::True),
+            "false" => Some(NixBoolean::False),
+            _ => None,
+        })
+        .flatten()
+}
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 @@
+use rnix::{SyntaxNode, types::{TypedNode, self}};
+
+fn ast_from_text<N: TypedNode>(text: &str) -> N {
+    let parse = rnix::parse(text);
+    let node = match parse.node().descendants().find_map(N::cast) {
+        Some(it) => it,
+        None => {
+            panic!("Failed to make ast node `{}` from text {}", std::any::type_name::<N>(), text)
+        }
+    };
+    node
+}
+
+pub fn parenthesize(node: &SyntaxNode) -> types::Paren {
+    ast_from_text(&format!("({})", node))
+}
+
+pub fn unary_not(node: &SyntaxNode) -> types::UnaryOp {
+    ast_from_text(&format!("!{}", node))
+}