fixed #13906 - ErrorMessage::FileLocation did not simplify did not simplify the file in all cases

firewave · firewave · commit 1a5a0926bea4 · 2025-06-02T15:11:17.000+02:00
diff --git a/Makefile b/Makefile
@@ -746,7 +746,7 @@ test/testconstructors.o: test/testconstructors.cpp lib/addoninfo.h lib/check.h l
 test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcppcheck.cpp
 
-test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h lib/xml.h test/fixture.h
+test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testerrorlogger.cpp
 
 test/testexceptionsafety.o: test/testexceptionsafety.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkexceptionsafety.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp
@@ -701,13 +701,20 @@ std::string ErrorLogger::callStackToString(const std::list<ErrorMessage::FileLoc
     return str;
 }
 
+ErrorMessage::FileLocation::FileLocation(const std::string &file, int line, unsigned int column)
+    : fileIndex(0), line(line), column(column), mOrigFileName(file), mFileName(Path::simplifyPath(file))
+{}
+
+ErrorMessage::FileLocation::FileLocation(const std::string &file, std::string info, int line, unsigned int column)
+    : fileIndex(0), line(line), column(column), mOrigFileName(file), mFileName(Path::simplifyPath(file)), mInfo(std::move(info))
+{}
 
 ErrorMessage::FileLocation::FileLocation(const Token* tok, const TokenList* tokenList)
-    : fileIndex(tok->fileIndex()), line(tok->linenr()), column(tok->column()), mOrigFileName(tokenList->getOrigFile(tok)), mFileName(tokenList->file(tok))
+    : fileIndex(tok->fileIndex()), line(tok->linenr()), column(tok->column()), mOrigFileName(tokenList->getOrigFile(tok)), mFileName(Path::simplifyPath(tokenList->file(tok)))
 {}
 
 ErrorMessage::FileLocation::FileLocation(const Token* tok, std::string info, const TokenList* tokenList)
-    : fileIndex(tok->fileIndex()), line(tok->linenr()), column(tok->column()), mOrigFileName(tokenList->getOrigFile(tok)), mFileName(tokenList->file(tok)), mInfo(std::move(info))
+    : fileIndex(tok->fileIndex()), line(tok->linenr()), column(tok->column()), mOrigFileName(tokenList->getOrigFile(tok)), mFileName(Path::simplifyPath(tokenList->file(tok))), mInfo(std::move(info))
 {}
 
 std::string ErrorMessage::FileLocation::getfile(bool convert) const
@@ -726,7 +733,6 @@ std::string ErrorMessage::FileLocation::getOrigFile(bool convert) const
 
 void ErrorMessage::FileLocation::setfile(std::string file)
 {
-    // TODO: consistently simplify mFileName
     mFileName = Path::simplifyPath(std::move(file));
 }
 
diff --git a/lib/errorlogger.h b/lib/errorlogger.h
@@ -57,12 +57,8 @@ class CPPCHECKLIB ErrorMessage {
      */
     class CPPCHECKLIB WARN_UNUSED FileLocation {
     public:
-        FileLocation(const std::string &file, int line, unsigned int column)
-            : fileIndex(0), line(line), column(column), mOrigFileName(file), mFileName(file) {}
-
-        FileLocation(const std::string &file, std::string info, int line, unsigned int column)
-            : fileIndex(0), line(line), column(column), mOrigFileName(file), mFileName(file), mInfo(std::move(info)) {}
-
+        FileLocation(const std::string &file, int line, unsigned int column);
+        FileLocation(const std::string &file, std::string info, int line, unsigned int column);
         FileLocation(const Token* tok, const TokenList* tokenList);
         FileLocation(const Token* tok, std::string info, const TokenList* tokenList);
 
diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp
@@ -86,11 +86,14 @@ void TokenList::deallocateTokens()
         mTokensFrontBack->front = nullptr;
         mTokensFrontBack->back = nullptr;
     }
+    // TODO: clear mOrigFiles?
     mFiles.clear();
 }
 
+// TODO: also update mOrigFiles?
 int TokenList::appendFileIfNew(std::string fileName)
 {
+    assert(mTokensFrontBack->front == nullptr); // has no effect if tokens have already been created
     assert(!fileName.empty());
 
     // Has this file been tokenized already?
@@ -347,8 +350,10 @@ void TokenList::createTokens(simplecpp::TokenList&& tokenList)
         // TODO: this points to mFiles when called from createTokens(std::istream &, const std::string&)
         mOrigFiles = mFiles = tokenList.getFiles();
     }
