new lint: useless_has_attr

3 files changed, 88 insertions, 0 deletions

diff --git a/lib/src/lints.rs b/lib/src/lints.rs
index 582cabe..7d73277 100644
--- a/lib/src/lints.rs
+++ b/lib/src/lints.rs
@@ -19,4 +19,5 @@ lints! {
     faster_zipattrswith,
     deprecated_to_path,
     bool_simplification,
+    useless_has_attr,
 }
diff --git a/lib/src/lints/useless_has_attr.rs b/lib/src/lints/useless_has_attr.rs
new file mode 100644
index 0000000..aae560a
--- /dev/null
+++ b/lib/src/lints/useless_has_attr.rs
@@ -0,0 +1,83 @@
+use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion};
+
+use if_chain::if_chain;
+use macros::lint;
+use rnix::{
+    types::{BinOp, BinOpKind, IfElse, Select, TypedNode},
+    NodeOrToken, SyntaxElement, SyntaxKind,
+};
+
+/// ## What it does
+/// Checks for expressions that use the "has attribute" operator: `?`,
+/// where the `or` operator would suffice.
+///
+/// ## Why is this bad?
+/// The `or` operator is more readable.
+///
+/// ## Example
+/// ```nix
+/// if x ? a then x.a else some_default
+/// ```
+///
+/// Use `or` instead:
+///
+/// ```nix
+/// x.a or some_default
+/// ```
+#[lint(
+    name = "useless_has_attr",
+    note = "This `if` expression can be simplified with `or`",
+    code = 19,
+    match_with = SyntaxKind::NODE_IF_ELSE
+)]
+struct UselessHasAttr;
+
+impl Rule for UselessHasAttr {
+    fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option<Report> {
+        if_chain! {
+            if let NodeOrToken::Node(node) = node;
+            if let Some(if_else_expr) = IfElse::cast(node.clone());
+            if let Some(condition_expr) = if_else_expr.condition();
+            if let Some(default_expr) = if_else_expr.else_body();
+            if let Some(cond_bin_expr) = BinOp::cast(condition_expr.clone());
+            if let Some(BinOpKind::IsSet) = cond_bin_expr.operator();
+
+            // set ? attr_path
+            // ^^^--------------- lhs
+            //      ^^^^^^^^^^--- rhs
+            if let Some(set) = cond_bin_expr.lhs();
+            if let Some(attr_path) = cond_bin_expr.rhs();
+
+            // check if body of the `if` expression is of the form `set.attr_path`
+            if let Some(body_expr) = if_else_expr.body();
+            if let Some(body_select_expr) = Select::cast(body_expr.clone());
+            let expected_body = make::select(&set, &attr_path);
+
+            // text comparison will do for now
+            if body_select_expr.node().text() == expected_body.node().text();
+            then {
+                let at = node.text_range();
+                // `or` is tightly binding, we need to parenthesize non-literal exprs
+                let default_with_parens = match default_expr.kind() {
+                    SyntaxKind::NODE_LIST
+                    | SyntaxKind::NODE_PAREN
+                    | SyntaxKind::NODE_STRING
+                    | SyntaxKind::NODE_ATTR_SET
+                    | SyntaxKind::NODE_IDENT => default_expr,
+                    _ => make::parenthesize(&default_expr).node().clone(),
+                };
+                let replacement = make::or_default(&set, &attr_path, &default_with_parens).node().clone();
+                let message = format!(
+                    "Consider using `{}` instead of this `if` expression",
+                    replacement
+                );
+                Some(
+                    self.report()
+                        .suggest(at, message, Suggestion::new(at, replacement)),
+                )
+            } else {
+                None
+            }
+        }
+    }
+}
diff --git a/lib/src/make.rs b/lib/src/make.rs
index c36fa7e..2c33ac8 100644
--- a/lib/src/make.rs
+++ b/lib/src/make.rs
@@ -88,3 +88,7 @@ pub fn empty() -> types::Root {
 pub fn binary(lhs: &SyntaxNode, op: &str, rhs: &SyntaxNode) -> types::BinOp {
     ast_from_text(&format!("{} {} {}", lhs, op, rhs))
 }
+
+pub fn or_default(set: &SyntaxNode, index: &SyntaxNode, default: &SyntaxNode) -> types::OrDefault {
+    ast_from_text(&format!("{}.{} or {}", set, index, default))
+}