cleaned up handling of specifying multiple libraries at once (#6593)

Author: firewave

cli/cmdlineparser.cpp

@@ -870,7 +870,14 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
 
             // --library
             else if (std::strncmp(argv[i], "--library=", 10) == 0) {
-                mSettings.libraries.emplace_back(argv[i] + 10);
+                std::list<std::string> libs = splitString(argv[i] + 10, ',');
+                for (auto& l : libs) {
+                    if (l.empty()) {
+                        mLogger.printError("empty library specified.");
+                        return Result::Fail;
+                    }
+                    mSettings.libraries.emplace_back(std::move(l));
+                }
             }
 
             // Set maximum number of #ifdef configurations to check

lib/library.cpp

@@ -184,23 +184,8 @@ static void gettokenlistfromvalid(const std::string& valid, bool cpp, TokenList&
 
 Library::Error Library::load(const char exename[], const char path[], bool debug)
 {
-    // TODO: remove handling of multiple libraries at once?
     if (std::strchr(path,',') != nullptr) {
-        if (debug)
-            std::cout << "handling multiple libraries '" + std::string(path) + "'" << std::endl;
-        std::string p(path);
-        for (;;) {
-            const std::string::size_type pos = p.find(',');
-            if (pos == std::string::npos)
-                break;
-            const Error &e = load(exename, p.substr(0,pos).c_str());
-            if (e.errorcode != ErrorCode::OK)
-                return e;
-            p = p.substr(pos+1);
-        }
-        if (!p.empty())
-            return load(exename, p.c_str());
-        return Error();
+        throw std::runtime_error("handling of multiple libraries not supported");
     }
 
     const bool is_abs_path = Path::isAbsolute(path);

lib/utils.cpp

@@ -20,6 +20,7 @@
 
 #include <algorithm>
 #include <cctype>
+#include <cstring>
 #include <iterator>
 #include <stack>
 #include <utility>
@@ -184,3 +185,21 @@ std::string replaceEscapeSequences(const std::string &source) {
     return result;
 }
 
+
+std::list<std::string> splitString(const std::string& str, char sep)
+{
+    if (std::strchr(str.c_str(), sep) == nullptr)
+        return {str};
+
+    std::list<std::string> l;
+    std::string p(str);
+    for (;;) {
+        const std::string::size_type pos = p.find(sep);
+        if (pos == std::string::npos)
+            break;
+        l.push_back(p.substr(0,pos));
+        p = p.substr(pos+1);
+    }
+    l.push_back(std::move(p));
+    return l;
+}

lib/utils.h

@@ -30,6 +30,7 @@
 #include <functional>
 #include <initializer_list>
 #include <limits>
+#include <list>
 #include <stdexcept>
 #include <string>
 #include <type_traits>
@@ -390,4 +391,12 @@ static inline T* empty_if_null(T* p)
     return p ? p : "";
 }
 
+/**
+ * Split string by given sperator.
+ * @param str The string to split
+ * @param sep The seperator
+ * @return The list of seperate strings (including empty ones). The whole input string if no seperator found.
+ */
+CPPCHECKLIB std::list<std::string> splitString(const std::string& str, char sep);
+
 #endif

test/cli/other_test.py

@@ -1728,3 +1728,27 @@ def test_lib_lookup_nofile(tmpdir):
         "looking for library '{}/cfg/gtk.cfg'".format(exepath),
         'Checking {} ...'.format(test_file)
     ]
+
+
+def test_lib_lookup_multi(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    exitcode, stdout, _, exe = cppcheck_ex(['--library=posix,gnu', '--debug-lookup', test_file])
+    exepath = os.path.dirname(exe)
+    if sys.platform == 'win32':
+        exepath = exepath.replace('\\', '/')
+    assert exitcode == 0, stdout
+    lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
+    assert lines == [
+        "looking for library 'posix'",
+        "looking for library 'posix.cfg'",
+        "looking for library '{}/posix.cfg'".format(exepath),
+        "looking for library '{}/cfg/posix.cfg'".format(exepath),
+        "looking for library 'gnu'",
+        "looking for library 'gnu.cfg'",
+        "looking for library '{}/gnu.cfg'".format(exepath),
+        "looking for library '{}/cfg/gnu.cfg'".format(exepath),
+        'Checking {} ...'.format(test_file)
+    ]

test/testcmdlineparser.cpp

@@ -364,6 +364,9 @@ class TestCmdlineParser : public TestFixture {
         TEST_CASE(signedCharUnsignedChar);
         TEST_CASE(library);
         TEST_CASE(libraryMissing);
+        TEST_CASE(libraryMultiple);
+        TEST_CASE(libraryMultipleEmpty);
+        TEST_CASE(libraryMultipleEmpty2);
         TEST_CASE(suppressXml);
         TEST_CASE(suppressXmlEmpty);
         TEST_CASE(suppressXmlMissing);
@@ -2447,6 +2450,30 @@ class TestCmdlineParser : public TestFixture {
         ASSERT_EQUALS("cppcheck: Failed to load library configuration file 'posix2'. File not found\n", logger->str());
     }
 
+    void libraryMultiple() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--library=posix,gnu", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS(2, settings->libraries.size());
+        auto it = settings->libraries.cbegin();
+        ASSERT_EQUALS("posix", *it++);
+        ASSERT_EQUALS("gnu", *it);
+    }
+
+    void libraryMultipleEmpty() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--library=posix,,gnu", "file.cpp"};
+        ASSERT_EQUALS(false, parser->fillSettingsFromArgs(3, argv));
+        ASSERT_EQUALS("cppcheck: error: empty library specified.\n", logger->str());
+    }
+
+    void libraryMultipleEmpty2() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--library=posix,gnu,", "file.cpp"};
+        ASSERT_EQUALS(false, parser->fillSettingsFromArgs(3, argv));
+        ASSERT_EQUALS("cppcheck: error: empty library specified.\n", logger->str());
+    }
+
     void suppressXml() {
         REDIRECT;
         ScopedFile file("suppress.xml",

test/testutils.cpp

@@ -41,6 +41,7 @@ class TestUtils : public TestFixture {
         TEST_CASE(trim);
         TEST_CASE(findAndReplace);
         TEST_CASE(replaceEscapeSequences);
+        TEST_CASE(splitString);
     }
 
     void isValidGlobPattern() const {
@@ -438,6 +439,50 @@ class TestUtils : public TestFixture {
         ASSERT_EQUALS("\\", ::replaceEscapeSequences("\\\\"));
         ASSERT_EQUALS("\"", ::replaceEscapeSequences("\\\""));
     }
+
+    void splitString() const {
+        {
+            const auto l = ::splitString("test", ',');
+            ASSERT_EQUALS(1, l.size());
+            ASSERT_EQUALS("test", *l.cbegin());
+        }
+        {
+            const auto l = ::splitString("test,test", ';');
+            ASSERT_EQUALS(1, l.size());
+            ASSERT_EQUALS("test,test", *l.cbegin());
+        }
+        {
+            const auto l = ::splitString("test,test", ',');
+            ASSERT_EQUALS(2, l.size());
+            auto it = l.cbegin();
+            ASSERT_EQUALS("test", *it++);
+            ASSERT_EQUALS("test", *it);
+        }
+        {
+            const auto l = ::splitString("test,test,", ',');
+            ASSERT_EQUALS(3, l.size());
+            auto it = l.cbegin();
+            ASSERT_EQUALS("test", *it++);
+            ASSERT_EQUALS("test", *it++);
+            ASSERT_EQUALS("", *it);
+        }
+        {
+            const auto l = ::splitString("test,,test", ',');
+            ASSERT_EQUALS(3, l.size());
+            auto it = l.cbegin();
+            ASSERT_EQUALS("test", *it++);
+            ASSERT_EQUALS("", *it++);
+            ASSERT_EQUALS("test", *it);
+        }
+        {
+            const auto l = ::splitString(",test,test", ',');
+            ASSERT_EQUALS(3, l.size());
+            auto it = l.cbegin();
+            ASSERT_EQUALS("", *it++);
+            ASSERT_EQUALS("test", *it++);
+            ASSERT_EQUALS("test", *it);
+        }
+    }
 };
 
 REGISTER_TEST(TestUtils)