Merge branch 'sonarview' into 'master'

Address string_view conversions flagged by SonarQube

See merge request OpenMW/openmw!4693

apps/openmw/mwscript/miscextensions.cpp

@@ -615,7 +615,7 @@ namespace MWScript
 
                 long key;
 
-                if (const auto k = ::Misc::StringUtils::toNumeric<long>(effect.data());
+                if (const auto k = ::Misc::StringUtils::toNumeric<long>(effect);
                     k.has_value() && *k >= 0 && *k <= 32767)
                     key = *k;
                 else

components/l10n/manager.cpp

@@ -14,7 +14,7 @@ namespace L10n
         mPreferredLocales.clear();
         if (gmstHasPriority)
             mPreferredLocales.push_back(icu::Locale("gmst"));
-        std::set<std::string> langSet;
+        std::set<std::string_view, std::less<>> langSet;
         for (const auto& lang : langs)
         {
             if (langSet.contains(lang))
@@ -31,10 +31,10 @@ namespace L10n
                 msg << " " << l.getName();
         }
         for (auto& [key, context] : mCache)
-            updateContext(key.first, *context);
+            updateContext(std::get<0>(key), *context);
     }
 
-    void Manager::readLangData(const std::string& name, MessageBundles& ctx, const icu::Locale& lang)
+    void Manager::readLangData(std::string_view name, MessageBundles& ctx, const icu::Locale& lang)
     {
         std::string langName(lang.getName());
         langName += ".yaml";
@@ -58,7 +58,7 @@ namespace L10n
         }
     }
 
-    void Manager::updateContext(const std::string& name, MessageBundles& ctx)
+    void Manager::updateContext(std::string_view name, MessageBundles& ctx)
     {
         icu::Locale fallbackLocale = ctx.getFallbackLocale();
         ctx.setPreferredLocales(mPreferredLocales);
@@ -89,9 +89,9 @@ namespace L10n
     }
 
     std::shared_ptr<const MessageBundles> Manager::getContext(
-        const std::string& contextName, const std::string& fallbackLocaleName)
+        std::string_view contextName, const std::string& fallbackLocaleName)
     {
-        std::pair<std::string, std::string> key(contextName, fallbackLocaleName);
+        std::tuple<std::string_view, std::string_view> key(contextName, fallbackLocaleName);
         auto it = mCache.find(key);
         if (it != mCache.end())
             return it->second;
@@ -102,7 +102,7 @@ namespace L10n
         for (char c : contextName)
             valid = valid && allowedChar(c);
         if (!valid)
-            throw std::runtime_error(std::string("Invalid l10n context name: ") + contextName);
+            throw std::runtime_error("Invalid l10n context name: " + std::string(contextName));
         icu::Locale fallbackLocale(fallbackLocaleName.c_str());
         std::shared_ptr<MessageBundles> ctx = std::make_shared<MessageBundles>(mPreferredLocales, fallbackLocale);
         ctx->setGmstLoader(mGmstLoader);

components/l10n/manager.hpp

@@ -27,20 +27,20 @@ namespace L10n
         void setGmstLoader(std::function<std::string(std::string_view)> fn) { mGmstLoader = std::move(fn); }
 
         std::shared_ptr<const MessageBundles> getContext(
-            const std::string& contextName, const std::string& fallbackLocale = "en");
+            std::string_view contextName, const std::string& fallbackLocale = "en");
 
-        std::string getMessage(const std::string& contextName, std::string_view key)
+        std::string getMessage(std::string_view contextName, std::string_view key)
         {
             return getContext(contextName)->formatMessage(key, {}, {});
         }
 
     private:
-        void readLangData(const std::string& name, MessageBundles& ctx, const icu::Locale& lang);
+        void readLangData(std::string_view name, MessageBundles& ctx, const icu::Locale& lang);
-        void updateContext(const std::string& name, MessageBundles& ctx);
+        void updateContext(std::string_view name, MessageBundles& ctx);
 
         const VFS::Manager* mVFS;
         std::vector<icu::Locale> mPreferredLocales;
-        std::map<std::pair<std::string, std::string>, std::shared_ptr<MessageBundles>> mCache;
+        std::map<std::tuple<std::string, std::string>, std::shared_ptr<MessageBundles>, std::less<>> mCache;
         std::function<std::string(std::string_view)> mGmstLoader;
     };
 

