diff --git a/tools/tidy/CMakeLists.txt b/tools/tidy/CMakeLists.txt index b14993b61..aad57dbd2 100644 --- a/tools/tidy/CMakeLists.txt +++ b/tools/tidy/CMakeLists.txt @@ -18,7 +18,9 @@ add_library( src/style/EnforceModuleInstantiationPrefix.cpp src/style/OnlyANSIPortDecl.cpp src/synthesis/XilinxDoNotCareValues.cpp - src/synthesis/CastSignedIndex.cpp) + src/synthesis/CastSignedIndex.cpp + src/style/NoDotStarInPortConnection.cpp + src/style/NoImplicitPortNameInPortConnection.cpp) target_include_directories(slang_tidy_obj_lib PUBLIC include ../../include) target_link_libraries(slang_tidy_obj_lib PUBLIC slang::slang) diff --git a/tools/tidy/include/TidyDiags.h b/tools/tidy/include/TidyDiags.h index e69098588..4befa9b1c 100644 --- a/tools/tidy/include/TidyDiags.h +++ b/tools/tidy/include/TidyDiags.h @@ -22,5 +22,7 @@ inline constexpr DiagCode EnforceModuleInstantiationPrefix(DiagSubsystem::Tidy, inline constexpr DiagCode OnlyANSIPortDecl(DiagSubsystem::Tidy, 8); inline constexpr DiagCode XilinxDoNotCareValues(DiagSubsystem::Tidy, 9); inline constexpr DiagCode CastSignedIndex(DiagSubsystem::Tidy, 10); +inline constexpr DiagCode NoDotStarInPortConnection(DiagSubsystem::Tidy, 11); +inline constexpr DiagCode NoImplicitPortNameInPortConnection(DiagSubsystem::Tidy, 12); } // namespace slang::diag diff --git a/tools/tidy/src/TidyConfig.cpp b/tools/tidy/src/TidyConfig.cpp index d95d09fab..6653494da 100644 --- a/tools/tidy/src/TidyConfig.cpp +++ b/tools/tidy/src/TidyConfig.cpp @@ -24,6 +24,8 @@ TidyConfig::TidyConfig() { styleChecks.emplace("NoOldAlwaysSyntax", CheckStatus::ENABLED); styleChecks.emplace("EnforceModuleInstantiationPrefix", CheckStatus::ENABLED); styleChecks.emplace("OnlyANSIPortDecl", CheckStatus::ENABLED); + styleChecks.emplace("NoDotStarInPortConnection", CheckStatus::ENABLED); + styleChecks.emplace("NoImplicitPortNameInPortConnection", CheckStatus::ENABLED); checkKinds.insert({slang::TidyKind::Style, styleChecks}); auto synthesisChecks = std::unordered_map(); diff --git a/tools/tidy/src/style/NoDotStarInPortConnection.cpp b/tools/tidy/src/style/NoDotStarInPortConnection.cpp new file mode 100644 index 000000000..cbd3e090f --- /dev/null +++ b/tools/tidy/src/style/NoDotStarInPortConnection.cpp @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "ASTHelperVisitors.h" +#include "TidyDiags.h" + +#include "slang/syntax/AllSyntax.h" +#include "slang/syntax/SyntaxVisitor.h" + +using namespace slang; +using namespace slang::ast; +using namespace slang::syntax; + +namespace no_dot_start_in_port_connection { + +struct PortConnectionVisitor : public SyntaxVisitor { + void handle(const WildcardPortConnectionSyntax&) { found = true; } + +public: + bool found{false}; +}; + +struct MainVisitor : public TidyVisitor, ASTVisitor { + explicit MainVisitor(Diagnostics& diagnostics) : TidyVisitor(diagnostics) {} + + void handle(const InstanceBodySymbol& symbol) { + NEEDS_SKIP_SYMBOL(symbol) + PortConnectionVisitor visitor; + symbol.getSyntax()->visit(visitor); + if (visitor.found) + diags.add(diag::NoDotStarInPortConnection, symbol.location); + } +}; +} // namespace no_dot_start_in_port_connection + +using namespace no_dot_start_in_port_connection; + +class NoDotStarInPortConnection : public TidyCheck { +public: + [[maybe_unused]] explicit NoDotStarInPortConnection(TidyKind kind) : TidyCheck(kind) {} + + bool check(const RootSymbol& root) override { + MainVisitor visitor(diagnostics); + root.visit(visitor); + if (!diagnostics.empty()) + return false; + return true; + } + + DiagCode diagCode() const override { return diag::NoDotStarInPortConnection; } + + std::string diagString() const override { return "use of .* in port connection list"; } + + DiagnosticSeverity diagSeverity() const override { return DiagnosticSeverity::Warning; } + + std::string name() const override { return "NoDotStarInPortConnection"; } + + std::string description() const override { return shortDescription(); } + + std::string shortDescription() const override { + return "Checks if in a module instantiation any port is connected using .* syntax."; + } +}; + +REGISTER(NoDotStarInPortConnection, NoDotStarInPortConnection, TidyKind::Style) diff --git a/tools/tidy/src/style/NoImplicitPortNameInPortConnection.cpp b/tools/tidy/src/style/NoImplicitPortNameInPortConnection.cpp new file mode 100644 index 000000000..c587fc76e --- /dev/null +++ b/tools/tidy/src/style/NoImplicitPortNameInPortConnection.cpp @@ -0,0 +1,71 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "ASTHelperVisitors.h" +#include "TidyDiags.h" +#include + +#include "slang/syntax/AllSyntax.h" +#include "slang/syntax/SyntaxVisitor.h" + +using namespace slang; +using namespace slang::ast; +using namespace slang::syntax; + +namespace no_implicit_port_name_in_port_connection { + +struct PortConnectionVisitor : public SyntaxVisitor { + void handle(const PortConnectionSyntax& syntax) { + if (syntax.toString().find('(') == std::string::npos) + found = true; + } + +public: + bool found{false}; +}; + +struct MainVisitor : public TidyVisitor, ASTVisitor { + explicit MainVisitor(Diagnostics& diagnostics) : TidyVisitor(diagnostics) {} + + void handle(const InstanceBodySymbol& symbol) { + NEEDS_SKIP_SYMBOL(symbol) + PortConnectionVisitor visitor; + symbol.getSyntax()->visit(visitor); + if (visitor.found) + diags.add(diag::NoImplicitPortNameInPortConnection, symbol.location); + } +}; +} // namespace no_implicit_port_name_in_port_connection + +using namespace no_implicit_port_name_in_port_connection; + +class NoImplicitPortNameInPortConnection : public TidyCheck { +public: + [[maybe_unused]] explicit NoImplicitPortNameInPortConnection(TidyKind kind) : TidyCheck(kind) {} + + bool check(const RootSymbol& root) override { + MainVisitor visitor(diagnostics); + root.visit(visitor); + if (!diagnostics.empty()) + return false; + return true; + } + + DiagCode diagCode() const override { return diag::NoImplicitPortNameInPortConnection; } + + std::string diagString() const override { + return "port name not specified. Please use .port_name(net) syntax."; + } + + DiagnosticSeverity diagSeverity() const override { return DiagnosticSeverity::Warning; } + + std::string name() const override { return "NoImplicitPortNameInPortConnection"; } + + std::string description() const override { return shortDescription(); } + + std::string shortDescription() const override { + return "Checks if in a module instantiation any port is connected using .port_name syntax."; + } +}; + +REGISTER(NoImplicitPortNameInPortConnection, NoImplicitPortNameInPortConnection, TidyKind::Style) diff --git a/tools/tidy/tests/CMakeLists.txt b/tools/tidy/tests/CMakeLists.txt index 84daacafb..a1f88eaeb 100644 --- a/tools/tidy/tests/CMakeLists.txt +++ b/tools/tidy/tests/CMakeLists.txt @@ -18,7 +18,9 @@ add_executable( EnforceModuleInstantiationTest.cpp OnlyANSIPortDecl.cpp XilinxDoNotCareValuesTest.cpp - CastSignedIndexTest.cpp) + CastSignedIndexTest.cpp + NoDotStarInPortConnectionTest.cpp + NoImplicitPortNameInPortConnectionTest.cpp) target_link_libraries(tidy_unittests PRIVATE Catch2::Catch2 slang_tidy_obj_lib) target_compile_definitions(tidy_unittests PRIVATE UNITTESTS) diff --git a/tools/tidy/tests/NoDotStarInPortConnectionTest.cpp b/tools/tidy/tests/NoDotStarInPortConnectionTest.cpp new file mode 100644 index 000000000..3e58fcfea --- /dev/null +++ b/tools/tidy/tests/NoDotStarInPortConnectionTest.cpp @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "Test.h" +#include "TidyFactory.h" + +TEST_CASE("NoDotStarInPortConnection: Use of dot star in module port connection") { + auto tree = SyntaxTree::fromText(R"( +module test (input clk, input rst); +endmodule + +module top (); + logic clk, rst; + test t (.clk(clk), .*); +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("NoDotStarInPortConnection"); + bool result = visitor->check(root); + CHECK_FALSE(result); +} + +TEST_CASE("NoDotStarInPortConnection: Module port connection port by port") { + auto tree = SyntaxTree::fromText(R"( +module test (input clk, input rst); +endmodule + +module top (); + logic clk, rst; + test t (.clk, .rst(rst)); +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("NoDotStarInPortConnection"); + bool result = visitor->check(root); + CHECK(result); +} diff --git a/tools/tidy/tests/NoImplicitPortNameInPortConnectionTest.cpp b/tools/tidy/tests/NoImplicitPortNameInPortConnectionTest.cpp new file mode 100644 index 000000000..384bffa2d --- /dev/null +++ b/tools/tidy/tests/NoImplicitPortNameInPortConnectionTest.cpp @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "Test.h" +#include "TidyFactory.h" + +TEST_CASE("NoImplicitPortNameInPortConnection: Only port name specified in module port connection") { + auto tree = SyntaxTree::fromText(R"( +module test (input clk, input rst); +endmodule + +module top (); + logic clk, rst; + test t (.clk, .rst(rst)); +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("NoImplicitPortNameInPortConnection"); + bool result = visitor->check(root); + CHECK_FALSE(result); +} + +TEST_CASE("NoImplicitPortNameInPortConnection: Module port connection port by port") { + auto tree = SyntaxTree::fromText(R"( +module test (input cl()k, input rst); +endmodule + +module top (); + logic clk, rst; + test t (.clk(clk), .rst(rst)); +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("NoImplicitPortNameInPortConnection"); + bool result = visitor->check(root); + CHECK(result); +}