-    else
+    else {
+        // TODO: clear mOrigFiles?
         mFiles.clear();
+    }
 
     for (const simplecpp::Token *tok = tokenList.cfront(); tok;) {
 
diff --git a/test/helpers.h b/test/helpers.h
@@ -106,6 +106,16 @@ class SimpleTokenList
             throw std::runtime_error("creating tokens failed");
     }
 
+    template<size_t size>
+    explicit SimpleTokenList(const char (&code)[size], const std::string& file0, Standards::Language lang = Standards::Language::CPP)
+        : list{settings, lang}
+    {
+        std::istringstream iss(code);
+        list.appendFileIfNew(file0);
+        if (!list.createTokens(iss))
+            throw std::runtime_error("creating tokens failed");
+    }
+
     Token* front() {
         return list.front();
     }
@@ -114,6 +124,10 @@ class SimpleTokenList
         return list.front();
     }
 
+    const TokenList& get() const {
+        return list;
+    }
+
 private:
     const Settings settings;
     TokenList list;
diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp
@@ -21,6 +21,7 @@
 #include "errorlogger.h"
 #include "errortypes.h"
 #include "fixture.h"
+#include "helpers.h"
 #include "suppressions.h"
 
 #include <list>
@@ -43,6 +44,7 @@ class TestErrorLogger : public TestFixture {
     void run() override {
         TEST_CASE(PatternSearchReplace);
         TEST_CASE(FileLocationConstruct);
+        TEST_CASE(FileLocationConstructFile);
         TEST_CASE(FileLocationSetFile);
         TEST_CASE(FileLocationSetFile2);
         TEST_CASE(ErrorMessageConstruct);
@@ -55,7 +57,7 @@ class TestErrorLogger : public TestFixture {
         TEST_CASE(CustomFormatLocations);
         TEST_CASE(ToXmlV2);
         TEST_CASE(ToXmlV2RemarkComment);
-        TEST_CASE(ToXmlV2Locations);
+        TEST_CASE(ToXmlLocations);
         TEST_CASE(ToXmlV2Encoding);
         TEST_CASE(FromXmlV2);
         TEST_CASE(ToXmlV3);
@@ -115,20 +117,116 @@ class TestErrorLogger : public TestFixture {
     }
 
     void FileLocationConstruct() const {
-        const ErrorMessage::FileLocation loc("foo.cpp", 1, 2);
-        ASSERT_EQUALS("foo.cpp", loc.getOrigFile());
-        ASSERT_EQUALS("foo.cpp", loc.getfile());
-        ASSERT_EQUALS(1, loc.line);
-        ASSERT_EQUALS(2, loc.column);
-        ASSERT_EQUALS("[foo.cpp:1]", loc.stringify(false));
-        ASSERT_EQUALS("[foo.cpp:1:2]", loc.stringify(true));
+        {
+            const ErrorMessage::FileLocation loc("foo.cpp", 1, 2);
+            ASSERT_EQUALS("foo.cpp", loc.getOrigFile(false));
+            ASSERT_EQUALS("foo.cpp", loc.getfile(false));
+            ASSERT_EQUALS(1, loc.line);
+            ASSERT_EQUALS(2, loc.column);
+            ASSERT_EQUALS("[foo.cpp:1]", loc.stringify(false));
+            ASSERT_EQUALS("[foo.cpp:1:2]", loc.stringify(true));
+            ASSERT_EQUALS("", loc.getinfo());
+        }
+        {
+            const ErrorMessage::FileLocation loc("foo.cpp", "info", 1, 2);
+            ASSERT_EQUALS("foo.cpp", loc.getOrigFile(false));
+            ASSERT_EQUALS("foo.cpp", loc.getfile(false));
+            ASSERT_EQUALS(1, loc.line);
+            ASSERT_EQUALS(2, loc.column);
+            ASSERT_EQUALS("[foo.cpp:1]", loc.stringify(false));
+            ASSERT_EQUALS("[foo.cpp:1:2]", loc.stringify(true));
+            ASSERT_EQUALS("info", loc.getinfo());
+        }
+        {
+            const SimpleTokenList tokenlist("a", "dir/a.cpp");
+            {
+                const ErrorMessage::FileLocation loc(tokenlist.front(), &tokenlist.get());
+                ASSERT_EQUALS("dir/a.cpp", loc.getOrigFile(false));
+                ASSERT_EQUALS("dir/a.cpp", loc.getfile(false));
+                ASSERT_EQUALS(1, loc.line);
+                ASSERT_EQUALS(1, loc.column);
+#if defined(_WIN32)
+                ASSERT_EQUALS("[dir\\a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir\\a.cpp:1:1]", loc.stringify(true));
+#else
+                ASSERT_EQUALS("[dir/a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir/a.cpp:1:1]", loc.stringify(true));
+#endif
+                ASSERT_EQUALS("", loc.getinfo());
+            }
+            {
+                const ErrorMessage::FileLocation loc(tokenlist.front(), "info", &tokenlist.get());
+                ASSERT_EQUALS("dir/a.cpp", loc.getOrigFile(false));
+                ASSERT_EQUALS("dir/a.cpp", loc.getfile(false));
+                ASSERT_EQUALS(1, loc.line);
+                ASSERT_EQUALS(1, loc.column);
+#if defined(_WIN32)
+                ASSERT_EQUALS("[dir\\a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir\\a.cpp:1:1]", loc.stringify(true));
+#else
+                ASSERT_EQUALS("[dir/a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir/a.cpp:1:1]", loc.stringify(true));
+#endif
+                ASSERT_EQUALS("info", loc.getinfo());
+            }
+        }
+        {
+            const SimpleTokenList tokenlist("a", "dir\\a.cpp");
+            {
+                const ErrorMessage::FileLocation loc(tokenlist.front(), &tokenlist.get());
+                ASSERT_EQUALS("dir\\a.cpp", loc.getOrigFile(false));
+                ASSERT_EQUALS("dir/a.cpp", loc.getfile(false));
+                ASSERT_EQUALS(1, loc.line);
+                ASSERT_EQUALS(1, loc.column);
+#if defined(_WIN32)
+                ASSERT_EQUALS("[dir\\a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir\\a.cpp:1:1]", loc.stringify(true));
+#else
+                ASSERT_EQUALS("[dir/a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir/a.cpp:1:1]", loc.stringify(true));
+#endif
+                ASSERT_EQUALS("", loc.getinfo());
+            }
+            {
+                const ErrorMessage::FileLocation loc(tokenlist.front(), "info", &tokenlist.get());
+                ASSERT_EQUALS("dir\\a.cpp", loc.getOrigFile(false));
+                ASSERT_EQUALS("dir/a.cpp", loc.getfile(false));
+                ASSERT_EQUALS(1, loc.line);
+                ASSERT_EQUALS(1, loc.column);
+#if defined(_WIN32)
+                ASSERT_EQUALS("[dir\\a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir\\a.cpp:1:1]", loc.stringify(true));
+#else
+                ASSERT_EQUALS("[dir/a.cpp:1]", loc.stringify(false));
+                ASSERT_EQUALS("[dir/a.cpp:1:1]", loc.stringify(true));
+#endif
+                ASSERT_EQUALS("info", loc.getinfo());
+            }
+        }
+    }
+
+    void FileLocationConstructFile() const {
+        ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation("dir/a.cpp", 1, 1).getfile(false));
+        ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation("dir\\a.cpp", 1, 1).getfile(false));
+        ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation("dir/a.cpp", "info", 1, 1).getfile(false));
+        ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation("dir\\a.cpp", "info", 1, 1).getfile(false));
+        {
+            const SimpleTokenList tokenlist("a", "dir/a.cpp");
+            ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation(tokenlist.front(), &tokenlist.get()).getfile(false));
+            ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation(tokenlist.front(), "info", &tokenlist.get()).getfile(false));
+        }
+        {
+            const SimpleTokenList tokenlist("a", "dir\\a.cpp");
+            ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation(tokenlist.front(), &tokenlist.get()).getfile(false));
+            ASSERT_EQUALS("dir/a.cpp", ErrorMessage::FileLocation(tokenlist.front(), "info", &tokenlist.get()).getfile(false));
+        }
     }
 
     void FileLocationSetFile() const {
         ErrorMessage::FileLocation loc("foo1.cpp", 0, 0);
         loc.setfile("foo.cpp");
-        ASSERT_EQUALS("foo1.cpp", loc.getOrigFile());
-        ASSERT_EQUALS("foo.cpp", loc.getfile());
+        ASSERT_EQUALS("foo1.cpp", loc.getOrigFile(false));
+        ASSERT_EQUALS("foo.cpp", loc.getfile(false));
         ASSERT_EQUALS(0, loc.line);
         ASSERT_EQUALS(0, loc.column);
         // TODO: the following looks wrong - there is no line or column 0
@@ -139,8 +237,8 @@ class TestErrorLogger : public TestFixture {
     void FileLocationSetFile2() const {
         ErrorMessage::FileLocation loc("foo1.cpp", SuppressionList::Suppression::NO_LINE, 0); // TODO: should not depend on Suppression
         loc.setfile("foo.cpp");
-        ASSERT_EQUALS("foo1.cpp", loc.getOrigFile());
-        ASSERT_EQUALS("foo.cpp", loc.getfile());
+        ASSERT_EQUALS("foo1.cpp", loc.getOrigFile(false));
+        ASSERT_EQUALS("foo.cpp", loc.getfile(false));
         ASSERT_EQUALS(SuppressionList::Suppression::NO_LINE, loc.line);
         ASSERT_EQUALS(0, loc.column);
         ASSERT_EQUALS("[foo.cpp]", loc.stringify(false));
@@ -295,7 +393,7 @@ class TestErrorLogger : public TestFixture {
         ASSERT_EQUALS("        <error id=\"id\" severity=\"warning\" msg=\"\" verbose=\"\" remark=\"remark\"/>", msg.toXML());
     }
 
-    void ToXmlV2Locations() const {
+    void ToXmlLocations() const {
         const ErrorMessage::FileLocation dir1loc{"dir1/a.cpp", 1, 1};
         const ErrorMessage::FileLocation dir2loc{"dir2\\a.cpp", 1, 1};
         ErrorMessage::FileLocation dir3loc{"dir/a.cpp", 1, 1};
@@ -310,7 +408,7 @@ class TestErrorLogger : public TestFixture {
         message += "            <location file=\"bar.cpp\" line=\"8\" column=\"1\" info=\"\\303\\244\"/>\n";
         message += "            <location file=\"foo.cpp\" line=\"5\" column=\"1\"/>\n";
         message += "            <location file=\"dir1/a.cpp\" line=\"1\" column=\"1\"/>\n";
-        message += "            <location file=\"dir2\\a.cpp\" line=\"1\" column=\"1\"/>\n";
+        message += "            <location file=\"dir2/a.cpp\" line=\"1\" column=\"1\"/>\n";
         message += "            <location file=\"dir3/a.cpp\" line=\"1\" column=\"1\"/>\n";
         message += "            <location file=\"dir4/a.cpp\" line=\"1\" column=\"1\"/>\n";
         message += "        </error>";
@@ -360,10 +458,10 @@ class TestErrorLogger : public TestFixture {
         ASSERT_EQUALS("Verbose error", msg.verboseMessage());
         ASSERT_EQUALS(456u, msg.hash);
         ASSERT_EQUALS(2u, msg.callStack.size());
-        ASSERT_EQUALS("foo.cpp", msg.callStack.front().getfile());
+        ASSERT_EQUALS("foo.cpp", msg.callStack.front().getfile(false));
         ASSERT_EQUALS(5, msg.callStack.front().line);
         ASSERT_EQUALS(2u, msg.callStack.front().column);
-        ASSERT_EQUALS("bar.cpp", msg.callStack.back().getfile());
+        ASSERT_EQUALS("bar.cpp", msg.callStack.back().getfile(false));
         ASSERT_EQUALS(8, msg.callStack.back().line);
         ASSERT_EQUALS(1u, msg.callStack.back().column);
     }

Original file line number	Diff line number	Diff line change
`@@ -86,11 +86,14 @@ void TokenList::deallocateTokens()`
`86`	`86`	`mTokensFrontBack->front = nullptr;`
`87`	`87`	`mTokensFrontBack->back = nullptr;`
`88`	`88`	`}`
	`89`	`+ // TODO: clear mOrigFiles?`
`89`	`90`	`mFiles.clear();`
`90`	`91`	`}`
`91`	`92`
	`93`	`+// TODO: also update mOrigFiles?`
`92`	`94`	`int TokenList::appendFileIfNew(std::string fileName)`
`93`	`95`	`{`
	`96`	`+ assert(mTokensFrontBack->front == nullptr); // has no effect if tokens have already been created`
`94`	`97`	`assert(!fileName.empty());`
`95`	`98`
`96`	`99`	`// Has this file been tokenized already?`
`@@ -347,8 +350,10 @@ void TokenList::createTokens(simplecpp::TokenList&& tokenList)`
`347`	`350`	`// TODO: this points to mFiles when called from createTokens(std::istream &, const std::string&)`
`348`	`351`	`mOrigFiles = mFiles = tokenList.getFiles();`
`349`	`352`	`}`
`350`		`- else`
	`353`	`+ else {`
	`354`	`+ // TODO: clear mOrigFiles?`
`351`	`355`	`mFiles.clear();`
	`356`	`+ }`
`352`	`357`
`353`	`358`	`for (const simplecpp::Token *tok = tokenList.cfront(); tok;) {`
`354`	`359`