Fix clang tidy on ci

.clang-tidy

@@ -12,28 +12,58 @@ Checks: >-
   modernize-*,
   performance-*,
   readability-*,
+  -bugprone-assignment-in-if-condition,
+  -bugprone-casting-through-void,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-empty-catch,
+  -bugprone-implicit-widening-of-multiplication-result,
   -bugprone-narrowing-conversions,
+  -bugprone-optional-value-conversion,
+  -bugprone-reserved-identifier,
+  -bugprone-switch-missing-default-case,
   -bugprone-too-small-loop-variable,
+  -bugprone-unchecked-optional-access,
+  -bugprone-unhandled-exception-at-new,
+  -clang-analyzer-core.NonNullParamChecker,
+  -clang-analyzer-cplusplus.NewDeleteLeaks,
+  -clang-analyzer-optin.core.EnumCastOutOfRange,
   -clang-analyzer-osx.*,
+  -misc-confusable-identifiers,
+  -misc-const-correctness,
+  -misc-include-cleaner,
   -misc-non-private-member-variables-in-classes,
+  -misc-no-recursion,
   -misc-unused-parameters,
+  -misc-use-anonymous-namespace,
   -modernize-avoid-c-arrays,
+  -modernize-macro-to-enum,
   -modernize-raw-string-literal,
   -modernize-return-braced-init-list,
   -modernize-use-default-member-init,
   -modernize-use-emplace,
   -modernize-use-nodiscard,
   -modernize-use-override,
   -modernize-use-trailing-return-type,
+  -performance-avoid-endl,
+  -performance-enum-size,
+  -performance-move-const-arg,
+  -performance-no-int-to-ptr,
+  -readability-avoid-nested-conditional-operator,
+  -readability-avoid-return-with-void-value,
   -readability-braces-around-statements,
   -readability-convert-member-functions-to-static,
   -readability-else-after-return,
+  -readability-function-cognitive-complexity,
+  -readability-identifier-length,
   -readability-implicit-bool-conversion,
   -readability-isolate-declaration,
   -readability-magic-numbers,
   -readability-named-parameter,
+  -readability-redundant-casting,
   -readability-static-accessed-through-instance,
+  -readability-suspicious-call-argument,
   -readability-uppercase-literal-suffix,
+  -readability-use-anyofallof,
   -readability-identifier-naming,
   -clang-diagnostic-inconsistent-missing-override
 CheckOptions:
@@ -43,5 +73,8 @@ CheckOptions:
     value: 'Point<.*?>;MapPoint;Position;Extent;DrawPoint'
   - key: performance-unnecessary-value-param.AllowedTypes
     value: 'Point<.*?>;MapPoint;Position;Extent;DrawPoint'
-AnalyzeTemporaryDtors: false
+  - key: readability-function-cognitive-complexity.IgnoreMacros
+    value: true
+  - key: readability-simplify-boolean-expr.IgnoreMacros
+    value: true
 ...

extras/audioDrivers/SDL/AudioSDL.cpp

