Check for genvar and generate region (#649)

* Lint: add legacy-genvar-declaration rule * Waive legacy-genvar-declaration in generate_label_prefix test * Lint: add legacy-generate-region rule * Waive legacy-genvar-declaration in conflicting rules tests
chipsalliance · Jan 27, 2021 · 7c65904 · 7c65904
1 parent b343939
commit 7c65904
Show file tree

Hide file tree

Showing 14 changed files with 488 additions and 0 deletions.
diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD
@@ -36,6 +36,8 @@ cc_library(
         ":generate_label_prefix_rule",
         ":generate_label_rule",
         ":interface_name_style_rule",
+        ":legacy_genvar_declaration_rule",
+        ":legacy_generate_region_rule",
         ":line_length_rule",
         ":macro_name_style_rule",
         ":mismatched_labels_rule",
@@ -1826,6 +1828,80 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "legacy_genvar_declaration_rule",
+    srcs = ["legacy_genvar_declaration_rule.cc"],
+    hdrs = ["legacy_genvar_declaration_rule.h"],
+    deps = [
+        "//common/analysis:citation",
+        "//common/analysis:lint_rule_status",
+        "//common/analysis:syntax_tree_lint_rule",
+        "//common/analysis/matcher",
+        "//common/analysis/matcher:bound_symbol_manager",
+        "//common/strings:naming_utils",
+        "//common/text:symbol",
+        "//common/text:syntax_tree_context",
+        "//verilog/CST:type",
+        "//verilog/CST:verilog_matchers",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint_rule_registry",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "legacy_genvar_declaration_rule_test",
+    srcs = ["legacy_genvar_declaration_rule_test.cc"],
+    deps = [
+        ":legacy_genvar_declaration_rule",
+        "//common/analysis:linter_test_utils",
+        "//common/analysis:syntax_tree_linter_test_utils",
+        "//common/text:symbol",
+        "//verilog/CST:verilog_nonterminals",
+        "//verilog/analysis:verilog_analyzer",
+        "//verilog/parser:verilog_token_enum",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
+cc_library(
+    name = "legacy_generate_region_rule",
+    srcs = ["legacy_generate_region_rule.cc"],
+    hdrs = ["legacy_generate_region_rule.h"],
+    deps = [
+        "//common/analysis:citation",
+        "//common/analysis:lint_rule_status",
+        "//common/analysis:syntax_tree_lint_rule",
+        "//common/analysis/matcher",
+        "//common/analysis/matcher:bound_symbol_manager",
+        "//common/strings:naming_utils",
+        "//common/text:symbol",
+        "//common/text:syntax_tree_context",
+        "//verilog/CST:type",
+        "//verilog/CST:verilog_matchers",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint_rule_registry",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "legacy_generate_region_rule_test",
+    srcs = ["legacy_generate_region_rule_test.cc"],
+    deps = [
+        ":legacy_generate_region_rule",
+        "//common/analysis:linter_test_utils",
+        "//common/analysis:syntax_tree_linter_test_utils",
+        "//common/text:symbol",
+        "//verilog/CST:verilog_nonterminals",
+        "//verilog/analysis:verilog_analyzer",
+        "//verilog/parser:verilog_token_enum",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "uvm_macro_semicolon_rule",
     srcs = ["uvm_macro_semicolon_rule.cc"],

diff --git a/verilog/analysis/checkers/legacy_generate_region_rule.cc b/verilog/analysis/checkers/legacy_generate_region_rule.cc
@@ -0,0 +1,77 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/legacy_generate_region_rule.h"
+
+#include <set>
+#include <string>
+
+#include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
+#include "common/analysis/citation.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/matcher/bound_symbol_manager.h"
+#include "common/analysis/matcher/matcher.h"
+#include "common/strings/naming_utils.h"
+#include "common/text/symbol.h"
+#include "common/text/tree_utils.h"
+#include "verilog/CST/verilog_matchers.h"
+#include "verilog/analysis/lint_rule_registry.h"
+
+namespace verilog {
+namespace analysis {
+
+VERILOG_REGISTER_LINT_RULE(LegacyGenerateRegionRule);
+
+using verible::GetStyleGuideCitation;
+using verible::LintRuleStatus;
+using verible::LintViolation;
+using verible::SyntaxTreeContext;
+using verible::matcher::Matcher;
+using verible::matcher::EqualTagPredicate;
+using verible::FindFirstSubtree;
+
+absl::string_view LegacyGenerateRegionRule::Name() {
+  return "legacy-generate-region";
+}
+const char LegacyGenerateRegionRule::kTopic[] = "generate-constructs";
+const char LegacyGenerateRegionRule::kMessage[] =
+    "Do not use generate regions.";
+
+std::string LegacyGenerateRegionRule::GetDescription(
+    DescriptionType description_type) {
+  return absl::StrCat("Checks that there are no generate regions. See ",
+                      GetStyleGuideCitation(kTopic), ".");
+}
+
+void LegacyGenerateRegionRule::HandleNode(
+    const verible::SyntaxTreeNode& node,
+    const verible::SyntaxTreeContext& context) {
+
+  const auto tag = static_cast<verilog::NodeEnum>(node.Tag().tag);
+  if (tag == NodeEnum::kGenerateRegion) {
+    const auto* generate_keyword = ABSL_DIE_IF_NULL(FindFirstSubtree(&node,
+        EqualTagPredicate<verible::SymbolKind::kLeaf, verilog_tokentype,
+                          verilog_tokentype::TK_generate>));
+    const auto& leaf = verible::SymbolCastToLeaf(*generate_keyword);
+    violations_.insert(LintViolation(leaf.get(), kMessage));
+  }
+}
+
+LintRuleStatus LegacyGenerateRegionRule::Report() const {
+  return LintRuleStatus(violations_, Name(), GetStyleGuideCitation(kTopic));
+}
+
+}  // namespace analysis
+}  // namespace verilog
diff --git a/verilog/analysis/checkers/legacy_generate_region_rule.h b/verilog/analysis/checkers/legacy_generate_region_rule.h
@@ -0,0 +1,58 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_LEGACY_GENERATE_REGION_RULE_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_LEGACY_GENERATE_REGION_RULE_H_
+
+#include <set>
+#include <string>
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/syntax_tree_lint_rule.h"
+#include "common/text/concrete_syntax_tree.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+// LegacyGenerateRegionRule checks that generate regions are not used.
+class LegacyGenerateRegionRule : public verible::SyntaxTreeLintRule {
+ public:
+  using rule_type = verible::SyntaxTreeLintRule;
+  static absl::string_view Name();
+
+  // Returns the description of the rule implemented formatted for either the
+  // helper flag or markdown depending on the parameter type.
+  static std::string GetDescription(DescriptionType);
+
+  void HandleNode(const verible::SyntaxTreeNode& node,
+                  const verible::SyntaxTreeContext& context) override;
+
+  verible::LintRuleStatus Report() const override;
+
+ private:
+  // Link to style guide rule.
+  static const char kTopic[];
+
+  // Diagnostic message.
+  static const char kMessage[];
+
+  std::set<verible::LintViolation> violations_;
+};
+
+}  // namespace analysis
+}  // namespace verilog
+
+#endif  // VERIBLE_VERILOG_ANALYSIS_CHECKERS_LEGACY_GENERATE_REGION_RULE_H_
diff --git a/verilog/analysis/checkers/legacy_generate_region_rule_test.cc b/verilog/analysis/checkers/legacy_generate_region_rule_test.cc
@@ -0,0 +1,61 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/legacy_generate_region_rule.h"
+
+#include <initializer_list>
+
+#include "gtest/gtest.h"
+#include "common/analysis/linter_test_utils.h"
+#include "common/analysis/syntax_tree_linter_test_utils.h"
+#include "verilog/CST/verilog_nonterminals.h"
+#include "verilog/analysis/verilog_analyzer.h"
+#include "verilog/parser/verilog_token_enum.h"
+
+namespace verilog {
+namespace analysis {
+namespace {
+
+using verible::LintTestCase;
+using verible::RunLintTestCases;
+
+TEST(LegacyGenerateRegionRuleTest, ValidCases) {
+  const std::initializer_list<LintTestCase> kTestCases = {
+    {""},
+    {"module M;\n"
+     "for (genvar k = 0; k < FooParam; k++) begin : gen_loop\n"
+     "  // code\n"
+     "end\n"
+     "endmodule\n"}
+  };
+  RunLintTestCases<VerilogAnalyzer, LegacyGenerateRegionRule>(kTestCases);
+}
+
+TEST(LegacyGenerateRegionRuleTest, InvalidCases) {
+  constexpr int kToken = TK_generate;
+  const std::initializer_list<LintTestCase> kTestCases = {
+    {"module M;\n",
+     {kToken, "generate"}, "\n"
+     "  for (genvar k = 0; k < FooParam; k++) begin : gen_loop\n"
+     "    // code\n"
+     "  end\n"
+     "endgenerate\n"
+     "endmodule\n"}
+  };
+  RunLintTestCases<VerilogAnalyzer, LegacyGenerateRegionRule>(kTestCases);
+}
+
+}  // namespace
+}  // namespace analysis
+}  // namespace verilog
diff --git a/verilog/analysis/checkers/legacy_genvar_declaration_rule.cc b/verilog/analysis/checkers/legacy_genvar_declaration_rule.cc
@@ -0,0 +1,77 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/legacy_genvar_declaration_rule.h"
+
+#include <set>
+#include <string>
+
+#include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
+#include "common/analysis/citation.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/matcher/bound_symbol_manager.h"
+#include "common/analysis/matcher/matcher.h"
+#include "common/strings/naming_utils.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/CST/identifier.h"
+#include "verilog/CST/verilog_matchers.h"
+#include "verilog/analysis/lint_rule_registry.h"
+
+namespace verilog {
+namespace analysis {
+
+VERILOG_REGISTER_LINT_RULE(LegacyGenvarDeclarationRule);
+
+using verible::GetStyleGuideCitation;
+using verible::LintRuleStatus;
+using verible::LintViolation;
+using verible::SyntaxTreeContext;
+using verible::matcher::Matcher;
+
+absl::string_view LegacyGenvarDeclarationRule::Name() {
+  return "legacy-genvar-declaration";
+}
+const char LegacyGenvarDeclarationRule::kTopic[] = "generate-constructs";
+const char LegacyGenvarDeclarationRule::kMessage[] =
+    "Do not use separate genvar declaration.";
+
+std::string LegacyGenvarDeclarationRule::GetDescription(
+    DescriptionType description_type) {
+  return absl::StrCat("Checks that there are no separate ",
+                      Codify("genvar", description_type), " declarations. See ",
+                      GetStyleGuideCitation(kTopic), ".");
+}
+
+void LegacyGenvarDeclarationRule::HandleNode(
+    const verible::SyntaxTreeNode& node,
+    const verible::SyntaxTreeContext& context) {
+
+  const auto tag = static_cast<verilog::NodeEnum>(node.Tag().tag);
+  if (tag == NodeEnum::kGenvarDeclaration) {
+    const auto identifier_matches = FindAllSymbolIdentifierLeafs(node);
+    for (const auto& match: identifier_matches) {
+      const auto& leaf = verible::SymbolCastToLeaf(*match.match);
+      violations_.insert(LintViolation(leaf.get(), kMessage));
+    }
+  }
+}
+
+LintRuleStatus LegacyGenvarDeclarationRule::Report() const {
+  return LintRuleStatus(violations_, Name(), GetStyleGuideCitation(kTopic));
+}
+
+}  // namespace analysis
+}  // namespace verilog