Merge pull request #2586 from Talha-Dmr/fix/generate-label-prefix-configurable
verilog: make generate-label-prefix configurable via style_regex
diff --git a/verible/verilog/analysis/checkers/BUILD b/verible/verilog/analysis/checkers/BUILD
index bd3a98b..2151eaa 100644
--- a/verible/verilog/analysis/checkers/BUILD
+++ b/verible/verilog/analysis/checkers/BUILD
@@ -817,6 +817,7 @@
"//verible/common/analysis:syntax-tree-lint-rule",
"//verible/common/analysis/matcher",
"//verible/common/analysis/matcher:bound-symbol-manager",
+ "//verible/common/text:config-utils",
"//verible/common/text:symbol",
"//verible/common/text:syntax-tree-context",
"//verible/common/text:token-info",
@@ -826,7 +827,9 @@
"//verible/verilog/CST:verilog-nonterminals",
"//verible/verilog/analysis:descriptions",
"//verible/verilog/analysis:lint-rule-registry",
+ "@abseil-cpp//absl/status",
"@abseil-cpp//absl/strings",
+ "@re2",
],
alwayslink = 1,
)
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
index c8822ce..e678ad5 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
@@ -14,12 +14,17 @@
#include "verible/verilog/analysis/checkers/generate-label-prefix-rule.h"
+#include <memory>
+#include <string>
#include <string_view>
-#include "absl/strings/match.h"
+#include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/matcher/bound-symbol-manager.h"
#include "verible/common/analysis/matcher/matcher.h"
+#include "verible/common/text/config-utils.h"
#include "verible/common/text/symbol.h"
#include "verible/common/text/syntax-tree-context.h"
#include "verible/common/text/token-info.h"
@@ -38,22 +43,36 @@
// Register the lint rule
VERILOG_REGISTER_LINT_RULE(GenerateLabelPrefixRule);
-static constexpr std::string_view kMessage =
- "All generate block labels must start with g_ or gen_";
+static constexpr std::string_view kDefaultStyleRegex = "(g_|gen_).*";
-// TODO(fangism): and be lower_snake_case?
-// TODO(fangism): generalize to a configurable pattern and
-// rename this class/rule to GenerateLabelNamingStyle?
+GenerateLabelPrefixRule::GenerateLabelPrefixRule()
+ : style_regex_(
+ std::make_unique<re2::RE2>(kDefaultStyleRegex, re2::RE2::Quiet)) {}
const LintRuleDescriptor &GenerateLabelPrefixRule::GetDescriptor() {
static const LintRuleDescriptor d{
.name = "generate-label-prefix",
.topic = "generate-constructs",
- .desc = "Checks that every generate block label starts with g_ or gen_.",
+ .desc =
+ "Checks that every generate block label matches the regex defined by "
+ "style_regex. The default regex requires labels to start with g_ or "
+ "gen_. Refer to https://github.com/chipsalliance/verible/tree/master/"
+ "verilog/tools/lint#readme for more detail on verible regex "
+ "patterns.",
+ // NOLINTNEXTLINE(misc-include-cleaner)
+ .param = {{"style_regex", std::string(kDefaultStyleRegex),
+ "A regex used to check generate label style."}},
};
return d;
}
+std::string GenerateLabelPrefixRule::CreateViolationMessage() const {
+ return absl::StrCat(
+ "Generate block label does not match the naming convention defined by "
+ "regex pattern: ",
+ style_regex_->pattern());
+}
+
// Matches begin statements
static const Matcher &BlockMatcher() {
static const Matcher matcher(NodekGenerateBlock());
@@ -84,15 +103,24 @@
}
if (label != nullptr) {
- if (!(absl::StartsWith(label->text(), "g_") ||
- absl::StartsWith(label->text(), "gen_"))) {
- violations_.insert(verible::LintViolation(*label, kMessage, context));
+ if (!RE2::FullMatch(label->text(), *style_regex_)) {
+ violations_.insert(verible::LintViolation(
+ *label, CreateViolationMessage(), context));
}
}
}
}
}
+// NOLINTNEXTLINE(misc-include-cleaner)
+absl::Status GenerateLabelPrefixRule::Configure(
+ std::string_view configuration) {
+ using verible::config::SetRegex;
+ absl::Status s = verible::ParseNameValues(
+ configuration, {{"style_regex", SetRegex(&style_regex_)}});
+ return s;
+}
+
verible::LintRuleStatus GenerateLabelPrefixRule::Report() const {
return verible::LintRuleStatus(violations_, GetDescriptor());
}
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
index 9090cf3..e18df9c 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
@@ -15,8 +15,13 @@
#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
+#include <memory>
#include <set>
+#include <string>
+#include <string_view>
+#include "absl/status/status.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/syntax-tree-lint-rule.h"
#include "verible/common/text/symbol.h"
@@ -27,20 +32,28 @@
namespace analysis {
// GenerateLabelPrefixRule checks that all generate block labels start
-// with g_ or gen_
+// with g_ or gen_ (configurable via style_regex)
class GenerateLabelPrefixRule : public verible::SyntaxTreeLintRule {
public:
using rule_type = verible::SyntaxTreeLintRule;
+ GenerateLabelPrefixRule();
+
static const LintRuleDescriptor &GetDescriptor();
+ std::string CreateViolationMessage() const;
+
void HandleSymbol(const verible::Symbol &symbol,
const verible::SyntaxTreeContext &context) final;
verible::LintRuleStatus Report() const final;
+ absl::Status Configure(std::string_view configuration) final;
+
private:
std::set<verible::LintViolation> violations_;
+
+ std::unique_ptr<re2::RE2> style_regex_;
};
} // namespace analysis
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
index b46bfe4..cb43501 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
@@ -27,6 +27,7 @@
namespace {
using verible::LintTestCase;
+using verible::RunConfiguredLintTestCases;
using verible::RunLintTestCases;
TEST(GenerateLabelPrefixRuleTest, Various) {
@@ -229,6 +230,40 @@
RunLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(kTestCases);
}
+TEST(GenerateLabelPrefixRuleTest, CustomRegex) {
+ const std::initializer_list<LintTestCase> kTestCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "for (genvar i=0; i<5; i++) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kTestCases, "style_regex:my_.*");
+
+ const std::initializer_list<LintTestCase> kFailCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "g_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "gen_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kFailCases, "style_regex:my_.*");
+}
+
} // namespace
} // namespace analysis
} // namespace verilog
