Streamlined readability-* clang-tidy checks

Addressed CI complaints

Addressed review comments, CI issues

Reverted files exempted from linting

Fixed missing comma

Fixed clang-format escape

Disabled spurious readability-non-const-parameter

Addressed review comment

Moved NOLINT inside namespace per review

Remove const from function argument decls


Fixed rebase and formatting issues


Fixed macOS CI issues

.clang-tidy

@@ -3,13 +3,7 @@ Checks: >
   clang-analyzer-*,
   modernize-*,
   portability-*,
-  readability-container-size-empty,
+  readability-*,
-  readability-identifier-naming,
-  readability-isolate-declaration,
-  readability-qualified-auto,
-  readability-redundant-access-specifiers,
-  readability-redundant-*,
-  readability-simplify-*,
   -clang-analyzer-core.NonNullParamChecker,
   -clang-analyzer-core.NullDereference,
   -clang-analyzer-nullability.NullablePassedToNonnull,
@@ -28,10 +22,15 @@ Checks: >
   -modernize-use-trailing-return-type,
   -readability-braces-around-statements,
   -readability-container-contains,
+  -readability-convert-member-functions-to-static,
+  -readability-else-after-return,
   -readability-function-cognitive-complexity,
+  -readability-function-size,
+  -readability-identifier-length,
+  -readability-implicit-bool-conversion,
   -readability-magic-numbers,
-  -readability-make-member-function-const,
+  -readability-named-parameter,
-  -readability-implicit-bool-conversion
+  -readability-uppercase-literal-suffix,
 
 CheckOptions:
   - { key: readability-identifier-naming.ClassCase,           value: CamelCase }

include/SFML/Audio/SoundSource.hpp

@@ -35,6 +35,7 @@
 
 namespace sf
 {
+// NOLINTBEGIN(readability-make-member-function-const)
 ////////////////////////////////////////////////////////////
 /// \brief Base class defining a sound's properties
 ///
@@ -291,6 +292,7 @@ protected:
     unsigned int m_source; //!< OpenAL source identifier
 };
 
+// NOLINTEND(readability-make-member-function-const)
 } // namespace sf
 
 

src/SFML/Audio/SoundFileReaderOgg.cpp

@@ -64,7 +64,7 @@ long tell(void* data)
     return static_cast<long>(stream->tell());
 }
 
-static ov_callbacks callbacks = {&read, &seek, nullptr, &tell};
+ov_callbacks callbacks = {&read, &seek, nullptr, &tell};
 } // namespace
 
 namespace sf::priv

src/SFML/Audio/SoundSource.cpp

@@ -34,6 +34,7 @@
 
 namespace sf
 {
+// NOLINTBEGIN(readability-make-member-function-const)
 ////////////////////////////////////////////////////////////
 SoundSource::SoundSource()
 {
@@ -204,5 +205,6 @@ SoundSource::Status SoundSource::getStatus() const
 
     return Stopped;
 }
+// NOLINTEND(readability-make-member-function-const)
 
 } // namespace sf

src/SFML/System/Vector2.cpp

@@ -67,13 +67,13 @@ Angle Vector2<T>::angle() const
 
 ////////////////////////////////////////////////////////////
 template <typename T>
