fix: parent relative paths, and rework on the whole path extraction mechanics

Tal500 · Tal Hadad · commit 903cf7fa76ca · 2025-05-21T13:06:47.000+03:00
Include also another integration test function to check header in parent dir usage (was failing before)
diff --git a/integration_test.py b/integration_test.py
@@ -35,14 +35,16 @@ def __test_relative_header_create_source(dir, include1, include2, is_include1_sy
 
 @pytest.mark.parametrize("with_pragma_once", (False, True))
 @pytest.mark.parametrize("is_sys", (False, True))
-def test_relative_header_1(tmpdir, with_pragma_once, is_sys):
+def test_relative_header_1(record_property, tmpdir, with_pragma_once, is_sys):
     _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
 
     test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=is_sys, is_include2_sys=is_sys)
 
     args = ([format_include_path_arg(tmpdir)] if is_sys else []) + [test_file]
 
-    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
 
     if with_pragma_once:
         assert stderr == ''
@@ -51,14 +53,16 @@ def test_relative_header_1(tmpdir, with_pragma_once, is_sys):
 
 @pytest.mark.parametrize("inv", (False, True))
 @pytest.mark.parametrize("source_relative", (False, True))
-def test_relative_header_2(tmpdir, inv, source_relative):
+def test_relative_header_2(record_property, tmpdir, inv, source_relative):
     header_file, _ = __test_relative_header_create_header(tmpdir)
 
     test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file, inv=inv)
 
     args = ["test.c" if source_relative else test_file]
 
     _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
     assert stderr == ''
     if source_relative and not inv:
         assert '#line 8 "test.h"' in stdout
@@ -68,7 +72,7 @@ def test_relative_header_2(tmpdir, inv, source_relative):
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
 @pytest.mark.parametrize("source_relative", (False, True))
-def test_relative_header_3(tmpdir, is_sys, inv, source_relative):
+def test_relative_header_3(record_property, tmpdir, is_sys, inv, source_relative):
     test_subdir = os.path.join(tmpdir, "test_subdir")
     os.mkdir(test_subdir)
     header_file, _ = __test_relative_header_create_header(test_subdir)
@@ -78,6 +82,8 @@ def test_relative_header_3(tmpdir, is_sys, inv, source_relative):
     args = ["test.c" if source_relative else test_file]
 
     _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
 
     if is_sys:
         assert "missing header: Header not found" in stderr
@@ -91,7 +97,7 @@ def test_relative_header_3(tmpdir, is_sys, inv, source_relative):
 @pytest.mark.parametrize("use_short_path", (False, True))
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
-def test_relative_header_4(tmpdir, use_short_path, is_sys, inv):
+def test_relative_header_4(record_property, tmpdir, use_short_path, is_sys, inv):
     test_subdir = os.path.join(tmpdir, "test_subdir")
     os.mkdir(test_subdir)
     header_file, _ = __test_relative_header_create_header(test_subdir)
@@ -102,5 +108,36 @@ def test_relative_header_4(tmpdir, use_short_path, is_sys, inv):
 
     args = [format_include_path_arg(test_subdir), test_file]
 
-    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
     assert stderr == ''