components/l10n/messagebundles.cpp

@@ -9,6 +9,50 @@
 
 namespace L10n
 {
+    namespace
+    {
+        std::string getErrorText(const UParseError& parseError)
+        {
+            icu::UnicodeString preContext(parseError.preContext), postContext(parseError.postContext);
+            std::string parseErrorString;
+            preContext.toUTF8String(parseErrorString);
+            postContext.toUTF8String(parseErrorString);
+            return parseErrorString;
+        }
+
+        template <class... Args>
+        bool checkSuccess(const icu::ErrorCode& status, const UParseError& parseError, Args const&... message)
+        {
+            if (status.isFailure())
+            {
+                std::string errorText = getErrorText(parseError);
+                if (!errorText.empty())
+                {
+                    (Log(Debug::Error) << ... << message)
+                        << ": " << status.errorName() << " in \"" << errorText << "\"";
+                }
+                else
+                {
+                    (Log(Debug::Error) << ... << message) << ": " << status.errorName();
+                }
+            }
+            return status.isSuccess();
+        }
+
+        std::string loadGmst(
+            const std::function<std::string(std::string_view)>& gmstLoader, const icu::MessageFormat* message)
+        {
+            icu::UnicodeString gmstNameUnicode;
+            std::string gmstName;
+            icu::ErrorCode success;
+            message->format(nullptr, nullptr, 0, gmstNameUnicode, success);
+            gmstNameUnicode.toUTF8String(gmstName);
+            if (gmstLoader)
+                return gmstLoader(gmstName);
+            return "GMST:" + gmstName;
+        }
+    }
+
     MessageBundles::MessageBundles(const std::vector<icu::Locale>& preferredLocales, icu::Locale& fallbackLocale)
         : mFallbackLocale(fallbackLocale)
     {
@@ -39,33 +83,6 @@ namespace L10n
         }
     }
 
-    std::string getErrorText(const UParseError& parseError)
-    {
-        icu::UnicodeString preContext(parseError.preContext), postContext(parseError.postContext);
-        std::string parseErrorString;
-        preContext.toUTF8String(parseErrorString);
-        postContext.toUTF8String(parseErrorString);
-        return parseErrorString;
-    }
-
-    static bool checkSuccess(
-        const icu::ErrorCode& status, const std::string& message, const UParseError parseError = UParseError())
-    {
-        if (status.isFailure())
-        {
-            std::string errorText = getErrorText(parseError);
-            if (!errorText.empty())
-            {
-                Log(Debug::Error) << message << ": " << status.errorName() << " in \"" << errorText << "\"";
-            }
-            else
-            {
-                Log(Debug::Error) << message << ": " << status.errorName();
-            }
-        }
-        return status.isSuccess();
-    }
-
     void MessageBundles::load(std::istream& input, const icu::Locale& lang)
     {
         YAML::Node data = YAML::Load(input);
@@ -80,20 +97,19 @@ namespace L10n
             icu::ErrorCode status;
             UParseError parseError;
             icu::MessageFormat message(pattern, langOrEn, parseError, status);
-            if (checkSuccess(status, std::string("Failed to create message ") + key + " for locale " + lang.getName(),
+            if (checkSuccess(status, parseError, "Failed to create message ", key, " for locale ", lang.getName()))
-                    parseError))
             {
-                mBundles[localeName].insert(std::make_pair(key, message));
+                mBundles[localeName].emplace(key, message);
             }
         }
     }
 
