Enabled more clang-tidy checks (#439)

* Enabled more clang-tidy checks * Enabled more clang-tidy checks * Enabled more clang-tidy checks * Enabled more clang-tidy checks * Enabled more clang-tidy checks
mikekazakov · Oct 24, 2024 · 7c0b60b · 7c0b60b
1 parent 1ed0a4a
commit 7c0b60b
Show file tree

Hide file tree

Showing 219 changed files with 818 additions and 893 deletions.
diff --git a/Source/.clang-tidy b/Source/.clang-tidy
@@ -111,37 +111,37 @@ Checks: >
   readability-implicit-bool-conversion,
   readability-inconsistent-declaration-parameter-name,
   readability-isolate-declaration,
-  # readability-magic-numbers,
-  # readability-make-member-function-const,
-  # readability-math-missing-parentheses,
-  # readability-misleading-indentation,
-  # readability-misplaced-array-index,
-  # readability-named-parameter,
-  # readability-non-const-parameter,
-  # readability-operators-representation,
-  # readability-qualified-auto,
-  # readability-redundant-access-specifiers,
-  # readability-redundant-casting,
-  # readability-redundant-control-flow,
-  # readability-redundant-declaration,
-  # readability-redundant-function-ptr-dereference,
-  # readability-redundant-inline-specifier,
-  # readability-redundant-member-init,
-  # readability-redundant-preprocessor,
-  # readability-redundant-smartptr-get,
-  # readability-redundant-string-cstr,
-  # readability-redundant-string-init,
-  # readability-reference-to-constructed-temporary,
-  # readability-simplify-boolean-expr,
-  # readability-simplify-subscript-expr,
+  # readability-magic-numbers,            <-- large
+  readability-make-member-function-const,
+  readability-math-missing-parentheses,
+  readability-misleading-indentation,
+  readability-misplaced-array-index,
+  readability-named-parameter,
+  # readability-non-const-parameter,      <-- would be nice some day, but that's massive
+  # readability-operators-representation, <-- not sure about the benefits
+  # readability-qualified-auto,           <-- would be nice some day, but that's massive
+  readability-redundant-access-specifiers,
+  readability-redundant-casting,
+  readability-redundant-control-flow,
+  readability-redundant-declaration,
+  readability-redundant-function-ptr-dereference,
+  readability-redundant-inline-specifier,
+  readability-redundant-member-init,
+  readability-redundant-preprocessor,
+  readability-redundant-smartptr-get,
+  readability-redundant-string-cstr,
+  readability-redundant-string-init,
+  readability-reference-to-constructed-temporary,
+  readability-simplify-boolean-expr,
+  readability-simplify-subscript-expr,
   readability-static-accessed-through-instance,
-  # readability-static-definition-in-anonymous-namespace,
-  # readability-string-compare,
-  # readability-suspicious-call-argument,
-  # readability-uniqueptr-delete-release,
-  # readability-uppercase-literal-suffix,
-  # readability-use-anyofallof,
-  # readability-use-std-min-max,
+  readability-static-definition-in-anonymous-namespace,
+  readability-string-compare,
+  readability-suspicious-call-argument,
+  readability-uniqueptr-delete-release,
+  # readability-uppercase-literal-suffix, <-- controversial
+  readability-use-anyofallof,
+  readability-use-std-min-max,
 
 CheckOptions:
   - key:    modernize-loop-convert.MinConfidence

diff --git a/Source/Base/include/Base/StringsBulk.h b/Source/Base/include/Base/StringsBulk.h
@@ -74,7 +74,7 @@ class StringsBulk::Iterator
     Iterator operator--(int) noexcept;
     Iterator &operator+=(long) noexcept;
     Iterator &operator-=(long) noexcept;
-    long operator-(const Iterator &) noexcept;
+    long operator-(const Iterator &) const noexcept;
 
     bool operator==(const Iterator &) const noexcept;
     bool operator!=(const Iterator &) const noexcept;

diff --git a/Source/Base/source/CFDefaultsCPP.cpp b/Source/Base/source/CFDefaultsCPP.cpp
@@ -89,7 +89,7 @@ std::optional<bool> CFDefaultsGetOptionalBool(CFStringRef _key) noexcept
     const Boolean v = CFPreferencesGetAppBooleanValue(_key, kCFPreferencesCurrentApplication, &has);
     if( !has )
         return {};
-    return v ? true : false;
+    return v != 0;
 }
 
 void CFDefaultsSetBool(CFStringRef _key, bool _value) noexcept

