From f01177e2854de928c15c266dbbf8e8868e65c1b5 Mon Sep 17 00:00:00 2001
From: Yingchi Long <i@lyc.dev>
Date: Wed, 24 Apr 2024 18:30:37 +0800
Subject: [PATCH] libnixf: diagnose redundant paren (#448)

---
 .../include/nixf/Basic/DiagnosticKinds.inc    |  1 +
 libnixf/src/Parse/ParseSimple.cpp             | 44 +++++++++++++++++++
 libnixf/test/Parse/ParseSimple.cpp            | 39 ++++++++++++++++
 3 files changed, 84 insertions(+)

diff --git a/libnixf/include/nixf/Basic/DiagnosticKinds.inc b/libnixf/include/nixf/Basic/DiagnosticKinds.inc
index 887c872fd..d19c6a5d8 100644
--- a/libnixf/include/nixf/Basic/DiagnosticKinds.inc
+++ b/libnixf/include/nixf/Basic/DiagnosticKinds.inc
@@ -10,6 +10,7 @@ DIAG("lex-float-leading-zero", FloatLeadingZero, Warning,
      "float begins with extra zeros `{}` is nixf extension")
 DIAG("parse-expected", Expected, Error, "expected {}")
 DIAG("parse-int-too-big", IntTooBig, Error, "this integer is too big for nix interpreter")
+DIAG("parse-redundant-paren", RedundantParen, Warning, "redundant parentheses")
 DIAG("parse-attrpath-extra-dot", AttrPathExtraDot, Error,
      "extra `.` at the end of attrpath")
 DIAG("parse-select-extra-dot", SelectExtraDot, Error,
diff --git a/libnixf/src/Parse/ParseSimple.cpp b/libnixf/src/Parse/ParseSimple.cpp
index 08dffec1f..ebd3c6dd5 100644
--- a/libnixf/src/Parse/ParseSimple.cpp
+++ b/libnixf/src/Parse/ParseSimple.cpp
@@ -5,6 +5,35 @@
 using namespace nixf;
 using namespace nixf::detail;
 
+namespace {
+
+/// \brief  Whether the node could be produced by non-term \p expr_simple
+bool mayProducedBySimple(Node::NodeKind NK) {
+  switch (NK) {
+  case Node::NK_ExprVar:
+  case Node::NK_ExprInt:
+  case Node::NK_ExprFloat:
+  case Node::NK_ExprSPath:
+  case Node::NK_ExprString:
+  case Node::NK_ExprPath:
+  case Node::NK_ExprParen:
+  case Node::NK_ExprAttrs:
+  case Node::NK_ExprList:
+    return true;
+  default:
+    break;
+  }
+  return false;
+}
+
+bool mayProducedBySimple(const Expr *E) {
+  if (!E)
+    return false;
+  return mayProducedBySimple(E->kind());
+}
+
+} // namespace
+
 std::shared_ptr<ExprParen> Parser::parseExprParen() {
   Token L = peek();
   auto LParen = std::make_shared<Misc>(L.range());
@@ -18,12 +47,27 @@ std::shared_ptr<ExprParen> Parser::parseExprParen() {
   if (ExpectResult ER = expect(tok_r_paren); ER.ok()) {
     consume(); // )
     auto RParen = std::make_shared<Misc>(ER.tok().range());
+    if (mayProducedBySimple(Expr.get())) {
+      Diagnostic &D =
+          Diags.emplace_back(Diagnostic::DK_RedundantParen, LParen->range());
+      D.tag(DiagnosticTag::Faded);
+      Fix &F = D.fix("remove ( and )");
+      F.edit(TextEdit::mkRemoval(LParen->range()));
+      F.edit(TextEdit::mkRemoval(RParen->range()));
+    }
     return std::make_shared<ExprParen>(
         LexerCursorRange{L.lCur(), ER.tok().rCur()}, std::move(Expr),
         std::move(LParen), std::move(RParen));
   } else { // NOLINT(readability-else-after-return)
     ER.diag().note(Note::NK_ToMachThis, L.range())
         << std::string(tok::spelling(tok_l_paren));
+    if (mayProducedBySimple(Expr.get())) {
+      Diagnostic &D =
+          Diags.emplace_back(Diagnostic::DK_RedundantParen, LParen->range());
+      D.tag(DiagnosticTag::Faded);
+      Fix &F = D.fix("remove (");
+      F.edit(TextEdit::mkRemoval(LParen->range()));
+    }
     return std::make_shared<ExprParen>(
         LexerCursorRange{L.lCur(), LastToken->rCur()}, std::move(Expr),
         std::move(LParen),
diff --git a/libnixf/test/Parse/ParseSimple.cpp b/libnixf/test/Parse/ParseSimple.cpp
index f1c19d6d2..6d10aff18 100644
--- a/libnixf/test/Parse/ParseSimple.cpp
+++ b/libnixf/test/Parse/ParseSimple.cpp
@@ -337,6 +337,26 @@ TEST(Parser, ParenExpr) {
   std::vector<Diagnostic> Diags;
   auto AST = nixf::parse(Src, Diags);
   ASSERT_TRUE(AST);
+  ASSERT_EQ(Diags.size(), 1);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_RedundantParen);
+
+  // Check the fix.
+
+  const Diagnostic &D = Diags[0];
+
+  ASSERT_EQ(D.fixes().size(), 1);
+
+  const Fix &F = D.fixes()[0];
+
+  ASSERT_EQ(F.message(), "remove ( and )");
+  ASSERT_EQ(F.edits().size(), 2);
+  ASSERT_TRUE(F.edits()[0].isRemoval());
+  ASSERT_EQ(F.edits()[0].oldRange().lCur().offset(), 0);
+  ASSERT_EQ(F.edits()[0].oldRange().rCur().offset(), 1);
+
+  // Also remove )
+  ASSERT_EQ(F.edits()[1].oldRange().lCur().offset(), 2);
+  ASSERT_EQ(F.edits()[1].oldRange().rCur().offset(), 3);
   ASSERT_EQ(AST->kind(), Node::NK_ExprParen);
   ASSERT_TRUE(AST->range().lCur().isAt(0, 0, 0));
   ASSERT_TRUE(AST->range().rCur().isAt(0, 3, 3));
@@ -355,6 +375,25 @@ TEST(Parser, ParenExprMissingRParen) {
   std::vector<Diagnostic> Diags;
   auto AST = nixf::parse(Src, Diags);
   ASSERT_TRUE(AST);
+
+  ASSERT_EQ(Diags.size(), 2);
+
+  {
+    ASSERT_EQ(Diags[1].kind(), Diagnostic::DK_RedundantParen);
+    // Check the fix.
+    const Diagnostic &D = Diags[1];
+
+    ASSERT_EQ(D.fixes().size(), 1);
+
+    const Fix &F = D.fixes()[0];
+
+    ASSERT_EQ(F.message(), "remove (");
+    ASSERT_EQ(F.edits().size(), 1);
+    ASSERT_TRUE(F.edits()[0].isRemoval());
+    ASSERT_EQ(F.edits()[0].oldRange().lCur().offset(), 0);
+    ASSERT_EQ(F.edits()[0].oldRange().rCur().offset(), 1);
+  }
+
   ASSERT_EQ(AST->kind(), Node::NK_ExprParen);
   ASSERT_TRUE(AST->range().lCur().isAt(0, 0, 0));
   ASSERT_TRUE(AST->range().rCur().isAt(0, 2, 2));