Avoid internal symbol in tidy port suffix rule (#1089)

Do not look at the internal symbol, scrutinize the name of the port directly. This way we don't crash on explicit ports, as those have the internal symbol pointer set to null.
MikePopoloski · Aug 14, 2024 · d3e77f3 · d3e77f3
1 parent 8174a94
commit d3e77f3
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 5 deletions.
diff --git a/tools/tidy/src/style/EnforcePortSuffix.cpp b/tools/tidy/src/style/EnforcePortSuffix.cpp
@@ -22,9 +22,7 @@ struct MainVisitor : public TidyVisitor, ASTVisitor<MainVisitor, true, false> {
         if (port.isNullPort)
             return;
 
-        auto symbol = port.internalSymbol;
-
-        if (symbol->name == checkConfig.clkName || symbol->name == checkConfig.resetName)
+        if (port.name == checkConfig.clkName || port.name == checkConfig.resetName)
             return;
 
         std::vector<std::string> const* suffixes;
@@ -38,10 +36,10 @@ struct MainVisitor : public TidyVisitor, ASTVisitor<MainVisitor, true, false> {
 
         bool matched = suffixes->empty(); // no error is thrown without a suffix
         for (auto& suffix : *suffixes) {
-            matched |= symbol->name.ends_with(suffix);
+            matched |= port.name.ends_with(suffix);
         }
         if (!matched) {
-            auto& diag = diags.add(diag::EnforcePortSuffix, port.location) << symbol->name;
+            auto& diag = diags.add(diag::EnforcePortSuffix, port.location) << port.name;
             if (suffixes->size() == 1) {
                 diag << fmt::format("\"{}\"", suffixes->front());
             }

diff --git a/tools/tidy/tests/EnforcePortSuffixTest.cpp b/tools/tidy/tests/EnforcePortSuffixTest.cpp
@@ -195,3 +195,49 @@ endmodule
     bool result = visitor->check(root);
     CHECK(result);
 }
+
+TEST_CASE("EnforcePortSuffix: Explicit ports") {
+    auto tree = SyntaxTree::fromText(R"(
+module top (
+    input .data_i({a1, b1}),
+    output .data_o({a2, b2})
+);
+    logic a1, a1, a2, b2;
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
+    auto& root = compilation.getRoot();
+
+    TidyConfig config;
+    Registry::setConfig(config);
+    Registry::setSourceManager(compilation.getSourceManager());
+    auto visitor = Registry::create("EnforcePortSuffix");
+    bool result = visitor->check(root);
+    CHECK(result);
+}
+
+TEST_CASE("EnforcePortSuffix: Explicit port with incorrect suffix") {
+    auto tree = SyntaxTree::fromText(R"(
+module top (
+    input .data_i({a1, b1}),
+    input .data_o({a2, b2})
+);
+    logic a1, a1, a2, b2;
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
+    auto& root = compilation.getRoot();
+
+    TidyConfig config;
+    Registry::setConfig(config);
+    Registry::setSourceManager(compilation.getSourceManager());
+    auto visitor = Registry::create("EnforcePortSuffix");
+    bool result = visitor->check(root);
+    CHECK_FALSE(result);
+}