From ee540039a1d6e866d00a4afe708a6b167af68172 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 22 May 2025 20:54:23 +0200 Subject: [PATCH 1/2] Address string_view conversions flagged by SonarQube --- apps/openmw/mwscript/miscextensions.cpp | 2 +- components/l10n/manager.cpp | 12 +-- components/l10n/manager.hpp | 10 +-- components/l10n/messagebundles.cpp | 100 ++++++++++++------------ components/l10n/messagebundles.hpp | 13 ++- components/lua/l10n.cpp | 2 +- 6 files changed, 74 insertions(+), 65 deletions(-) diff --git a/apps/openmw/mwscript/miscextensions.cpp b/apps/openmw/mwscript/miscextensions.cpp index af81fd2d3c..fd2064463e 100644 --- a/apps/openmw/mwscript/miscextensions.cpp +++ b/apps/openmw/mwscript/miscextensions.cpp @@ -615,7 +615,7 @@ namespace MWScript long key; - if (const auto k = ::Misc::StringUtils::toNumeric(effect.data()); + if (const auto k = ::Misc::StringUtils::toNumeric(effect); k.has_value() && *k >= 0 && *k <= 32767) key = *k; else diff --git a/components/l10n/manager.cpp b/components/l10n/manager.cpp index 27a4d603d4..b176e4aa7a 100644 --- a/components/l10n/manager.cpp +++ b/components/l10n/manager.cpp @@ -14,7 +14,7 @@ namespace L10n mPreferredLocales.clear(); if (gmstHasPriority) mPreferredLocales.push_back(icu::Locale("gmst")); - std::set langSet; + std::set> langSet; for (const auto& lang : langs) { if (langSet.contains(lang)) @@ -34,7 +34,7 @@ namespace L10n updateContext(key.first, *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 Manager::getContext( - const std::string& contextName, const std::string& fallbackLocaleName) + std::string_view contextName, const std::string& fallbackLocaleName) { - std::pair key(contextName, fallbackLocaleName); + std::pair 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 ctx = std::make_shared(mPreferredLocales, fallbackLocale); ctx->setGmstLoader(mGmstLoader); diff --git a/components/l10n/manager.hpp b/components/l10n/manager.hpp index 4b047fa9d7..7022057178 100644 --- a/components/l10n/manager.hpp +++ b/components/l10n/manager.hpp @@ -27,20 +27,20 @@ namespace L10n void setGmstLoader(std::function fn) { mGmstLoader = std::move(fn); } std::shared_ptr 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 updateContext(const std::string& name, MessageBundles& ctx); + void readLangData(std::string_view name, MessageBundles& ctx, const icu::Locale& lang); + void updateContext(std::string_view name, MessageBundles& ctx); const VFS::Manager* mVFS; std::vector mPreferredLocales; - std::map, std::shared_ptr> mCache; + std::map, std::shared_ptr, std::less<>> mCache; std::function mGmstLoader; }; diff --git a/components/l10n/messagebundles.cpp b/components/l10n/messagebundles.cpp index e8a56a9bb3..665704e30d 100644 --- a/components/l10n/messagebundles.cpp +++ b/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 + 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& 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& 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(), - parseError)) + if (checkSuccess(status, parseError, "Failed to create message ", key, " for locale ", lang.getName())) { - 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 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& argNames, const std::vector& args) const { @@ -158,7 +160,7 @@ namespace L10n message->format(argNames.data(), args.data(), static_cast(args.size()), result, success); else message->format(nullptr, nullptr, static_cast(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(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(args.size()), result, success); else defaultMessage.format(nullptr, nullptr, static_cast(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; } diff --git a/components/l10n/messagebundles.hpp b/components/l10n/messagebundles.hpp index f142d3f2ac..15ccbaa630 100644 --- a/components/l10n/messagebundles.hpp +++ b/components/l10n/messagebundles.hpp @@ -10,6 +10,8 @@ #include #include +#include + namespace L10n { /** @@ -41,18 +43,23 @@ namespace L10n void setPreferredLocales(const std::vector& preferredLocales); const std::vector& 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 fn) { mGmstLoader = std::move(fn); } private: + template + using StringMap = std::unordered_map>; // icu::Locale isn't hashable (or comparable), so we use the string form instead, which is canonicalized - std::unordered_map> mBundles; + StringMap> mBundles; const icu::Locale mFallbackLocale; std::vector mPreferredLocaleStrings; std::vector mPreferredLocales; std::function 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; }; } diff --git a/components/lua/l10n.cpp b/components/lua/l10n.cpp index 8153efd5b5..856ab8a808 100644 --- a/components/lua/l10n.cpp +++ b/components/lua/l10n.cpp @@ -66,7 +66,7 @@ namespace LuaUtil }; return sol::make_object( - lua, [manager](const std::string& contextName, sol::optional fallbackLocale) { + lua, [manager](std::string_view contextName, sol::optional fallbackLocale) { if (fallbackLocale) return L10nContext{ manager->getContext(contextName, *fallbackLocale) }; else From 75845bf8633998532beeadf61573919b02db0003 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Wed, 30 Jul 2025 19:41:14 +0200 Subject: [PATCH 2/2] Work around compiler versions that haven't addressed defect 3865 --- components/l10n/manager.cpp | 4 ++-- components/l10n/manager.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/l10n/manager.cpp b/components/l10n/manager.cpp index b176e4aa7a..1f75c5b073 100644 --- a/components/l10n/manager.cpp +++ b/components/l10n/manager.cpp @@ -31,7 +31,7 @@ namespace L10n msg << " " << l.getName(); } for (auto& [key, context] : mCache) - updateContext(key.first, *context); + updateContext(std::get<0>(key), *context); } void Manager::readLangData(std::string_view name, MessageBundles& ctx, const icu::Locale& lang) @@ -91,7 +91,7 @@ namespace L10n std::shared_ptr Manager::getContext( std::string_view contextName, const std::string& fallbackLocaleName) { - std::pair key(contextName, fallbackLocaleName); + std::tuple key(contextName, fallbackLocaleName); auto it = mCache.find(key); if (it != mCache.end()) return it->second; diff --git a/components/l10n/manager.hpp b/components/l10n/manager.hpp index 7022057178..89a9bd4b05 100644 --- a/components/l10n/manager.hpp +++ b/components/l10n/manager.hpp @@ -40,7 +40,7 @@ namespace L10n const VFS::Manager* mVFS; std::vector mPreferredLocales; - std::map, std::shared_ptr, std::less<>> mCache; + std::map, std::shared_ptr, std::less<>> mCache; std::function mGmstLoader; };