-Vector2<T> Vector2<T>::rotatedBy(Angle angle) const
+Vector2<T> Vector2<T>::rotatedBy(Angle phi) const
 {
     static_assert(std::is_floating_point_v<T>, "Vector2::rotatedBy() is only supported for floating point types");
 
     // No zero vector assert, because rotating a zero vector is well-defined (yields always itself)
-    T cos = std::cos(static_cast<T>(angle.asRadians()));
+    T cos = std::cos(static_cast<T>(phi.asRadians()));
-    T sin = std::sin(static_cast<T>(angle.asRadians()));
+    T sin = std::sin(static_cast<T>(phi.asRadians()));
 
     // Don't manipulate x and y separately, otherwise they're overwritten too early
     return Vector2<T>(cos * x - sin * y, sin * x + cos * y);

src/SFML/Window/DRM/DRMContext.cpp

@@ -64,13 +64,13 @@ int             contextCount   = 0;
 EGLDisplay      display        = EGL_NO_DISPLAY;
 int             waitingForFlip = 0;
 
-static void pageFlipHandler(int /* fd */, unsigned int /* frame */, unsigned int /* sec */, unsigned int /* usec */, void* data)
+void pageFlipHandler(int /* fd */, unsigned int /* frame */, unsigned int /* sec */, unsigned int /* usec */, void* data)
 {
     int* temp = static_cast<int*>(data);
     *temp     = 0;
 }
 
-static bool waitForFlip(int timeout)
+bool waitForFlip(int timeout)
 {
     while (waitingForFlip)
     {

src/SFML/Window/DRM/InputImplUDev.cpp

@@ -28,6 +28,7 @@
 #include <SFML/System/Err.hpp>
 #include <SFML/Window/DRM/InputImplUDev.hpp>
 
+#include <algorithm>
 #include <cerrno>
 #include <cstdio>
 #include <cstdlib>
@@ -664,13 +665,9 @@ void InputImpl::setMousePosition(const Vector2i& position, const WindowBase& /*r
 ////////////////////////////////////////////////////////////
 bool InputImpl::isTouchDown(unsigned int finger)
 {
-    for (const auto& slot : touchSlots)
+    return std::any_of(touchSlots.cbegin(),
-    {
+                       touchSlots.cend(),
-        if (slot.id == static_cast<int>(finger))
+                       [finger](const TouchSlot& slot) { return slot.id == static_cast<int>(finger); });
-            return true;
-    }
-
-    return false;
 }
 
 

src/SFML/Window/GlContext.cpp

@@ -913,7 +913,7 @@ void GlContext::initialize(const ContextSettings& requestedSettings)
 
 
 ////////////////////////////////////////////////////////////
-void GlContext::checkSettings(const ContextSettings& requestedSettings)
+void GlContext::checkSettings(const ContextSettings& requestedSettings) const
 {
     // Perform checks to inform the user if they are getting a context they might not have expected
 

src/SFML/Window/GlContext.hpp

@@ -311,7 +311,7 @@ private:
     /// \param requestedSettings Requested settings during context creation
     ///
     ////////////////////////////////////////////////////////////
-    void checkSettings(const ContextSettings& requestedSettings);
+    void checkSettings(const ContextSettings& requestedSettings) const;
 
     ////////////////////////////////////////////////////////////
     // Member data

src/SFML/Window/OSX/HIDInputManager.hpp

@@ -257,7 +257,7 @@ private:
     /// \see isKeyPressed, isMouseButtonPressed
     ///
     ////////////////////////////////////////////////////////////
-    bool isPressed(IOHIDElements& elements);
+    bool isPressed(IOHIDElements& elements) const;
 
     ////////////////////////////////////////////////////////////
     /// \brief Convert a HID key usage to its corresponding scancode

src/SFML/Window/OSX/HIDInputManager.mm

@@ -34,8 +34,8 @@
 
 namespace
 {
-static const std::uint8_t unknownVirtualCode = 0xff;
+const std::uint8_t unknownVirtualCode = 0xff;
-static const bool         isIsoKeyboard      = (KBGetLayoutType(LMGetKbdType()) == kKeyboardISO);
+const bool         isIsoKeyboard      = (KBGetLayoutType(LMGetKbdType()) == kKeyboardISO);
 }
 
 namespace sf::priv
@@ -962,7 +962,7 @@ CFSetRef HIDInputManager::copyDevices(std::uint32_t page, std::uint32_t usage)
 
 
 ////////////////////////////////////////////////////////////
-bool HIDInputManager::isPressed(IOHIDElements& elements)
+bool HIDInputManager::isPressed(IOHIDElements& elements) const
 {
     bool pressed = false;
     for (auto it = elements.begin(); it != elements.end() && !pressed; /* noop */)

src/SFML/Window/Unix/ClipboardImpl.cpp

@@ -40,6 +40,7 @@
 namespace
 {
 // Filter the events received by windows (only allow those matching a specific window)
+// NOLINTNEXTLINE(readability-non-const-parameter)
 Bool checkEvent(::Display*, XEvent* event, XPointer userData)
 {
     // Just check if the event matches the window

src/SFML/Window/Unix/WindowImplX11.cpp

@@ -92,6 +92,7 @@ constexpr unsigned long eventMask = FocusChangeMask | ButtonPressMask | ButtonRe
 constexpr unsigned int maxTrialsCount = 5;
 
 // Filter the events received by windows (only allow those matching a specific window)
+// NOLINTNEXTLINE(readability-non-const-parameter)
 Bool checkEvent(::Display*, XEvent* event, XPointer userData)
 {
     // Just check if the event matches the window
@@ -350,13 +351,9 @@ bool isWMAbsolutePositionGood()
     if (!ewmhSupported())
         return false;
 
-    for (const sf::String& name : wmAbsPosGood)
+    return std::any_of(std::begin(wmAbsPosGood),
-    {
+                       std::end(wmAbsPosGood),
-        if (name == windowManagerName)
+                       [&](const sf::String& name) { return name == windowManagerName; });
-            return true;
-    }
-
-    return false;
 }
 } // namespace WindowsImplX11Impl
 } // namespace

src/SFML/Window/Win32/JoystickImpl.cpp

@@ -31,6 +31,7 @@
 #include <SFML/System/Win32/WindowsHeader.hpp>
 #include <SFML/Window/JoystickImpl.hpp>
 
+#include <algorithm>
 #include <cmath>
 #include <cstring>
 #include <iomanip>
@@ -473,13 +474,9 @@ void JoystickImpl::cleanupDInput()
 bool JoystickImpl::isConnectedDInput(unsigned int index)
 {
     // Check if a joystick with the given index is in the connected list
-    for (const JoystickRecord& record : joystickList)
+    return std::any_of(joystickList.cbegin(),
-    {
+                       joystickList.cend(),
-        if (record.index == index)
+                       [index](const JoystickRecord& record) { return record.index == index; });
-            return true;
-    }
-
-    return false;
 }
 
 

src/SFML/Window/WindowImpl.cpp

@@ -295,7 +295,7 @@ void WindowImpl::processSensorEvents()
 ////////////////////////////////////////////////////////////
 bool WindowImpl::createVulkanSurface([[maybe_unused]] const VkInstance&            instance,
                                      [[maybe_unused]] VkSurfaceKHR&                surface,
-                                     [[maybe_unused]] const VkAllocationCallbacks* allocator)
+                                     [[maybe_unused]] const VkAllocationCallbacks* allocator) const
 {
 #if defined(SFML_VULKAN_IMPLEMENTATION_NOT_AVAILABLE)
 

src/SFML/Window/WindowImpl.hpp

@@ -248,7 +248,7 @@ public:
     /// \return True if surface creation was successful, false otherwise
     ///
     ////////////////////////////////////////////////////////////
-    bool createVulkanSurface(const VkInstance& instance, VkSurfaceKHR& surface, const VkAllocationCallbacks* allocator);
+    bool createVulkanSurface(const VkInstance& instance, VkSurfaceKHR& surface, const VkAllocationCallbacks* allocator) const;
 
 protected:
     ////////////////////////////////////////////////////////////