fix: use both absolute and relative header paths in header matching (#362)

Co-authored-by: Tal Hadad <[email protected]>

Author: Tal500

.github/workflows/CI-unixish.yml

@@ -30,7 +30,18 @@ jobs:
       run: |
         sudo apt-get update
         sudo apt-get install libc++-18-dev
-          
+
+    - name: Install missing software on macos
+      if: contains(matrix.os, 'macos')
+      run: |
+        brew install python3
+
+    - name: Install missing Python packages
+      run: |
+        python3 -m pip config set global.break-system-packages true
+        python3 -m pip install pip --upgrade
+        python3 -m pip install pytest
+
     - name: make simplecpp
       run: make -j$(nproc)
 
@@ -41,6 +52,10 @@ jobs:
       run: |
         make -j$(nproc) selfcheck
 
+    - name: integration test
+      run: |
+        python3 -m pytest integration_test.py
+
     - name: Run CMake
       run: |
         cmake -S . -B cmake.output

.github/workflows/CI-windows.yml

@@ -26,7 +26,18 @@ jobs:
 
       - name: Setup msbuild.exe
         uses: microsoft/setup-msbuild@v2
-        
+
+      - name: Set up Python 3.13
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.13'
+          check-latest: true
+
+      - name: Install missing Python packages
+        run: |
+          python -m pip install pip --upgrade || exit /b !errorlevel!
+          python -m pip install pytest || exit /b !errorlevel!
+
       - name: Run cmake
         if: matrix.os == 'windows-2019'
         run: |
@@ -48,4 +59,9 @@ jobs:
       - name: Selfcheck
         run: |
           .\${{ matrix.config }}\simplecpp.exe simplecpp.cpp -e || exit /b !errorlevel!
+
+      - name: integration test
+        run: |
+          set SIMPLECPP_EXE_PATH=.\${{ matrix.config }}\simplecpp.exe
+          python -m pytest integration_test.py || exit /b !errorlevel!
         

.gitignore

@@ -32,3 +32,6 @@ testrunner
 # CLion
 /.idea
 /cmake-build-*
+
+# python
+__pycache__/

integration_test.py

@@ -0,0 +1,94 @@
+## test with python -m pytest integration_test.py
+
+import os
+import pytest
+from testutils import simplecpp, format_include_path_arg, format_include
+
+def __test_relative_header_create_header(dir, with_pragma_once=True):
+    header_file = os.path.join(dir, 'test.h')
+    with open(header_file, 'wt') as f:
+        f.write(f"""
+                {"#pragma once" if with_pragma_once else ""}
+                #ifndef TEST_H_INCLUDED
+                #define TEST_H_INCLUDED
+                #else
+                #error header_was_already_included
+                #endif
+                """)
+    return header_file, "error: #error header_was_already_included"
+
+def __test_relative_header_create_source(dir, include1, include2, is_include1_sys=False, is_include2_sys=False, inv=False):
+    if inv:
+        return __test_relative_header_create_source(dir, include1=include2, include2=include1, is_include1_sys=is_include2_sys, is_include2_sys=is_include1_sys)
+    ## otherwise
+
+    src_file = os.path.join(dir, 'test.c')
+    with open(src_file, 'wt') as f:
+        f.write(f"""
+                #undef TEST_H_INCLUDED
+                #include {format_include(include1, is_include1_sys)}
+                #include {format_include(include2, is_include2_sys)}
+                """)
+    return src_file
+
+@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):
+    _, 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)
+
+    if with_pragma_once:
+        assert stderr == ''
+    else:
+        assert double_include_error in stderr
+
+@pytest.mark.parametrize("inv", (False, True))
+def test_relative_header_2(tmpdir, inv):
+    header_file, _ = __test_relative_header_create_header(tmpdir)
+
+    test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file, inv=inv)
+
+    args = [test_file]
+
+    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    assert stderr == ''
+
+@pytest.mark.parametrize("is_sys", (False, True))
+@pytest.mark.parametrize("inv", (False, True))
+def test_relative_header_3(tmpdir, is_sys, inv):
+    test_subdir = os.path.join(tmpdir, "test_subdir")
+    os.mkdir(test_subdir)
+    header_file, _ = __test_relative_header_create_header(test_subdir)
+
+    test_file = __test_relative_header_create_source(tmpdir, "test_subdir/test.h", header_file, is_include1_sys=is_sys, inv=inv)
+
+    args = [test_file]
+
+    _, _, stderr = simplecpp(args, cwd=tmpdir)
+
+    if is_sys:
+        assert "missing header: Header not found" in stderr
+    else:
+        assert stderr == ''
+
+@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):
+    test_subdir = os.path.join(tmpdir, "test_subdir")
+    os.mkdir(test_subdir)
+    header_file, _ = __test_relative_header_create_header(test_subdir)
+    if use_short_path:
+        header_file = "test_subdir/test.h"
+
+    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]
+
+    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    assert stderr == ''

