Add forbid-implicit-declartions lint rule

Signed-off-by: Lukasz Dalek <[email protected]>

verilog/analysis/checkers/BUILD

@@ -28,6 +28,7 @@ cc_library(
         ":explicit_task_lifetime_rule",
         ":forbid_consecutive_null_statements_rule",
         ":forbid_defparam_rule",
+        ":forbid_implicit_declarations_rule",
         ":forbidden_anonymous_enums_rule",
         ":forbidden_anonymous_structs_unions_rule",
         ":forbidden_macro_rule",
@@ -307,6 +308,44 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "forbid_implicit_declarations_rule",
+    srcs = ["forbid_implicit_declarations_rule.cc"],
+    hdrs = ["forbid_implicit_declarations_rule.h"],
+    deps = [
+        "//common/analysis:citation",
+        "//common/analysis:lint_rule_status",
+        "//common/analysis:syntax_tree_lint_rule",
+        "//common/analysis/matcher",
+        "//common/analysis/matcher:bound_symbol_manager",
+        "//common/analysis/matcher:core_matchers",
+        "//common/analysis/matcher:matcher_builders",
+        "//common/text:symbol",
+        "//common/text:syntax_tree_context",
+        "//verilog/CST:identifier",
+        "//verilog/CST:verilog_matchers",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint_rule_registry",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "forbid_implicit_declarations_rule_test",
+    srcs = ["forbid_implicit_declarations_rule_test.cc"],
+    deps = [
+        ":forbid_implicit_declarations_rule",
+        "//common/analysis:linter_test_utils",
+        "//common/analysis:syntax_tree_linter_test_utils",
+        "//common/text:symbol",
+        "//verilog/CST:verilog_nonterminals",
+        "//verilog/CST:verilog_treebuilder_utils",
+        "//verilog/analysis:verilog_analyzer",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "mismatched_labels_rule",
     srcs = ["mismatched_labels_rule.cc"],

verilog/analysis/checkers/forbid_implicit_declarations_rule.cc

@@ -0,0 +1,160 @@
+// Copyright 2017-2020 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/forbid_implicit_declarations_rule.h"
+
+#include <set>
+#include <string>
+
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+#include "common/analysis/citation.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/matcher/bound_symbol_manager.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/analysis/descriptions.h"
+#include "verilog/analysis/lint_rule_registry.h"
+#include "verilog/CST/identifier.h"
+
+namespace verilog {
+namespace analysis {
+
+using verible::GetStyleGuideCitation;
+using verible::LintRuleStatus;
+using verible::LintViolation;
+using verible::SyntaxTreeContext;
+
+// Register ForbidImplicitDeclarationsRule
+VERILOG_REGISTER_LINT_RULE(ForbidImplicitDeclarationsRule);
+
+// forbid-implicit-net-declarations?
+absl::string_view ForbidImplicitDeclarationsRule::Name() {
+  return "forbid-implicit-declarations";
+}
+const char ForbidImplicitDeclarationsRule::kTopic[] = "implicit-declarations";
+const char ForbidImplicitDeclarationsRule::kMessage[] =
+    "Nets must be declared explicitly.";
+
+std::string ForbidImplicitDeclarationsRule::GetDescription(DescriptionType description_type) {
+  return absl::StrCat("Checks that there are no occurrences of "
+                      "implicitly declared nets.");
+}
+
+const absl::string_view ForbidImplicitDeclarationsRule::GetIdentifier(
+    const verible::matcher::BoundSymbolManager& manager,
+    const std::string& id) const {
+  const auto* identifier_symbol = manager.FindSymbol(id);
+  const auto* identifier_leaf = AutoUnwrapIdentifier(*ABSL_DIE_IF_NULL(identifier_symbol));
+  return ABSL_DIE_IF_NULL(identifier_leaf)->get().text;
+}
+
+// checks that we are in correct scope
+void ForbidImplicitDeclarationsRule::CheckAndPopScope(const verible::Symbol& symbol,
+                                                const SyntaxTreeContext& context) {
+  while ((scopes.size() > 0) &&
+         (!ContainsAncestor(scopes.top().symbol_, context))) {
+    scopes.pop();
+  }
+}
+
+void ForbidImplicitDeclarationsRule::DetectScope(const verible::Symbol& symbol,
+                                                 const SyntaxTreeContext& context) {
+  verible::matcher::BoundSymbolManager manager;
+
+  if (net_scope_matcher_.Matches(symbol, &manager)) {
+    CheckAndPopScope(symbol, context);
+    scopes.push(ScopeType(&symbol));
+  }
+}
+
+void ForbidImplicitDeclarationsRule::DetectDeclarations(const verible::Symbol& symbol,
+                                                        const SyntaxTreeContext& context) {
+  verible::matcher::BoundSymbolManager manager;
+  // If matched variable in scope
+  if (net_declaration_matcher_.Matches(symbol, &manager)) {
+    // Top of the matched tree stack
+    CHECK_GE(context.size(), 1);
+    const auto& top = context.top(); // scope
+    CHECK_EQ(static_cast<verilog::NodeEnum>(top.Tag().tag),
+             verilog::NodeEnum::kModuleItemList);
+
+    // This could be omitted if we had matchers for begining and ending of scopes
+    // e.g. something like kBegin/kEnd. But we would have to modify parser grammar for
+    // some of them, e.g. "module". Leaving like this for now.
+    CheckAndPopScope(symbol, context);
+
+    const auto* decls = ABSL_DIE_IF_NULL(manager.GetAs<verible::SyntaxTreeNode>("decls"));
+    for (const auto& itr : decls->children()) {
+      const auto& kind = itr.get()->Kind();
+      if (kind != verible::SymbolKind::kNode) continue;
+      const auto& tag = static_cast<verilog::NodeEnum>(itr.get()->Tag().tag);
+      if ((tag != verilog::NodeEnum::kNetVariable) &&
+          (tag != verilog::NodeEnum::kNetDeclarationAssignment)) continue;
+      const auto& leaf = ABSL_DIE_IF_NULL(GetLeftmostLeaf(*itr))->get();
+      CHECK_EQ(leaf.token_enum, SymbolIdentifier);
+      const auto& identifier = leaf.text;
+
+      // TODO: check parent scope for net names overlap?
+      CHECK_GE(scopes.size(), 1);
+      auto& scope = scopes.top().declared_nets_;
+      scope[identifier] = &top;
+    }
+  }
+}
+
+void ForbidImplicitDeclarationsRule::DetectReference(const verible::Symbol& symbol,
+                                                     const SyntaxTreeContext& context) {
+  verible::matcher::BoundSymbolManager manager;
+
+  if (net_assignment_matcher_.Matches(symbol, &manager)) {
+    const auto* lval = ABSL_DIE_IF_NULL(manager.GetAs<verible::SyntaxTreeLeaf>("lval"));
+    const verible::TokenInfo& lval_token = lval->get();
+    CHECK_EQ(lval_token.token_enum, SymbolIdentifier);
+    const auto& identifier = lval_token.text;
+
+    CheckAndPopScope(symbol, context);
+
+    CHECK_GE(scopes.size(), 1);
+    auto& scope = scopes.top().declared_nets_;
+    const auto* node = scope[identifier];
+
+    // If there's no decleration nor exists common ancestor then it must be
+    // implicit declaration.
+    // TODO: Check necessity of call to ContainsAncestor(). CheckAndPopScope()
+    // is propably enough.
+    if ((node == nullptr) || (!ContainsAncestor(node, context))) {
+      violations_.insert(LintViolation(*lval, kMessage, context));
+    }
+  }
+}
+
+void ForbidImplicitDeclarationsRule::HandleSymbol(const verible::Symbol& symbol,
+                                                  const SyntaxTreeContext& context) {
+  // Detects and pushes on stack new scopes
+  DetectScope(symbol, context);
+
+  // Detects nets declarations
+  DetectDeclarations(symbol, context);
+
+  // Detect referenced nets and checks for declarations
+  DetectReference(symbol, context);
+}
+
+LintRuleStatus ForbidImplicitDeclarationsRule::Report() const {
+  return LintRuleStatus(violations_, Name(), GetStyleGuideCitation(kTopic));
+}
+
+}  // namespace analysis
+}  // namespace verilog

verilog/analysis/checkers/forbid_implicit_declarations_rule.h

@@ -0,0 +1,123 @@
+// Copyright 2017-2020 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_FORBID_IMPLICIT_DECLARATIONS_RULE_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_FORBID_IMPLICIT_DECLARATIONS_RULE_H_
+
+#include <set>
+#include <string>
+#include <stack>
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/matcher/core_matchers.h"
+#include "common/analysis/matcher/matcher.h"
+#include "common/analysis/matcher/matcher_builders.h"
+#include "common/analysis/syntax_tree_lint_rule.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/CST/verilog_matchers.h"  // IWYU pragma: keep
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+// ForbidImplicitDeclarationsRule detect implicitly declared nets
+class ForbidImplicitDeclarationsRule : public verible::SyntaxTreeLintRule {
+ public:
+  using rule_type = verible::SyntaxTreeLintRule;
+  static absl::string_view Name();
+
+  // Returns the description of the rule implemented formatted for either the
+  // helper flag or markdown depending on the parameter type.
+  static std::string GetDescription(DescriptionType);
+
+  void HandleSymbol(const verible::Symbol& symbol,
+                    const verible::SyntaxTreeContext& context) override;
+
+  verible::LintRuleStatus Report() const override;
+
+ private:
+  // Link to style guide rule.
+  static const char kTopic[];
+
+  // Diagnostic message.
+  static const char kMessage[];
+
+  using Matcher = verible::matcher::Matcher;
+
+  // Checks that top of the stack is our ancestor. If not
+  // pops until find one
+  void CheckAndPopScope(const verible::Symbol& symbol,
+                  const verible::SyntaxTreeContext& context);
+
+  // Pushes new scopes on stack
+  void DetectScope(const verible::Symbol& symbol,
+                   const verible::SyntaxTreeContext& context);
+
+  // Detects nets declarations adds them to map
+  void DetectDeclarations(const verible::Symbol& symbol,
+                          const verible::SyntaxTreeContext& context);
+
+  void DetectReference(const verible::Symbol& symbol,
+                       const verible::SyntaxTreeContext& context);
+
+  bool ContainsAncestor(const verible::Symbol* const symbol,
+                        const verible::SyntaxTreeContext& context) const {
+    CHECK_NOTNULL(symbol);
+
+    // check for common ancestors
+    for (const auto& iter : context) {
+      if (iter == symbol) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  const absl::string_view GetIdentifier(
+      const verible::matcher::BoundSymbolManager& manager,
+      const std::string& id) const;
+
+  // Matches scopes with nets
+  // TODO: description, other scopes
+  const Matcher net_scope_matcher_ = verible::matcher::AnyOf(
+      NodekModuleItemList());
+
+  // Matches nets declarations
+  const Matcher net_declaration_matcher_ =
+      NodekNetDeclaration(PathkNetVariableDeclarationAssign().Bind("decls"));
+
+  // Detects nets assignments
+  const Matcher net_assignment_matcher_ = NodekContinuousAssignmentStatement(
+      PathkAssignmentList(PathkNetVariableAssignment(
+          LValueOfAssignment(
+              PathkReference(UnqualifiedReferenceHasId().Bind("lval"))))));
+
+  std::set<verible::LintViolation> violations_;
+
+  using DeclType = std::map<absl::string_view, const verible::Symbol*>;
+
+  struct ScopeType {
+    ScopeType(const verible::Symbol* symbol) : symbol_(symbol) {};
+    const verible::Symbol* const symbol_;
+    DeclType declared_nets_;
+  };
+
+  std::stack<ScopeType> scopes;
+};
+
+}  // namespace analysis
+}  // namespace verilog
+
+#endif  // VERIBLE_VERILOG_ANALYSIS_CHECKERS_FORBID_IMPLICIT_DECLARATIONS_RULE_H_

verilog/analysis/checkers/forbid_implicit_declarations_rule_test.cc

@@ -0,0 +1,54 @@
+// Copyright 2017-2020 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/forbid_implicit_declarations_rule.h"
+
+#include <initializer_list>
+
+#include "gtest/gtest.h"
+#include "common/analysis/linter_test_utils.h"
+#include "common/analysis/syntax_tree_linter_test_utils.h"
+#include "common/text/symbol.h"
+#include "verilog/CST/verilog_nonterminals.h"
+#include "verilog/CST/verilog_treebuilder_utils.h"
+#include "verilog/analysis/verilog_analyzer.h"
+
+namespace verilog {
+namespace analysis {
+namespace {
+
+using verible::LintTestCase;
+using verible::RunLintTestCases;
+
+TEST(ForbidImplicitDeclarationsRule, FunctionFailures) {
+  auto kToken = SymbolIdentifier;
+  const std::initializer_list<LintTestCase> ForbidImplicitDeclarationsTestCases = {
+      {""},
+      {"module m;\nendmodule\n"},
+      {"module m;\nassign ", {kToken, "a1"}, " = 1'b0;\nendmodule"},
+      {"module m;\nwire a1; assign a1 = 1'b0;\nendmodule"},
+      {"module m;\nwire a1;\nmodule foo;\nassign ", {kToken, "a1"}, " = 1'b0;\nendmodule;\nendmodule"},
+      {"module m;\nwire a1;\nmodule foo;\nendmodule;\nassign a1 = 1'b0;\nendmodule"},
+      {"module m;\nwire a1;\nbegin\nend\nassign a1 = 1'b0;\nendmodule"},
+      {"module m;\nwire a1;\nbegin\nassign ", {kToken, "a1"}, " = 1'b0;\nend\nassign a1 = 1'b0;\nendmodule"},
+      {"module m;\nwire a1;\nbegin\nwire a1; assign a1 = 1'b0;\nend\nassign a1 = 1'b0;\nendmodule"},
+  };
+
+  RunLintTestCases<VerilogAnalyzer, ForbidImplicitDeclarationsRule>(
+      ForbidImplicitDeclarationsTestCases);
+}
+
+}  // namespace
+}  // namespace analysis
+}  // namespace verilog