diff --git a/Source/Base/source/CFStackAllocator.cpp b/Source/Base/source/CFStackAllocator.cpp
@@ -28,7 +28,7 @@ CFAllocatorRef CFStackAllocator::Construct() noexcept
     return CFAllocatorCreate(kCFAllocatorUseContext, &context);
 }
 
-void *CFStackAllocator::DoAlloc(CFIndex _alloc_size, CFOptionFlags, void *_info) noexcept
+void *CFStackAllocator::DoAlloc(CFIndex _alloc_size, CFOptionFlags /*_hint*/, void *_info) noexcept
 {
     assert(_alloc_size >= 0);
     constexpr long alignment = 16;

diff --git a/Source/Base/source/StringsBulk.cpp b/Source/Base/source/StringsBulk.cpp
@@ -16,18 +16,18 @@ struct StringsBulk::Ctrl {
     // offsets: count * 4 bytes
     // null-terminated strings
 
-    [[nodiscard]] inline const uint32_t *Offsets() const noexcept
+    [[nodiscard]] const uint32_t *Offsets() const noexcept
     {
         const auto raw = reinterpret_cast<const char *>(this);
         return reinterpret_cast<const uint32_t *>(raw + sizeof(Ctrl));
     }
 
-    [[nodiscard]] inline const char *Get(size_t _index) const noexcept
+    [[nodiscard]] const char *Get(size_t _index) const noexcept
     {
         return reinterpret_cast<const char *>(this) + Offsets()[_index];
     }
 
-    [[nodiscard]] inline size_t StrLen(size_t _index) const noexcept
+    [[nodiscard]] size_t StrLen(size_t _index) const noexcept
     {
         if( _index + 1 < count )
             return Offsets()[_index + 1] - Offsets()[_index] - 1;
@@ -189,7 +189,7 @@ StringsBulk::Ctrl *StringsBulk::Allocate(size_t _number_of_strings, size_t _tota
 {
     assert(_number_of_strings != 0);
 
-    const size_t bytes = sizeof(Ctrl) + sizeof(uint32_t) * _number_of_strings + _total_chars;
+    const size_t bytes = sizeof(Ctrl) + (sizeof(uint32_t) * _number_of_strings) + _total_chars;
 
     const auto ctrl = reinterpret_cast<Ctrl *>(malloc(bytes));
     if( ctrl == nullptr )
@@ -349,7 +349,7 @@ StringsBulk::Iterator &StringsBulk::Iterator::operator-=(long _d) noexcept
     return *this;
 }
 
-long StringsBulk::Iterator::operator-(const Iterator &_rhs) noexcept
+long StringsBulk::Iterator::operator-(const Iterator &_rhs) const noexcept
 {
     return long(m_Index) - long(_rhs.m_Index);
 }
@@ -400,7 +400,7 @@ StringsBulk StringsBulk::NonOwningBuilder::Build() const
 
     const auto ctrl = StringsBulk::Allocate(strings_num, total_chars);
     auto offsets = reinterpret_cast<uint32_t *>(reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl));
-    char *storage = reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl) + sizeof(uint32_t) * strings_num;
+    char *storage = reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl) + (sizeof(uint32_t) * strings_num);
     for( size_t index = 0; index < strings_num; ++index ) {
         offsets[index] = uint32_t(storage - reinterpret_cast<char *>(ctrl));
         const auto string_bytes = m_Strings[index].length();
@@ -448,7 +448,7 @@ StringsBulk StringsBulk::Builder::Build() const
 
     const auto ctrl = StringsBulk::Allocate(strings_num, total_chars);
     auto offsets = reinterpret_cast<uint32_t *>(reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl));
-    char *storage = reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl) + sizeof(uint32_t) * strings_num;
+    char *storage = reinterpret_cast<char *>(ctrl) + sizeof(StringsBulk::Ctrl) + (sizeof(uint32_t) * strings_num);
     for( size_t index = 0; index < strings_num; ++index ) {
         offsets[index] = uint32_t(storage - reinterpret_cast<char *>(ctrl));
         const auto string_bytes = m_Strings[index].length() + 1;

diff --git a/Source/Base/source/algo.cpp b/Source/Base/source/algo.cpp
@@ -110,7 +110,7 @@ std::vector<std::string> SplitByDelimiters(std::string_view _str, std::string_vi
     std::string next;
     for( auto c : _str ) {
         if( _delims.contains(c) ) {
-            if( !next.empty() || _compress == false ) {
+            if( !next.empty() || !_compress ) {
                 res.emplace_back(std::move(next));
                 next = {};
             }
@@ -120,7 +120,7 @@ std::vector<std::string> SplitByDelimiters(std::string_view _str, std::string_vi
         }
     }
 
-    if( !next.empty() || (_compress == false && !_str.empty()) ) {
+    if( !next.empty() || (!_compress && !_str.empty()) ) {
         res.emplace_back(std::move(next));
     }
 
@@ -133,7 +133,7 @@ std::vector<std::string> SplitByDelimiter(std::string_view _str, char _delim, bo
     std::string next;
     for( auto c : _str ) {
         if( c == _delim ) {
-            if( !next.empty() || _compress == false ) {
+            if( !next.empty() || !_compress ) {
                 res.emplace_back(std::move(next));
                 next = {};
             }
@@ -143,7 +143,7 @@ std::vector<std::string> SplitByDelimiter(std::string_view _str, char _delim, bo
         }
     }
 
-    if( !next.empty() || (_compress == false && !_str.empty()) ) {
+    if( !next.empty() || (!_compress && !_str.empty()) ) {
         res.emplace_back(std::move(next));
     }
 

diff --git a/Source/Base/tests/StringsBulk_UT.cpp b/Source/Base/tests/StringsBulk_UT.cpp
@@ -60,7 +60,7 @@ TEST_CASE(PREFIX "random strings")
         const auto l = rand() % 1000;
         string s(l, ' ');
         for( int j = 0; j < l; ++j )
-            s[j] = static_cast<unsigned char>(j % 255 + 1);
+            s[j] = static_cast<unsigned char>((j % 255) + 1);
         v.emplace_back(s);
     }
     StringsBulk::Builder sbb;
@@ -87,7 +87,7 @@ TEST_CASE(PREFIX "non-owning builder")
         const auto l = rand() % 1000;
         string s(l, ' ');
         for( int j = 0; j < l; ++j )
-            s[j] = static_cast<unsigned char>(j % 255 + 1);
+            s[j] = static_cast<unsigned char>((j % 255) + 1);
         v.emplace_back(s);
     }
     StringsBulk::NonOwningBuilder sbb;

diff --git a/Source/CUI/source/CommandPopover.mm b/Source/CUI/source/CommandPopover.mm
@@ -1036,7 +1036,7 @@ - (void)showRelativeToRect:(NSRect)_positioning_rect
         else if( _alignment == NCCommandPopoverAlignment::Right )
             return NSMaxX(rect_on_screen) - sx;
         else
-            return NSMinX(rect_on_screen) + (rect_on_screen.size.width - sx) / 2.;
+            return NSMinX(rect_on_screen) + ((rect_on_screen.size.width - sx) / 2.);
     }();
     const double y = NSMinY(rect_on_screen) - m_ContentSize.height;
     NSScreen *screen = _positioning_view.window.screen;

diff --git a/Source/CUI/source/ProcessSheetController.mm b/Source/CUI/source/ProcessSheetController.mm
@@ -92,7 +92,7 @@ - (void)Show
 {
     // consider using modal dialog here.
 
-    if( m_Running == true )
+    if( m_Running )
         return;
     dispatch_to_main_queue_after(g_ShowDelay, [=] {
         if( m_ClientClosed )
@@ -110,7 +110,7 @@ - (void)Close
 
 - (void)Discard
 {
-    if( m_Running == false )
+    if( !m_Running )
         return;
 
     dispatch_to_main_queue([=] { [self.window close]; });

diff --git a/Source/Config/source/ConfigImpl.cpp b/Source/Config/source/ConfigImpl.cpp
@@ -280,7 +280,7 @@ void ConfigImpl::SetInternal(std::string_view _path, const Value &_value)
     if( _path.empty() )
         return;
 
-    if( ReplaceOrInsert(_path, _value) == true ) {
+    if( ReplaceOrInsert(_path, _value) ) {
         FireObservers(_path);
         MarkDirty();
     }
@@ -401,7 +401,7 @@ void ConfigImpl::FireObservers(std::string_view _path) const
     if( const auto observers = FindObservers(_path) ) {
         for( const auto &observer : observers->observers ) {
             const auto lock = std::lock_guard{observer->lock};
-            if( observer->was_removed == false ) {
+            if( !observer->was_removed ) {
                 observer->callback();
             }
         }
@@ -419,7 +419,7 @@ base::intrusive_ptr<const ConfigImpl::Observers> ConfigImpl::FindObservers(std::
 
 void ConfigImpl::MarkDirty()
 {
-    if( m_WriteScheduled.test_and_set() == false ) {
+    if( !m_WriteScheduled.test_and_set() ) {
         m_OverwritesDumpExecutor->Execute([this] { WriteOverwrites(); });
     }
 }
@@ -456,14 +456,14 @@ void ConfigImpl::ResetToDefaults()
 
 void ConfigImpl::Commit()
 {
-    if( m_WriteScheduled.test_and_set() == true ) {
+    if( m_WriteScheduled.test_and_set() ) {
         WriteOverwrites();
     }
 }
 
 void ConfigImpl::OverwritesDidChange()
 {
-    if( m_ReadScheduled.test_and_set() == false ) {
+    if( !m_ReadScheduled.test_and_set() ) {
         m_OverwritesReloadExecutor->Execute([this] { ReloadOverwrites(); });
     }
 }

diff --git a/Source/NimbleCommander/NimbleCommander/Bootstrap/AppDelegate+MainWindowCreation.mm b/Source/NimbleCommander/NimbleCommander/Bootstrap/AppDelegate+MainWindowCreation.mm
@@ -337,7 +337,7 @@ static bool RestoreFilePanelStateFromLastOpenedWindow(MainWindowFilePanelState *
                                             VFSHost &_host)
 {
     // at this moment we (thankfully) care only about sanboxed versions
-    if( nc::base::AmISandboxed() == false )
+    if( !nc::base::AmISandboxed() )
         return true;
 
     if( _host.IsNativeFS() )
@@ -350,7 +350,7 @@ static bool RestoreFilePanelStateFromLastOpenedWindow(MainWindowFilePanelState *
                                                     const std::string &_directory_path,
                                                     VFSHost &_host)
 {
-    if( nc::base::AmISandboxed() == false )
+    if( !nc::base::AmISandboxed() )
         return true;
 
     if( _host.IsNativeFS() )

diff --git a/Source/NimbleCommander/NimbleCommander/Bootstrap/AppDelegate.mm b/Source/NimbleCommander/NimbleCommander/Bootstrap/AppDelegate.mm
@@ -824,7 +824,7 @@ - (IBAction)onMainMenuPerformShowFavorites:(id) [[maybe_unused]] _sender
         apt->SetProgressCallback([](double _progress) { g_Me.dock.SetProgress(_progress); });
         return apt;
     }();
-    return *apt.get();
+    return *apt;
 }
 
 - (nc::core::Dock &)dock
@@ -880,7 +880,7 @@ - (NCMainWindowController *)windowForExternalRevealRequest
 static void DoTemporaryFileStoragePurge()
 {
     assert(g_TemporaryFileStorage != nullptr);
-    const auto deadline = time(nullptr) - 60 * 60 * 24; // 24 hours back
+    const auto deadline = time(nullptr) - (60 * 60 * 24); // 24 hours back
     g_TemporaryFileStorage->Purge(deadline);
 
     dispatch_after(6h, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0), DoTemporaryFileStoragePurge);

diff --git a/Source/NimbleCommander/NimbleCommander/Bootstrap/Interactions.mm b/Source/NimbleCommander/NimbleCommander/Bootstrap/Interactions.mm
@@ -19,9 +19,7 @@ bool AskUserToResetDefaults()
     [alert addButtonWithTitle:NSLocalizedString(@"OK", "")];
     [alert addButtonWithTitle:NSLocalizedString(@"Cancel", "")];
     [alert.buttons objectAtIndex:0].keyEquivalent = @"";
-    if( [alert runModal] == NSAlertFirstButtonReturn )
-        return true;
-    return false;
+    return [alert runModal] == NSAlertFirstButtonReturn;
 }
 
 bool AskToExitWithRunningOperations()

diff --git a/Source/NimbleCommander/NimbleCommander/Core/ActionsShortcutsManager.mm b/Source/NimbleCommander/NimbleCommander/Core/ActionsShortcutsManager.mm
@@ -423,7 +423,7 @@
 // clang-format on
 
 template <size_t size, typename T, size_t... indexes>
-static constexpr auto make_array_n_impl(T &&value, std::index_sequence<indexes...>)
+static constexpr auto make_array_n_impl(T &&value, std::index_sequence<indexes...> /*unused*/)
 {
     return std::array<std::decay_t<T>, size>{(static_cast<void>(indexes), value)..., std::forward<T>(value)};
 }

diff --git a/Source/NimbleCommander/NimbleCommander/Core/SandboxManager.mm b/Source/NimbleCommander/NimbleCommander/Core/SandboxManager.mm
@@ -225,10 +225,7 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
             return true;
 
     // special treating for /Volumes dir - can browse it by default, but not dirs inside it
-    if( p == "/Volumes" )
-        return true;
-
-    return false;
+    return p == "/Volumes";
 }
 
 bool SandboxManager::CanAccessFolder(const std::string &_path) const
@@ -270,7 +267,5 @@ - (BOOL)panel:(id) [[maybe_unused]] _sender shouldEnableURL:(NSURL *)_url
 
 bool SandboxManager::EnsurePathAccess(const std::string &_path)
 {
-    if( !SandboxManager::Instance().CanAccessFolder(_path) && !SandboxManager::Instance().AskAccessForPathSync(_path) )
-        return false;
-    return true;
+    return SandboxManager::Instance().CanAccessFolder(_path) || SandboxManager::Instance().AskAccessForPathSync(_path);
 }
diff --git a/Source/NimbleCommander/NimbleCommander/Core/ServicesHandler.mm b/Source/NimbleCommander/NimbleCommander/Core/ServicesHandler.mm
@@ -73,7 +73,7 @@
 
 static bool IsASingleDirectoryPath(const std::vector<std::string> &_paths, VFSHost &_native_host)
 {
-    return _paths.size() == 1 && _native_host.IsDirectory(_paths[0].c_str(), 0);
+    return _paths.size() == 1 && _native_host.IsDirectory(_paths[0], 0);
 }
 
 void ServicesHandler::RevealItem(NSPasteboard *_pboard,

diff --git a/Source/NimbleCommander/NimbleCommander/Core/SimpleComboBoxPersistentDataSource.mm b/Source/NimbleCommander/NimbleCommander/Core/SimpleComboBoxPersistentDataSource.mm
@@ -40,7 +40,7 @@ - (void)dealloc
         Value arr(rapidjson::kArrayType);
         for( auto &s : m_Items )
             arr.PushBack(Value(s.UTF8String, g_CrtAllocator), g_CrtAllocator);
-        StateConfig().Set(m_ConfigPath.c_str(), arr);
+        StateConfig().Set(m_ConfigPath, arr);
     }
 }
 

diff --git a/Source/NimbleCommander/NimbleCommander/Core/TemporaryNativeFileChangesSentinel.cpp b/Source/NimbleCommander/NimbleCommander/Core/TemporaryNativeFileChangesSentinel.cpp
@@ -14,7 +14,7 @@ static std::optional<std::vector<uint8_t>> CalculateFileHash(const std::string &
 {
     const int chunk_sz = 1 * 1024 * 1024;
     VFSFilePtr file;
-    int rc = nc::bootstrap::NativeVFSHostInstance().CreateFile(_path.c_str(), file, nullptr);
+    int rc = nc::bootstrap::NativeVFSHostInstance().CreateFile(_path, file, nullptr);
     if( rc != 0 )
         return std::nullopt;