simplecpp.cpp

@@ -43,6 +43,8 @@
 #ifdef SIMPLECPP_WINDOWS
 #include <windows.h>
 #undef ERROR
+#else
+#include <unistd.h>
 #endif
 
 #if __cplusplus >= 201103L
@@ -147,6 +149,11 @@ static unsigned long long stringToULL(const std::string &s)
     return ret;
 }
 
+static bool startsWith(const std::string &s, const std::string &p)
+{
+    return (s.size() >= p.size()) && std::equal(p.begin(), p.end(), s.begin());
+}
+
 static bool endsWith(const std::string &s, const std::string &e)
 {
     return (s.size() >= e.size()) && std::equal(e.rbegin(), e.rend(), s.rbegin());
@@ -2680,6 +2687,46 @@ static bool isCpp17OrLater(const simplecpp::DUI &dui)
     return !std_ver.empty() && (std_ver >= "201703L");
 }
 
+
+static std::string currentDirectoryOSCalc() {
+#ifdef SIMPLECPP_WINDOWS
+    TCHAR NPath[MAX_PATH];
+    GetCurrentDirectory(MAX_PATH, NPath);
+    return NPath;
+#else
+    const std::size_t size = 1024;
+    char the_path[size];
+    getcwd(the_path, size);
+    return the_path;
+#endif
+}
+
+static const std::string& currentDirectory() {
+    static const std::string curdir = simplecpp::simplifyPath(currentDirectoryOSCalc());
+    return curdir;
+}
+
+static std::string toAbsolutePath(const std::string& path) {
+    if (path.empty()) {
+        return path;// preserve error file path that is indicated by an empty string
+    }
+    if (!isAbsolutePath(path)) {
+        return currentDirectory() + "/" + path;
+    }
+    // otherwise
+    return 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);
+    }
+    // otherwise
+    return std::make_pair("", false);
+}
+
 static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader);
 static void simplifyHasInclude(simplecpp::TokenList &expr, const simplecpp::DUI &dui)
 {
@@ -3098,9 +3145,12 @@ static std::string openHeader(std::ifstream &f, const std::string &path)
 
 static std::string getRelativeFileName(const std::string &sourcefile, const std::string &header)
 {
+    std::string path;
     if (sourcefile.find_first_of("\\/") != std::string::npos)
-        return simplecpp::simplifyPath(sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header);
-    return simplecpp::simplifyPath(header);
+        path = sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header;
+    else
+        path = header;
+    return simplecpp::simplifyPath(path);
 }
 
 static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header)
