Skip to content

Commit

Permalink
libnixf: diagnose redundant paren (nix-community#448)
Browse files Browse the repository at this point in the history
  • Loading branch information
inclyc authored Apr 24, 2024
1 parent c58edbf commit f01177e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
1 change: 1 addition & 0 deletions libnixf/include/nixf/Basic/DiagnosticKinds.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions libnixf/src/Parse/ParseSimple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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),
Expand Down
39 changes: 39 additions & 0 deletions libnixf/test/Parse/ParseSimple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down

0 comments on commit f01177e

Please sign in to comment.