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

.github/workflows/CI-unixish.yml

@@ -53,7 +53,7 @@ jobs:
 
     - name: integration test
       run: |
-        python3 -m pytest integration_test.py
+        python3 -m pytest integration_test.py -vv
 
     - name: Run CMake
       run: |

.github/workflows/CI-windows.yml

@@ -57,5 +57,5 @@ jobs:
       - name: integration test
         run: |
           set SIMPLECPP_EXE_PATH=.\${{ matrix.config }}\simplecpp.exe
-          python -m pytest integration_test.py || exit /b !errorlevel!
+          python -m pytest integration_test.py -vv || exit /b !errorlevel!
 

integration_test.py

@@ -35,40 +35,48 @@ 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 == ''
     else:
         assert double_include_error in stderr
 
+@pytest.mark.parametrize("with_pragma_once", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
 @pytest.mark.parametrize("source_relative", (False, True))
-def test_relative_header_2(tmpdir, inv, source_relative):
-    header_file, _ = __test_relative_header_create_header(tmpdir)
+def test_relative_header_2(record_property, tmpdir, with_pragma_once, inv, source_relative):
+    header_file, 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", header_file, inv=inv)
 
     args = ["test.c" if source_relative else test_file]
 
     _, stdout, stderr = simplecpp(args, cwd=tmpdir)
-    assert stderr == ''
-    if source_relative and not inv:
-        assert '#line 8 "test.h"' in stdout
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
+    if with_pragma_once:
+        assert stderr == ''
+        if inv:
+            assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+        else:
+            assert '#line 8 "test.h"' in stdout
     else:
-        assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+        assert double_include_error in stderr
 
 @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,20 +86,23 @@ 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
     else:
         assert stderr == ''
-        if source_relative and not inv:
-            assert '#line 8 "test_subdir/test.h"' in stdout
-        else:
+        if inv:
             assert f'#line 8 "{pathlib.PurePath(test_subdir).as_posix()}/test.h"' in stdout
+        else:
+            assert '#line 8 "test_subdir/test.h"' in stdout
 
 @pytest.mark.parametrize("use_short_path", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (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, relative_include_dir, is_sys, inv):
     test_subdir = os.path.join(tmpdir, "test_subdir")
     os.mkdir(test_subdir)
     header_file, _ = __test_relative_header_create_header(test_subdir)
@@ -100,7 +111,69 @@ def test_relative_header_4(tmpdir, use_short_path, is_sys, inv):
 
     test_file = __test_relative_header_create_source(tmpdir, header_file, "test.h", is_include2_sys=is_sys, inv=inv)
 
-    args = [format_include_path_arg(test_subdir), test_file]
+    args = [format_include_path_arg("test_subdir" if relative_include_dir else 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 == ''
+    if (use_short_path and not inv) or (relative_include_dir and inv):
+        assert '#line 8 "test_subdir/test.h"' in stdout
+    else:
+        assert f'#line 8 "{pathlib.PurePath(test_subdir).as_posix()}/test.h"' in stdout
+
+@pytest.mark.parametrize("with_pragma_once", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (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, relative_include_dir, 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(".." if relative_include_dir else 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 (relative_include_dir or not is_sys) and 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
+
+@pytest.mark.parametrize("with_pragma_once", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (False, True))
+@pytest.mark.parametrize("is_sys", (False, True))
+@pytest.mark.parametrize("inv", (False, True))
+def test_relative_header_6(record_property, tmpdir, with_pragma_once, relative_include_dir, is_sys, inv): # test relative paths with .. that is resolved only by an include dir
+    ## in this test, both the header and the source file are at the same dir, but there is a dummy inclusion dir as a subdir
+    header_file, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
+
+    test_subdir = os.path.join(tmpdir, "test_subdir")
+    os.mkdir(test_subdir)
+    test_file = __test_relative_header_create_source(tmpdir, header_file, "../test.h", is_include2_sys=is_sys, inv=inv)
+
+    args = [format_include_path_arg("test_subdir" if relative_include_dir else test_subdir), "test.c"]
+
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
+    if with_pragma_once:
+        assert stderr == ''
+        if relative_include_dir and 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

simplecpp.cpp

@@ -2719,14 +2719,36 @@ 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);
+static std::string dirPath(const std::string& path, bool withTrailingSlash=true) {
+    const std::size_t lastSlash = path.find_last_of("\\/");
+    if (lastSlash == std::string::npos) {
+        return "";
     }
-    // otherwise
-    return std::make_pair("", false);
+    return path.substr(0, lastSlash + (withTrailingSlash ? 1U : 0U));
+}
+
+static std::string omitPathTrailingSlash(const std::string& path) {
+    if (endsWith(path, "/")) {
+        return path.substr(0, path.size() - 1U);
+    }
+    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(currentPrefix, false);
+    }
+    const std::size_t size = currentPrefix.size();
+    std::string relativeFromMeetingPath = normalizedAbsolutePath.substr(size, normalizedAbsolutePath.size() - size);
+    if (startsWith_(relativeFromMeetingPath, "/")) {
+        // 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);
@@ -3147,12 +3169,17 @@ static std::string openHeader(std::ifstream &f, const std::string &path)
 
 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 baseFileSimplified = simplecpp::simplifyPath(baseFile);
+    const std::string baseFileAbsolute = isAbsolutePath(baseFileSimplified) ?
+        baseFileSimplified :
+        simplecpp::simplifyPath(currentDirectory() + "/" + baseFileSimplified);
+
+    const std::string headerSimplified = simplecpp::simplifyPath(header);
+    const std::string path = isAbsolutePath(headerSimplified) ?
+        headerSimplified :
+        simplecpp::simplifyPath(dirPath(baseFileAbsolute) + headerSimplified);
+
+    return extractRelativePathFromAbsolute(path);
 }
 
 static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header)
@@ -3174,8 +3201,9 @@ 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 = simplecpp::simplifyPath(basePath + simplifiedHeader);
+    // preserve absoluteness/relativieness of the including dir
+    return isAbsolutePath(includePath) ? absoluteSimplifiedHeaderPath : extractRelativePathFromAbsolute(absoluteSimplifiedHeaderPath);
 }
 
 static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
@@ -3210,22 +3238,18 @@ 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;
     }
-    // otherwise
     return "";
 }
 
@@ -3267,6 +3291,16 @@ static bool hasFile(const std::map<std::string, simplecpp::TokenList *> &filedat
     return !getFileIdPath(filedata, sourcefile, header, dui, systemheader).empty();
 }
 
+static void safeInsertTokenListToMap(std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &header2, simplecpp::TokenList *tokens, const std::string &header, const std::string &sourcefile, bool systemheader, const char* contextDesc)
+{
+    const bool inserted = filedata.insert(std::make_pair(header2, tokens)).second;
+    if (!inserted) {
+        std::cerr << "error in " << contextDesc << " - attempt to add a tokenized file to the file map, but this file is already in the map! Details:" <<
+            "header: " << header << " header2: " << header2 << " source: " << sourcefile << " systemheader: " << systemheader << std::endl;
+        std::abort();
+    }
+}
+
 std::map<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::TokenList &rawtokens, std::vector<std::string> &filenames, const simplecpp::DUI &dui, simplecpp::OutputList *outputList)
 {
 #ifdef SIMPLECPP_WINDOWS
@@ -3343,7 +3377,7 @@ std::map<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::To
         TokenList *tokens = new TokenList(header2, filenames, outputList);
         if (dui.removeComments)
             tokens->removeComments();
-        ret[header2] = tokens;
+        safeInsertTokenListToMap(ret, header2, tokens, header, rawtok->location.file(), systemheader, "simplecpp::load");
         if (tokens->front())
             filelist.push_back(tokens->front());
     }
@@ -3630,7 +3664,7 @@ void simplecpp::preprocess(simplecpp::TokenList &output, const simplecpp::TokenL
                         TokenList * const tokens = new TokenList(header2, files, outputList);
                         if (dui.removeComments)
                             tokens->removeComments();
-                        filedata[header2] = tokens;
+                        safeInsertTokenListToMap(filedata, header2, tokens, header, rawtok->location.file(), systemheader, "simplecpp::preprocess");
                     }
                 }
                 if (header2.empty()) {