From 87c80895cd7eb9fa7c09a343352f16b471f57fce Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Thu, 5 Jan 2023 16:16:35 +0300
Subject: feat(tvix/eval): skip & warn for useless parenthesis

Change-Id: I567ca0682012b9d09f1217e57a104ac5671f8d82
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7771
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
---
 tvix/eval/src/compiler/optimiser.rs | 26 ++++++++++++++++++++++++++
 tvix/eval/src/warnings.rs           |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/tvix/eval/src/compiler/optimiser.rs b/tvix/eval/src/compiler/optimiser.rs
index 615de32357..af0ad2256e 100644
--- a/tvix/eval/src/compiler/optimiser.rs
+++ b/tvix/eval/src/compiler/optimiser.rs
@@ -9,6 +9,7 @@ use ast::Expr;
 pub(super) fn optimise_expr(c: &mut Compiler, slot: LocalIdx, expr: ast::Expr) -> ast::Expr {
     match expr {
         Expr::BinOp(_) => optimise_bin_op(c, slot, expr),
+        Expr::Paren(_) => optimise_paren(c, expr),
         _ => expr.to_owned(),
     }
 }
@@ -123,3 +124,28 @@ fn optimise_bin_op(c: &mut Compiler, slot: LocalIdx, expr: ast::Expr) -> ast::Ex
 
     expr
 }
+
+/// Detect useless parenthesis around primitive expressions.
+fn optimise_paren(c: &mut Compiler, expr: ast::Expr) -> ast::Expr {
+    if let Expr::Paren(inner) = &expr {
+        let inner = inner.expr().unwrap();
+
+        if let Expr::Paren(_) = &inner {
+            c.emit_warning(&expr, WarningKind::UselessParens);
+            return optimise_paren(c, inner);
+        }
+
+        if let Expr::Literal(_)
+        | Expr::Str(_)
+        | Expr::Select(_)
+        | Expr::List(_)
+        | Expr::AttrSet(_)
+        | Expr::Ident(_) = &inner
+        {
+            c.emit_warning(&expr, WarningKind::UselessParens);
+            return inner;
+        }
+    }
+
+    expr
+}
diff --git a/tvix/eval/src/warnings.rs b/tvix/eval/src/warnings.rs
index cfe5044a8c..26efd24d1e 100644
--- a/tvix/eval/src/warnings.rs
+++ b/tvix/eval/src/warnings.rs
@@ -17,6 +17,7 @@ pub enum WarningKind {
     DeadCode,
     EmptyInherit,
     EmptyLet,
+    UselessParens,
 
     /// Tvix internal warning for features triggered by users that are
     /// not actually implemented yet, but do not cause runtime failures.
@@ -105,6 +106,10 @@ impl EvalWarning {
                 format!("this `let`-expression contains no bindings")
             }
 
+            WarningKind::UselessParens => {
+                format!("these parenthesis can be removed")
+            }
+
             WarningKind::NotImplemented(what) => {
                 format!("feature not yet implemented in tvix: {}", what)
             }
@@ -125,6 +130,7 @@ impl EvalWarning {
             WarningKind::DeadCode => "W008",
             WarningKind::EmptyInherit => "W009",
             WarningKind::EmptyLet => "W010",
+            WarningKind::UselessParens => "W011",
 
             WarningKind::NotImplemented(_) => "W999",
         }
-- 
cgit 1.4.1