From 371d1b52274fbf87f639b748d4a7d04b4b9c8871 Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Sun, 14 Jan 2024 15:22:11 +0800 Subject: [PATCH] libnixf: address comments from @Origami404 (#296) https://github.com/nix-community/nixd/pull/296#discussion_r1451664918 --- libnixf/include/nixf/Basic/Range.h | 40 +++++++++++++++++---- libnixf/lib/Parse/test/Parser.cpp | 56 +++++++++++++++--------------- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/libnixf/include/nixf/Basic/Range.h b/libnixf/include/nixf/Basic/Range.h index d46e73d7c..280f6b37d 100644 --- a/libnixf/include/nixf/Basic/Range.h +++ b/libnixf/include/nixf/Basic/Range.h @@ -5,16 +5,44 @@ namespace nixf { -struct Point { +/// \brief A point in the source file. +/// +/// This class is used to represent a point in the source file. And it shall be +/// constructed by Lexer, to keep Line & Column information correct. +/// \see Lexer::consume(std::size_t) +class Point { int64_t Line; int64_t Column; std::size_t Offset; -}; + friend class Lexer; + Point(int64_t Line, int64_t Column, std::size_t Offset) + : Line(Line), Column(Column), Offset(Offset) {} -inline bool operator==(const Point &LHS, const Point &RHS) { - return LHS.Line == RHS.Line && LHS.Column == RHS.Column && - LHS.Offset == RHS.Offset; -} +public: + friend bool operator==(const Point &LHS, const Point &RHS) { + return LHS.Line == RHS.Line && LHS.Column == RHS.Column && + LHS.Offset == RHS.Offset; + } + Point() = default; + + /// \brief Check if the point is at the given position. + [[nodiscard]] bool isAt(int64_t Line, int64_t Column, + std::size_t Offset) const { + return this->Line == Line && this->Column == Column && + this->Offset == Offset; + } + + /// \brief Line number, starting from 0. + /// + /// Currently we only accept LF as the line terminator. + [[nodiscard]] int64_t line() const { return Line; } + + /// \brief Column number, starting from 0. + [[nodiscard]] int64_t column() const { return Column; } + + /// \brief Offset in the source file, starting from 0. + [[nodiscard]] std::size_t offset() const { return Offset; } +}; class RangeTy { Point Begin; diff --git a/libnixf/lib/Parse/test/Parser.cpp b/libnixf/lib/Parse/test/Parser.cpp index 98fd9fbe0..6dfa663ba 100644 --- a/libnixf/lib/Parse/test/Parser.cpp +++ b/libnixf/lib/Parse/test/Parser.cpp @@ -30,8 +30,8 @@ TEST(Parser, Float) { ASSERT_EQ(Expr->kind(), Node::NK_ExprFloat); ASSERT_EQ(static_cast(Expr.get())->value(), 1.0); ASSERT_EQ(Diags.diags().size(), 0); - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 3, 3)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 3, 3)); } TEST(Parser, FloatLeading) { @@ -45,8 +45,8 @@ TEST(Parser, FloatLeading) { // Check the diagnostic. ASSERT_EQ(Diags.diags().size(), 1); auto &D = Diags.diags()[0]; - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 4, 4)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 4, 4)); ASSERT_EQ(D->kind(), Diagnostic::DK_FloatLeadingZero); ASSERT_EQ(D->args().size(), 1); ASSERT_EQ(D->args()[0], "01"); @@ -60,8 +60,8 @@ TEST(Parser, FloatLeading00) { ASSERT_EQ(Expr->kind(), Node::NK_ExprFloat); ASSERT_EQ(static_cast(Expr.get())->value(), 0.5); ASSERT_EQ(Diags.diags().size(), 1); - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 4, 4)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 4, 4)); } TEST(Parser, StringSimple) { @@ -71,12 +71,12 @@ TEST(Parser, StringSimple) { ASSERT_TRUE(Expr); ASSERT_EQ(Expr->kind(), Node::NK_ExprString); auto Parts = static_cast(Expr.get())->parts(); - ASSERT_EQ(Parts->range().begin(), Point(0, 1, 1)); - ASSERT_EQ(Parts->range().end(), Point(0, 4, 4)); + ASSERT_TRUE(Parts->range().begin().isAt(0, 1, 1)); + ASSERT_TRUE(Parts->range().end().isAt(0, 4, 4)); ASSERT_EQ(Parts->fragments().size(), 1); ASSERT_EQ(Diags.diags().size(), 0); - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 5, 5)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 5, 5)); } TEST(Parser, StringMissingDQuote) { @@ -86,15 +86,15 @@ TEST(Parser, StringMissingDQuote) { ASSERT_TRUE(Expr); ASSERT_EQ(Expr->kind(), Node::NK_ExprString); auto Parts = static_cast(Expr.get())->parts(); - ASSERT_EQ(Parts->range().begin(), Point(0, 1, 1)); - ASSERT_EQ(Parts->range().end(), Point(0, 4, 4)); + ASSERT_TRUE(Parts->range().begin().isAt(0, 1, 1)); + ASSERT_TRUE(Parts->range().end().isAt(0, 4, 4)); ASSERT_EQ(Parts->fragments().size(), 1); // Check the diagnostic. ASSERT_EQ(Diags.diags().size(), 1); auto &D = Diags.diags()[0]; - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 4, 4)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 4, 4)); ASSERT_EQ(D->kind(), Diagnostic::DK_Expected); ASSERT_EQ(D->args().size(), 1); ASSERT_EQ(D->args()[0], "\""); @@ -102,8 +102,8 @@ TEST(Parser, StringMissingDQuote) { // Check the note. ASSERT_EQ(D->notes().size(), 1); auto &N = D->notes()[0]; - ASSERT_EQ(N->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(N->range().end(), Point(0, 1, 1)); + ASSERT_TRUE(N->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(N->range().end().isAt(0, 1, 1)); ASSERT_EQ(N->kind(), Note::NK_ToMachThis); ASSERT_EQ(N->args().size(), 1); ASSERT_EQ(N->args()[0], "\""); @@ -111,8 +111,8 @@ TEST(Parser, StringMissingDQuote) { // Check fix-it hints. ASSERT_EQ(D->fixes().size(), 1); const auto &F = D->fixes()[0]; - ASSERT_EQ(F.oldRange().begin(), Point(0, 4, 4)); - ASSERT_EQ(F.oldRange().end(), Point(0, 4, 4)); + ASSERT_TRUE(F.oldRange().begin().isAt(0, 4, 4)); + ASSERT_TRUE(F.oldRange().end().isAt(0, 4, 4)); ASSERT_EQ(F.newText(), "\""); } @@ -123,8 +123,8 @@ TEST(Parser, StringInterpolation) { ASSERT_TRUE(Expr); ASSERT_EQ(Expr->kind(), Node::NK_ExprString); auto Parts = static_cast(Expr.get())->parts(); - ASSERT_EQ(Parts->range().begin(), Point(0, 1, 1)); - ASSERT_EQ(Parts->range().end(), Point(0, 13, 13)); + ASSERT_TRUE(Parts->range().begin().isAt(0, 1, 1)); + ASSERT_TRUE(Parts->range().end().isAt(0, 13, 13)); ASSERT_EQ(Parts->fragments().size(), 3); ASSERT_EQ(Parts->fragments()[0].kind(), InterpolablePart::SPK_Escaped); @@ -132,8 +132,8 @@ TEST(Parser, StringInterpolation) { ASSERT_EQ(Parts->fragments()[2].kind(), InterpolablePart::SPK_Escaped); ASSERT_EQ(Diags.diags().size(), 0); - ASSERT_EQ(Expr->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(Expr->range().end(), Point(0, 14, 14)); + ASSERT_TRUE(Expr->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(Expr->range().end().isAt(0, 14, 14)); } TEST(Parser, IndentedString) { @@ -156,8 +156,8 @@ TEST(Parser, IndentedString) { // Check the diagnostic. ASSERT_EQ(Diags.diags().size(), 1); auto &D = Diags.diags()[0]; - ASSERT_EQ(D->range().begin(), Point(7, 3, 39)); - ASSERT_EQ(D->range().end(), Point(7, 3, 39)); + ASSERT_TRUE(D->range().begin().isAt(7, 3, 39)); + ASSERT_TRUE(D->range().end().isAt(7, 3, 39)); ASSERT_EQ(D->kind(), Diagnostic::DK_Expected); ASSERT_EQ(D->args().size(), 1); ASSERT_EQ(D->args()[0], "''"); @@ -165,8 +165,8 @@ TEST(Parser, IndentedString) { // Check the note. ASSERT_EQ(D->notes().size(), 1); auto &N = D->notes()[0]; - ASSERT_EQ(N->range().begin(), Point(0, 0, 0)); - ASSERT_EQ(N->range().end(), Point(0, 2, 2)); + ASSERT_TRUE(N->range().begin().isAt(0, 0, 0)); + ASSERT_TRUE(N->range().end().isAt(0, 2, 2)); ASSERT_EQ(N->kind(), Note::NK_ToMachThis); ASSERT_EQ(N->args().size(), 1); ASSERT_EQ(N->args()[0], "''"); @@ -174,8 +174,8 @@ TEST(Parser, IndentedString) { // Check fix-it hints. ASSERT_EQ(D->fixes().size(), 1); const auto &F = D->fixes()[0]; - ASSERT_EQ(F.oldRange().begin(), Point(7, 3, 39)); - ASSERT_EQ(F.oldRange().end(), Point(7, 3, 39)); + ASSERT_TRUE(F.oldRange().begin().isAt(7, 3, 39)); + ASSERT_TRUE(F.oldRange().end().isAt(7, 3, 39)); ASSERT_EQ(F.newText(), "''"); }