Add extra & pedantic warnings and fix the code (#4491)

* Remove discarded qualifiers in return types

* Add more warnings and fix underlying enums

.clang-format

@@ -23,6 +23,7 @@ IncludeCategories:
     Priority:        2
 IndentWidth:     4
 SpacesBeforeTrailingComments: 1
+Standard:        c++20
 TabWidth:        4
 UseTab:          ForIndentation
 ...

CMakeLists.txt

@@ -16,7 +16,7 @@ project(tfs CXX)
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 
 if (NOT WIN32)
-    add_compile_options(-Wall -Werror -pipe -fvisibility=hidden)
+    add_compile_options(-Wall -Wextra -Wnon-virtual-dtor -pedantic -Werror -pipe -fvisibility=hidden)
 endif ()
 
 if (CMAKE_COMPILER_IS_GNUCXX)

src/enums.h

@@ -413,10 +413,7 @@ enum PlayerSex_t : uint8_t
 	PLAYERSEX_LAST = PLAYERSEX_MALE
 };
 
-enum Vocation_t : uint16_t
-{
-	VOCATION_NONE = 0
-};
+inline constexpr uint16_t VOCATION_NONE = 0;
 
 enum ReturnValue
 {

src/iomapserialize.cpp

@@ -318,15 +318,17 @@ bool IOMapSerialize::saveHouseInfo()
 
 		std::string listText;
 		if (house->getAccessList(GUEST_LIST, listText) && !listText.empty()) {
-			if (!stmt.addRow(fmt::format("{:d}, {}, {:s}", house->getId(), GUEST_LIST, db.escapeString(listText)))) {
+			if (!stmt.addRow(fmt::format("{:d}, {:d}, {:s}", house->getId(), tfs::to_underlying(GUEST_LIST),
+			                             db.escapeString(listText)))) {
 				return false;
 			}
 
 			listText.clear();
 		}
 
 		if (house->getAccessList(SUBOWNER_LIST, listText) && !listText.empty()) {
-			if (!stmt.addRow(fmt::format("{:d}, {}, {:s}", house->getId(), SUBOWNER_LIST, db.escapeString(listText)))) {
+			if (!stmt.addRow(fmt::format("{:d}, {:d}, {:s}", house->getId(), tfs::to_underlying(SUBOWNER_LIST),
+			                             db.escapeString(listText)))) {
 				return false;
 			}
 

src/iomarket.cpp

@@ -21,7 +21,7 @@ MarketOfferList IOMarket::getActiveOffers(MarketAction_t action, uint16_t itemId
 
 	DBResult_ptr result = Database::getInstance().storeQuery(fmt::format(
 	    "SELECT `id`, `amount`, `price`, `created`, `anonymous`, (SELECT `name` FROM `players` WHERE `id` = `player_id`) AS `player_name` FROM `market_offers` WHERE `sale` = {:d} AND `itemtype` = {:d}",
-	    action, itemId));
+	    tfs::to_underlying(action), itemId));
 	if (!result) {
 		return offerList;
 	}
@@ -53,7 +53,7 @@ MarketOfferList IOMarket::getOwnOffers(MarketAction_t action, uint32_t playerId)
 
 	DBResult_ptr result = Database::getInstance().storeQuery(fmt::format(
 	    "SELECT `id`, `amount`, `price`, `created`, `itemtype` FROM `market_offers` WHERE `player_id` = {:d} AND `sale` = {:d}",
-	    playerId, action));
+	    playerId, tfs::to_underlying(action)));
 	if (!result) {
 		return offerList;
 	}
@@ -76,7 +76,7 @@ HistoryMarketOfferList IOMarket::getOwnHistory(MarketAction_t action, uint32_t p
 
 	DBResult_ptr result = Database::getInstance().storeQuery(fmt::format(
 	    "SELECT `itemtype`, `amount`, `price`, `expires_at`, `state` FROM `market_history` WHERE `player_id` = {:d} AND `sale` = {:d}",
-	    playerId, action));
+	    playerId, tfs::to_underlying(action)));
 	if (!result) {
 		return offerList;
 	}
