Add convenience function: TextStructureView::GetRangeForToken().

There are various places in which we need the line column range for a token. Put that in one place. Signed-off-by: Henner Zeller <[email protected]>
Practical-UVM-Step-By-Step · Nov 2, 2021 · 85f7289 · 85f7289
1 parent 612c1ba
commit 85f7289
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 40 deletions.
diff --git a/common/analysis/file_analyzer.cc b/common/analysis/file_analyzer.cc
@@ -104,26 +104,25 @@ absl::Status FileAnalyzer::Parse(Parser* parser) {
 std::string FileAnalyzer::TokenErrorMessage(
     const TokenInfo& error_token) const {
   // TODO(fangism): accept a RejectedToken to get an explanation message.
-  const absl::string_view base_text = Data().Contents();
   std::ostringstream output_stream;
   if (!error_token.isEOF()) {
     // TODO(hzeller): simply print LineColumnRange ?
-    const auto left = Data().GetLineColAtOffset(error_token.left(base_text));
-    auto right = Data().GetLineColAtOffset(error_token.right(base_text));
-    --right.column;  // Point to last character, not one-past-the-end.
-    output_stream << "token: \"" << error_token.text() << "\" at " << left;
-    if (left.line == right.line) {
+    LineColumnRange range = Data().GetRangeForToken(error_token);
+    --range.end.column;  // Point to last character, not one-past-the-end.
+    output_stream << "token: \"" << error_token.text() << "\" at "
+                  << range.start;
+    if (range.start.line == range.end.line) {
       // Only print upper bound if it differs by > 1 character.
-      if (left.column + 1 < right.column) {
+      if (range.start.column + 1 < range.end.column) {
         // .column is 0-based index, so +1 to get 1-based index.
-        output_stream << '-' << right.column + 1;
+        output_stream << '-' << range.end.column + 1;
       }
     } else {
       // Already prints 1-based index.
-      output_stream << '-' << right;
+      output_stream << '-' << range.end;
     }
   } else {
-    const auto end = Data().GetLineColAtOffset(base_text.length());
+    const auto end = Data().GetLineColAtOffset(Data().Contents().length());
     output_stream << "token: <<EOF>> at " << end;
   }
   return output_stream.str();
@@ -141,25 +140,15 @@ std::vector<std::string> FileAnalyzer::TokenErrorMessages() const {
 void FileAnalyzer::ExtractLinterTokenErrorDetail(
     const RejectedToken& error_token,
     const ReportLinterErrorFunction& error_report) const {
-  const absl::string_view base_text = Data().Contents();
-
-  const LineColumn start =
-      error_token.token_info.isEOF()
-          ? Data().GetLineColAtOffset(base_text.length())
-          : Data().GetLineColAtOffset(error_token.token_info.left(base_text));
-  const LineColumn end =
-      error_token.token_info.isEOF()
-          ? start
-          : Data().GetLineColAtOffset(error_token.token_info.right(base_text));
-
+  const LineColumnRange range = Data().GetRangeForToken(error_token.token_info);
   absl::string_view context_line = "";
   const auto& lines = Data().Lines();
-  if (start.line < static_cast<int>(lines.size())) {
-    context_line = lines[start.line];
+  if (range.start.line < static_cast<int>(lines.size())) {
+    context_line = lines[range.start.line];
   }
   // TODO(b/63893567): Explain syntax errors by inspecting state stack.
   error_report(
-      filename_, {start, end}, error_token.phase,
+      filename_, range, error_token.phase,
       error_token.token_info.isEOF() ? "<EOF>" : error_token.token_info.text(),
       context_line, error_token.explanation);
 }

diff --git a/common/strings/line_column_map.h b/common/strings/line_column_map.h
@@ -38,21 +38,30 @@ struct LineColumn {
   int line;    // 0-based index
   int column;  // 0-based index
 
-  bool operator==(const LineColumn& r) const {
+  constexpr bool operator==(const LineColumn& r) const {
     return line == r.line && column == r.column;
   }
+  constexpr bool operator<(const LineColumn& r) const {
+    if (line < r.line) return true;
+    if (line > r.line) return false;
+    return column < r.column;
+  }
+  constexpr bool operator>=(const LineColumn& r) const { return !(*this < r); }
 };
 
 std::ostream& operator<<(std::ostream&, const LineColumn&);
 
 // A complete range.
 struct LineColumnRange {
-  LineColumn start;
-  LineColumn end;
+  LineColumn start;  // Inclusive
+  LineColumn end;    // Exclusive
 
-  bool operator==(const LineColumnRange& r) const {
+  constexpr bool operator==(const LineColumnRange& r) const {
     return start == r.start && end == r.end;
   }
+  constexpr bool PositionInRange(const LineColumn& pos) const {
+    return pos >= start && pos < end;
+  }
 };
 
 std::ostream& operator<<(std::ostream&, const LineColumnRange&);

diff --git a/common/strings/line_column_map_test.cc b/common/strings/line_column_map_test.cc
@@ -179,5 +179,32 @@ TEST(LineColumnMapTest, Lookup) {
   }
 }
 
+TEST(LineColumnTest, LineColumnComparison) {
+  constexpr LineColumn before_line{.line = 41, .column = 1};
+  constexpr LineColumn before_col{.line = 42, .column = 1};
+  constexpr LineColumn center{.line = 42, .column = 8};
+  constexpr LineColumn after_col{.line = 42, .column = 8};
+
+  EXPECT_LT(before_line, center);
+  EXPECT_LT(before_col, center);
+  EXPECT_EQ(center, center);
+  EXPECT_GE(center, center);
+  EXPECT_GE(after_col, center);
+}
+
+TEST(LineColumnTest, LineColumnRangeComparison) {
+  constexpr LineColumnRange range{{.line = 42, .column = 17},
+                                  {.line = 42, .column = 22}};
+
+  constexpr LineColumn before{.line = 42, .column = 16};
+  constexpr LineColumn inside_start{.line = 42, .column = 17};
+  constexpr LineColumn inside_end{.line = 42, .column = 21};
+  constexpr LineColumn outside_after_end{.line = 42, .column = 22};
+
+  EXPECT_FALSE(range.PositionInRange(before));
+  EXPECT_TRUE(range.PositionInRange(inside_start));
+  EXPECT_TRUE(range.PositionInRange(inside_end));
+  EXPECT_FALSE(range.PositionInRange(outside_after_end));
+}
 }  // namespace
 }  // namespace verible
diff --git a/common/text/text_structure.cc b/common/text/text_structure.cc
@@ -127,6 +127,12 @@ TokenRange TextStructureView::TokenRangeSpanningOffsets(size_t lower,
   return make_range(left, right);
 }
 
+LineColumnRange TextStructureView::GetRangeForToken(
+    const TokenInfo& token) const {
+  return {GetLineColAtOffset(token.left(Contents())),
+          GetLineColAtOffset(token.right(Contents()))};
+}
+
 TokenRange TextStructureView::TokenRangeOnLine(size_t lineno) const {
   if (lineno + 1 < line_token_map_.size()) {
     return make_range(line_token_map_[lineno], line_token_map_[lineno + 1]);

diff --git a/common/text/text_structure.h b/common/text/text_structure.h
@@ -95,10 +95,14 @@ class TextStructureView {
 
   const LineColumnMap& GetLineColumnMap() const { return line_column_map_; }
 
+  // Given a byte offset, return the line/column
   LineColumn GetLineColAtOffset(int bytes_offset) const {
     return line_column_map_.GetLineColAtOffset(contents_, bytes_offset);
   }
 
+  // Convenience function: Given the token, return the range it covers.
+  LineColumnRange GetRangeForToken(const TokenInfo& token) const;
+
   const std::vector<TokenSequence::const_iterator>& GetLineTokenMap() const {
     return line_token_map_;
   }

diff --git a/verilog/tools/ls/document-symbol-filler.cc b/verilog/tools/ls/document-symbol-filler.cc
@@ -147,11 +147,8 @@ verible::lsp::Range DocumentSymbolFiller::RangeFromLeaf(
 
 verible::lsp::Range DocumentSymbolFiller::RangeFromToken(
     const verible::TokenInfo &token) const {
-  verible::LineColumn start =
-      text_view_.GetLineColAtOffset(token.left(text_view_.Contents()));
-  verible::LineColumn end =
-      text_view_.GetLineColAtOffset(token.right(text_view_.Contents()));
-  return {.start = {.line = start.line, .character = start.column},
-          .end = {.line = end.line, .character = end.column}};
+  const verible::LineColumnRange range = text_view_.GetRangeForToken(token);
+  return {.start = {.line = range.start.line, .character = range.start.column},
+          .end = {.line = range.end.line, .character = range.end.column}};
 }
 }  // namespace verilog
diff --git a/verilog/tools/ls/verible-lsp-adapter.cc b/verilog/tools/ls/verible-lsp-adapter.cc
@@ -30,16 +30,14 @@ static verible::lsp::Diagnostic ViolationToDiagnostic(
     const verilog::LintViolationWithStatus &v,
     const verible::TextStructureView &text) {
   const verible::LintViolation &violation = *v.violation;
-  verible::LineColumn start =
-      text.GetLineColAtOffset(violation.token.left(text.Contents()));
-  verible::LineColumn end =
-      text.GetLineColAtOffset(violation.token.right(text.Contents()));
+  const verible::LineColumnRange range = text.GetRangeForToken(violation.token);
   const char *fix_msg = violation.autofixes.empty() ? "" : " (fix available)";
   return verible::lsp::Diagnostic{
       .range =
           {
-              .start = {.line = start.line, .character = start.column},
-              .end = {.line = end.line, .character = end.column},
+              .start = {.line = range.start.line,
+                        .character = range.start.column},
+              .end = {.line = range.end.line, .character = range.end.column},
           },
       .message = absl::StrCat(violation.reason, " ", v.status->url, "[",
                               v.status->lint_rule_name, "]", fix_msg),