From 40867728c023a9f95f346671b51be68002cc7552 Mon Sep 17 00:00:00 2001 From: Akshay Date: Tue, 21 Sep 2021 16:41:24 +0530 Subject: add suggestion to bool_comparison --- lib/src/lib.rs | 42 ++++++++++++++++--- lib/src/lints/bool_comparison.rs | 88 ++++++++++++++++++++++++---------------- lib/src/make.rs | 20 +++++++++ 3 files changed, 110 insertions(+), 40 deletions(-) create mode 100644 lib/src/make.rs 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; @@ -12,32 +14,60 @@ pub trait Rule { pub struct Diagnostic { pub at: TextRange, pub message: String, + pub suggestion: Option, } 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>(at: TextRange, fix: E) -> Self { + Self { + at, + fix: fix.into() + } } } #[derive(Debug, Default)] pub struct Report { pub diagnostics: Vec, + 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::>(); -// } -// } +// not entirely accurate, underhanded nix programmers might write `true = false` +fn boolean_ident(node: &SyntaxNode) -> Option { + 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(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::(), 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)) +} -- cgit v1.2.3