Merge pull request #2062 from kbrunham-intel/feat/add_dryrun_option
Added --verify option to verilog-formatter.
diff --git a/common/analysis/linter_test_utils.h b/common/analysis/linter_test_utils.h
index 7c7a0b0..88b401f 100644
--- a/common/analysis/linter_test_utils.h
+++ b/common/analysis/linter_test_utils.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.
@@ -143,7 +143,7 @@
template <class AnalyzerType, class RuleClass>
void RunApplyFixCases(std::initializer_list<AutoFixInOut> tests,
- absl::string_view configuration) {
+ absl::string_view configuration = "") {
using rule_type = typename RuleClass::rule_type;
auto rule_generator = [&configuration]() -> std::unique_ptr<rule_type> {
std::unique_ptr<rule_type> instance(new RuleClass());
diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD
index 1b06db6..f61323d 100644
--- a/verilog/analysis/checkers/BUILD
+++ b/verilog/analysis/checkers/BUILD
@@ -62,6 +62,7 @@
":signal-name-style-rule",
":struct-union-name-style-rule",
":suggest-parentheses-rule",
+ ":suspicious-semicolon-rule",
":token-stream-lint-rule",
":truncated-numeric-literal-rule",
":undersized-binary-literal-rule",
@@ -2123,3 +2124,28 @@
"@com_google_googletest//:gtest_main",
],
)
+
+cc_library(
+ name = "suspicious-semicolon-rule",
+ srcs = ["suspicious_semicolon_rule.cc"],
+ hdrs = ["suspicious_semicolon_rule.h"],
+ deps = [
+ "//common/analysis:lint-rule-status",
+ "//common/analysis:syntax-tree-lint-rule",
+ "//verilog/CST:verilog-matchers",
+ "//verilog/analysis:descriptions",
+ "//verilog/analysis:lint-rule-registry",
+ ],
+ alwayslink = 1,
+)
+
+cc_test(
+ name = "suspicious-semicolon-rule_test",
+ srcs = ["suspicious_semicolon_rule_test.cc"],
+ deps = [
+ ":suspicious-semicolon-rule",
+ "//common/analysis:syntax-tree-linter-test-utils",
+ "//verilog/analysis:verilog-analyzer",
+ "@com_google_googletest//:gtest_main",
+ ],
+)
diff --git a/verilog/analysis/checkers/suspicious_semicolon_rule.cc b/verilog/analysis/checkers/suspicious_semicolon_rule.cc
new file mode 100644
index 0000000..043c21a
--- /dev/null
+++ b/verilog/analysis/checkers/suspicious_semicolon_rule.cc
@@ -0,0 +1,81 @@
+// 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.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/suspicious_semicolon_rule.h"
+
+#include "common/analysis/matcher/matcher.h"
+#include "verilog/CST/verilog_matchers.h"
+#include "verilog/CST/verilog_nonterminals.h"
+#include "verilog/analysis/lint_rule_registry.h"
+
+namespace verilog {
+namespace analysis {
+
+using verible::matcher::Matcher;
+
+VERILOG_REGISTER_LINT_RULE(SuspiciousSemicolon);
+
+static constexpr absl::string_view kMessage =
+ "Potentially unintended semicolon";
+
+const LintRuleDescriptor &SuspiciousSemicolon::GetDescriptor() {
+ static const LintRuleDescriptor d{
+ .name = "suspicious-semicolon",
+ .topic = "bugprone",
+ .desc =
+ "Checks that there are no suspicious semicolons that might affect "
+ "code behaviour but escape quick visual inspection"};
+ return d;
+}
+
+static const Matcher &NullStatementMatcher() {
+ static const Matcher matcher(NodekNullStatement());
+ return matcher;
+}
+
+void SuspiciousSemicolon::HandleNode(
+ const verible::SyntaxTreeNode &node,
+ const verible::SyntaxTreeContext &context) {
+ verible::matcher::BoundSymbolManager manager;
+ if (!NullStatementMatcher().Matches(node, &manager)) return;
+
+ // Waive @(posedge clk);
+ // But catch always_ff @(posedge clk);
+ const bool parent_is_proc_timing_ctrl_statement =
+ context.DirectParentIs(NodeEnum::kProceduralTimingControlStatement);
+ if (!context.IsInside(NodeEnum::kAlwaysStatement) &&
+ parent_is_proc_timing_ctrl_statement) {
+ return;
+ }
+
+ if (!parent_is_proc_timing_ctrl_statement &&
+ !context.DirectParentIsOneOf(
+ {NodeEnum::kForeachLoopStatement, NodeEnum::kWhileLoopStatement,
+ NodeEnum::kForLoopStatement, NodeEnum::kForeverLoopStatement,
+ NodeEnum::kIfBody, NodeEnum::kElseBody})) {
+ return;
+ }
+
+ violations_.insert(verible::LintViolation(
+ node, kMessage, context,
+ {verible::AutoFix("Remove ';'",
+ {verible::StringSpanOfSymbol(node), ""})}));
+}
+
+verible::LintRuleStatus SuspiciousSemicolon::Report() const {
+ return verible::LintRuleStatus(violations_, GetDescriptor());
+}
+
+} // namespace analysis
+} // namespace verilog
diff --git a/verilog/analysis/checkers/suspicious_semicolon_rule.h b/verilog/analysis/checkers/suspicious_semicolon_rule.h
new file mode 100644
index 0000000..a03a407
--- /dev/null
+++ b/verilog/analysis/checkers/suspicious_semicolon_rule.h
@@ -0,0 +1,62 @@
+// 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.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_SUSPICIOUS_SEMICOLON_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_SUSPICIOUS_SEMICOLON_H_
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/syntax_tree_lint_rule.h"
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+/*
+ * Detect suspicious semicolons. Inspired by clang-tidy's
+ * bugprone-suspicious-semicolon check.
+ *
+ * This rule detects extra semicolons that modify code behaviour
+ * while having a good chance to escape quick visual inspection.
+ *
+ * A couple of examples:
+ *
+ * if (condition);
+ * `uvm_fatal(...);
+ *
+ * while (condition); begin
+ * doSomething();
+ * end
+ *
+ * Reference:
+ * https://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-semicolon.html#bugprone-suspicious-semicolon
+ */
+class SuspiciousSemicolon : public verible::SyntaxTreeLintRule {
+ public:
+ using rule_type = verible::SyntaxTreeLintRule;
+
+ static const LintRuleDescriptor &GetDescriptor();
+
+ void HandleNode(const verible::SyntaxTreeNode &node,
+ const verible::SyntaxTreeContext &context) final;
+
+ verible::LintRuleStatus Report() const final;
+
+ private:
+ std::set<verible::LintViolation> violations_;
+};
+
+} // namespace analysis
+} // namespace verilog
+
+#endif // VERIBLE_VERILOG_ANALYSIS_CHECKERS_SUSPICIOUS_SEMICOLON_H_
diff --git a/verilog/analysis/checkers/suspicious_semicolon_rule_test.cc b/verilog/analysis/checkers/suspicious_semicolon_rule_test.cc
new file mode 100644
index 0000000..3e422f5
--- /dev/null
+++ b/verilog/analysis/checkers/suspicious_semicolon_rule_test.cc
@@ -0,0 +1,83 @@
+// 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.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/suspicious_semicolon_rule.h"
+
+#include "common/analysis/syntax_tree_linter_test_utils.h"
+#include "verilog/analysis/verilog_analyzer.h"
+
+namespace verilog {
+namespace analysis {
+namespace {
+
+static constexpr int kToken = ';';
+TEST(SuspiciousSemicolon, DetectSuspiciousSemicolons) {
+ const std::initializer_list<verible::LintTestCase>
+ kSuspiciousSemicolonTestCases = {
+ {"module m; initial begin if(x)", {kToken, ";"}, " end endmodule"},
+ {"module m; initial begin if(x) x; else",
+ {kToken, ";"},
+ " y; end endmodule"},
+ {"module m; initial begin while(x)", {kToken, ";"}, " end endmodule"},
+ {"module m; initial begin forever", {kToken, ";"}, " end endmodule"},
+ {"module m; always_ff @(posedge clk)", {kToken, ";"}, " endmodule"},
+ {"module m; initial begin for(;;)", {kToken, ";"}, " end endmodule"},
+ {"module m; initial begin foreach (array[i])",
+ {kToken, ";"},
+ " end endmodule"},
+ };
+
+ verible::RunLintTestCases<VerilogAnalyzer, SuspiciousSemicolon>(
+ kSuspiciousSemicolonTestCases);
+}
+
+TEST(SuspiciousSemicolon, ShouldNotComplain) {
+ const std::initializer_list<verible::LintTestCase>
+ kSuspiciousSemicolonTestCases = {
+ {""},
+ {"module m; initial begin if(x) begin end end endmodule"},
+ {"module m; @(posedge clk); endmodule"},
+ {"module m; always_ff @(posedge clk) begin ; end endmodule"},
+ {"module m; endmodule;"},
+ {"class c; int x;; endclass"},
+ };
+
+ verible::RunLintTestCases<VerilogAnalyzer, SuspiciousSemicolon>(
+ kSuspiciousSemicolonTestCases);
+}
+
+TEST(SuspiciousSemicolon, ApplyAutoFix) {
+ const std::initializer_list<verible::AutoFixInOut>
+ kSuspiciousSemicolonTestCases = {
+ {"module m; initial begin if(x); end endmodule",
+ "module m; initial begin if(x) end endmodule"},
+ {"module m; initial begin if(x) x; else; y; end endmodule",
+ "module m; initial begin if(x) x; else y; end endmodule"},
+ {"module m; initial begin while(x); end endmodule",
+ "module m; initial begin while(x) end endmodule"},
+ {"module m; initial begin forever; end endmodule",
+ "module m; initial begin forever end endmodule"},
+ {"module m; always_ff @(posedge clk); endmodule",
+ "module m; always_ff @(posedge clk) endmodule"},
+ {"module m; initial begin foreach (array[i]); end endmodule",
+ "module m; initial begin foreach (array[i]) end endmodule"},
+ };
+
+ verible::RunApplyFixCases<VerilogAnalyzer, SuspiciousSemicolon>(
+ kSuspiciousSemicolonTestCases);
+}
+
+} // namespace
+} // namespace analysis
+} // namespace verilog
diff --git a/verilog/tools/kythe/scope_resolver.h b/verilog/tools/kythe/scope_resolver.h
index 4571ca3..5deba61 100644
--- a/verilog/tools/kythe/scope_resolver.h
+++ b/verilog/tools/kythe/scope_resolver.h
@@ -140,8 +140,8 @@
absl::flat_hash_map<std::string, absl::flat_hash_set<ScopedVname>>
variable_to_scoped_vname_;
- // Mapping from scope to all its members.
- absl::flat_hash_map<SignatureDigest, absl::flat_hash_set<VName>>
+ // Mapping from scope to all its members. NOTE: requires pointer stability!
+ absl::node_hash_map<SignatureDigest, absl::flat_hash_set<VName>>
scope_to_vnames_;
// Maps the scope to the human readable description. Available only when debug
diff --git a/verilog/tools/lint/BUILD b/verilog/tools/lint/BUILD
index 13a4a24..04fd143 100644
--- a/verilog/tools/lint/BUILD
+++ b/verilog/tools/lint/BUILD
@@ -69,6 +69,7 @@
("signal-name-style", "signal_name_style", False),
("struct-union-name-style", "struct_name_style", True),
("struct-union-name-style", "union_name_style", True),
+ ("suspicious-semicolon", "suspicious_semicolon", False),
("v2001-generate-begin", "generate_begin_module", True),
("void-cast", "void-cast", True),
("undersized-binary-literal", "undersized_binary_literal", True),
diff --git a/verilog/tools/lint/testdata/forbid_consecutive_null_statements.sv b/verilog/tools/lint/testdata/forbid_consecutive_null_statements.sv
index 273d124..492a415 100644
--- a/verilog/tools/lint/testdata/forbid_consecutive_null_statements.sv
+++ b/verilog/tools/lint/testdata/forbid_consecutive_null_statements.sv
@@ -1,4 +1,3 @@
module forbid_consecutive_null_statements;
- always_ff @(posedge foo)
;; // [Style: consecutive-null-statements] [forbid-consecutive-null-statements]
endmodule
diff --git a/verilog/tools/lint/testdata/suspicious_semicolon.sv b/verilog/tools/lint/testdata/suspicious_semicolon.sv
new file mode 100644
index 0000000..ab49fb6
--- /dev/null
+++ b/verilog/tools/lint/testdata/suspicious_semicolon.sv
@@ -0,0 +1,6 @@
+module suspicious_semicolon ();
+ initial begin
+ if (x);
+ $display("Hi");
+ end
+endmodule
diff --git a/verilog/tools/ls/README.md b/verilog/tools/ls/README.md
index bd626d4..373b6da 100644
--- a/verilog/tools/ls/README.md
+++ b/verilog/tools/ls/README.md
@@ -116,7 +116,7 @@
Here is a simple setup: put this in your `~/.emacs` file
and make sure the binary is in your `$PATH` (or use full path).
-```lisp
+```elisp
(require 'lsp-mode)
(add-to-list 'lsp-language-id-configuration '(verilog-mode . "verilog"))
(lsp-register-client
@@ -127,6 +127,15 @@
(add-hook 'verilog-mode-hook 'lsp)
```
+It is also possible to automatically configure `eglot` and `lsp-mode` using
+the [verilog-ext](https://github.com/gmlarumbe/verilog-ext.git) package:
+```elisp
+(require 'verilog-ext)
+(verilog-ext-mode-setup)
+(verilog-ext-eglot-set-server 've-verible-ls) ;`eglot' config
+(verilog-ext-lsp-set-server 've-verible-ls) ; `lsp' config
+```
+
### Kakoune
First, go to [kak-lsp](https://github.com/kak-lsp/kak-lsp) project and follow the installation and configuration steps.
