Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding regex configuration to the parameter-name-style rule #2213

Merged
merged 3 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,6 @@ cc_library(
"//common/analysis:syntax-tree-lint-rule",
"//common/analysis/matcher",
"//common/analysis/matcher:bound-symbol-manager",
"//common/strings:naming-utils",
"//common/text:config-utils",
"//common/text:symbol",
"//common/text:syntax-tree-context",
Expand All @@ -1686,6 +1685,7 @@ cc_library(
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:string_view",
"@com_googlesource_code_re2//:re2",
],
alwayslink = 1,
)
Expand All @@ -1699,6 +1699,9 @@ cc_test(
"//common/analysis:syntax-tree-linter-test-utils",
"//verilog/analysis:verilog-analyzer",
"//verilog/parser:verilog-token-enum",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:string_view",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
],
Expand Down
203 changes: 154 additions & 49 deletions verilog/analysis/checkers/parameter_name_style_rule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
#include "verilog/analysis/checkers/parameter_name_style_rule.h"

#include <cstdint>
#include <memory>
#include <set>
#include <string>
#include <utility>
#include <vector>

#include "absl/status/status.h"
Expand All @@ -26,11 +26,11 @@
#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/config_utils.h"
#include "common/text/symbol.h"
#include "common/text/syntax_tree_context.h"
#include "common/text/token_info.h"
#include "re2/re2.h"
#include "verilog/CST/parameters.h"
#include "verilog/CST/verilog_matchers.h"
#include "verilog/analysis/descriptions.h"
Expand All @@ -40,26 +40,47 @@
namespace verilog {
namespace analysis {

VERILOG_REGISTER_LINT_RULE(ParameterNameStyleRule);

using verible::LintRuleStatus;
using verible::LintViolation;
using verible::SyntaxTreeContext;
using Matcher = verible::matcher::Matcher;

// Register ParameterNameStyleRule.
VERILOG_REGISTER_LINT_RULE(ParameterNameStyleRule);
// Upper Camel Case (may end in _[0-9]+)
static constexpr absl::string_view kUpperCamelCaseRegex =
"([A-Z0-9]+[a-z0-9]*)+(_[0-9]+)?";
// ALL_CAPS
static constexpr absl::string_view kAllCapsRegex = "[A-Z_0-9]+";

static constexpr absl::string_view kLocalparamDefaultRegex = "";
static constexpr absl::string_view kParameterDefaultRegex = "";

ParameterNameStyleRule::ParameterNameStyleRule()
: localparam_style_regex_(std::make_unique<re2::RE2>(
absl::StrCat("(", kUpperCamelCaseRegex, ")"), re2::RE2::Quiet)),
parameter_style_regex_(std::make_unique<re2::RE2>(
absl::StrCat("(", kUpperCamelCaseRegex, ")|(", kAllCapsRegex, ")"),
re2::RE2::Quiet)) {}

const LintRuleDescriptor &ParameterNameStyleRule::GetDescriptor() {
static const LintRuleDescriptor d{
.name = "parameter-name-style",
.topic = "constants",
.desc =
"Checks that non-type parameter and localparam names follow at least "
"one of the naming conventions from a choice of "
"CamelCase and ALL_CAPS, ORed together with the pipe-symbol(|). "
"Empty configuration: no style enforcement.",
.param = {{"localparam_style", "CamelCase", "Style of localparam name"},
"Checks that parameter and localparm names conform to a naming "
"convention based on a choice of 'CamelCase', 'ALL_CAPS' and a user "
"defined regex ORed together. Empty configurtaion: no style "
"enforcement. Refer to "
"https://github.com/chipsalliance/verible/tree/master/verilog/tools/"
"lint#readme for more detail on verible regex patterns.",
.param = {{"localparam_style", "CamelCase", "Style of localparam names"},
{"parameter_style", "CamelCase|ALL_CAPS",
"Style of parameter names"}},
"Style of parameter names."},
{"localparam_style_regex", std::string(kLocalparamDefaultRegex),
"A regex used to check localparam name style."},
{"parameter_style_regex", std::string(kParameterDefaultRegex),
"A regex used to check parameter name style."}},
};
return d;
}
Expand All @@ -69,21 +90,16 @@ static const Matcher &ParamDeclMatcher() {
return matcher;
}

std::string ParameterNameStyleRule::ViolationMsg(absl::string_view symbol_type,
uint32_t allowed_bitmap) {
// TODO(hzeller): there are multiple places in this file referring to the
// same string representations of these options.
static constexpr std::pair<uint32_t, const char *> kBitNames[] = {
{kUpperCamelCase, "CamelCase"}, {kAllCaps, "ALL_CAPS"}};
std::string bit_list;
for (const auto &b : kBitNames) {
if (allowed_bitmap & b.first) {
if (!bit_list.empty()) bit_list.append(" or ");
bit_list.append(b.second);
}
}
return absl::StrCat("Non-type ", symbol_type, " names must be styled with ",
bit_list);
std::string ParameterNameStyleRule::CreateLocalparamViolationMessage() {
return absl::StrCat(
"Localparam name does not match the naming convention ",
"defined by regex pattern: ", localparam_style_regex_->pattern());
}

std::string ParameterNameStyleRule::CreateParameterViolationMessage() {
return absl::StrCat(
"Parameter name does not match the naming convention ",
"defined by regex pattern: ", parameter_style_regex_->pattern());
}

void ParameterNameStyleRule::HandleSymbol(const verible::Symbol &symbol,
Expand All @@ -92,43 +108,132 @@ void ParameterNameStyleRule::HandleSymbol(const verible::Symbol &symbol,
if (ParamDeclMatcher().Matches(symbol, &manager)) {
if (IsParamTypeDeclaration(symbol)) return;

const auto param_decl_token = GetParamKeyword(symbol);
const verilog_tokentype param_decl_token = GetParamKeyword(symbol);

auto identifiers = GetAllParameterNameTokens(symbol);

for (const auto *id : identifiers) {
const auto param_name = id->text();
uint32_t observed_style = 0;
if (verible::IsUpperCamelCaseWithDigits(param_name)) {
observed_style |= kUpperCamelCase;
}
if (verible::IsNameAllCapsUnderscoresDigits(param_name)) {
observed_style |= kAllCaps;
}
if (param_decl_token == TK_localparam && localparam_allowed_style_ &&
(observed_style & localparam_allowed_style_) == 0) {
violations_.insert(LintViolation(
*id, ViolationMsg("localparam", localparam_allowed_style_),
context));
} else if (param_decl_token == TK_parameter && parameter_allowed_style_ &&
(observed_style & parameter_allowed_style_) == 0) {
violations_.insert(LintViolation(
*id, ViolationMsg("parameter", parameter_allowed_style_), context));
const auto name = id->text();
switch (param_decl_token) {
case TK_localparam:
if (!RE2::FullMatch(name, *localparam_style_regex_)) {
violations_.insert(LintViolation(
*id, CreateLocalparamViolationMessage(), context));
}
break;

case TK_parameter:
if (!RE2::FullMatch(name, *parameter_style_regex_)) {
violations_.insert(
LintViolation(*id, CreateParameterViolationMessage(), context));
}
break;

default:
break;
}
}
}
}

absl::Status ParameterNameStyleRule::AppendRegex(
std::unique_ptr<re2::RE2> *rule_regex, absl::string_view regex_str) {
if (*rule_regex == nullptr) {
*rule_regex = std::make_unique<re2::RE2>(absl::StrCat("(", regex_str, ")"),
re2::RE2::Quiet);
} else {
*rule_regex = std::make_unique<re2::RE2>(
absl::StrCat((*rule_regex)->pattern(), "|(", regex_str, ")"),
re2::RE2::Quiet);
}

if ((*rule_regex)->ok()) {
return absl::OkStatus();
}

std::string error_msg = absl::StrCat("Failed to parse regular expression: ",
(*rule_regex)->error());
return absl::InvalidArgumentError(error_msg);
}

absl::Status ParameterNameStyleRule::ConfigureRegex(
std::unique_ptr<re2::RE2> *rule_regex, uint32_t config_style,
std::unique_ptr<re2::RE2> *config_style_regex) {
absl::Status s;

int config_style_regex_length = (*config_style_regex)->pattern().length();

// If no rule is set, no style enforcement
if (config_style == 0 && config_style_regex_length == 0) {
// Set the regex pattern to match everything
*rule_regex = std::make_unique<re2::RE2>(".*", re2::RE2::Quiet);
return absl::OkStatus();
}

// Clear rule_regex
*rule_regex = nullptr;

// Append UpperCamelCase regex to rule_regex (if enabled)
if (config_style & kUpperCamelCase) {
s = AppendRegex(rule_regex, kUpperCamelCaseRegex);
if (!s.ok()) {
return s;
}
}

// Append ALL_CAPS regex to rule_regex (if enabled)
if (config_style & kAllCaps) {
s = AppendRegex(rule_regex, kAllCapsRegex);
if (!s.ok()) {
return s;
}
}

// Append regex from config_style_regex (if provided by the user)
if (config_style_regex_length > 0) {
s = AppendRegex(rule_regex, (*config_style_regex)->pattern());
}

return s;
}

absl::Status ParameterNameStyleRule::Configure(
absl::string_view configuration) {
// TODO(issue #133) include bitmap choices in generated documentation.
static const std::vector<absl::string_view> choices = {
"CamelCase", "ALL_CAPS"}; // same sequence as enum StyleChoicesBits
// same sequence as enum StyleChoicesBits
static const std::vector<absl::string_view> choices = {"CamelCase",
"ALL_CAPS"};

uint32_t localparam_style = kUpperCamelCase;
uint32_t parameter_style = kUpperCamelCase | kAllCaps;
std::unique_ptr<re2::RE2> localparam_style_regex =
std::make_unique<re2::RE2>(kLocalparamDefaultRegex, re2::RE2::Quiet);
std::unique_ptr<re2::RE2> parameter_style_regex =
std::make_unique<re2::RE2>(kParameterDefaultRegex, re2::RE2::Quiet);

using verible::config::SetNamedBits;
return verible::ParseNameValues(
using verible::config::SetRegex;

absl::Status s = verible::ParseNameValues(
configuration,
{{"localparam_style", SetNamedBits(&localparam_allowed_style_, choices)},
{"parameter_style", SetNamedBits(&parameter_allowed_style_, choices)}});
{{"localparam_style", SetNamedBits(&localparam_style, choices)},
{"parameter_style", SetNamedBits(&parameter_style, choices)},
{"localparam_style_regex", SetRegex(&localparam_style_regex)},
{"parameter_style_regex", SetRegex(&parameter_style_regex)}});

if (!s.ok()) {
return s;
}

// Form a regex to use based on *_style, and *_style_regex
s = ConfigureRegex(&localparam_style_regex_, localparam_style,
&localparam_style_regex);
if (!s.ok()) {
return s;
}

s = ConfigureRegex(&parameter_style_regex_, parameter_style,
&parameter_style_regex);
return s;
}

LintRuleStatus ParameterNameStyleRule::Report() const {
Expand Down
35 changes: 25 additions & 10 deletions verilog/analysis/checkers/parameter_name_style_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_PARAMETER_NAME_STYLE_RULE_H_

#include <cstdint>
#include <memory>
#include <set>
#include <string>

Expand All @@ -25,42 +26,56 @@
#include "common/analysis/syntax_tree_lint_rule.h"
#include "common/text/symbol.h"
#include "common/text/syntax_tree_context.h"
#include "re2/re2.h"
#include "verilog/analysis/descriptions.h"

namespace verilog {
namespace analysis {

// ParameterNameStyleRule checks that each non-type parameter/localparam
// follows the correct naming convention.
// parameter should follow UpperCamelCase (preferred) or ALL_CAPS.
// localparam should follow UpperCamelCase.
// follows the correct naming convention matching a regex pattern.
class ParameterNameStyleRule : public verible::SyntaxTreeLintRule {
public:
using rule_type = verible::SyntaxTreeLintRule;

ParameterNameStyleRule();

static const LintRuleDescriptor &GetDescriptor();

absl::Status Configure(absl::string_view configuration) final;
std::string CreateLocalparamViolationMessage();
std::string CreateParameterViolationMessage();

void HandleSymbol(const verible::Symbol &symbol,
const verible::SyntaxTreeContext &context) final;

verible::LintRuleStatus Report() const final;

absl::Status Configure(absl::string_view configuration) final;

const RE2 *localparam_style_regex() const {
return localparam_style_regex_.get();
}
const RE2 *parameter_style_regex() const {
return parameter_style_regex_.get();
}

private:
// Format diagnostic message.
static std::string ViolationMsg(absl::string_view symbol_type,
uint32_t allowed_bitmap);
absl::Status AppendRegex(std::unique_ptr<re2::RE2> *rule_regex,
absl::string_view regex_str);
absl::Status ConfigureRegex(std::unique_ptr<re2::RE2> *rule_regex,
uint32_t config_style,
std::unique_ptr<re2::RE2> *config_style_regex);

enum StyleChoicesBits {
kUpperCamelCase = (1 << 0),
kAllCaps = (1 << 1),
};

uint32_t localparam_allowed_style_ = kUpperCamelCase;
uint32_t parameter_allowed_style_ = kUpperCamelCase | kAllCaps;

std::set<verible::LintViolation> violations_;

// Regex's to check the style against
std::unique_ptr<re2::RE2> localparam_style_regex_;
std::unique_ptr<re2::RE2> parameter_style_regex_;
};

} // namespace analysis
Expand Down
Loading
Loading