@@ -142,7 +142,7 @@ driver::RawSoundHandle AudioSDL::LoadEffect(const std::string& filepath)
 
 driver::RawSoundHandle AudioSDL::LoadEffect(const std::vector<char>& data, const std::string& /*ext*/)
 {
-    SDL_RWops* rwOps = SDL_RWFromConstMem(&data[0], static_cast<int>(data.size()));
+    SDL_RWops* rwOps = SDL_RWFromConstMem(data.data(), static_cast<int>(data.size()));
     Mix_Chunk* sound = Mix_LoadWAV_RW(rwOps, true); //-V601
 
     if(sound == nullptr)

libs/common/include/Rect.h

@@ -20,8 +20,11 @@ template<typename T>
 struct RectBase
 {
     using position_type = Point<T>;
+    // Deactivated to bug in clang-tidy
+    // NOLINTBEGIN(modernize-type-traits)
     using extent_elem_type =
       typename std::conditional_t<std::is_integral_v<T>, std::make_unsigned<T>, std::common_type<T>>::type;
+    // NOLINTEND(modernize-type-traits)
     using extent_type = Point<extent_elem_type>;
     T left, top, right, bottom;
     constexpr RectBase() : RectBase(position_type::all(0), extent_type::all(0)) {}

libs/common/include/helpers/serializeContainers.h

@@ -35,8 +35,11 @@ namespace detail {
     void pushContainer(Serializer& ser, const T& container, long)
     {
         using Type = typename T::value_type;
+        // Deactivated to bug in clang-tidy
+        // NOLINTBEGIN(modernize-type-traits)
         using Integral =
           typename std::conditional_t<std::is_enum_v<Type>, std::underlying_type<Type>, std::common_type<Type>>::type;
+        // NOLINTEND(modernize-type-traits)
         for(const auto el : container)
         {
             // Cast also required for bool vector -.-

libs/s25main/Debug.cpp

@@ -132,10 +132,10 @@ bool captureBacktrace(DebugInfo::stacktrace_t& stacktrace, LPCONTEXT ctx) noexce
     return true;
 }
 #else
-bool captureBacktrace(DebugInfo::stacktrace_t& stacktrace, void*) noexcept
+bool captureBacktrace(DebugInfo::stacktrace_t& stacktrace, void*) noexcept(false)
 {
 #    if RTTR_BACKTRACE_HAS_FUNCTION
-    const auto num_frames = backtrace(&stacktrace[0], stacktrace.size());
+    const auto num_frames = backtrace(stacktrace.data(), stacktrace.size());
     stacktrace.resize(num_frames);
     return true;
 #    else
@@ -182,7 +182,7 @@ DebugInfo::~DebugInfo()
     sock.Close();
 }
 
-DebugInfo::stacktrace_t DebugInfo::GetStackTrace(void* ctx) noexcept
+DebugInfo::stacktrace_t DebugInfo::GetStackTrace(void* ctx) noexcept(false)
 {
 #ifndef RTTR_CONTEXT_PTR_TYPE
     using RTTR_CONTEXT_PTR_TYPE = void*;
@@ -267,7 +267,7 @@ bool DebugInfo::SendStackTrace(const stacktrace_t& stacktrace)
         endStacktrace.push_back(reinterpret_cast<littleVoid_t::value_type>(ptr));
 
     unsigned stacktraceLen = sizeof(littleVoid_t) * endStacktrace.size();
-    return SendString(reinterpret_cast<const char*>(&endStacktrace[0]), stacktraceLen);
+    return SendString(reinterpret_cast<const char*>(endStacktrace.data()), stacktraceLen);
 }
 
 bool DebugInfo::SendReplay()

libs/s25main/Debug.h

@@ -21,7 +21,7 @@ class DebugInfo
     DebugInfo();
     ~DebugInfo();
 
-    static stacktrace_t GetStackTrace(void* ctx = nullptr) noexcept;
+    static stacktrace_t GetStackTrace(void* ctx = nullptr) noexcept(false);
 
     bool Send(const void* buffer, size_t length);
     bool SendSigned(int32_t i);

libs/s25main/GameCommands.h

@@ -447,7 +447,7 @@ class SetInventorySetting : public Coords
 
 protected:
     SetInventorySetting(const MapPoint pt, boost_variant2<GoodType, Job> what, const InventorySetting state)
-        : Coords(GCType::SetInventorySetting, pt), what(std::move(what)), state(state)
+        : Coords(GCType::SetInventorySetting, pt), what(what), state(state)
     {}
     SetInventorySetting(Serializer& ser) : Coords(GCType::SetInventorySetting, ser)
 
@@ -806,7 +806,7 @@ class TradeOverLand : public Coords
 protected:
     /// Note: Can only trade wares or figures!
     TradeOverLand(const MapPoint pt, boost_variant2<GoodType, Job> what, const uint32_t count)
-        : Coords(GCType::Trade, pt), what(std::move(what)), count(count)
+        : Coords(GCType::Trade, pt), what(what), count(count)
     {}
     TradeOverLand(Serializer& ser) : Coords(GCType::Trade, ser)
     {

libs/s25main/GamePlayer.cpp

@@ -160,7 +160,7 @@ void GamePlayer::Serialize(SerializedGameData& sgd) const
     sgd.PushEnum<uint8_t>(ps);
 
     // Nur richtige Spieler serialisieren
-    if(!(ps == PlayerState::Occupied || ps == PlayerState::AI))
+    if(ps != PlayerState::Occupied && ps != PlayerState::AI)
         return;
 
     sgd.PushBool(isDefeated);
@@ -234,7 +234,7 @@ void GamePlayer::Deserialize(SerializedGameData& sgd)
     // Ehemaligen PS auslesen
     auto origin_ps = sgd.Pop<PlayerState>();
     // Nur richtige Spieler serialisieren
-    if(!(origin_ps == PlayerState::Occupied || origin_ps == PlayerState::AI))
+    if(origin_ps != PlayerState::Occupied && origin_ps != PlayerState::AI)
         return;
 
     isDefeated = sgd.PopBool();
@@ -614,14 +614,14 @@ bool GamePlayer::FindCarrierForRoad(RoadSegment* rs) const
     {
         // dann braucht man Träger UND Boot
         best[0] = FindWarehouse(*rs->GetF1(), FW::HasWareAndFigure(GoodType::Boat, Job::Helper, false), false, false,
-                                &length[0], rs);
+                                length.data(), rs);
         // 2. Flagge des Weges
         best[1] = FindWarehouse(*rs->GetF2(), FW::HasWareAndFigure(GoodType::Boat, Job::Helper, false), false, false,
                                 &length[1], rs);
     } else
     {
         // 1. Flagge des Weges
-        best[0] = FindWarehouse(*rs->GetF1(), FW::HasFigure(Job::Helper, false), false, false, &length[0], rs);
+        best[0] = FindWarehouse(*rs->GetF1(), FW::HasFigure(Job::Helper, false), false, false, length.data(), rs);
         // 2. Flagge des Weges
         best[1] = FindWarehouse(*rs->GetF2(), FW::HasFigure(Job::Helper, false), false, false, &length[1], rs);
     }
@@ -878,7 +878,7 @@ nofCarrier* GamePlayer::OrderDonkey(RoadSegment* road) const
     std::array<nobBaseWarehouse*, 2> best;
 
     // 1. Flagge des Weges
-    best[0] = FindWarehouse(*road->GetF1(), FW::HasFigure(Job::PackDonkey, false), false, false, &length[0], road);
+    best[0] = FindWarehouse(*road->GetF1(), FW::HasFigure(Job::PackDonkey, false), false, false, length.data(), road);
     // 2. Flagge des Weges
     best[1] = FindWarehouse(*road->GetF2(), FW::HasFigure(Job::PackDonkey, false), false, false, &length[1], road);
 

libs/s25main/Replay.cpp

@@ -155,11 +155,11 @@ bool Replay::StartRecording(const boost::filesystem::path& filepath, const MapIn
             RTTR_Assert(!mapInfo.savegame);
             file_.WriteUnsignedInt(mapInfo.mapData.uncompressedLength);
             file_.WriteUnsignedInt(mapInfo.mapData.data.size());
-            file_.WriteRawData(&mapInfo.mapData.data[0], mapInfo.mapData.data.size());
+            file_.WriteRawData(mapInfo.mapData.data.data(), mapInfo.mapData.data.size());
             file_.WriteUnsignedInt(mapInfo.luaData.uncompressedLength);
             file_.WriteUnsignedInt(mapInfo.luaData.data.size());
             if(!mapInfo.luaData.data.empty())
-                file_.WriteRawData(&mapInfo.luaData.data[0], mapInfo.luaData.data.size());
+                file_.WriteRawData(mapInfo.luaData.data.data(), mapInfo.luaData.data.size());
             break;
         case MapType::Savegame: mapInfo.savegame->Save(file_, GetMapName()); break;
     }
@@ -269,11 +269,11 @@ bool Replay::LoadGameData(MapInfo& mapInfo)
             case MapType::OldMap:
                 mapInfo.mapData.uncompressedLength = file_.ReadUnsignedInt();
                 mapInfo.mapData.data.resize(file_.ReadUnsignedInt());
-                file_.ReadRawData(&mapInfo.mapData.data[0], mapInfo.mapData.data.size());
+                file_.ReadRawData(mapInfo.mapData.data.data(), mapInfo.mapData.data.size());
                 mapInfo.luaData.uncompressedLength = file_.ReadUnsignedInt();
                 mapInfo.luaData.data.resize(file_.ReadUnsignedInt());
                 if(!mapInfo.luaData.data.empty())
-                    file_.ReadRawData(&mapInfo.luaData.data[0], mapInfo.luaData.data.size());
+                    file_.ReadRawData(mapInfo.luaData.data.data(), mapInfo.luaData.data.size());
                 break;
             case MapType::Savegame:
                 mapInfo.savegame = std::make_unique<Savegame>();

libs/s25main/SavedFile.cpp

@@ -38,7 +38,7 @@ void SavedFile::WriteExtHeader(BinaryFile& file, const std::string& mapName)
     mapName_ = mapName;
 
     // Program version
-    file.WriteRawData(&revision[0], revision.size());
+    file.WriteRawData(revision.data(), revision.size());
     s25util::time64_t tmpTime = libendian::ConvertEndianess<false>::fromNative(saveTime_);
     file.WriteRawData(&tmpTime, sizeof(tmpTime));
     file.WriteShortString(mapName);
@@ -57,7 +57,7 @@ bool SavedFile::ReadFileHeader(BinaryFile& file)
         throw std::range_error("Program signature is to long!");
     try
     {
-        file.ReadRawData(&read_signature[0], signature.size());
+        file.ReadRawData(read_signature.data(), signature.size());
 
         // Signatur überprüfen
         if(!std::equal(signature.begin(), signature.end(), read_signature.begin()))
@@ -88,7 +88,7 @@ bool SavedFile::ReadFileHeader(BinaryFile& file)
 
 bool SavedFile::ReadExtHeader(BinaryFile& file)
 {
-    file.ReadRawData(&revision[0], revision.size());
+    file.ReadRawData(revision.data(), revision.size());
     file.ReadRawData(&saveTime_, sizeof(saveTime_));
     saveTime_ = libendian::ConvertEndianess<false>::toNative(saveTime_);
     mapName_ = file.ReadShortString();

libs/s25main/Ware.h

@@ -99,7 +99,7 @@ class Ware : public GameObject
     noRoadNode* GetLocation() { return location; }
     const noRoadNode* GetLocation() const { return location; }
     /// Ist die Ware eine LostWare (Ware, die kein Ziel mehr hat und irgendwo sinnlos rumliegt)?
-    bool IsLostWare() const { return ((goal ? false : true) && state != State::OnShip); }
+    bool IsLostWare() const { return ((goal == nullptr) && state != State::OnShip); }
     /// Informiert Ware, dass eine Schiffsreise beginnt
     void StartShipJourney();
     /// Informiert Ware, dass Schiffsreise beendet ist und die Ware nun in einem Hafengebäude liegt

libs/s25main/ai/aijh/Jobs.h

@@ -57,7 +57,7 @@ class JobWithTarget
 {
 public:
     JobWithTarget() : target(MapPoint::Invalid()) {}
-    inline MapPoint GetTarget() const { return target; }
+    MapPoint GetTarget() const { return target; }
     void SetTarget(MapPoint newTarget) { target = newTarget; }
 
 protected:
@@ -72,8 +72,8 @@ class BuildJob : public AIJob, public JobWithTarget
     {}
 
     void ExecuteJob() override;
-    inline BuildingType GetType() const { return type; }
-    inline MapPoint GetAround() const { return around; }
+    BuildingType GetType() const { return type; }
+    MapPoint GetAround() const { return around; }
 
 private:
     BuildingType type;

libs/s25main/buildings/nobHarborBuilding.cpp

@@ -1255,7 +1255,7 @@ void nobHarborBuilding::CancelSeaAttacker(nofAttacker* attacker)
 unsigned nobHarborBuilding::CalcDistributionPoints(const GoodType type) const
 {
     // Ist überhaupt eine Expedition im Gang und ein entsprechender Warentyp
-    if(!expedition.active || !(type == GoodType::Boards || type == GoodType::Stones))
+    if(!expedition.active || (type != GoodType::Boards && type != GoodType::Stones))
         return 0;
 
     unsigned ordered_boards = 0, ordered_stones = 0;

libs/s25main/controls/ctrlChat.cpp

@@ -117,7 +117,7 @@ void ctrlChat::Draw_()
     for(unsigned i = 0; i < show_lines; ++i)
     {
         DrawPoint curTextPos = textPos;
-        if(PrimaryChatLine* line = get_if<PrimaryChatLine>(&chat_lines[i + pos]))
+        if(auto* line = get_if<PrimaryChatLine>(&chat_lines[i + pos]))
         {
             // Zeit, Spieler und danach Textnachricht
             if(!line->time_string.empty())
@@ -154,8 +154,8 @@ void ctrlChat::WrapLine(unsigned short i)
 
     // Breite von Zeitstring und Spielername berechnen (falls vorhanden)
     unsigned short prefix_width =
-      (line.time_string.length() ? font->getWidth(line.time_string) : 0)
-      + (line.player.length() ? (bracket1_size + bracket2_size + font->getWidth(line.player)) : 0);
+      (!line.time_string.empty() ? font->getWidth(line.time_string) : 0)
+      + (!line.player.empty() ? (bracket1_size + bracket2_size + font->getWidth(line.player)) : 0);
 
     // Reicht die Breite des Textfeldes noch nichtmal dafür aus?
     if(prefix_width > GetSize().x - 2 - SCROLLBAR_WIDTH)

libs/s25main/controls/ctrlEdit.cpp

@@ -34,7 +34,7 @@ void ctrlEdit::SetText(const std::string& text)
 {
     text_ = s25util::utf8to32(text);
     if(numberOnly_)
-        helpers::erase_if(text_, [](char32_t c) { return !(c >= '0' && c <= '9'); });
+        helpers::erase_if(text_, [](char32_t c) { return c < '0' || c > '9'; });
     if(maxLength_ > 0 && text_.size() > maxLength_)
         text_.resize(maxLength_);
 
@@ -168,7 +168,7 @@ void ctrlEdit::Draw_()
 void ctrlEdit::AddChar(char32_t c)
 {
     // Number-only text fields accept numbers only ;)
-    if(numberOnly_ && !(c >= '0' && c <= '9'))
+    if(numberOnly_ && (c < '0' || c > '9'))
         return;
 
     if(maxLength_ > 0 && text_.size() >= maxLength_)
@@ -184,12 +184,12 @@ void ctrlEdit::AddChar(char32_t c)
  */
 void ctrlEdit::RemoveChar()
 {
-    if(cursorPos_ > 0 && text_.length() > 0)
+    if(cursorPos_ > 0 && !text_.empty())
     {
         text_.erase(cursorPos_ - 1, 1);
 
         // View verschieben
-        while(text_.length() > 0 && text_.length() <= viewStart_)
+        while(!text_.empty() && text_.length() <= viewStart_)
             --viewStart_;
 
         CursorLeft();

libs/s25main/controls/ctrlProgress.cpp

@@ -32,9 +32,9 @@ ctrlProgress::ctrlProgress(Window* parent, const unsigned id, const DrawPoint& p
 
     Extent btSize = Extent::all(size.y);
     AddImageButton(0, DrawPoint(0, 0), btSize, tc, LOADER.GetImageN(btMinusRes, button_minus),
-                   (button_minus_tooltip.length() ? button_minus_tooltip : _("Less")));
+                   (!button_minus_tooltip.empty() ? button_minus_tooltip : _("Less")));
     AddImageButton(1, DrawPoint(size.x - btSize.x, 0), btSize, tc, LOADER.GetImageN(btPlusRes, button_plus),
-                   (button_plus_tooltip.length() ? button_plus_tooltip : _("More")));
+                   (!button_plus_tooltip.empty() ? button_plus_tooltip : _("More")));
 
     // Hide left and right 3D border by making the buttons overlap the bar
     padding_.x -= 2;

libs/s25main/desktops/dskCredits.cpp

@@ -28,7 +28,6 @@ using namespace std::chrono_literals;
 const auto PAGE_TIME = 12900ms;
 /// Duration for fading between pages
 const auto FADING_TIME = 2s;
-using std::chrono::duration_cast;
 
 namespace {
 enum