Skip to content

Commit

Permalink
linter: make constraint-name-style configurable
Browse files Browse the repository at this point in the history
Make constraint-name-style rule regex-configurable while maintaining its
default behavior of requiring constraint names to end in '_c'.
  • Loading branch information
IEncinas10 committed Nov 26, 2024
1 parent fd287ef commit de368db
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 17 deletions.
4 changes: 3 additions & 1 deletion verible/verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1533,16 +1533,18 @@ 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",
"//verible/verilog/CST:constraints",
"//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,
)
Expand Down
34 changes: 23 additions & 11 deletions verible/verilog/analysis/checkers/constraint-name-style-rule.cc
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -15,13 +15,16 @@
#include "verible/verilog/analysis/checkers/constraint-name-style-rule.h"

#include <set>
#include <string>

#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"
Expand All @@ -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(&regex)}});
}

static const Matcher &ConstraintMatcher() {
static const Matcher matcher(NodekConstraintDeclaration());
return matcher;
Expand All @@ -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());
}
Expand Down
27 changes: 23 additions & 4 deletions verible/verilog/analysis/checkers/constraint-name-style-rule.h
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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 <memory>
#include <set>
#include <string>

#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"
Expand All @@ -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<verible::LintViolation> violations_;
std::unique_ptr<re2::RE2> regex = std::make_unique<re2::RE2>(kSuffix);

std::string FormatReason() const;
};

} // namespace analysis
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -27,8 +27,12 @@ namespace analysis {
namespace {

using verible::LintTestCase;
using verible::RunConfiguredLintTestCases;
using verible::RunLintTestCases;

// Lower snake case, starts with `c_`
#define kPrefix "c+(_[a-z0-9]+)+"

// Tests that ConstraintNameStyleRule correctly accepts valid names.
TEST(ConstraintNameStyleRuleTest, AcceptTests) {
const std::initializer_list<LintTestCase> kTestCases = {
Expand All @@ -49,6 +53,26 @@ TEST(ConstraintNameStyleRuleTest, AcceptTests) {
RunLintTestCases<VerilogAnalyzer, ConstraintNameStyleRule>(kTestCases);
}

TEST(ConstraintNameStyleRuleTest, VariousPrefixTests) {
constexpr int kToken = SymbolIdentifier;
const std::initializer_list<LintTestCase> 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"},
};
RunConfiguredLintTestCases<VerilogAnalyzer, ConstraintNameStyleRule>(
kTestCases, "pattern:" kPrefix);
}

// Tests that ConstraintNameStyleRule rejects invalid names.
TEST(ConstraintNameStyleRuleTest, RejectTests) {
constexpr int kToken = SymbolIdentifier;
Expand Down

0 comments on commit de368db

Please sign in to comment.