@@ -242,7 +242,7 @@ void IOMarket::createOffer(uint32_t playerId, MarketAction_t action, uint32_t it
 {
 	Database::getInstance().executeQuery(fmt::format(
 	    "INSERT INTO `market_offers` (`player_id`, `sale`, `itemtype`, `amount`, `price`, `created`, `anonymous`) VALUES ({:d}, {:d}, {:d}, {:d}, {:d}, {:d}, {:d})",
-	    playerId, action, itemId, amount, price, time(nullptr), anonymous));
+	    playerId, tfs::to_underlying(action), itemId, amount, price, time(nullptr), anonymous));
 }
 
 void IOMarket::acceptOffer(uint32_t offerId, uint16_t amount)
@@ -256,12 +256,13 @@ void IOMarket::deleteOffer(uint32_t offerId)
 	Database::getInstance().executeQuery(fmt::format("DELETE FROM `market_offers` WHERE `id` = {:d}", offerId));
 }
 
-void IOMarket::appendHistory(uint32_t playerId, MarketAction_t type, uint16_t itemId, uint16_t amount, uint64_t price,
+void IOMarket::appendHistory(uint32_t playerId, MarketAction_t action, uint16_t itemId, uint16_t amount, uint64_t price,
                              time_t timestamp, MarketOfferState_t state)
 {
 	g_databaseTasks.addTask(fmt::format(
 	    "INSERT INTO `market_history` (`player_id`, `sale`, `itemtype`, `amount`, `price`, `expires_at`, `inserted`, `state`) VALUES ({:d}, {:d}, {:d}, {:d}, {:d}, {:d}, {:d}, {:d})",
-	    playerId, type, itemId, amount, price, timestamp, time(nullptr), state));
+	    playerId, tfs::to_underlying(action), itemId, amount, price, timestamp, time(nullptr),
+	    tfs::to_underlying(state)));
 }
 
 bool IOMarket::moveOfferToHistory(uint32_t offerId, MarketOfferState_t state)
@@ -292,7 +293,7 @@ void IOMarket::updateStatistics()
 {
 	DBResult_ptr result = Database::getInstance().storeQuery(fmt::format(
 	    "SELECT `sale` AS `sale`, `itemtype` AS `itemtype`, COUNT(`price`) AS `num`, MIN(`price`) AS `min`, MAX(`price`) AS `max`, SUM(`price`) AS `sum` FROM `market_history` WHERE `state` = {:d} GROUP BY `itemtype`, `sale`",
-	    OFFERSTATE_ACCEPTED));
+	    tfs::to_underlying(OFFERSTATE_ACCEPTED)));
 	if (!result) {
 		return;
 	}

src/podium.h

@@ -31,7 +31,7 @@ class Podium final : public Item
 	}
 	void setFlags(uint8_t newFlags) { flags = newFlags; }
 
-	const Direction getDirection() const { return direction; }
+	Direction getDirection() const { return direction; }
 	void setDirection(Direction newDirection) { direction = newDirection; }
 
 protected:

src/server.h

@@ -10,6 +10,8 @@
 class ServiceBase
 {
 public:
+	virtual ~ServiceBase() = default;
+
 	virtual bool is_single_socket() const = 0;
 	virtual bool is_checksummed() const = 0;
 	virtual uint8_t get_protocol_identifier() const = 0;

src/spells.h

@@ -146,9 +146,9 @@ class Spell : public BaseSpell
 		return !vocationSpellMap.empty() && vocationSpellMap.find(vocationId) != vocationSpellMap.end();
 	}
 
-	const SpellGroup_t getGroup() const { return group; }
+	SpellGroup_t getGroup() const { return group; }
 	void setGroup(SpellGroup_t g) { group = g; }
-	const SpellGroup_t getSecondaryGroup() const { return secondaryGroup; }
+	SpellGroup_t getSecondaryGroup() const { return secondaryGroup; }
 	void setSecondaryGroup(SpellGroup_t g) { secondaryGroup = g; }
 
 	uint32_t getCooldown() const { return cooldown; }

src/tools.h

@@ -72,4 +72,18 @@ int64_t OTSYS_TIME();
 
 SpellGroup_t stringToSpellGroup(const std::string& value);
 
+namespace tfs {
+
+#if __has_cpp_attribute(__cpp_lib_to_underlying)
+
+inline constexpr auto to_underlying(auto e) noexcept { return std::to_underlying(e); }
+
+#else
+
+inline constexpr auto to_underlying(auto e) noexcept { return static_cast<std::underlying_type_t<decltype(e)>>(e); }
+
+#endif
+
+} // namespace tfs
+
 #endif // FS_TOOLS_H