From 9ecc0670fe64c6df68289f979be488e6772cdc80 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:21:39 -0700 Subject: [PATCH] chore: clean up clang-tidy warnings (#50918) * chore: use emplace and use it correctly Co-authored-by: David Sanders * chore: redundant cast to the same type [google-readability-casting] Co-authored-by: David Sanders * chore: do not create objects with +new [google-objc-avoid-nsobject-new] Co-authored-by: David Sanders * chore: default arguments on virtual or override methods are prohibited [google-default-arguments] Co-authored-by: David Sanders * chore: warning: C-style casts are discouraged; use static_cast [google-readability-casting] CFLocaleGetValue already returns CFTypeRef so that redundant static_cast was removed Co-authored-by: David Sanders * chore: refactor block to avoid use after move warning from clang-tidy Looks like clang-tidy couldn't tell these were two mutually exclusive branches so there was no actual issue, but refactoring is cleaner anyway since it makes it more DRY. Co-authored-by: David Sanders * chore: C-style casts are discouraged; use static_cast [google-readability-casting] No cast needed here, everything is already the correct type Co-authored-by: David Sanders * chore: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting] Co-authored-by: David Sanders * chore: use '= default' to define a trivial destructor [modernize-use-equals-default] Co-authored-by: David Sanders * chore: use range-based for loop instead [modernize-loop-convert] Co-authored-by: David Sanders * chore: redundant void argument list [modernize-redundant-void-arg] Co-authored-by: David Sanders * chore: address code review feedback Co-authored-by: David Sanders * chore: use auto Co-authored-by: Charles Kerr Co-authored-by: David Sanders --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: David Sanders --- shell/browser/api/electron_api_app.cc | 10 ++++------ shell/browser/browser_mac.mm | 2 +- shell/browser/browser_process_impl.cc | 8 ++++---- shell/browser/mac/electron_application.mm | 2 +- shell/browser/native_window.h | 2 +- shell/browser/native_window_mac.h | 2 +- shell/browser/native_window_views.cc | 8 ++++---- shell/browser/ui/cocoa/electron_bundle_mover.mm | 7 +++---- shell/browser/ui/cocoa/electron_ns_window.mm | 11 ++++------- shell/browser/ui/message_box_mac.mm | 2 +- .../browser/ui/views/win_caption_button_container.cc | 2 +- shell/browser/ui/win/notify_icon_host.cc | 10 +++------- shell/common/gin_converters/guid_converter.h | 2 +- shell/common/platform_util_mac.mm | 10 +++------- 14 files changed, 32 insertions(+), 46 deletions(-) diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 948844e599..e0e7aef28c 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -989,17 +989,15 @@ std::string App::GetLocaleCountryCode() { WCHAR locale_name[LOCALE_NAME_MAX_LENGTH] = {0}; if (GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SISO3166CTRYNAME, - (LPWSTR)&locale_name, - sizeof(locale_name) / sizeof(WCHAR)) || + locale_name, std::size(locale_name)) || GetLocaleInfoEx(LOCALE_NAME_SYSTEM_DEFAULT, LOCALE_SISO3166CTRYNAME, - (LPWSTR)&locale_name, - sizeof(locale_name) / sizeof(WCHAR))) { + locale_name, std::size(locale_name))) { base::WideToUTF8(locale_name, wcslen(locale_name), ®ion); } #elif BUILDFLAG(IS_MAC) CFLocaleRef locale = CFLocaleCopyCurrent(); - CFStringRef value = CFStringRef( - static_cast(CFLocaleGetValue(locale, kCFLocaleCountryCode))); + auto value = + static_cast(CFLocaleGetValue(locale, kCFLocaleCountryCode)); if (value != nil) { char temporaryCString[3]; const CFIndex kCStringSize = sizeof(temporaryCString); diff --git a/shell/browser/browser_mac.mm b/shell/browser/browser_mac.mm index 31f4f1141c..c5453506fd 100644 --- a/shell/browser/browser_mac.mm +++ b/shell/browser/browser_mac.mm @@ -184,7 +184,7 @@ std::vector Browser::GetRecentDocuments() { std::vector documents; documents.reserve([recentURLs count]); for (NSURL* url in recentURLs) - documents.push_back(std::string([url.path UTF8String])); + documents.emplace_back([url.path UTF8String]); return documents; } diff --git a/shell/browser/browser_process_impl.cc b/shell/browser/browser_process_impl.cc index c33b9caae9..3a33345589 100644 --- a/shell/browser/browser_process_impl.cc +++ b/shell/browser/browser_process_impl.cc @@ -455,9 +455,9 @@ void BrowserProcessImpl::CreateOSCryptAsync() { // The DPAPI key provider requires OSCrypt::Init to have already been called // to initialize the key storage. This happens in // BrowserMainPartsWin::PreCreateMainMessageLoop. - providers.emplace_back(std::make_pair( + providers.emplace_back( /*precedence=*/10u, - std::make_unique(local_state()))); + std::make_unique(local_state())); #endif // BUILDFLAG(IS_WIN) #if BUILDFLAG(IS_LINUX) @@ -499,9 +499,9 @@ void BrowserProcessImpl::CreateOSCryptAsync() { #if BUILDFLAG(IS_MAC) if (base::FeatureList::IsEnabled(features::kUseKeychainKeyProvider)) { - providers.emplace_back(std::make_pair( + providers.emplace_back( /*precedence=*/10u, - std::make_unique())); + std::make_unique()); } #endif // BUILDFLAG(IS_MAC) diff --git a/shell/browser/mac/electron_application.mm b/shell/browser/mac/electron_application.mm index 13853870c3..4bd3fd6838 100644 --- a/shell/browser/mac/electron_application.mm +++ b/shell/browser/mac/electron_application.mm @@ -208,7 +208,7 @@ inline void dispatch_sync_main(dispatch_block_t block) { forEventClass:kInternetEventClass andEventID:kAEGetURL]; - handoffLock_ = [NSCondition new]; + handoffLock_ = [[NSCondition alloc] init]; } - (void)handleURLEvent:(NSAppleEventDescriptor*)event diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 6f8dc7420d..a2715b1ba1 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -96,7 +96,7 @@ class NativeWindow : public views::WidgetDelegate { virtual void SetFullScreen(bool fullscreen) = 0; virtual bool IsFullscreen() const = 0; - virtual void SetBounds(const gfx::Rect& bounds, bool animate = false) = 0; + virtual void SetBounds(const gfx::Rect& bounds, bool animate) = 0; virtual gfx::Rect GetBounds() const = 0; void SetShape(const std::vector& rects); void SetSize(const gfx::Size& size, bool animate = false); diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index 95f89b7ffd..68c603c7b7 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -59,7 +59,7 @@ class NativeWindowMac : public NativeWindow, bool IsMinimized() const override; void SetFullScreen(bool fullscreen) override; bool IsFullscreen() const override; - void SetBounds(const gfx::Rect& bounds, bool animate = false) override; + void SetBounds(const gfx::Rect& bounds, bool animate) override; gfx::Rect GetBounds() const override; bool IsNormal() const override; gfx::Rect GetNormalBounds() const override; diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 390d5e4604..1c52d002d4 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1269,7 +1269,7 @@ void NativeWindowViews::SetBackgroundColor(SkColor background_color) { SetClassLongPtr(GetAcceleratedWidget(), GCLP_HBRBACKGROUND, reinterpret_cast(brush)); if (previous_brush) - DeleteObject((HBRUSH)previous_brush); + DeleteObject(reinterpret_cast(previous_brush)); InvalidateRect(GetAcceleratedWidget(), nullptr, 1); #endif SkColor compositor_color = background_color; @@ -1468,11 +1468,11 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) { // the ::GetWindowLongPtr or ::SetWindowLongPtr functions with "nIndex" set // to "GWLP_HWNDPARENT" which actually means the window owner. HWND hwndParent = parent ? parent->GetAcceleratedWidget() : nullptr; - if (hwndParent == - (HWND)::GetWindowLongPtr(GetAcceleratedWidget(), GWLP_HWNDPARENT)) + if (hwndParent == reinterpret_cast(::GetWindowLongPtr( + GetAcceleratedWidget(), GWLP_HWNDPARENT))) return; ::SetWindowLongPtr(GetAcceleratedWidget(), GWLP_HWNDPARENT, - (LONG_PTR)hwndParent); + reinterpret_cast(hwndParent)); // Ensures the visibility if (IsVisible()) { WINDOWPLACEMENT wp; diff --git a/shell/browser/ui/cocoa/electron_bundle_mover.mm b/shell/browser/ui/cocoa/electron_bundle_mover.mm index bbc5b3bb78..28e6386975 100644 --- a/shell/browser/ui/cocoa/electron_bundle_mover.mm +++ b/shell/browser/ui/cocoa/electron_bundle_mover.mm @@ -155,10 +155,9 @@ bool AuthorizedInstall(NSString* srcPath, NSString* dstPath, bool* canceled) { AuthorizationItem myItems = {kAuthorizationRightExecute, 0, nullptr, 0}; AuthorizationRights myRights = {1, &myItems}; - AuthorizationFlags myFlags = - (AuthorizationFlags)(kAuthorizationFlagInteractionAllowed | - kAuthorizationFlagExtendRights | - kAuthorizationFlagPreAuthorize); + AuthorizationFlags myFlags = kAuthorizationFlagInteractionAllowed | + kAuthorizationFlagExtendRights | + kAuthorizationFlagPreAuthorize; err = AuthorizationCopyRights(myAuthorizationRef, &myRights, nullptr, myFlags, nullptr); diff --git a/shell/browser/ui/cocoa/electron_ns_window.mm b/shell/browser/ui/cocoa/electron_ns_window.mm index 507b0e877d..02534abb58 100644 --- a/shell/browser/ui/cocoa/electron_ns_window.mm +++ b/shell/browser/ui/cocoa/electron_ns_window.mm @@ -49,7 +49,7 @@ static StartGridAnimationIMP g_orig_startGridAnimation = nullptr; static void Patched_startGridAnimation(id self, SEL _cmd, id animation, - void (^completionHandler)(void)) { + void (^completionHandler)()) { if (completionHandler) completionHandler(); } @@ -83,8 +83,7 @@ MouseDownImpl g_nsnextstepframe_mousedown; @implementation SwizzledMethodsClass - (void)swiz_nsthemeframe_mouseDown:(NSEvent*)event { if ([self.window respondsToSelector:@selector(shell)]) { - electron::NativeWindowMac* shell = - (electron::NativeWindowMac*)[(id)self.window shell]; + electron::NativeWindowMac* shell = [(id)self.window shell]; if (shell && !shell->has_frame()) [self cr_mouseDownOnFrameView:event]; } @@ -93,8 +92,7 @@ MouseDownImpl g_nsnextstepframe_mousedown; - (void)swiz_nsnextstepframe_mouseDown:(NSEvent*)event { if ([self.window respondsToSelector:@selector(shell)]) { - electron::NativeWindowMac* shell = - (electron::NativeWindowMac*)[(id)self.window shell]; + electron::NativeWindowMac* shell = [(id)self.window shell]; if (shell && !shell->has_frame()) { [self cr_mouseDownOnFrameView:event]; } @@ -104,8 +102,7 @@ MouseDownImpl g_nsnextstepframe_mousedown; - (void)swiz_nsview_swipeWithEvent:(NSEvent*)event { if ([self.window respondsToSelector:@selector(shell)]) { - electron::NativeWindowMac* shell = - (electron::NativeWindowMac*)[(id)self.window shell]; + electron::NativeWindowMac* shell = [(id)self.window shell]; if (shell) { if (event.deltaY == 1.0) { shell->NotifyWindowSwipe("up"); diff --git a/shell/browser/ui/message_box_mac.mm b/shell/browser/ui/message_box_mac.mm index 3adf5f720e..7d232b6b1f 100644 --- a/shell/browser/ui/message_box_mac.mm +++ b/shell/browser/ui/message_box_mac.mm @@ -80,7 +80,7 @@ NSAlert* CreateNSAlert(const MessageBoxSettings& settings) { // TODO(@codebytere): This behavior violates HIG & should be deprecated. if (settings.cancel_id == settings.default_id) { - [(NSButton*)[ns_buttons objectAtIndex:settings.default_id] highlight:YES]; + [[ns_buttons objectAtIndex:settings.default_id] highlight:YES]; } } diff --git a/shell/browser/ui/views/win_caption_button_container.cc b/shell/browser/ui/views/win_caption_button_container.cc index 501c9f5f99..9e369eee88 100644 --- a/shell/browser/ui/views/win_caption_button_container.cc +++ b/shell/browser/ui/views/win_caption_button_container.cc @@ -87,7 +87,7 @@ WinCaptionButtonContainer::WinCaptionButtonContainer(WinFrameView* frame_view) UpdateButtonToolTipsForWindowControlsOverlay(); } -WinCaptionButtonContainer::~WinCaptionButtonContainer() {} +WinCaptionButtonContainer::~WinCaptionButtonContainer() = default; int WinCaptionButtonContainer::NonClientHitTest(const gfx::Point& point) const { DCHECK(HitTestPoint(point)) diff --git a/shell/browser/ui/win/notify_icon_host.cc b/shell/browser/ui/win/notify_icon_host.cc index c59d9f6b93..0d894b90cd 100644 --- a/shell/browser/ui/win/notify_icon_host.cc +++ b/shell/browser/ui/win/notify_icon_host.cc @@ -198,12 +198,10 @@ NotifyIcon* NotifyIconHost::CreateNotifyIcon(std::optional guid) { guid_str = guid_str.substr(1, guid_str.length() - 2); } - unsigned char* uid_cstr = (unsigned char*)guid_str.c_str(); + auto* uid_cstr = reinterpret_cast(guid_str.data()); RPC_STATUS result = UuidFromStringA(uid_cstr, &uid); if (result != RPC_S_INVALID_STRING_UUID) { - for (NotifyIcons::const_iterator i(notify_icons_.begin()); - i != notify_icons_.end(); ++i) { - auto* current_win_icon = static_cast(*i); + for (const NotifyIcon* current_win_icon : notify_icons_) { if (current_win_icon->guid() == uid) { LOG(WARNING) << "Guid already in use. Existing tray entry will be replaced."; @@ -256,9 +254,7 @@ LRESULT CALLBACK NotifyIconHost::WndProc(HWND hwnd, NotifyIcon* win_icon = nullptr; // Find the selected status icon. - for (NotifyIcons::const_iterator i(notify_icons_.begin()); - i != notify_icons_.end(); ++i) { - auto* current_win_icon = static_cast(*i); + for (NotifyIcon* current_win_icon : notify_icons_) { if (current_win_icon->icon_id() == wparam) { win_icon = current_win_icon; break; diff --git a/shell/common/gin_converters/guid_converter.h b/shell/common/gin_converters/guid_converter.h index a2c771efeb..8996af1615 100644 --- a/shell/common/gin_converters/guid_converter.h +++ b/shell/common/gin_converters/guid_converter.h @@ -76,7 +76,7 @@ struct Converter { if (guid[0] == '{' && guid[guid.length() - 1] == '}') { guid = guid.substr(1, guid.length() - 2); } - unsigned char* uid_cstr = (unsigned char*)guid.c_str(); + auto* uid_cstr = reinterpret_cast(guid.data()); RPC_STATUS result = UuidFromStringA(uid_cstr, &uid); if (result == RPC_S_INVALID_STRING_UUID) { return false; diff --git a/shell/common/platform_util_mac.mm b/shell/common/platform_util_mac.mm index 153334f7f1..ffcb20c4f1 100644 --- a/shell/common/platform_util_mac.mm +++ b/shell/common/platform_util_mac.mm @@ -167,13 +167,9 @@ void OpenExternal(const GURL& url, configuration:configuration completionHandler:^(NSRunningApplication* _Nullable app, NSError* _Nullable error) { - if (error) { - runner->PostTask(FROM_HERE, base::BindOnce(std::move(copied_callback), - "Failed to open URL")); - } else { - runner->PostTask(FROM_HERE, - base::BindOnce(std::move(copied_callback), "")); - } + std::string err_msg = error ? "Failed to open URL" : ""; + runner->PostTask(FROM_HERE, base::BindOnce(std::move(copied_callback), + std::move(err_msg))); }]; }