Skip to content

Commit

Permalink
linter: make constraint-name-style configurable
Browse files Browse the repository at this point in the history
Up until now, constraint-name-style rule required constraint names to
end in '_c'. This makes it configurable, so that users can require
constraint names to start with 'c_' instead.
  • Loading branch information
IEncinas10 committed Sep 23, 2023
1 parent 470e0b9 commit 5e9b7da
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 20 deletions.
1 change: 1 addition & 0 deletions verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,7 @@ cc_library(
"//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",
"//common/text:token-info",
Expand Down
46 changes: 34 additions & 12 deletions 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 @@ -24,9 +24,11 @@
#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 "constraint_name_style_rule.h"
#include "verilog/CST/constraints.h"
#include "verilog/CST/verilog_matchers.h"
#include "verilog/analysis/descriptions.h"
Expand All @@ -44,27 +46,32 @@ 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() {
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.",
"convention and end with _c or start with c_.",
.param = {{"check_suffix", "true"}},
};
return d;
}

static const Matcher& ConstraintMatcher() {
absl::Status ConstraintNameStyleRule::Configure(
absl::string_view configuration) {
return verible::ParseNameValues(
configuration,
{{"check_suffix", verible::config::SetBool(&check_suffix)}});
}

static const Matcher &ConstraintMatcher() {
static const Matcher matcher(NodekConstraintDeclaration());
return matcher;
}

void ConstraintNameStyleRule::HandleSymbol(const verible::Symbol& symbol,
const SyntaxTreeContext& context) {
void ConstraintNameStyleRule::HandleSymbol(const verible::Symbol &symbol,
const SyntaxTreeContext &context) {
verible::matcher::BoundSymbolManager manager;
if (ConstraintMatcher().Matches(symbol, &manager)) {
// Since an out-of-line definition is always followed by a forward
Expand All @@ -75,19 +82,34 @@ void ConstraintNameStyleRule::HandleSymbol(const verible::Symbol& symbol,
return;
}

const auto* identifier_token =
const auto *identifier_token =
GetSymbolIdentifierFromConstraintDeclaration(symbol);
if (!identifier_token) return;

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));
!CheckNameStyle(constraint_name)) {
violations_.insert(
LintViolation(*identifier_token, FormatReason(), context));
}
}
}

bool ConstraintNameStyleRule::CheckNameStyle(absl::string_view name) const {
return check_suffix
? name.size() > suffix.size() && absl::EndsWith(name, suffix)
: name.size() > prefix.size() && absl::StartsWith(name, prefix);
}

absl::string_view ConstraintNameStyleRule::FormatReason() const {
return check_suffix ? "Constraint names must by styled with lower_snake_case "
"and end with _c"
: "Constraint names must by styled with lower_snake_case "
"and start with c_";
;
}

LintRuleStatus ConstraintNameStyleRule::Report() const {
return LintRuleStatus(violations_, GetDescriptor());
}
Expand Down
30 changes: 23 additions & 7 deletions 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 Down Expand Up @@ -27,22 +27,38 @@
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 correct
// naming convention.
//
// 1. Constraints should be named with lower_snake_case
// 2. Constraints should either end with _c or start with c_. This is
// configurable and the default is to require constraints to end with _c
class ConstraintNameStyleRule : public verible::SyntaxTreeLintRule {
public:
using rule_type = verible::SyntaxTreeLintRule;

static const LintRuleDescriptor& GetDescriptor();
static const LintRuleDescriptor &GetDescriptor();

void HandleSymbol(const verible::Symbol& symbol,
const verible::SyntaxTreeContext& context) final;
absl::Status Configure(absl::string_view configuration) final;
void HandleSymbol(const verible::Symbol &symbol,
const verible::SyntaxTreeContext &context) final;

verible::LintRuleStatus Report() const final;

bool CheckNameStyle(absl::string_view name) const;

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

absl::string_view FormatReason() const;

/*
* Determines whether the constraint name should end with "_c" or start with
* "c_"
*/
bool check_suffix = true;
static constexpr absl::string_view suffix = "_c";
static constexpr absl::string_view prefix = "c_";
};

} // namespace analysis
Expand Down
22 changes: 21 additions & 1 deletion verilog/analysis/checkers/constraint_name_style_rule_test.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 Down Expand Up @@ -29,6 +29,7 @@ namespace analysis {
namespace {

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

// Tests that ConstraintNameStyleRule correctly accepts valid names.
Expand All @@ -51,6 +52,25 @@ 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_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, "check_suffix:false");
}

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

0 comments on commit 5e9b7da

Please sign in to comment.