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

Override style guide citations #817

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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
63 changes: 61 additions & 2 deletions verilog/analysis/verilog_linter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_replace.h"
#include "absl/strings/string_view.h"
#include "common/analysis/line_lint_rule.h"
#include "common/analysis/line_linter.h"
Expand Down Expand Up @@ -524,10 +525,60 @@ absl::Status PrintRuleInfo(std::ostream* os,
return absl::OkStatus();
}

static void AppendCitation(const absl::string_view& rule_raw,
CustomCitationMap& citations) {
const size_t colon_pos = rule_raw.find(':');
if (!colon_pos || colon_pos == absl::string_view::npos) return;
const size_t rule_id_end = colon_pos;
const size_t rule_citation_beg = colon_pos + 1;
absl::string_view rule_id = rule_raw.substr(0, rule_id_end);
absl::string_view citation =
rule_raw.substr(rule_citation_beg, rule_raw.size() - rule_citation_beg);
rule_id = absl::StripAsciiWhitespace(rule_id);
citation = absl::StripAsciiWhitespace(citation);

citations[rule_id] =
absl::StrReplaceAll(citation, {{"\\\r\n", "\r\n"}, {"\\\n", "\n"}});
}

CustomCitationMap ParseCitations(absl::string_view content) {
CustomCitationMap citations;
size_t rule_begin = 0;
size_t rule_end = content.find("\n");
while (rule_end != std::string::npos) {
// check unix like escape seq
if (rule_end == 0 || content[rule_end - 1] == '\\') {
rule_end = content.find("\n", rule_end + 1);
continue;
}
// check windows like escape seq
if (rule_end == 1 ||
(content[rule_end - 2] == '\\' && content[rule_end - 1] == '\r')) {
rule_end = content.find("\n", rule_end + 1);
continue;
}

auto rule_subs = content.substr(rule_begin, rule_end - rule_begin);
AppendCitation(rule_subs, citations);
rule_begin = rule_end + 1;

rule_end = content.find("\n", rule_begin);
}
return citations;
}

void GetLintRuleDescriptionsHelpFlag(std::ostream* os,
absl::string_view flag_value) {
absl::string_view flag_value,
const CustomCitationMap& citations) {
// Set up the map.
auto rule_map = analysis::GetAllRuleDescriptionsHelpFlag();
for (const auto& [rule_id, citation] : citations) {
auto rule = rule_map.find(rule_id);
if (rule != rule_map.end()) {
rule->second.description = citation;
}
}

for (const auto& rule_id : analysis::kDefaultRuleSet) {
rule_map[rule_id].default_enabled = true;
}
Expand All @@ -548,12 +599,20 @@ void GetLintRuleDescriptionsHelpFlag(std::ostream* os,
}
}

