adding more tests, cleaning the code

chipsalliance · May 21, 2021 · b58a5b3 · b58a5b3
1 parent 198b208
commit b58a5b3
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 35 deletions.
diff --git a/verilog/analysis/verilog_linter.cc b/verilog/analysis/verilog_linter.cc
@@ -333,32 +333,41 @@ absl::Status PrintRuleInfo(std::ostream* os,
   return absl::OkStatus();
 }
 
-static void AppendCitation(CustomCitationMap& citations,
-                           const absl::string_view& rule_raw) {
-  const size_t eq_pos = rule_raw.find(':');
-  if (!eq_pos || eq_pos == absl::string_view::npos) return;
-  const size_t rule_id_end = eq_pos;
-  const size_t rule_citation_beg = eq_pos + 1;
+static void AppendCitation(const absl::string_view& rule_raw,
+                           CustomCitationMap& citations) {
+  const size_t colon_pos = rule_raw.find(':');
+  if (!colon_pos || colon_pos == absl::string_view::npos) return;
+  const size_t rule_id_end = colon_pos;
+  const size_t rule_citation_beg = colon_pos + 1;
   absl::string_view rule_id = rule_raw.substr(0, rule_id_end);
   absl::string_view citation =
       rule_raw.substr(rule_citation_beg, rule_raw.size() - rule_citation_beg);
+  rule_id = absl::StripAsciiWhitespace(rule_id);
+  citation = absl::StripAsciiWhitespace(citation);
 
   citations[rule_id] = absl::StrReplaceAll(citation, {{"\\\n", "\n"}});
 }
 
-CustomCitationMap ParseCitations(absl::string_view text) {
+CustomCitationMap ParseCitations(absl::string_view content) {
+#ifdef __WIN32
+  const char* newline = "\r\n";
+  constexpr size_t step = 2;
+#elif __linux__
+  const char* newline = "\n";
+  constexpr size_t step = 1;
+#endif
   CustomCitationMap citations;
   size_t rule_begin = 0;
-  size_t rule_end = text.find('\n');
+  size_t rule_end = content.find(newline);
   while (rule_end != std::string::npos) {
-    if (!rule_end || text[rule_end - 1] == '\\') {
-      rule_end = text.find('\n', rule_end + 1);
+    if (rule_end == 0 || content[rule_end - 1] == '\\') {
+      rule_end = content.find(newline, rule_end + step);
       continue;
     }
-    auto rule_subs = text.substr(rule_begin, rule_end - rule_begin);
-    AppendCitation(citations, rule_subs);
-    rule_begin = rule_end + 1;
-    rule_end = text.find('\n', rule_begin);
+    auto rule_subs = content.substr(rule_begin, rule_end - rule_begin);
+    AppendCitation(rule_subs, citations);
+    rule_begin = rule_end + step;
+    rule_end = content.find(newline, rule_begin);
   }
   return citations;
 }
@@ -368,16 +377,13 @@ void GetLintRuleDescriptionsHelpFlag(std::ostream* os,
                                      const CustomCitationMap& citations) {
   // Set up the map.
   auto rule_map = analysis::GetAllRuleDescriptionsHelpFlag();
-  if (!citations.empty()) {
-    for (const auto& [rule_id, citation] : citations) {
-      auto rule = rule_map.find(rule_id);
-      if (rule != rule_map.end()) {
-        // we can move here, cause process
-        // exits after this function
-        rule->second.description = std::move(citation);
-      }
+  for (const auto& [rule_id, citation] : citations) {
+    auto rule = rule_map.find(rule_id);
+    if (rule != rule_map.end()) {
+      rule->second.description = citation;
     }
   }
+
   for (const auto& rule_id : analysis::kDefaultRuleSet) {
     rule_map[rule_id].default_enabled = true;
   }
@@ -405,14 +411,10 @@ void GetLintRuleDescriptionsMarkdown(std::ostream* os,
     rule_map[rule_id].default_enabled = true;
   }
 
-  if (!citations.empty()) {
-    for (const auto& [rule_id, citation] : citations) {
-      auto rule = rule_map.find(rule_id);
-      if (rule != rule_map.end()) {
-        // we can move here, cause process
-        // exits after this function
-        rule->second.description = std::move(citation);
-      }
+  for (const auto& [rule_id, citation] : citations) {
+    auto rule = rule_map.find(rule_id);
+    if (rule != rule_map.end()) {
+      rule->second.description = citation;
     }
   }
 

diff --git a/verilog/analysis/verilog_linter.h b/verilog/analysis/verilog_linter.h
@@ -138,7 +138,7 @@ void GetLintRuleDescriptionsHelpFlag(std::ostream*, absl::string_view,
 void GetLintRuleDescriptionsMarkdown(std::ostream*, const CustomCitationMap&);
 
 // Parse the file with custom citations
-CustomCitationMap ParseCitations(absl::string_view);
+CustomCitationMap ParseCitations(absl::string_view content);
 
 }  // namespace verilog
 

diff --git a/verilog/analysis/verilog_linter_test.cc b/verilog/analysis/verilog_linter_test.cc
@@ -384,18 +384,25 @@ TEST_F(VerilogLinterTest, MultiByteUTF8CharactersAreOnlyCountedOnce) {
 }
 
 TEST(VerilogLinterDocumentationTest, ParseCustomCitations) {
+#ifdef __WIN32
+#define NEWLINE "\r\n"
+#elif __linux__
+#define NEWLINE "\n"
+#endif
   absl::string_view citations_raw =
-      "struct-union-name-style:one line citation is ok\n"
-      "signal-name-style:multi line citation\\\n"
-      "is also\\\n"
-      "correct\n";
+      "struct-union-name-style:one line citation is ok" NEWLINE
+      "signal-name-style:multi line citation\\" NEWLINE "is also\\" NEWLINE
+      "correct" NEWLINE "stripping-left :the spaces" NEWLINE
+      "stripping-right: the spaces" NEWLINE;
 
   auto citations = verilog::ParseCitations(citations_raw);
   ASSERT_TRUE(!citations.empty());
   EXPECT_TRUE(absl::StrContains(citations["struct-union-name-style"],
                                 "one line citation is ok"));
   EXPECT_TRUE(
       absl::StrContains(citations["signal-name-style"], "\nis also\ncorrect"));
+  EXPECT_TRUE(absl::StrContains(citations["stripping-left"], "the spaces"));
+  EXPECT_TRUE(citations["stripping-right"] == "the spaces");
 }
 
 TEST(VerilogLinterDocumentationTest, AllRulesHelpDescriptions) {