From 2ac4392dffc5eb4108ef372ce2374f48a4fa3976 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sun, 6 Feb 2022 23:11:24 +0000 Subject: [PATCH] Add cppcoreguidelines-explicit-virtual-functions clang-tidy check Fix the warnings it emits --- CMakeLists.txt | 1 + src/api/api_database.h | 34 ++++++++++--------- src/api/game/game.h | 24 +++++++------ src/api/plugin.h | 24 ++++++------- .../api/interface/api_game_operations_test.h | 2 +- .../api/interface/create_game_handle_test.h | 4 +-- .../api/interface/database_interface_test.h | 2 +- src/tests/api/internals/metadata_list_test.h | 2 +- src/tests/api/internals/plugin_test.h | 28 ++++++++------- 9 files changed, 64 insertions(+), 57 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index eeffafdf..8bc72442 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -487,6 +487,7 @@ if(RUN_CLANG_TIDY) set(CLANG_TIDY_COMMON_CHECKS "cppcoreguidelines-avoid-c-arrays" "cppcoreguidelines-c-copy-assignment-signature" + "cppcoreguidelines-explicit-virtual-functions" "cppcoreguidelines-interfaces-global-init" "cppcoreguidelines-macro-usage" "cppcoreguidelines-narrowing-conventions" diff --git a/src/api/api_database.h b/src/api/api_database.h index cabe0a83..d3548d6f 100644 --- a/src/api/api_database.h +++ b/src/api/api_database.h @@ -41,41 +41,43 @@ namespace loot { struct ApiDatabase final : public DatabaseInterface { explicit ApiDatabase(std::shared_ptr conditionEvaluator); - void LoadLists(const std::filesystem::path& masterlist_path, - const std::filesystem::path& userlist_path = "", - const std::filesystem::path& masterlist_prelude_path = ""); + void LoadLists( + const std::filesystem::path& masterlist_path, + const std::filesystem::path& userlist_path = "", + const std::filesystem::path& masterlist_prelude_path = "") override; void WriteUserMetadata(const std::filesystem::path& outputFile, - const bool overwrite) const; + const bool overwrite) const override; void WriteMinimalList(const std::filesystem::path& outputFile, - const bool overwrite) const; + const bool overwrite) const override; - std::vector GetKnownBashTags() const; + std::vector GetKnownBashTags() const override; std::vector GetGeneralMessages( - bool evaluateConditions = false) const; + bool evaluateConditions = false) const override; - std::vector GetGroups(bool includeUserMetadata = true) const; - std::vector GetUserGroups() const; + std::vector GetGroups(bool includeUserMetadata = true) const override; + std::vector GetUserGroups() const override; void SetUserGroups(const std::vector& groups); - std::vector GetGroupsPath(const std::string& fromGroupName, - const std::string& toGroupName) const; + std::vector GetGroupsPath( + const std::string& fromGroupName, + const std::string& toGroupName) const override; std::optional GetPluginMetadata( const std::string& plugin, bool includeUserMetadata = true, - bool evaluateConditions = false) const; + bool evaluateConditions = false) const override; std::optional GetPluginUserMetadata( const std::string& plugin, - bool evaluateConditions = false) const; + bool evaluateConditions = false) const override; - void SetPluginUserMetadata(const PluginMetadata& pluginMetadata); + void SetPluginUserMetadata(const PluginMetadata& pluginMetadata) override; - void DiscardPluginUserMetadata(const std::string& plugin); + void DiscardPluginUserMetadata(const std::string& plugin) override; - void DiscardAllUserMetadata(); + void DiscardAllUserMetadata() override; private: std::shared_ptr conditionEvaluator_; diff --git a/src/api/game/game.h b/src/api/game/game.h index 2164576e..60f8d1d7 100644 --- a/src/api/game/game.h +++ b/src/api/game/game.h @@ -52,29 +52,31 @@ public: // Game Interface Methods // //////////////////////////// - std::shared_ptr GetDatabase(); + std::shared_ptr GetDatabase() override; - bool IsValidPlugin(const std::string& plugin) const; + bool IsValidPlugin(const std::string& plugin) const override; void LoadPlugins(const std::vector& plugins, - bool loadHeadersOnly); + bool loadHeadersOnly) override; std::shared_ptr GetPlugin( - const std::string& pluginName) const; + const std::string& pluginName) const override; - std::vector> GetLoadedPlugins() const; + std::vector> GetLoadedPlugins() + const override; - void IdentifyMainMasterFile(const std::string& masterFile); + void IdentifyMainMasterFile(const std::string& masterFile) override; - std::vector SortPlugins(const std::vector& plugins); + std::vector SortPlugins( + const std::vector& plugins) override; - void LoadCurrentLoadOrderState(); + void LoadCurrentLoadOrderState() override; - bool IsPluginActive(const std::string& pluginName) const; + bool IsPluginActive(const std::string& pluginName) const override; - std::vector GetLoadOrder() const; + std::vector GetLoadOrder() const override; - void SetLoadOrder(const std::vector& loadOrder); + void SetLoadOrder(const std::vector& loadOrder) override; private: void CacheArchives(); diff --git a/src/api/plugin.h b/src/api/plugin.h index 9c3d8fc0..21675a00 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -47,21 +47,21 @@ public: std::filesystem::path pluginPath, const bool headerOnly); - std::string GetName() const; - std::optional GetHeaderVersion() const; - std::optional GetVersion() const; - std::vector GetMasters() const; - std::vector GetBashTags() const; - std::optional GetCRC() const; + std::string GetName() const override; + std::optional GetHeaderVersion() const override; + std::optional GetVersion() const override; + std::vector GetMasters() const override; + std::vector GetBashTags() const override; + std::optional GetCRC() const override; - bool IsMaster() const; + bool IsMaster() const override; - bool IsLightPlugin() const; + bool IsLightPlugin() const override; - bool IsValidAsLightPlugin() const; - bool IsEmpty() const; - bool LoadsArchive() const; - bool DoFormIDsOverlap(const PluginInterface& plugin) const; + bool IsValidAsLightPlugin() const override; + bool IsEmpty() const override; + bool LoadsArchive() const override; + bool DoFormIDsOverlap(const PluginInterface& plugin) const override; size_t GetOverlapSize( const std::vector> plugins) const; diff --git a/src/tests/api/interface/api_game_operations_test.h b/src/tests/api/interface/api_game_operations_test.h index 7b84a996..cb57e7a8 100644 --- a/src/tests/api/interface/api_game_operations_test.h +++ b/src/tests/api/interface/api_game_operations_test.h @@ -46,7 +46,7 @@ protected: errorMessage("Obsolete. Remove this and install Enhanced Weather."), generalMasterlistMessage("A general masterlist message.") {} - virtual void SetUp() { + virtual void SetUp() override { CommonGameTestFixture::SetUp(); ASSERT_FALSE(std::filesystem::exists(masterlistPath)); diff --git a/src/tests/api/interface/create_game_handle_test.h b/src/tests/api/interface/create_game_handle_test.h index b17a7d06..ce863122 100644 --- a/src/tests/api/interface/create_game_handle_test.h +++ b/src/tests/api/interface/create_game_handle_test.h @@ -42,7 +42,7 @@ protected: localPathJunctionLink(localPath.string() + ".junction"), originalWorkingDirectory(std::filesystem::current_path()) {} - void SetUp() { + void SetUp() override { using std::filesystem::file_type; using std::filesystem::status; CommonGameTestFixture::SetUp(); @@ -70,7 +70,7 @@ protected: std::filesystem::current_path(dataPath.parent_path().parent_path()); } - void TearDown() { std::filesystem::current_path(originalWorkingDirectory); } + void TearDown() override { std::filesystem::current_path(originalWorkingDirectory); } std::shared_ptr handle_; diff --git a/src/tests/api/interface/database_interface_test.h b/src/tests/api/interface/database_interface_test.h index 05328a7a..55968e30 100644 --- a/src/tests/api/interface/database_interface_test.h +++ b/src/tests/api/interface/database_interface_test.h @@ -41,7 +41,7 @@ protected: generalUserlistMessage("A general userlist message."), db_(nullptr) {} - void SetUp() { + void SetUp() override { ApiGameOperationsTest::SetUp(); db_ = handle_->GetDatabase(); diff --git a/src/tests/api/internals/metadata_list_test.h b/src/tests/api/internals/metadata_list_test.h index 9390fac8..4484de3d 100644 --- a/src/tests/api/internals/metadata_list_test.h +++ b/src/tests/api/internals/metadata_list_test.h @@ -40,7 +40,7 @@ protected: {metadataFilesPath / "invalid" / "non_map_root.yaml", metadataFilesPath / "invalid" / "non_unique.yaml"}) {} - inline virtual void SetUp() { + inline virtual void SetUp() override { CommonGameTestFixture::SetUp(); using std::filesystem::copy; diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index c4e3b8dc..5489bbae 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -43,7 +43,7 @@ protected: GetArchiveFileExtension(GetParam())), game_(GetParam(), dataPath.parent_path(), localPath) {} - void SetUp() { + void SetUp() override { CommonGameTestFixture::SetUp(); game_.LoadCurrentLoadOrderState(); @@ -138,21 +138,23 @@ private: class OtherPluginType final : public PluginInterface { public: - std::string GetName() const { return ""; } - std::optional GetHeaderVersion() const { return 0.0f; } - std::optional GetVersion() const { return std::nullopt; } - std::vector GetMasters() const { + std::string GetName() const override { return ""; } + std::optional GetHeaderVersion() const override { return 0.0f; } + std::optional GetVersion() const override { + return std::nullopt; + } + std::vector GetMasters() const override { return std::vector(); } - std::vector GetBashTags() const { return std::vector(); } - std::optional GetCRC() const { return std::nullopt; } + std::vector GetBashTags() const override { return std::vector(); } + std::optional GetCRC() const override { return std::nullopt; } - bool IsMaster() const { return false; } - bool IsLightPlugin() const { return false; } - bool IsValidAsLightPlugin() const { return false; } - bool IsEmpty() const { return false; } - bool LoadsArchive() const { return false; } - bool DoFormIDsOverlap(const PluginInterface&) const { return true; } + bool IsMaster() const override { return false; } + bool IsLightPlugin() const override { return false; } + bool IsValidAsLightPlugin() const override { return false; } + bool IsEmpty() const override { return false; } + bool LoadsArchive() const override { return false; } + bool DoFormIDsOverlap(const PluginInterface&) const override { return true; } }; // Pass an empty first argument, as it's a prefix for the test instantation,