From 2191f1198d3a1476a22b1d52870712511817e83d Mon Sep 17 00:00:00 2001 From: Ignacio Encinas Date: Sat, 23 Nov 2024 11:22:18 +0100 Subject: [PATCH] linter: make constraint-name-style configurable Make constraint-name-style rule regex-configurable while maintaining its default behavior of requiring constraint names to be lower snake case ending in `_c` --- verible/verilog/analysis/checkers/BUILD | 4 ++- .../checkers/constraint-name-style-rule.cc | 34 +++++++++++++------ .../checkers/constraint-name-style-rule.h | 27 ++++++++++++--- .../constraint-name-style-rule_test.cc | 24 ++++++++++++- 4 files changed, 72 insertions(+), 17 deletions(-) diff --git a/verible/verilog/analysis/checkers/BUILD b/verible/verilog/analysis/checkers/BUILD index 231f2708a..839ea1394 100644 --- a/verible/verilog/analysis/checkers/BUILD +++ b/verible/verilog/analysis/checkers/BUILD @@ -1533,7 +1533,7 @@ cc_library( "//verible/common/analysis:syntax-tree-lint-rule", "//verible/common/analysis/matcher", "//verible/common/analysis/matcher:bound-symbol-manager", - "//verible/common/strings:naming-utils", + "//verible/common/text:config-utils", "//verible/common/text:symbol", "//verible/common/text:syntax-tree-context", "//verible/common/text:token-info", @@ -1541,8 +1541,10 @@ cc_library( "//verible/verilog/CST:verilog-matchers", "//verible/verilog/analysis:descriptions", "//verible/verilog/analysis:lint-rule-registry", + "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:string_view", + "@com_googlesource_code_re2//:re2", ], alwayslink = 1, ) diff --git a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc index ac832af33..83272570c 100644 --- a/verible/verilog/analysis/checkers/constraint-name-style-rule.cc +++ b/verible/verilog/analysis/checkers/constraint-name-style-rule.cc @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2023 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. @@ -15,13 +15,16 @@ #include "verible/verilog/analysis/checkers/constraint-name-style-rule.h" #include +#include -#include "absl/strings/match.h" +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.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/strings/naming-utils.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" @@ -41,20 +44,24 @@ using verible::matcher::Matcher; // Register ConstraintNameStyleRule. VERILOG_REGISTER_LINT_RULE(ConstraintNameStyleRule); -static constexpr absl::string_view kMessage = - "Constraint names must by styled with lower_snake_case and end with _c."; - const LintRuleDescriptor &ConstraintNameStyleRule::GetDescriptor() { static const LintRuleDescriptor d{ .name = "constraint-name-style", .topic = "constraints", .desc = - "Check that constraint names follow the lower_snake_case " - "convention and end with _c.", + "Check that constraint names follow the required name style " + "specified by a regular expression.", + .param = {{"pattern", kSuffix.data()}}, }; return d; } +absl::Status ConstraintNameStyleRule::Configure( + absl::string_view configuration) { + return verible::ParseNameValues( + configuration, {{"pattern", verible::config::SetRegex(®ex)}}); +} + static const Matcher &ConstraintMatcher() { static const Matcher matcher(NodekConstraintDeclaration()); return matcher; @@ -78,13 +85,18 @@ void ConstraintNameStyleRule::HandleSymbol(const verible::Symbol &symbol, const absl::string_view constraint_name = identifier_token->text(); - if (!verible::IsLowerSnakeCaseWithDigits(constraint_name) || - !absl::EndsWith(constraint_name, "_c")) { - violations_.insert(LintViolation(*identifier_token, kMessage, context)); + if (!RE2::FullMatch(constraint_name, *regex)) { + violations_.insert( + LintViolation(*identifier_token, FormatReason(), context)); } } } +std::string ConstraintNameStyleRule::FormatReason() const { + return absl::StrCat("Constraint names must obey the following regex: ", + regex->pattern()); +} + LintRuleStatus ConstraintNameStyleRule::Report() const { return LintRuleStatus(violations_, GetDescriptor()); } diff --git a/verible/verilog/analysis/checkers/constraint-name-style-rule.h b/verible/verilog/analysis/checkers/constraint-name-style-rule.h index cb934ec55..a049ad21b 100644 --- a/verible/verilog/analysis/checkers/constraint-name-style-rule.h +++ b/verible/verilog/analysis/checkers/constraint-name-style-rule.h @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2023 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. @@ -15,8 +15,13 @@ #ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_CONSTRAINT_NAME_STYLE_RULE_H_ #define VERIBLE_VERILOG_ANALYSIS_CHECKERS_CONSTRAINT_NAME_STYLE_RULE_H_ +#include #include +#include +#include "absl/status/status.h" +#include "absl/strings/string_view.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" @@ -26,22 +31,36 @@ namespace verilog { namespace analysis { -// ConstraintNameStyleRule check each constraint name follows the correct naming -// convention. -// The constraints should be named with lower_snake_case and end with _c. +// ConstraintNameStyleRule checks that each constraint name follows the +// specified naming convention. +// +// This convention is set by providing a regular expression to be matched +// against. +// +// The default, `kSuffix` checks that the name is written in lower_snake_case +// and ends with `_c` class ConstraintNameStyleRule : public verible::SyntaxTreeLintRule { public: using rule_type = verible::SyntaxTreeLintRule; static const LintRuleDescriptor &GetDescriptor(); + absl::Status Configure(absl::string_view configuration) final; void HandleSymbol(const verible::Symbol &symbol, const verible::SyntaxTreeContext &context) final; verible::LintRuleStatus Report() const final; + std::string Pattern() const { return regex->pattern(); } + private: + // Lower snake case, ends with `_c` + static constexpr absl::string_view kSuffix = "([a-z0-9]+_)+c"; + std::set violations_; + std::unique_ptr regex = std::make_unique(kSuffix); + + std::string FormatReason() const; }; } // namespace analysis diff --git a/verible/verilog/analysis/checkers/constraint-name-style-rule_test.cc b/verible/verilog/analysis/checkers/constraint-name-style-rule_test.cc index bff515a0b..41c884599 100644 --- a/verible/verilog/analysis/checkers/constraint-name-style-rule_test.cc +++ b/verible/verilog/analysis/checkers/constraint-name-style-rule_test.cc @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2023 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. @@ -27,6 +27,7 @@ namespace analysis { namespace { using verible::LintTestCase; +using verible::RunConfiguredLintTestCases; using verible::RunLintTestCases; // Tests that ConstraintNameStyleRule correctly accepts valid names. @@ -49,6 +50,27 @@ TEST(ConstraintNameStyleRuleTest, AcceptTests) { RunLintTestCases(kTestCases); } +TEST(ConstraintNameStyleRuleTest, VariousPrefixTests) { + constexpr int kToken = SymbolIdentifier; + const std::initializer_list kTestCases = { + {"class foo; rand logic a; constraint c_foo { a == 16; } endclass"}, + {"class foo; rand logic a; constraint c_a { a == 16; } endclass"}, + {"class foo; rand logic a; constraint c_foo_bar { a == 16; } endclass"}, + {"class foo; rand logic a; constraint ", + {kToken, "c_"}, + " { a == 16; } endclass"}, + {"class foo; rand logic a; constraint ", + {kToken, "no_suffix"}, + " { a == 16; } endclass"}, + {"class foo; rand logic a; constraint ", + {kToken, "suffix_ok_but_we_want_prefix_c"}, + " { a == 16; } endclass"}, + }; + // Lower snake case, starts with `c_` + RunConfiguredLintTestCases( + kTestCases, "pattern:c+(_[a-z0-9]+)+"); +} + // Tests that ConstraintNameStyleRule rejects invalid names. TEST(ConstraintNameStyleRuleTest, RejectTests) { constexpr int kToken = SymbolIdentifier;