fixed #14658 - do not append extension in lookup if a relative path has been provided (#8432)

Author: firewave

lib/library.cpp

@@ -194,12 +194,13 @@ Library::Error Library::load(const char exename[], const char path[], bool debug
     }
 
     const bool is_abs_path = Path::isAbsolute(path);
+    const bool is_rel_path = Path::isRelative(path);
 
     std::string fullfilename(path);
 
     // TODO: what if the extension is not .cfg?
-    // only append extension when we provide the library name and not a path - TODO: handle relative paths?
-    if (!is_abs_path && Path::getFilenameExtension(fullfilename).empty())
+    // only append extension when we provide the library name and not a path
+    if (!is_abs_path && !is_rel_path && Path::getFilenameExtension(fullfilename).empty())
         fullfilename += ".cfg";
 
     std::string absolute_path;

lib/path.cpp

@@ -189,6 +189,12 @@ bool Path::isAbsolute(const std::string& path)
 #endif
 }
 
+bool Path::isRelative(const std::string& path)
+{
+    const std::string p = fromNativeSeparators(path);
+    return (p.find('/') != std::string::npos) && !isAbsolute(path);
+}
+
 std::string Path::getRelativePath(const std::string& absolutePath, const std::vector<std::string>& basePaths)
 {
     for (const std::string &bp : basePaths) {

lib/path.h

@@ -118,6 +118,13 @@ class CPPCHECKLIB Path {
      */
     static bool isAbsolute(const std::string& path);
 
+    /**
+     * @brief Check if given path is relative
+     * @param path Path to check
+     * @return true if given path is relative
+     */
+    static bool isRelative(const std::string& path);
+
     /**
      * @brief Create a relative path from an absolute one, if absolute path is inside the basePaths.
      * @param absolutePath Path to be made relative.

lib/platform.cpp

@@ -176,11 +176,12 @@ bool Platform::loadFromFile(const std::vector<std::string>& paths, const std::st
         std::cout << "looking for platform '" + filename + "'" << std::endl;
 
     const bool is_abs_path = Path::isAbsolute(filename);
+    const bool is_rel_path = Path::isRelative(filename);
 
     std::string fullfilename(filename);
     // TODO: what if extension is not .xml?
     // only append extension when we provide the library name is not a path - TODO: handle relative paths?
-    if (!is_abs_path && Path::getFilenameExtension(fullfilename).empty())
+    if (!is_abs_path && !is_rel_path && Path::getFilenameExtension(fullfilename).empty())
         fullfilename += ".xml";
 
     // TODO: use native separators

test/cli/lookup_test.py

@@ -205,14 +205,35 @@ def test_lib_lookup_relative_noext_notfound(tmpdir):
     assert exitcode == 1, stdout if stdout else stderr
     lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
     assert lines == [
-        "looking for library 'config/gnu.cfg'",
-        "looking for library '{}/config/gnu.cfg'".format(exepath),
-        "looking for library '{}/cfg/config/gnu.cfg'".format(exepath),
+        "looking for library 'config/gnu'",
+        "looking for library '{}/config/gnu'".format(exepath),
+        "looking for library '{}/cfg/config/gnu'".format(exepath),
         "library not found: 'config/gnu'",
         "cppcheck: Failed to load library configuration file 'config/gnu'. File not found"
     ]
 
 
+# TODO: this can never be found - bail out early?
+def test_lib_lookup_relative_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    exitcode, stdout, stderr, exe = cppcheck_ex(['--debug-lookup=library', '--library=config/gnu/', test_file])
+    exepath = os.path.dirname(exe)
+    if sys.platform == 'win32':
+        exepath = exepath.replace('\\', '/')
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
+    assert lines == [
+        "looking for library 'config/gnu/'",
+        "looking for library '{}/config/gnu/'".format(exepath),
+        "looking for library '{}/cfg/config/gnu/'".format(exepath),
+        "library not found: 'config/gnu/'",
+        "cppcheck: Failed to load library configuration file 'config/gnu/'. File not found"
+    ]
+
+
 def test_lib_lookup_absolute(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):
@@ -258,6 +279,48 @@ def test_lib_lookup_absolute_notfound(tmpdir):
     ]
 
 
+def test_lib_lookup_absolute_noext_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    cfg_file = os.path.join(tmpdir, 'test')
+
+    exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=library', '--library={}'.format(cfg_file), test_file])
+    exepath = os.path.dirname(exe)
+    if sys.platform == 'win32':
+        exepath = exepath.replace('\\', '/')
+    assert exitcode == 1, stdout
+    lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
+    assert lines == [
+        "looking for library '{}'".format(cfg_file),
+        "library not found: '{}'".format(cfg_file),
+        "cppcheck: Failed to load library configuration file '{}'. File not found".format(cfg_file)
+    ]
+
+
+# TODO: this can never be found - bail out early?
+def test_lib_lookup_absolute_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    cfg_file = os.path.join(tmpdir, 'test')
+    cfg_file_trail = cfg_file + os.path.sep
+
+    exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=library', '--library={}'.format(cfg_file_trail), test_file])
+    exepath = os.path.dirname(exe)
+    if sys.platform == 'win32':
+        exepath = exepath.replace('\\', '/')
+    assert exitcode == 1, stdout
+    lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
+    assert lines == [
+        "looking for library '{}'".format(cfg_file_trail),
+        "library not found: '{}'".format(cfg_file_trail),
+        "cppcheck: Failed to load library configuration file '{}'. File not found".format(cfg_file_trail)
+    ]
+
+
 def test_lib_lookup_nofile(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):
@@ -544,14 +607,38 @@ def test_platform_lookup_relative_noext_notfound(tmpdir):
     lines = stdout.splitlines()
     assert lines == [
         "looking for platform 'platform/none'",
-        "try to load platform file '{}/platform/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none.xml".format(cwd, cwd),
-        "try to load platform file '{}/platforms/platform/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none.xml".format(cwd, cwd),
-        "try to load platform file '{}/platform/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none.xml".format(exepath, exepath),
-        "try to load platform file '{}/platforms/platform/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none.xml".format(exepath, exepath),
+        "try to load platform file '{}/platform/none' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none".format(cwd, cwd),
+        "try to load platform file '{}/platforms/platform/none' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none".format(cwd, cwd),
+        "try to load platform file '{}/platform/none' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none".format(exepath, exepath),
+        "try to load platform file '{}/platforms/platform/none' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none".format(exepath, exepath),
         "cppcheck: error: unrecognized platform: 'platform/none'."
     ]
 
 
+# TODO: this can never be found - bail out early?
+def test_platform_lookup_relative_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    exitcode, stdout, stderr, exe = cppcheck_ex(['--debug-lookup=platform', '--platform=platform/none/', test_file])
+    cwd = os.getcwd()
+    exepath = os.path.dirname(exe)
+    if sys.platform == 'win32':
+        cwd = cwd.replace('\\', '/')
+        exepath = exepath.replace('\\', '/')
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = stdout.splitlines()
+    assert lines == [
+        "looking for platform 'platform/none/'",
+        "try to load platform file '{}/platform/none/' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none/".format(cwd, cwd),
+        "try to load platform file '{}/platforms/platform/none/' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none/".format(cwd, cwd),
+        "try to load platform file '{}/platform/none/' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platform/none/".format(exepath, exepath),
+        "try to load platform file '{}/platforms/platform/none/' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/platform/none/".format(exepath, exepath),
+        "cppcheck: error: unrecognized platform: 'platform/none/'."
+    ]
+
+
 def test_platform_lookup_absolute(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):
@@ -590,6 +677,42 @@ def test_platform_lookup_absolute_notfound(tmpdir):
     ]
 
 
+def test_platform_lookup_absolute_noext_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    platform_file = os.path.join(tmpdir, 'test')
+
+    exitcode, stdout, stderr = cppcheck(['--debug-lookup=platform', '--platform={}'.format(platform_file), test_file])
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = stdout.splitlines()
+    assert lines == [
+        "looking for platform '{}'".format(platform_file),
+        "try to load platform file '{}' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}".format(platform_file, platform_file),
+        "cppcheck: error: unrecognized platform: '{}'.".format(platform_file)
+    ]
+
+
+# TODO: this can never be found - bail out early?
+def test_platform_lookup_absolute_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    platform_file = os.path.join(tmpdir, 'test')
+    platform_file_trail = platform_file + os.path.sep
+
+    exitcode, stdout, stderr = cppcheck(['--debug-lookup=platform', '--platform={}'.format(platform_file_trail), test_file])
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = stdout.splitlines()
+    assert lines == [
+        "looking for platform '{}'".format(platform_file_trail),
+        "try to load platform file '{}' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}".format(platform_file_trail, platform_file_trail),
+        "cppcheck: error: unrecognized platform: '{}'.".format(platform_file_trail)
+    ]
+
+
 @pytest.mark.skip  # TODO: fails when not run from the root folder
 def test_platform_lookup_nofile(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
@@ -759,6 +882,7 @@ def test_addon_lookup_relative_notfound(tmpdir):
     ]
 
 
+# FIXME: an addon requires a file extension as we need to differentiate between .py and .json addons
 def test_addon_lookup_relative_noext_notfound(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):
@@ -777,6 +901,26 @@ def test_addon_lookup_relative_noext_notfound(tmpdir):
     ]
 
 
+# TODO: this can never be found - bail out early?
+def test_addon_lookup_relative_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=addon', '--addon=addon/misra/', test_file])
+    exepath = os.path.dirname(exe)
+    exepath_sep = exepath + os.path.sep
+    assert exitcode == 1, stdout
+    lines = stdout.splitlines()
+    assert lines == [
+        # TODO: should not append extension
+        "looking for addon 'addon/misra/.py'",
+        "looking for addon '{}addon/misra/.py'".format(exepath_sep),
+        "looking for addon '{}addons/addon/misra/.py'".format(exepath_sep),  # TODO: mixed separators
+        'Did not find addon addon/misra/.py'
+    ]
+
+
 def test_addon_lookup_absolute(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):
@@ -811,6 +955,43 @@ def test_addon_lookup_absolute_notfound(tmpdir):
     ]
 
 
+# FIXME: an addon requires a file extension as we need to differentiate between .py and .json addons
+def test_addon_lookup_absolute_noext_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    addon_file = os.path.join(tmpdir, 'test')
+    addon_file_py = os.path.join(tmpdir, 'test.py')  # TODO: do not add extension
+
+    exitcode, stdout, stderr = cppcheck(['--debug-lookup=addon', '--addon={}'.format(addon_file), test_file])
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = stdout.splitlines()
+    assert lines == [
+        "looking for addon '{}'".format(addon_file_py),
+        'Did not find addon {}'.format(addon_file_py)
+    ]
+
+
+# TODO: this can never be found - bail out early?
+def test_addon_lookup_absolute_noext_trailing_notfound(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    addon_file = os.path.join(tmpdir, 'test')
+    addon_file_trail = addon_file + os.path.sep
+    addon_file_trail_py = addon_file_trail + '.py'  # TODO: do not add extension
+
+    exitcode, stdout, stderr = cppcheck(['--debug-lookup=addon', '--addon={}'.format(addon_file_trail), test_file])
+    assert exitcode == 1, stdout if stdout else stderr
+    lines = stdout.splitlines()
+    assert lines == [
+        "looking for addon '{}'".format(addon_file_trail_py),
+        'Did not find addon {}'.format(addon_file_trail_py)
+    ]
+
+
 def test_addon_lookup_nofile(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt'):

test/testpath.cpp

@@ -57,6 +57,7 @@ class TestPath : public TestFixture {
         TEST_CASE(getAbsolutePath);
         TEST_CASE(exists);
         TEST_CASE(fromNativeSeparators);
+        TEST_CASE(isRelative);
     }
 
     void removeQuotationMarks() const {
@@ -618,6 +619,33 @@ class TestPath : public TestFixture {
         ASSERT_EQUALS("//lib/file.c", Path::fromNativeSeparators("\\\\lib\\file.c"));
         ASSERT_EQUALS("./lib/file.c", Path::fromNativeSeparators(".\\lib\\file.c"));
     }
+
+    void isRelative() const {
+        ASSERT_EQUALS(true, Path::isRelative("dir/file"));
+        ASSERT_EQUALS(true, Path::isRelative("dir\\file"));
+
+        // TODO: is this expected?
+        ASSERT_EQUALS(true, Path::isRelative("file/"));
+        ASSERT_EQUALS(true, Path::isRelative("file\\"));
+
+        ASSERT_EQUALS(false, Path::isRelative("file"));
+
+#ifdef _WIN32
+        // this is a relative path on Windows
+        ASSERT_EQUALS(true, Path::isRelative("/dir/file"));
+#else
+        ASSERT_EQUALS(false, Path::isRelative("/dir/file"));
+#endif
+
+#ifdef _WIN32
+        // TODO: this is not detected as absolute path in _WIN32 builds
+        ASSERT_EQUALS(false, Path::isRelative("c:\\dir\\file"));
+        ASSERT_EQUALS(false, Path::isRelative("c:/dir/file"));
+#else
+        ASSERT_EQUALS(true, Path::isRelative("c:\\dir\\file"));
+        ASSERT_EQUALS(true, Path::isRelative("c:/dir/file"));
+#endif
+    }
 };
 
 REGISTER_TEST(TestPath)