Fixes #13956: VisualStudio Importer: Define UNICODE=1;_UNICODE=1 if CharacterSet is Unicode

Makefile

@@ -767,7 +767,7 @@ test/testfunctions.o: test/testfunctions.cpp lib/addoninfo.h lib/check.h lib/che
 test/testgarbage.o: test/testgarbage.cpp lib/addoninfo.h lib/check.h lib/checkers.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/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testgarbage.cpp
 
-test/testimportproject.o: test/testimportproject.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.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 test/fixture.h test/redirect.h
+test/testimportproject.o: test/testimportproject.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.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/redirect.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testimportproject.cpp
 
 test/testincompletestatement.o: test/testincompletestatement.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/checkother.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

lib/importproject.cpp

@@ -36,6 +36,7 @@
 #include <stack>
 #include <unordered_set>
 #include <utility>
+#include <vector>
 
 #include "xml.h"
 
@@ -527,51 +528,11 @@ namespace {
         std::string platformStr;
     };
 
-    struct ItemDefinitionGroup {
-        explicit ItemDefinitionGroup(const tinyxml2::XMLElement *idg, std::string includePaths) : additionalIncludePaths(std::move(includePaths)) {
+    struct ConditionalGroup {
+        explicit ConditionalGroup(const tinyxml2::XMLElement *idg){
             const char *condAttr = idg->Attribute("Condition");
             if (condAttr)
-                condition = condAttr;
-            for (const tinyxml2::XMLElement *e1 = idg->FirstChildElement(); e1; e1 = e1->NextSiblingElement()) {
-                const char* name = e1->Name();
-                if (std::strcmp(name, "ClCompile") == 0) {
-                    enhancedInstructionSet = "StreamingSIMDExtensions2";
-                    for (const tinyxml2::XMLElement *e = e1->FirstChildElement(); e; e = e->NextSiblingElement()) {
-                        const char * const text = e->GetText();
-                        if (!text)
-                            continue;
-                        const char * const ename = e->Name();
-                        if (std::strcmp(ename, "PreprocessorDefinitions") == 0)
-                            preprocessorDefinitions = text;
-                        else if (std::strcmp(ename, "AdditionalIncludeDirectories") == 0) {
-                            if (!additionalIncludePaths.empty())
-                                additionalIncludePaths += ';';
-                            additionalIncludePaths += text;
-                        } else if (std::strcmp(ename, "LanguageStandard") == 0) {
-                            if (std::strcmp(text, "stdcpp14") == 0)
-                                cppstd = Standards::CPP14;
-                            else if (std::strcmp(text, "stdcpp17") == 0)
-                                cppstd = Standards::CPP17;
-                            else if (std::strcmp(text, "stdcpp20") == 0)
-                                cppstd = Standards::CPP20;
-                            else if (std::strcmp(text, "stdcpplatest") == 0)
-                                cppstd = Standards::CPPLatest;
-                        } else if (std::strcmp(ename, "EnableEnhancedInstructionSet") == 0) {
-                            enhancedInstructionSet = text;
-                        }
-                    }
-                }
-                else if (std::strcmp(name, "Link") == 0) {
-                    for (const tinyxml2::XMLElement *e = e1->FirstChildElement(); e; e = e->NextSiblingElement()) {
-                        const char * const text = e->GetText();
-                        if (!text)
-                            continue;
-                        if (std::strcmp(e->Name(), "EntryPointSymbol") == 0) {
-                            entryPointSymbol = text;
-                        }
-                    }
-                }
-            }
+                mCondition = condAttr;
         }
 
         static void replaceAll(std::string &c, const std::string &from, const std::string &to) {
@@ -585,9 +546,9 @@ namespace {
         // see https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditions
         // properties are .NET String objects and you can call any of its members on them
         bool conditionIsTrue(const ProjectConfiguration &p) const {
-            if (condition.empty())
+            if (mCondition.empty())
                 return true;
-            std::string c = '(' + condition + ");";
+            std::string c = '(' + mCondition + ");";
             replaceAll(c, "$(Configuration)", p.configuration);
             replaceAll(c, "$(Platform)", p.platformStr);
 
@@ -623,13 +584,75 @@ namespace {
             }
             return false;
         }
-        std::string condition;
+    private:
+        std::string mCondition;
+    };
+
+    struct ItemDefinitionGroup : ConditionalGroup {
+        explicit ItemDefinitionGroup(const tinyxml2::XMLElement *idg, std::string includePaths) : ConditionalGroup(idg), additionalIncludePaths(std::move(includePaths)) {
+            for (const tinyxml2::XMLElement *e1 = idg->FirstChildElement(); e1; e1 = e1->NextSiblingElement()) {
+                const char* name = e1->Name();
+                if (std::strcmp(name, "ClCompile") == 0) {
+                    enhancedInstructionSet = "StreamingSIMDExtensions2";
+                    for (const tinyxml2::XMLElement *e = e1->FirstChildElement(); e; e = e->NextSiblingElement()) {
+                        const char * const text = e->GetText();
+                        if (!text)
+                            continue;
+                        const char * const ename = e->Name();
+                        if (std::strcmp(ename, "PreprocessorDefinitions") == 0)
+                            preprocessorDefinitions = text;
+                        else if (std::strcmp(ename, "AdditionalIncludeDirectories") == 0) {
+                            if (!additionalIncludePaths.empty())
+                                additionalIncludePaths += ';';
+                            additionalIncludePaths += text;
+                        } else if (std::strcmp(ename, "LanguageStandard") == 0) {
+                            if (std::strcmp(text, "stdcpp14") == 0)
+                                cppstd = Standards::CPP14;
+                            else if (std::strcmp(text, "stdcpp17") == 0)
+                                cppstd = Standards::CPP17;
+                            else if (std::strcmp(text, "stdcpp20") == 0)
+                                cppstd = Standards::CPP20;
+                            else if (std::strcmp(text, "stdcpplatest") == 0)
+                                cppstd = Standards::CPPLatest;
+                        } else if (std::strcmp(ename, "EnableEnhancedInstructionSet") == 0) {
+                            enhancedInstructionSet = text;
+                        }
+                    }
+                }
+                else if (std::strcmp(name, "Link") == 0) {
+                    for (const tinyxml2::XMLElement *e = e1->FirstChildElement(); e; e = e->NextSiblingElement()) {
+                        const char * const text = e->GetText();
+                        if (!text)
+                            continue;
+                        if (std::strcmp(e->Name(), "EntryPointSymbol") == 0) {
+                            entryPointSymbol = text;
+                        }
+                    }
+                }
+            }
+        }
+
         std::string enhancedInstructionSet;
         std::string preprocessorDefinitions;
         std::string additionalIncludePaths;
         std::string entryPointSymbol; // TODO: use this
         Standards::cppstd_t cppstd = Standards::CPPLatest;
     };
+
+    struct ConfigurationPropertyGroup : ConditionalGroup {
+        explicit ConfigurationPropertyGroup(const tinyxml2::XMLElement *idg) : ConditionalGroup(idg) {
+            for (const tinyxml2::XMLElement *e = idg->FirstChildElement(); e; e = e->NextSiblingElement()) {
+                if (std::strcmp(e->Name(), "UseOfMfc") == 0) {
+                    useOfMfc = true;
+                } else if (std::strcmp(e->Name(), "CharacterSet") == 0) {
+                    useUnicode = std::strcmp(e->GetText(), "Unicode") == 0;
+                }
+            }
+        }
+
+        bool useOfMfc = false;
+        bool useUnicode = false;
+    };
 }
 
 static std::list<std::string> toStringList(const std::string &s)
@@ -648,17 +671,8 @@ static std::list<std::string> toStringList(const std::string &s)
     return ret;
 }
 
-static void importPropertyGroup(const tinyxml2::XMLElement *node, std::map<std::string,std::string,cppcheck::stricmp> &variables, std::string &includePath, bool *useOfMfc)
+static void importPropertyGroup(const tinyxml2::XMLElement *node, std::map<std::string, std::string, cppcheck::stricmp> &variables, std::string &includePath)
 {
-    if (useOfMfc) {
-        for (const tinyxml2::XMLElement *e = node->FirstChildElement(); e; e = e->NextSiblingElement()) {
-            if (std::strcmp(e->Name(), "UseOfMfc") == 0) {
-                *useOfMfc = true;
-                break;
-            }
-        }
-    }
-
     const char* labelAttribute = node->Attribute("Label");
     if (labelAttribute && std::strcmp(labelAttribute, "UserMacros") == 0) {
         for (const tinyxml2::XMLElement *propertyGroup = node->FirstChildElement(); propertyGroup; propertyGroup = propertyGroup->NextSiblingElement()) {
@@ -719,31 +733,39 @@ static void loadVisualStudioProperties(const std::string &props, std::map<std::s
                 }
             }
         } else if (std::strcmp(name,"PropertyGroup")==0) {
-            importPropertyGroup(node, variables, includePath, nullptr);
+            importPropertyGroup(node, variables, includePath);
         } else if (std::strcmp(name,"ItemDefinitionGroup")==0) {
             itemDefinitionGroupList.emplace_back(node, additionalIncludeDirectories);
         }
     }
 }
 
-bool ImportProject::importVcxproj(const std::string &filename, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache)
+bool ImportProject::importVcxproj(const std::string &filename,
+                                  std::map<std::string, std::string, cppcheck::stricmp> &variables,
+                                  const std::string &additionalIncludeDirectories,
+                                  const std::vector<std::string> &fileFilters,
+                                  std::vector<SharedItemsProject> &cache)
+{
+    tinyxml2::XMLDocument doc;
+    const tinyxml2::XMLError error = doc.LoadFile(filename.c_str());
+    if (error != tinyxml2::XML_SUCCESS) {
+        printError(std::string("Visual Studio project file is not a valid XML - ") + tinyxml2::XMLDocument::ErrorIDToName(error));
+        return false;
+    }
+    return importVcxproj(filename, doc, variables, additionalIncludeDirectories, fileFilters, cache);
+}
+
+bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache)
 {
     variables["ProjectDir"] = Path::simplifyPath(Path::getPathFromFilename(filename));
 
     std::list<ProjectConfiguration> projectConfigurationList;
     std::list<std::string> compileList;
     std::list<ItemDefinitionGroup> itemDefinitionGroupList;
+    std::vector<ConfigurationPropertyGroup> configurationPropertyGroups;
     std::string includePath;
     std::vector<SharedItemsProject> sharedItemsProjects;
 
-    bool useOfMfc = false;
-
-    tinyxml2::XMLDocument doc;
-    const tinyxml2::XMLError error = doc.LoadFile(filename.c_str());
-    if (error != tinyxml2::XML_SUCCESS) {
-        printError(std::string("Visual Studio project file is not a valid XML - ") + tinyxml2::XMLDocument::ErrorIDToName(error));
-        return false;
-    }
     const tinyxml2::XMLElement * const rootnode = doc.FirstChildElement();
     if (rootnode == nullptr) {
         printError("Visual Studio project file has no XML root node");
@@ -777,7 +799,12 @@ bool ImportProject::importVcxproj(const std::string &filename, std::map<std::str
         } else if (std::strcmp(name, "ItemDefinitionGroup") == 0) {
             itemDefinitionGroupList.emplace_back(node, additionalIncludeDirectories);
         } else if (std::strcmp(name, "PropertyGroup") == 0) {
-            importPropertyGroup(node, variables, includePath, &useOfMfc);
+            const char* labelAttribute = node->Attribute("Label");
+            if (labelAttribute && std::strcmp(labelAttribute, "Configuration") == 0) {
+                configurationPropertyGroups.emplace_back(node);
+            } else {
+                importPropertyGroup(node, variables, includePath);
+            }
         } else if (std::strcmp(name, "ImportGroup") == 0) {
             const char *labelAttribute = node->Attribute("Label");
             if (labelAttribute && std::strcmp(labelAttribute, "PropertySheets") == 0) {
@@ -853,7 +880,6 @@ bool ImportProject::importVcxproj(const std::string &filename, std::map<std::str
             fs.cfg = p.name;
             // TODO: detect actual MSC version
             fs.msc = true;
-            fs.useMfc = useOfMfc;
             fs.defines = "_WIN32=1";
             if (p.platform == ProjectConfiguration::Win32)
                 fs.platformType = Platform::Type::Win32W;
@@ -879,6 +905,17 @@ bool ImportProject::importVcxproj(const std::string &filename, std::map<std::str
                     fs.defines += ";__AVX512__";
                 additionalIncludePaths += ';' + i.additionalIncludePaths;
             }
+            bool useUnicode = false;
+            for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
+                if (!c.conditionIsTrue(p))
+                    continue;
+                // in msbuild the last definition wins
+                useUnicode = c.useUnicode;
+                fs.useMfc = c.useOfMfc;
+            }
+            if (useUnicode) {
+                fs.defines += ";UNICODE=1;_UNICODE=1";
+            }
             fsSetDefines(fs, fs.defines);
             fsSetIncludePaths(fs, Path::getPathFromFilename(filename), toStringList(includePath + ';' + additionalIncludePaths), variables);
             for (const auto &path : sharedItemsIncludePaths) {

lib/importproject.h

@@ -52,6 +52,7 @@ namespace cppcheck {
  * @brief Importing project settings.
  */
 class CPPCHECKLIB WARN_UNUSED ImportProject {
+    friend class TestImporter;
 public:
     enum class Type : std::uint8_t {
         NONE,
@@ -111,6 +112,7 @@ class CPPCHECKLIB WARN_UNUSED ImportProject {
     bool importSln(std::istream &istr, const std::string &path, const std::vector<std::string> &fileFilters);
     static SharedItemsProject importVcxitems(const std::string &filename, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache);
     bool importVcxproj(const std::string &filename, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache);
+    bool importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache);
     bool importBcb6Prj(const std::string &projectFilename);
 
     static void printError(const std::string &message);

test/testimportproject.cpp

@@ -22,6 +22,7 @@
 #include "redirect.h"
 #include "settings.h"
 #include "suppressions.h"
+#include "xml.h"
 
 #include <list>
 #include <map>
@@ -34,6 +35,8 @@ class TestImporter : public ImportProject {
 public:
     using ImportProject::importCompileCommands;
     using ImportProject::importCppcheckGuiProject;
+    using ImportProject::importVcxproj;
+    using ImportProject::SharedItemsProject;
 
     bool sourceFileExists(const std::string & /*file*/) override {
         return true;
@@ -71,6 +74,7 @@ class TestImportProject : public TestFixture {
         TEST_CASE(importCompileCommandsDirectoryInvalid); // 'directory' field not a string
         TEST_CASE(importCppcheckGuiProject);
         TEST_CASE(ignorePaths);
+        TEST_CASE(testVcxprojUnicode);
     }
 
     void setDefines() const {
@@ -455,6 +459,59 @@ class TestImportProject : public TestFixture {
         ASSERT_EQUALS(0, project.fileSettings.size());
     }
 
+    void testVcxprojUnicode() const
+    {
+        const char vcxproj[] = R"-(
+<?xml version="1.0" encoding="utf-8"?>
+<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <ItemGroup Label="ProjectConfigurations">
+    <ProjectConfiguration Include="Debug|Win32">
+      <Configuration>Debug</Configuration>
+      <Platform>Win32</Platform>
+    </ProjectConfiguration>
+    <ProjectConfiguration Include="Release|Win32">
+      <Configuration>Release</Configuration>
+      <Platform>Win32</Platform>
+    </ProjectConfiguration>
+  </ItemGroup>
+  <PropertyGroup Label="Configuration">
+    <!-- Only to test that the last configuration entry overwrites this -->
+    <CharacterSet>Unicode</CharacterSet>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
+    <ConfigurationType>Application</ConfigurationType>
+    <UseDebugLibraries>true</UseDebugLibraries>
+    <PlatformToolset>v143</PlatformToolset>
+    <CharacterSet>Unicode</CharacterSet>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Configuration">
+    <ConfigurationType>Application</ConfigurationType>
+    <UseDebugLibraries>false</UseDebugLibraries>
+    <PlatformToolset>v143</PlatformToolset>
+    <CharacterSet>NotSet</CharacterSet>
+    <UseOfMfc>Static</UseOfMfc>
+  </PropertyGroup>
+  <ItemGroup>
+    <ClCompile Include="main.cpp" />
+  </ItemGroup>
+</Project>
+)-";
+        tinyxml2::XMLDocument doc;
+        ASSERT_EQUALS(tinyxml2::XML_SUCCESS, doc.Parse(vcxproj, sizeof(vcxproj)));
+        TestImporter project;
+        std::map<std::string, std::string, cppcheck::stricmp> variables;
+        std::vector<TestImporter::SharedItemsProject> cache;
+        ASSERT_EQUALS(project.importVcxproj("test.vcxproj", doc, variables, {}, {}, cache), true);
+        ASSERT_EQUALS(project.fileSettings.size(), 2);
+        ASSERT(project.fileSettings.front().defines.find(";UNICODE=1;") != std::string::npos);
+        ASSERT(project.fileSettings.front().defines.find(";_UNICODE=1") != std::string::npos);
+        ASSERT(project.fileSettings.front().defines.find(";_UNICODE=1;") == std::string::npos); // No duplicates
+        ASSERT_EQUALS(project.fileSettings.front().useMfc, false);
+        ASSERT(project.fileSettings.back().defines.find(";UNICODE=1;") == std::string::npos);
+        ASSERT(project.fileSettings.back().defines.find(";_UNICODE=1") == std::string::npos);
+        ASSERT_EQUALS(project.fileSettings.back().useMfc, true);
+    }
+
     // TODO: test fsParseCommand()
 
     // TODO: test vcxproj conditions