@@ -3110,7 +3160,7 @@ static std::string openHeaderRelative(std::ifstream &f, const std::string &sourc
 
 static std::string getIncludePathFileName(const std::string &includePath, const std::string &header)
 {
-    std::string path = includePath;
+    std::string path = toAbsolutePath(includePath);
     if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\')
         path += '/';
     return path + header;
@@ -3119,9 +3169,9 @@ static std::string getIncludePathFileName(const std::string &includePath, const
 static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
 {
     for (std::list<std::string>::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) {
-        std::string simplePath = openHeader(f, getIncludePathFileName(*it, header));
-        if (!simplePath.empty())
-            return simplePath;
+        std::string path = openHeader(f, getIncludePathFileName(*it, header));
+        if (!path.empty())
+            return path;
     }
     return "";
 }
@@ -3131,49 +3181,76 @@ static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const
     if (isAbsolutePath(header))
         return openHeader(f, header);
 
-    std::string ret;
-
     if (systemheader) {
-        ret = openHeaderIncludePath(f, dui, header);
-        return ret;
+        // always return absolute path for systemheaders
+        return toAbsolutePath(openHeaderIncludePath(f, dui, header));
     }
 
+    std::string ret;
+
     ret = openHeaderRelative(f, sourcefile, header);
     if (ret.empty())
-        return openHeaderIncludePath(f, dui, header);
+        return toAbsolutePath(openHeaderIncludePath(f, dui, header));// in a similar way to system headers
     return ret;
 }
 
-static std::string getFileName(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader)
+static std::string findPathInMapBothRelativeAndAbsolute(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string& path) {
+    // here there are two possibilities - either we match this from absolute path or from a relative one
+    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
+    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;
+            }
+        }
+    } else {
+        const std::string absolutePath = toAbsolutePath(path);
+        if (filedata.find(absolutePath) != filedata.end())
+            return absolutePath;
+    }
+    // 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)
 {
     if (filedata.empty()) {
         return "";
     }
     if (isAbsolutePath(header)) {
-        return (filedata.find(header) != filedata.end()) ? simplecpp::simplifyPath(header) : "";
+        const std::string simplifiedHeaderPath = simplecpp::simplifyPath(header);
+        return (filedata.find(simplifiedHeaderPath) != filedata.end()) ? simplifiedHeaderPath : "";
     }
 
     if (!systemheader) {
-        const std::string relativeFilename = getRelativeFileName(sourcefile, header);
-        if (filedata.find(relativeFilename) != filedata.end())
-            return relativeFilename;
+        const std::string relativeOrAbsoluteFilename = getRelativeFileName(sourcefile, header);// unknown if absolute or relative, but always simplified
+        const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, relativeOrAbsoluteFilename);
+        if (!match.empty()) {
+            return match;
+        }
     }
 
     for (std::list<std::string>::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) {
-        std::string s = simplecpp::simplifyPath(getIncludePathFileName(*it, header));
-        if (filedata.find(s) != filedata.end())
-            return s;
+        const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, simplecpp::simplifyPath(getIncludePathFileName(*it, header)));
+        if (!match.empty()) {
+            return match;
+        }
     }
 
     if (systemheader && filedata.find(header) != filedata.end())
-        return header;
+        return header;// system header that its file wasn't found in the included paths but alreasy in the filedata - return this as is
 
     return "";
 }
 
 static bool hasFile(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader)
 {
-    return !getFileName(filedata, sourcefile, header, dui, systemheader).empty();
+    return !getFileIdPath(filedata, sourcefile, header, dui, systemheader).empty();
 }
 
 std::map<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::TokenList &rawtokens, std::vector<std::string> &filenames, const simplecpp::DUI &dui, simplecpp::OutputList *outputList)
@@ -3529,7 +3606,7 @@ void simplecpp::preprocess(simplecpp::TokenList &output, const simplecpp::TokenL
 
                 const bool systemheader = (inctok->str()[0] == '<');
                 const std::string header(realFilename(inctok->str().substr(1U, inctok->str().size() - 2U)));
-                std::string header2 = getFileName(filedata, rawtok->location.file(), header, dui, systemheader);
+                std::string header2 = getFileIdPath(filedata, rawtok->location.file(), header, dui, systemheader);
                 if (header2.empty()) {
                     // try to load file..
                     std::ifstream f;