-    const icu::MessageFormat* MessageBundles::findMessage(std::string_view key, const std::string& localeName) const
+    const icu::MessageFormat* MessageBundles::findMessage(std::string_view key, std::string_view localeName) const
     {
         auto iter = mBundles.find(localeName);
         if (iter != mBundles.end())
         {
-            auto message = iter->second.find(key.data());
+            auto message = iter->second.find(key);
             if (message != iter->second.end())
             {
                 return &(message->second);
@@ -116,20 +132,6 @@ namespace L10n
         return formatMessage(key, argNames, argValues);
     }
 
-    static std::string loadGmst(
-        const std::function<std::string(std::string_view)> gmstLoader, const icu::MessageFormat* message)
-    {
-        icu::UnicodeString gmstNameUnicode;
-        std::string gmstName;
-        icu::ErrorCode success;
-        message->format(nullptr, nullptr, 0, gmstNameUnicode, success);
-        gmstNameUnicode.toUTF8String(gmstName);
-        if (gmstLoader)
-            return gmstLoader(gmstName);
-        else
-            return "GMST:" + gmstName;
-    }
-
     std::string MessageBundles::formatMessage(std::string_view key, const std::vector<icu::UnicodeString>& argNames,
         const std::vector<icu::Formattable>& args) const
     {
@@ -158,7 +160,7 @@ namespace L10n
                 message->format(argNames.data(), args.data(), static_cast<std::int32_t>(args.size()), result, success);
             else
                 message->format(nullptr, nullptr, static_cast<std::int32_t>(args.size()), result, success);
-            checkSuccess(success, std::string("Failed to format message ") + key.data());
+            checkSuccess(success, {}, "Failed to format message ", key);
             result.toUTF8String(resultString);
             return resultString;
         }
@@ -171,7 +173,7 @@ namespace L10n
         icu::MessageFormat defaultMessage(
             icu::UnicodeString::fromUTF8(icu::StringPiece(key.data(), static_cast<std::int32_t>(key.size()))),
             defaultLocale, parseError, success);
-        if (!checkSuccess(success, std::string("Failed to create message ") + key.data(), parseError))
+        if (!checkSuccess(success, parseError, "Failed to create message ", key))
             // If we can't parse the key as a pattern, just return the key
             return std::string(key);
 
@@ -180,7 +182,7 @@ namespace L10n
                 argNames.data(), args.data(), static_cast<std::int32_t>(args.size()), result, success);
         else
             defaultMessage.format(nullptr, nullptr, static_cast<std::int32_t>(args.size()), result, success);
-        checkSuccess(success, std::string("Failed to format message ") + key.data());
+        checkSuccess(success, {}, "Failed to format message ", key);
         result.toUTF8String(resultString);
         return resultString;
     }

components/l10n/messagebundles.hpp

@@ -10,6 +10,8 @@
 #include <unicode/locid.h>
 #include <unicode/msgfmt.h>
 
+#include <components/misc/strings/algorithm.hpp>
+
 namespace L10n
 {
     /**
@@ -41,18 +43,23 @@ namespace L10n
         void setPreferredLocales(const std::vector<icu::Locale>& preferredLocales);
         const std::vector<icu::Locale>& getPreferredLocales() const { return mPreferredLocales; }
         void load(std::istream& input, const icu::Locale& lang);
-        bool isLoaded(const icu::Locale& loc) const { return mBundles.find(loc.getName()) != mBundles.end(); }
+        bool isLoaded(const icu::Locale& loc) const
+        {
+            return mBundles.find(std::string_view(loc.getName())) != mBundles.end();
+        }
         const icu::Locale& getFallbackLocale() const { return mFallbackLocale; }
         void setGmstLoader(std::function<std::string(std::string_view)> fn) { mGmstLoader = std::move(fn); }
 
     private:
+        template <class T>
+        using StringMap = std::unordered_map<std::string, T, Misc::StringUtils::StringHash, std::equal_to<>>;
         // icu::Locale isn't hashable (or comparable), so we use the string form instead, which is canonicalized
-        std::unordered_map<std::string, std::unordered_map<std::string, icu::MessageFormat>> mBundles;
+        StringMap<StringMap<icu::MessageFormat>> mBundles;
         const icu::Locale mFallbackLocale;
         std::vector<std::string> mPreferredLocaleStrings;
         std::vector<icu::Locale> mPreferredLocales;
         std::function<std::string(std::string_view)> mGmstLoader;
-        const icu::MessageFormat* findMessage(std::string_view key, const std::string& localeName) const;
+        const icu::MessageFormat* findMessage(std::string_view key, std::string_view localeName) const;
     };
 
 }

components/lua/l10n.cpp

@@ -66,7 +66,7 @@ namespace LuaUtil
               };
 
         return sol::make_object(
-            lua, [manager](const std::string& contextName, sol::optional<std::string> fallbackLocale) {
+            lua, [manager](std::string_view contextName, sol::optional<std::string> fallbackLocale) {
                 if (fallbackLocale)
                     return L10nContext{ manager->getContext(contextName, *fallbackLocale) };
                 else