void GetLintRuleDescriptionsMarkdown(std::ostream* os) {
void GetLintRuleDescriptionsMarkdown(std::ostream* os,
const CustomCitationMap& citations) {
auto rule_map = analysis::GetAllRuleDescriptionsMarkdown();
for (const auto& rule_id : analysis::kDefaultRuleSet) {
rule_map[rule_id].default_enabled = true;
}

for (const auto& [rule_id, citation] : citations) {
auto rule = rule_map.find(rule_id);
if (rule != rule_map.end()) {
rule->second.description = citation;
}
}

for (const auto& rule : rule_map) {
// Print the rule, description and if it is enabled by default.
*os << "### " << rule.first << "\n";
Expand Down
14 changes: 12 additions & 2 deletions verilog/analysis/verilog_linter.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
#include "verilog/analysis/verilog_linter_configuration.h"

namespace verilog {
// the structure used for custom citation
using CustomCitationMap = std::map<absl::string_view, std::string>;

struct LintViolationWithStatus {
const verible::LintViolation* violation;
Expand Down Expand Up @@ -246,10 +248,18 @@ absl::Status PrintRuleInfo(std::ostream*,
absl::string_view);

// Outputs the descriptions for every rule for the --help_rules flag.
void GetLintRuleDescriptionsHelpFlag(std::ostream*, absl::string_view);
// When custom citations are delivered substitute chosen rule description
// with citations specified by user
void GetLintRuleDescriptionsHelpFlag(std::ostream*, absl::string_view,
const CustomCitationMap&);

// Outputs the descriptions for every rule, formatted for markdown.
void GetLintRuleDescriptionsMarkdown(std::ostream*);
// When custom citations are delivered substitute chosen rule description
// with citations specified by user
void GetLintRuleDescriptionsMarkdown(std::ostream*, const CustomCitationMap&);

// Parse the file with custom citations
CustomCitationMap ParseCitations(absl::string_view content);

} // namespace verilog

Expand Down
67 changes: 65 additions & 2 deletions verilog/analysis/verilog_linter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,19 +413,68 @@ TEST_F(VerilogLinterTest, MultiByteUTF8CharactersAreOnlyCountedOnce) {
EXPECT_EQ(diagnostics.second, "");
}

TEST(VerilogLinterDocumentationTest, ParseCustomCitationsLinuxNewlineStyle) {
absl::string_view citations_raw =
"struct-union-name-style:one line citation is ok \n"
"signal-name-style:multi line citation\\\nis also\\\n"
"correct\nstripping-left :the spaces\n"
"stripping-right: the spaces\n";

auto citations = verilog::ParseCitations(citations_raw);
ASSERT_TRUE(!citations.empty());
EXPECT_TRUE(absl::StrContains(citations["struct-union-name-style"],
"one line citation is ok"));
EXPECT_TRUE(
absl::StrContains(citations["signal-name-style"], "\nis also\ncorrect"));
EXPECT_TRUE(citations["stripping-left"] == "the spaces");
EXPECT_TRUE(citations["stripping-right"] == "the spaces");
}

TEST(VerilogLinterDocumentationTest, ParseCustomCitationsWindowsNewlineStyle) {
absl::string_view citations_raw =
"struct-union-name-style:one line citation is ok \r\n"
"signal-name-style:multi line citation\\\r\nis also\\\r\n"
"correct\r\nstripping-left :the spaces\r\n"
"stripping-right: the spaces\r\n";

auto citations = verilog::ParseCitations(citations_raw);
ASSERT_TRUE(!citations.empty());
EXPECT_TRUE(absl::StrContains(citations["struct-union-name-style"],
"one line citation is ok"));
EXPECT_TRUE(absl::StrContains(citations["signal-name-style"],
"\r\nis also\r\ncorrect"));
EXPECT_TRUE(citations["stripping-left"] == "the spaces");
EXPECT_TRUE(citations["stripping-right"] == "the spaces");
}

TEST(VerilogLinterDocumentationTest, AllRulesHelpDescriptions) {
std::ostringstream stream;
verilog::GetLintRuleDescriptionsHelpFlag(&stream, "all");
verilog::GetLintRuleDescriptionsHelpFlag(&stream, "all", {});
// Spot-check a few patterns, must mostly make sure generation
// works without any fatal errors.
EXPECT_TRUE(absl::StrContains(stream.str(), "line-length"));
EXPECT_TRUE(absl::StrContains(stream.str(), "posix-eof"));
EXPECT_TRUE(absl::StrContains(stream.str(), "Enabled by default:"));
}

TEST(VerilogLinterDocumentationTest,
AllRulesHelpDescriptionsWithCustomCitations) {
std::ostringstream stream;
verilog::CustomCitationMap citations = {
{"struct-union-name-style", "one line citation is ok"},
{"signal-name-style", "multi line citation\nis also\ncorrect"}};

verilog::GetLintRuleDescriptionsHelpFlag(&stream, "all", citations);
// Spot-check a few patterns, must mostly make sure generation
// works without any fatal errors.
EXPECT_TRUE(absl::StrContains(stream.str(), "one line citation is ok"));
EXPECT_TRUE(absl::StrContains(stream.str(), "is also\ncorrect"));
EXPECT_TRUE(absl::StrContains(stream.str(), "Enabled by default:"));
}

TEST(VerilogLinterDocumentationTest, AllRulesMarkdown) {
std::ostringstream stream;
verilog::GetLintRuleDescriptionsMarkdown(&stream);
verilog::GetLintRuleDescriptionsMarkdown(&stream, {});
// Spot-check a few patterns, must mostly make sure generation
// works without any fatal errors.
EXPECT_TRUE(absl::StrContains(stream.str(), "line-length"));
Expand Down Expand Up @@ -906,5 +955,19 @@ TEST_F(ViolationFixerTest, PrintAppliedFixes) {
});
}

TEST(VerilogLinterDocumentationTest, AllRulesMarkdownWithCustomCitations) {
std::ostringstream stream;
verilog::CustomCitationMap citations = {
{"struct-union-name-style", "one line citation is ok"},
{"signal-name-style", "multi line citation\nis also\ncorrect"}};

verilog::GetLintRuleDescriptionsMarkdown(&stream, citations);
// Spot-check a few patterns, must mostly make sure generation
// works without any fatal errors.
EXPECT_TRUE(absl::StrContains(stream.str(), "one line citation is ok"));
EXPECT_TRUE(absl::StrContains(stream.str(), "is also\ncorrect"));
EXPECT_TRUE(absl::StrContains(stream.str(), "Enabled by default:"));
}

} // namespace
} // namespace verilog
2 changes: 1 addition & 1 deletion verilog/tools/lint/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ cc_binary(
visibility = ["//visibility:public"],
deps = [
"//common/util:enum_flags",
"//common/util:file_util",
"//common/util:init_command_line",
"//common/util:logging",
"//common/util:file_util",
"//verilog/analysis:verilog_linter",
"//verilog/analysis:verilog_linter_configuration",
"@com_google_absl//absl/flags:flag",
Expand Down
16 changes: 16 additions & 0 deletions verilog/tools/lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,22 @@ Additionally, the `--rules_config` flag can be used to read configuration stored
in a file. The syntax is the same as above, except the rules can be also
separated with the newline character.

## Custom citations

The `--lint_rule_citations` flag allows to load custom description for specified rules.
It accepts the path to file with prepared configuration.
The file should contain the rule set with appropriate description.
The rules are separeted with newline sign. When new lines are desired, just escape them with `\` sign.

### Example file configuration:

```
struct-union-name-style:multi\
line\
example
signal-name-style:single line example
```

## Waiving Lint Violations {#lint-waiver}

### In-file waiver comments
Expand Down
19 changes: 16 additions & 3 deletions verilog/tools/lint/verilog_lint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,12 @@ ABSL_FLAG(std::string, autofix_output_file, "",
"File to write a patch with autofixes to. If not set autofixes are "
"applied directly to the analyzed file. Relevant only when "
"--autofix option is enabled.");

ABSL_FLAG(std::string, lint_rule_citations, "",
"Path to lint rule citations to overwrite. "
"Please refer to the README file for information about its format.");
// LINT.ThenChange(README.md)

using verilog::CustomCitationMap;
using verilog::LinterConfiguration;

// LintOneFile returns 0, 1, or 2
Expand All @@ -104,16 +107,26 @@ int main(int argc, char** argv) {
absl::StrCat("usage: ", argv[0], " [options] <file> [<file>...]");
const auto args = verible::InitCommandLine(usage, &argc, &argv);

std::string custom_citations_file = absl::GetFlag(FLAGS_lint_rule_citations);
std::string content;
CustomCitationMap citations;
if (!custom_citations_file.empty()) {
const absl::Status config_read_status =
verible::file::GetContents(custom_citations_file, &content);
if (!config_read_status.ok()) return -1;
citations = verilog::ParseCitations(content);
}

std::string help_flag = absl::GetFlag(FLAGS_help_rules);
if (!help_flag.empty()) {
verilog::GetLintRuleDescriptionsHelpFlag(&std::cout, help_flag);
verilog::GetLintRuleDescriptionsHelpFlag(&std::cout, help_flag, citations);
return 0;
}

// In documentation generation mode, print documentation and exit immediately.
bool generate_markdown_flag = absl::GetFlag(FLAGS_generate_markdown);
if (generate_markdown_flag) {
verilog::GetLintRuleDescriptionsMarkdown(&std::cout);
verilog::GetLintRuleDescriptionsMarkdown(&std::cout, citations);
return 0;
}

Expand Down