+
+@pytest.mark.parametrize("with_pragma_once", (False, True))
+@pytest.mark.parametrize("is_sys", (False, True))
+@pytest.mark.parametrize("inv", (False, True))
+def test_relative_header_5(record_property, tmpdir, with_pragma_once, is_sys, inv): # test relative paths with ..
+    ## in this test, the subdir role is the opposite then the previous - it contains the test.c file, while the parent tmpdir contains the header file
+    header_file, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
+    if is_sys:
+        header_file_second_path = "test.h"
+    else:
+        header_file_second_path = "../test.h"
+
+    test_subdir = os.path.join(tmpdir, "test_subdir")
+    os.mkdir(test_subdir)
+    test_file = __test_relative_header_create_source(test_subdir, header_file, header_file_second_path, is_include2_sys=is_sys, inv=inv)
+
+    args = ([format_include_path_arg(tmpdir)] if is_sys else []) + ["test.c"]
+
+    _, stdout, stderr = simplecpp(args, cwd=test_subdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
+    if with_pragma_once:
+        assert stderr == ''
+        if inv:
+            assert '#line 8 "../test.h"' in stdout
+        else:
+            assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+    else:
+        assert double_include_error in stderr
diff --git a/simplecpp.cpp b/simplecpp.cpp
@@ -2719,14 +2719,40 @@ static std::string toAbsolutePath(const std::string& path) {
     return simplecpp::simplifyPath(path);
 }
 
-static std::pair<std::string, bool> extractRelativePathFromAbsolute(const std::string& absolutepath) {
-    static const std::string prefix = currentDirectory() + "/";
-    if (startsWith_(absolutepath, prefix)) {
-        const std::size_t size = prefix.size();
-        return std::make_pair(absolutepath.substr(size, absolutepath.size() - size), true);
+// templated function for compiler optimization opportunities
+template <bool withTrailingSlash>
+static std::string dirPath(const std::string& path) {
+    const std::size_t firstSlash = path.find_last_of("\\/");
+    if (firstSlash == std::string::npos) {
+        return "";
+    } else {
+        return path.substr(0, firstSlash + (withTrailingSlash ? 1U : 0U));
     }
-    // otherwise
-    return std::make_pair("", false);
+}
+
+static std::string omitPathTrailingSlash(const std::string& path) {
+    if (endsWith(path, "/")) {
+        return path.substr(0, path.size() - 1U);
+    } else {
+        return path;
+    }
+}
+
+static std::string extractRelativePathFromAbsolute(const std::string& absoluteSimplifiedPath, const std::string& prefixSimplifiedAbsoluteDir = currentDirectory()) {
+    const std::string normalizedAbsolutePath = omitPathTrailingSlash(absoluteSimplifiedPath);
+    std::string currentPrefix = omitPathTrailingSlash(prefixSimplifiedAbsoluteDir);
+    std::string leadingParenting = "";
+    while (!startsWith_(normalizedAbsolutePath, currentPrefix)) {
+        leadingParenting = "../" + leadingParenting;
+        currentPrefix = dirPath<false>(currentPrefix);
+    }
+    const std::size_t size = currentPrefix.size();
+    std::string relativeFromMeetingPath = normalizedAbsolutePath.substr(size, normalizedAbsolutePath.size() - size);
+    if (startsWith_(relativeFromMeetingPath.c_str(), "/")) {
+        // omit the leading slash
+        relativeFromMeetingPath = relativeFromMeetingPath.substr(1, relativeFromMeetingPath.size());
+    }
+    return leadingParenting + relativeFromMeetingPath;
 }
 
 static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader);
@@ -3145,19 +3171,25 @@ static std::string openHeader(std::ifstream &f, const std::string &path)
     return "";
 }
 
+// templated function for compiler optimization opportunities
+template <bool expandBaseWithAbsolutePath, bool exactRelative> 
 static std::string getRelativeFileName(const std::string &baseFile, const std::string &header)
 {
-    std::string path;
-    if (baseFile.find_first_of("\\/") != std::string::npos)
-        path = baseFile.substr(0, baseFile.find_last_of("\\/") + 1U) + header;
-    else
-        path = header;
-    return simplecpp::simplifyPath(path);
+    const std::string baseFileNormalized = (expandBaseWithAbsolutePath && !isAbsolutePath(baseFile)) ? (currentDirectory() + "/" + baseFile) : baseFile;
+    const std::string path = isAbsolutePath(header) ? header : (dirPath<true>(baseFileNormalized) + header);
+    const std::string simplifiedPath = simplecpp::simplifyPath(path);
+    if (exactRelative) {
+        const std::string absolutePath = expandBaseWithAbsolutePath ? simplifiedPath : toAbsolutePath(simplifiedPath);
+        const std::string absoluteBaseFile = expandBaseWithAbsolutePath ? baseFileNormalized : toAbsolutePath(baseFileNormalized);
+        return extractRelativePathFromAbsolute(absolutePath, dirPath<true>(absoluteBaseFile));
+    } else {
+        return simplifiedPath;
+    }
 }
 
 static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header)
 {
-    return openHeader(f, getRelativeFileName(sourcefile, header));
+    return openHeader(f, getRelativeFileName<false, false>(sourcefile, header));
 }
 
 // returns the simplified header path:
@@ -3174,8 +3206,8 @@ static std::string getIncludePathFileName(const std::string &includePath, const
     std::string basePath = toAbsolutePath(includePath);
     if (!basePath.empty() && basePath[basePath.size()-1U]!='/' && basePath[basePath.size()-1U]!='\\')
         basePath += '/';
-    const std::string absolutesimplifiedHeaderPath = basePath + simplifiedHeader;
-    return extractRelativePathFromAbsolute(absolutesimplifiedHeaderPath).first;
+    const std::string absoluteSimplifiedHeaderPath = basePath + simplifiedHeader;
+    return extractRelativePathFromAbsolute(absoluteSimplifiedHeaderPath);
 }
 
 static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
@@ -3210,23 +3242,20 @@ static std::string findPathInMapBothRelativeAndAbsolute(const std::map<std::stri
     if (filedata.find(path) != filedata.end()) {// try first to respect the exact match
         return path;
     }
+
     // otherwise - try to use the normalize to the correct representation
+    std::string alternativePath;
     if (isAbsolutePath(path)) {
-        const std::pair<std::string, bool> relativeExtractedResult = extractRelativePathFromAbsolute(path);
-        if (relativeExtractedResult.second) {
-            const std::string relativePath = relativeExtractedResult.first;
-            if (filedata.find(relativePath) != filedata.end()) {
-                return relativePath;
-            }
-        }
+        alternativePath = extractRelativePathFromAbsolute(simplecpp::simplifyPath(path));
     } else {
-        const std::string absolutePath = toAbsolutePath(path);
-        if (filedata.find(absolutePath) != filedata.end()) {
-            return absolutePath;
-        }
+        alternativePath = toAbsolutePath(path);
+    }
+
+    if (filedata.find(alternativePath) != filedata.end()) {
+        return alternativePath;
+    } else {
+        return "";
     }
-    // otherwise
-    return "";
 }
 
 static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader)
@@ -3243,7 +3272,7 @@ static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenLis
     }
 
     if (!systemheader) {
-        const std::string relativeOrAbsoluteFilename = getRelativeFileName(sourcefile, header);// unknown if absolute or relative, but always simplified
+        const std::string relativeOrAbsoluteFilename = getRelativeFileName<true, true>(sourcefile, header);// unknown if absolute or relative, but always simplified
         const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, relativeOrAbsoluteFilename);
         if (!match.empty()) {
             return match;
