libnixf: diagose "or" keyword used as a binary operator (#650)

nix-community · Feb 11, 2025 · 065dcb4 · 065dcb4
1 parent 06d1c85
commit 065dcb4
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 0 deletions.
diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py
@@ -93,6 +93,12 @@ class Diagnostic(TypedDict):
         "severity": "Error",
         "message": "operator is non-associative",
     },
+    {
+        "sname": "parse-kw-or-isnot-binary-op",
+        "cname": "KeywordOrIsNotBinaryOp",
+        "severity": "Error",
+        "message": "'or' keyword is not a binary operator",
+    },
     {
         "sname": "let-dynamic",
         "cname": "LetDynamic",

diff --git a/libnixf/src/Parse/ParseOp.cpp b/libnixf/src/Parse/ParseOp.cpp
@@ -150,6 +150,28 @@ std::shared_ptr<Expr> Parser::parseExprOpBP(unsigned LeftRBP) {
           Range, std::move(O), std::move(Prefix), std::move(Path));
       break;
     }
+    case tok_kw_or: {
+      // expr_op 'or' expr_op
+      // This is a wrong syntax, maybe the user meant '||'?
+      auto &D = Diags.emplace_back(Diagnostic::DK_KeywordOrIsNotBinaryOp,
+                                   Tok.range());
+      D.fix("replace 'or' with '||'").edit(TextEdit(Tok.range(), "||"));
+      consume();
+      assert(LastToken && "consume() should have set LastToken");
+      auto O = std::make_shared<Op>(Tok.range(), Tok.kind());
+
+      // Try to recover by replacing 'or' with '||'
+      auto RHS = parseExprOpBP(getBP(tok_op_or).second);
+      if (!RHS) {
+        diagNullExpr(Diags, LastToken->rCur(), "RHS of 'or'");
+        continue;
+      }
+
+      LexerCursorRange Range{Prefix->lCur(), RHS->rCur()};
+      Prefix = std::make_shared<ExprBinOp>(Range, std::move(O),
+                                           std::move(Prefix), std::move(RHS));
+      break;
+    }
     default:
       return Prefix;
     }

diff --git a/libnixf/test/Parse/ParseOp.cpp b/libnixf/test/Parse/ParseOp.cpp
@@ -156,6 +156,53 @@ TEST(Parser, Op_PipeOperator_NonAssociative) {
   ASSERT_EQ(Diags[0].kind(), nixf::Diagnostic::DK_OperatorNotAssociative);
 }
 
+TEST(Parser, OrKeywordAsBinaryOp) {
+  auto Src = R"(a or b)"sv;
+
+  std::vector<Diagnostic> Diags;
+  Parser P(Src, Diags);
+  auto AST = P.parseExpr();
+
+  ASSERT_TRUE(AST);
+
+  // Check that the diagnostic message is generated
+  ASSERT_EQ(Diags.size(), 1);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_KeywordOrIsNotBinaryOp);
+
+  // Check that the suggested fix is to replace 'or' with '||'
+  ASSERT_EQ(Diags[0].fixes().size(), 1);
+  ASSERT_EQ(Diags[0].fixes()[0].edits()[0].newText(), "||");
+}
+
+TEST(Parser, OrKeywordAsBinaryOp2) {
+  auto Src = R"(a or)"sv;
+
+  std::vector<Diagnostic> Diags;
+  Parser P(Src, Diags);
+  auto AST = P.parseExpr();
+
+  ASSERT_TRUE(AST);
+
+  // Check that the diagnostic message is generated
+  ASSERT_EQ(Diags.size(), 2);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_KeywordOrIsNotBinaryOp);
+  ASSERT_EQ(Diags[1].kind(), Diagnostic::DK_Expected);
+}
+
+TEST(Parser, OrKeywordAsBinaryOp3) {
+  auto Src = R"(a.b or (a or b))"sv;
+
+  std::vector<Diagnostic> Diags;
+  Parser P(Src, Diags);
+  auto AST = P.parseExpr();
+
+  ASSERT_TRUE(AST);
+
+  // Check that the diagnostic message is generated
+  ASSERT_EQ(Diags.size(), 1);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_KeywordOrIsNotBinaryOp);
+}
+
 TEST(Parser, Op_Issue647) {
   auto Src = R"(!)"sv;