Support multiple slang-tidy port suffixes for EnforcePortSuffix check (…

…#787)

tools/tidy/README.md

@@ -23,8 +23,9 @@ checks_config_section ::= "CheckConfigs:" [EOL] config+
 config                ::= ((config_tuple ",")* | config_tuple) END
 config_tuple          ::= config_name ":" config_value
 config_name           ::= identifier
-config_value          ::= identifier
-identifier            ::= {A-Za-z}+
+config_value          ::= expression | "[" ((expression ",")* expression)? "]"
+identifier            ::= { A-Z | a-z }+
+expression            ::= { A-Z | a-z | 0-9 | "_" }+
 EOL                   ::= '\n' | '\r' | '\r\n'
 END                   ::= EOL* | EOF
 ```

tools/tidy/include/TidyConfig.h

@@ -25,9 +25,9 @@ class TidyConfig {
         std::string clkName;
         std::string resetName;
         bool resetIsActiveHigh;
-        std::string inputPortSuffix;
-        std::string outputPortSuffix;
-        std::string inoutPortSuffix;
+        std::vector<std::string> inputPortSuffix;
+        std::vector<std::string> outputPortSuffix;
+        std::vector<std::string> inoutPortSuffix;
         std::string moduleInstantiationPrefix;
     };
 
@@ -40,6 +40,9 @@ class TidyConfig {
     /// Returns the check config object
     inline const CheckConfigs& getCheckConfigs() const { return checkConfigs; }
 
+    /// Returns the check config object
+    inline CheckConfigs& getCheckConfigs() { return checkConfigs; }
+
     // Returns a vector containing the file names that won't be checked by slang-tidy
     inline const std::vector<std::string>& getSkipFiles() const { return skipFiles; }
 
@@ -69,45 +72,34 @@ class TidyConfig {
     [[nodiscard]] bool toggleCheck(slang::TidyKind kind, const std::string& checkName,
                                    CheckStatus status);
 
-    /// Sets the value of a check config. Will throw an invalid_argument exception if the type of
-    /// value doesn't match the config type
-    template<typename T>
-    void setConfig(const std::string& configName, T value) {
+    /// Visits the value of a check config. Will throw an invalid_argument exception
+    /// if the configName is unknown
+    template<typename Visitor>
+    void visitConfig(std::string const& configName, Visitor visit) {
         if (configName == "clkName") {
-            innerSetConfig(checkConfigs.clkName, value);
+            visit(checkConfigs.clkName);
             return;
         }
         else if (configName == "resetName") {
-            innerSetConfig(checkConfigs.resetName, value);
+            visit(checkConfigs.resetName);
             return;
         }
         else if (configName == "resetIsActiveHigh") {
-            innerSetConfig(checkConfigs.resetIsActiveHigh, value);
+            visit(checkConfigs.resetIsActiveHigh);
             return;
         }
         else if (configName == "inputPortSuffix") {
-            innerSetConfig(checkConfigs.inputPortSuffix, value);
+            visit(checkConfigs.inputPortSuffix);
             return;
         }
         else if (configName == "outputPortSuffix") {
-            innerSetConfig(checkConfigs.outputPortSuffix, value);
+            visit(checkConfigs.outputPortSuffix);
             return;
         }
         else if (configName == "inoutPortSuffix") {
-            innerSetConfig(checkConfigs.inoutPortSuffix, value);
+            visit(checkConfigs.inoutPortSuffix);
             return;
         }
-
         SLANG_THROW(std::invalid_argument(fmt::format("The check: {} does not exist", configName)));
     }
-
-    template<typename T, typename U>
-    void innerSetConfig(T& config, U value) {
-        if constexpr (std::is_same_v<T, U>)
-            config = value;
-        else
-            SLANG_THROW(
-                std::invalid_argument(fmt::format("check config expected a {} found {}",
-                                                  slang::typeName<T>(), slang::typeName<U>())));
-    }
 };

tools/tidy/include/TidyConfigParser.h

@@ -51,9 +51,21 @@ class TidyConfigParser {
     /// Gets the next character from the file stream, skips whitespaces and tabs
     char nextChar();
 
-    /// Peeks the next character from the file stream
+    /// Peeks the next character from the file stream, skips whitespaces and tabs
     char peekChar();
 
+    /// Reads characters from the file stream and adds them to str
+    /// as long as pred for the extracted character is true.
+    /// Returns the first character for which pred is false.
+    template<typename Func, std::enable_if_t<std::is_invocable_r_v<bool, Func, char>, bool> = true>
+    char readIf(std::string& str, Func pred) {
+        char c;
+        while (pred(c = nextChar())) {
+            str.push_back(c);
+        }
+        return c;
+    }
+
     /// Parses config file and sets the state to parse checks or check configs
     void parseInitial();
 
@@ -63,6 +75,24 @@ class TidyConfigParser {
     /// Parses the check configs regions of the config file
     void parseCheckConfigs();
 
+    /// Parses multiple configValues by individually
+    /// parsing every single config values of type T
+    template<typename T>
+    void parseConfigValue(std::vector<T>& config, std::vector<std::string>&& configValues) {
+        config.clear();
+        for (auto& value : configValues) {
+            T tmp;
+            parseConfigValue(tmp, std::move(value));
+            config.emplace_back(std::move(tmp));
+        }
+    }
+
+    /// Parses configValue as a single string
+    void parseConfigValue(std::string& config, std::string&& configValue);
+
+    /// Parses configValue as a bool.
+    void parseConfigValue(bool& config, std::string&& configValue);
+
     /// Toggles all checks with the status provided
     void toggleAllChecks(TidyConfig::CheckStatus status);
 
@@ -74,7 +104,7 @@ class TidyConfigParser {
                      TidyConfig::CheckStatus status);
 
     /// Sets the check config with the provided value
-    void setCheckConfig(const std::string& configName, std::string configValue);
+    void setCheckConfig(const std::string& configName, std::vector<std::string> configValue);
 
     /// The name format of the checks provided by the user are required to be: this-is-my-check
     /// but the registered names in the TidyFactory are ThisIsMyCheck. This function translates from

tools/tidy/src/TidyConfig.cpp

@@ -11,9 +11,9 @@ TidyConfig::TidyConfig() {
     checkConfigs.clkName = "clk_i";
     checkConfigs.resetName = "rst_ni";
     checkConfigs.resetIsActiveHigh = true;
-    checkConfigs.inputPortSuffix = "_i";
-    checkConfigs.outputPortSuffix = "_o";
-    checkConfigs.inoutPortSuffix = "_io";
+    checkConfigs.inputPortSuffix = {"_i"};
+    checkConfigs.outputPortSuffix = {"_o"};
+    checkConfigs.inoutPortSuffix = {"_io"};
     checkConfigs.moduleInstantiationPrefix = "i_";
 
     auto styleChecks = std::unordered_map<std::string, CheckStatus>();

tools/tidy/src/TidyConfigParser.cpp

@@ -8,6 +8,15 @@
 
 #include "slang/util/OS.h"
 
+template<typename T>
+struct is_vector : std::false_type {};
+
+template<typename T>
+struct is_vector<std::vector<T>> : std::true_type {};
+
+template<typename T>
+inline constexpr bool is_vector_v = is_vector<T>::value;
+
 TidyConfigParser::TidyConfigParser(const std::filesystem::path& path) {
     parserState = ParserState::Initial;
     filePath = path.string();
@@ -56,6 +65,11 @@ char TidyConfigParser::nextChar() {
 }
 
 inline char TidyConfigParser::peekChar() {
+    while (fileStream.peek() == ' ' || fileStream.peek() == '\t') {
+        fileStream.get();
+    }
+    if (fileStream.eof())
+        return 0;
 #if defined(_WIN32)
     char ret = fileStream.peek();
     if (ret == '\r') {
@@ -93,7 +107,7 @@ void TidyConfigParser::parseFile() {
 }
 
 void TidyConfigParser::parseInitial() {
-    char currentChar = nextChar();
+    char currentChar = peekChar();
     if (currentChar == 0)
         reportErrorAndExit("Unexpected end of file");
 
@@ -102,10 +116,7 @@ void TidyConfigParser::parseInitial() {
             fmt::format("Unexpected token with ascii_code ({}): {}", +currentChar, currentChar));
 
     std::string str;
-    while (!fileStream.eof() && isalpha(currentChar)) {
-        str.push_back(currentChar);
-        currentChar = nextChar();
-    }
+    currentChar = readIf(str, isalpha);
 
     if (currentChar != ':')
         reportErrorAndExit(fmt::format("Expected token: ':', found: (ASCIICode: {}){}",
@@ -138,16 +149,14 @@ void TidyConfigParser::parseChecks() {
         char currentChar = nextChar();
 
         // If it is a new line ignore it and get the following char
-        if (currentChar == '\n')
+        while (currentChar == '\n')
             currentChar = nextChar();
 
         if (currentChar == '-')
             newCheckState = TidyConfig::CheckStatus::DISABLED;
         else if (isalpha(currentChar))
             checkGroup.push_back(currentChar);
         else if (currentChar == '*') {
-            if (checkGroup.size())
-                reportErrorAndExit("Unexpected '*'");
             toggleAllChecks(TidyConfig::CheckStatus::ENABLED);
             currentChar = nextChar();
             if (currentChar == '\n' || currentChar == 0) {
@@ -258,49 +267,63 @@ void TidyConfigParser::parseChecks() {
 
 void TidyConfigParser::parseCheckConfigs() {
     while (!fileStream.eof()) {
-        std::string optionName;
-        std::string optionValue;
-
         // Parse optional newline
         if (peekChar() == '\n')
             nextChar();
 
         // Parse option name
-        while (true) {
-            char currentChar = nextChar();
-            if (currentChar == ':')
-                break;
-            else if (isalpha(currentChar))
-                optionName.push_back(currentChar);
-            else
-                reportErrorAndExit(fmt::format("Expected ':' or '_' or a letter but found ({}){}",
-                                               +currentChar, currentChar));
+        std::string optionName;
+        char currentChar = readIf(optionName, isalpha);
+
+        if (currentChar != ':') {
+            reportErrorAndExit(fmt::format("Expected ':' or a letter but found ({}){}",
+                                           +currentChar, currentChar));
         }
 
-        // Parse option value
-        while (true) {
-            char currentChar = nextChar();
-            if (currentChar == ',') {
-                setCheckConfig(optionName, optionValue);
-                if (nextChar() != '\n') {
-                    reportErrorAndExit(fmt::format("Expected new line but found: ({}){}",
-                                                   +currentChar, currentChar));
-                }
-                break;
-            }
-            else if (isalpha(currentChar) || currentChar == '_') {
-                optionValue.push_back(currentChar);
+        // Parse multiple option values
+        std::vector<std::string> optionValues;
+
+        auto isOptionValueChar = [](char c) { return isalnum(c) || c == '_'; };
+
+        if (peekChar() == '[') {
+            currentChar = nextChar(); // skip '['
+
+            do {
+                std::string optionValue;
+                currentChar = readIf(optionValue, isOptionValueChar);
+                if (!optionValue.empty())
+                    optionValues.emplace_back(optionValue);
+            } while (currentChar == ',');
+
+            if (currentChar != ']') {
+                reportErrorAndExit(
+                    fmt::format("Expected ']' but found ({}){}", +currentChar, currentChar));
             }
-            else if (currentChar == '\n' || currentChar == 0) {
-                while (peekChar() == '\n')
-                    nextChar();
-                setCheckConfig(optionName, optionValue);
-                parserState = ParserState::Initial;
-                return;
+            currentChar = nextChar();
+        }
+        else { // Parse single option value
+            std::string optionValue;
+            currentChar = readIf(optionValue, isOptionValueChar);
+            optionValues.emplace_back(optionValue);
+        }
+
+        if (currentChar == ',') {
+            setCheckConfig(optionName, optionValues);
+            if (nextChar() != '\n') {
+                reportErrorAndExit(
+                    fmt::format("Expected new line but found: ({}){}", +currentChar, currentChar));
             }
-            else
-                reportErrorAndExit(fmt::format("Expected ',' new line or a letter but found ({}){}",
-                                               +currentChar, currentChar));
+        }
+        else if (currentChar == '\n' || currentChar == 0) {
+            while (peekChar() == '\n')
+                nextChar();
+            setCheckConfig(optionName, optionValues);
+            parserState = ParserState::Initial;
+            return;
+        }
+        else {
+            reportErrorAndExit(fmt::format("Expected ',' new line or a letter but found ({}){}",
+                                           +currentChar, currentChar));
         }
     }
 }
@@ -336,25 +359,44 @@ void TidyConfigParser::toggleCheck(const std::string& groupName, const std::stri
             fmt::format("Check name {} does not exist in group {}", checkName, groupName));
 }
 
-void TidyConfigParser::setCheckConfig(const std::string& configName, std::string configValue) {
-    auto set_config = [&](auto value) {
-        SLANG_TRY {
-            config.setConfig(configName, value);
-        }
-        SLANG_CATCH(std::invalid_argument & exception) {
-#if __cpp_exceptions
-            reportWarning(exception.what());
-#endif
-        }
-    };
+void TidyConfigParser::parseConfigValue(std::string& config, std::string&& configValue) {
+    config = std::move(configValue);
+}
 
+void TidyConfigParser::parseConfigValue(bool& config, std::string&& configValue) {
     std::transform(configValue.begin(), configValue.end(), configValue.begin(), ::tolower);
-
     if (configValue == "true" || configValue == "false") {
-        set_config(configValue == "true");
+        config = configValue == "true";
+    }
+    else if (configValue == "1" || configValue == "0") {
+        config = configValue == "1";
     }
     else {
-        set_config(configValue);
+        reportErrorAndExit(fmt::format("Expected boolean expression but got '{}'", configValue));
+    }
+}
+
+void TidyConfigParser::setCheckConfig(const std::string& configName,
+                                      std::vector<std::string> configValues) {
+    SLANG_TRY {
+        config.visitConfig(configName, [&](auto& v) {
+            if constexpr (is_vector_v<std::remove_cvref_t<decltype(v)>>) {
+                parseConfigValue(v, std::move(configValues));
+            }
+            else {
+                if (configValues.size() != 1) {
+                    reportErrorAndExit(
+                        fmt::format("Expected one configuration value for '{}' but got {}",
+                                    configName, configValues.size()));
+                }
+                parseConfigValue(v, std::move(configValues.front()));
+            }
+        });
+    }
+    SLANG_CATCH(std::invalid_argument & exception) {
+#if __cpp_exceptions
+        reportWarning(exception.what());
+#endif
     }
 }