From 9b1b0f5b079f1ac75f16f9439911826bf6143759 Mon Sep 17 00:00:00 2001 From: Norm Evangelista Date: Sun, 22 Jan 2023 12:51:07 -0800 Subject: [PATCH] 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 | 17 ++++++++--------- include/SFML/Audio/SoundSource.hpp | 2 ++ src/SFML/Audio/SoundFileReaderOgg.cpp | 2 +- src/SFML/Audio/SoundSource.cpp | 2 ++ src/SFML/System/Vector2.cpp | 6 +++--- src/SFML/Window/DRM/DRMContext.cpp | 4 ++-- src/SFML/Window/DRM/InputImplUDev.cpp | 11 ++++------- src/SFML/Window/GlContext.cpp | 2 +- src/SFML/Window/GlContext.hpp | 2 +- src/SFML/Window/OSX/HIDInputManager.hpp | 2 +- src/SFML/Window/OSX/HIDInputManager.mm | 6 +++--- src/SFML/Window/Unix/ClipboardImpl.cpp | 1 + src/SFML/Window/Unix/WindowImplX11.cpp | 11 ++++------- src/SFML/Window/Win32/JoystickImpl.cpp | 11 ++++------- src/SFML/Window/WindowImpl.cpp | 2 +- src/SFML/Window/WindowImpl.hpp | 2 +- 16 files changed, 39 insertions(+), 44 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index dc725a092..259be062f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -3,13 +3,7 @@ Checks: > clang-analyzer-*, modernize-*, portability-*, - readability-container-size-empty, - readability-identifier-naming, - readability-isolate-declaration, - readability-qualified-auto, - readability-redundant-access-specifiers, - readability-redundant-*, - readability-simplify-*, + readability-*, -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-implicit-bool-conversion + -readability-named-parameter, + -readability-uppercase-literal-suffix, CheckOptions: - { key: readability-identifier-naming.ClassCase, value: CamelCase } diff --git a/include/SFML/Audio/SoundSource.hpp b/include/SFML/Audio/SoundSource.hpp index c9d7561b1..72f16daf7 100644 --- a/include/SFML/Audio/SoundSource.hpp +++ b/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 diff --git a/src/SFML/Audio/SoundFileReaderOgg.cpp b/src/SFML/Audio/SoundFileReaderOgg.cpp index daf30e630..b9c6c0a11 100644 --- a/src/SFML/Audio/SoundFileReaderOgg.cpp +++ b/src/SFML/Audio/SoundFileReaderOgg.cpp @@ -64,7 +64,7 @@ long tell(void* data) return static_cast(stream->tell()); } -static ov_callbacks callbacks = {&read, &seek, nullptr, &tell}; +ov_callbacks callbacks = {&read, &seek, nullptr, &tell}; } // namespace namespace sf::priv diff --git a/src/SFML/Audio/SoundSource.cpp b/src/SFML/Audio/SoundSource.cpp index 8fa877ecf..93a547e08 100644 --- a/src/SFML/Audio/SoundSource.cpp +++ b/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 diff --git a/src/SFML/System/Vector2.cpp b/src/SFML/System/Vector2.cpp index 99dab6b6b..53b1a8d69 100644 --- a/src/SFML/System/Vector2.cpp +++ b/src/SFML/System/Vector2.cpp @@ -67,13 +67,13 @@ Angle Vector2::angle() const //////////////////////////////////////////////////////////// template -Vector2 Vector2::rotatedBy(Angle angle) const +Vector2 Vector2::rotatedBy(Angle phi) const { static_assert(std::is_floating_point_v, "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(angle.asRadians())); - T sin = std::sin(static_cast(angle.asRadians())); + T cos = std::cos(static_cast(phi.asRadians())); + T sin = std::sin(static_cast(phi.asRadians())); // Don't manipulate x and y separately, otherwise they're overwritten too early return Vector2(cos * x - sin * y, sin * x + cos * y); diff --git a/src/SFML/Window/DRM/DRMContext.cpp b/src/SFML/Window/DRM/DRMContext.cpp index 630115d4a..6a014f0ad 100644 --- a/src/SFML/Window/DRM/DRMContext.cpp +++ b/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(data); *temp = 0; } -static bool waitForFlip(int timeout) +bool waitForFlip(int timeout) { while (waitingForFlip) { diff --git a/src/SFML/Window/DRM/InputImplUDev.cpp b/src/SFML/Window/DRM/InputImplUDev.cpp index 01612f1ac..bc0382c25 100644 --- a/src/SFML/Window/DRM/InputImplUDev.cpp +++ b/src/SFML/Window/DRM/InputImplUDev.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -664,13 +665,9 @@ void InputImpl::setMousePosition(const Vector2i& position, const WindowBase& /*r //////////////////////////////////////////////////////////// bool InputImpl::isTouchDown(unsigned int finger) { - for (const auto& slot : touchSlots) - { - if (slot.id == static_cast(finger)) - return true; - } - - return false; + return std::any_of(touchSlots.cbegin(), + touchSlots.cend(), + [finger](const TouchSlot& slot) { return slot.id == static_cast(finger); }); } diff --git a/src/SFML/Window/GlContext.cpp b/src/SFML/Window/GlContext.cpp index 4e4e66db1..7ce9c0827 100644 --- a/src/SFML/Window/GlContext.cpp +++ b/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 diff --git a/src/SFML/Window/GlContext.hpp b/src/SFML/Window/GlContext.hpp index 58ef13be4..96f1c8981 100644 --- a/src/SFML/Window/GlContext.hpp +++ b/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 diff --git a/src/SFML/Window/OSX/HIDInputManager.hpp b/src/SFML/Window/OSX/HIDInputManager.hpp index 6fd0c1a23..0e6a4a805 100644 --- a/src/SFML/Window/OSX/HIDInputManager.hpp +++ b/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 diff --git a/src/SFML/Window/OSX/HIDInputManager.mm b/src/SFML/Window/OSX/HIDInputManager.mm index 989a2467b..633f0603c 100644 --- a/src/SFML/Window/OSX/HIDInputManager.mm +++ b/src/SFML/Window/OSX/HIDInputManager.mm @@ -34,8 +34,8 @@ namespace { -static const std::uint8_t unknownVirtualCode = 0xff; -static const bool isIsoKeyboard = (KBGetLayoutType(LMGetKbdType()) == kKeyboardISO); +const std::uint8_t unknownVirtualCode = 0xff; +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 */) diff --git a/src/SFML/Window/Unix/ClipboardImpl.cpp b/src/SFML/Window/Unix/ClipboardImpl.cpp index f66dc8c7c..1ac66c17b 100644 --- a/src/SFML/Window/Unix/ClipboardImpl.cpp +++ b/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 diff --git a/src/SFML/Window/Unix/WindowImplX11.cpp b/src/SFML/Window/Unix/WindowImplX11.cpp index a743b78e1..0f3666ae7 100644 --- a/src/SFML/Window/Unix/WindowImplX11.cpp +++ b/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) - { - if (name == windowManagerName) - return true; - } - - return false; + return std::any_of(std::begin(wmAbsPosGood), + std::end(wmAbsPosGood), + [&](const sf::String& name) { return name == windowManagerName; }); } } // namespace WindowsImplX11Impl } // namespace diff --git a/src/SFML/Window/Win32/JoystickImpl.cpp b/src/SFML/Window/Win32/JoystickImpl.cpp index b5f52618b..9ff28fe84 100644 --- a/src/SFML/Window/Win32/JoystickImpl.cpp +++ b/src/SFML/Window/Win32/JoystickImpl.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -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) - { - if (record.index == index) - return true; - } - - return false; + return std::any_of(joystickList.cbegin(), + joystickList.cend(), + [index](const JoystickRecord& record) { return record.index == index; }); } diff --git a/src/SFML/Window/WindowImpl.cpp b/src/SFML/Window/WindowImpl.cpp index d5b8f36ed..6db218ba3 100644 --- a/src/SFML/Window/WindowImpl.cpp +++ b/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) diff --git a/src/SFML/Window/WindowImpl.hpp b/src/SFML/Window/WindowImpl.hpp index e41f3a1bf..ba0c88dbb 100644 --- a/src/SFML/Window/WindowImpl.hpp +++ b/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: ////////////////////////////////////////////////////////////