Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new prefer_key_path rule #5548

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

SimplyDanny
Copy link
Collaborator

@SimplyDanny SimplyDanny commented Apr 29, 2024

This new rule triggers on code like

list.filter { $0.isValid }

which can be replaced by

list.filter(\.isValid)

as of Swift 5.2.

Automatic fixes fail once the argument label for the closure is named. The fix would need to know its name and insert it in the function call. But that's impossible for a syntax-based rule. We could do auto-fixes for simple cases like map and filter (which represent most of the findings in OSS projects and SwiftLint itself) at least. I'm not sure if partial fixes are a good thing though.

@SimplyDanny
Copy link
Collaborator Author

SimplyDanny commented Apr 29, 2024

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

@SwiftLintBot
Copy link

SwiftLintBot commented Apr 29, 2024

1327 Warnings
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Cache/Cache.swift:700:58: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Cache/PoiStringProvider.swift:252:67: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Cache/PoiStringProvider.swift:256:67: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/SourceList.swift:363:64: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/VideoList.swift:210:25: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/VideoList.swift:440:46: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/VideoList.swift:81:78: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/VideoList.swift:87:78: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /App/ShareExtension/ShareToBraveViewController.swift:78:51: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /App/ShareExtension/ShareToBraveViewController.swift:78:93: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Extensions/UIViewExtensions.swift:94:58: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Widgets.swift:54:61: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistCarplayController.swift:119:58: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistCarplayController.swift:366:63: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController.swift:448:47: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCarplayManager.swift:93:15: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Search/SearchEngines.swift:155:73: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Tab.swift:417:50: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Tab.swift:417:68: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift:479:80: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/Debug/Brave Search/BraveSearchDebugMenu.swift:78:22: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/Debug/Rewards Internals/QA/RewardsDebugSettingsViewController.swift:36:96: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/Features/ShieldsPrivacy/ClearDataSectionView.swift:57:88: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/General/SearchEngines/SearchSettingsTableViewController.swift:83:41: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/Tabs/PrivateTabsView.swift:76:58: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Settings/Tabs/PrivateTabsView.swift:77:57: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Shields/ShieldsSwitch.swift:216:39: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/Shields/ShieldsSwitch.swift:220:39: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/SiteStateListenerScriptHandler.swift:113:41: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Migration/Migration.swift:61:52: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/Brave/Migration/Migration.swift:87:27: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveNews/Composer/FeedDataSource.swift:884:55: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveUI/Popover/PopoverController.swift:329:35: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Chart/LineChartView.swift:262:35: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Chart/LineChartView.swift:44:33: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift:320:46: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift:353:46: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift:62:101: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift:239:128: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift:327:52: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift:498:14: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/SendTokenStore.swift:299:132: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/SwapTokenStore.swift:950:141: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift:282:126: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift:311:126: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/WalletUserAssetManager.swift:169:88: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Sources/BraveWallet/WalletUserAssetManager.swift:209:33: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:112:49: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:125:60: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:144:49: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:157:60: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:15:47: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:176:49: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:189:60: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:208:56: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:220:67: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:238:54: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:251:65: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:51:49: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:64:60: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:82:49: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/InitialSearchEnginesTests.swift:95:60: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/SolanaProviderScriptHandlerTests.swift:243:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /Tests/ClientTests/SolanaProviderScriptHandlerTests.swift:280:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Brave: /ThirdParty/Static/Static/DataSource.swift:172:36: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /Core/ContentBlockerRulesLists.swift:37:76: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /Core/CookieStorage.swift:138:53: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/AutofillLoginListViewModel.swift:133:18: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/CookieDebugViewController.swift:60:47: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/DownloadManager.swift:175:57: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/DownloadManager.swift:176:67: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/DownloadsListDataSource.swift:78:70: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/DownloadsListSectioningHelper.swift:83:81: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/NetworkProtectionStatusViewModel.swift:221:63: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/NetworkProtectionVPNSettingsViewModel.swift:56:18: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/RootDebugViewController.swift:313:18: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGo/TabPreviewsSource.swift:200:43: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:110:42: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:124:36: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:138:42: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:279:54: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:279:95: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:309:54: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:309:95: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:79:32: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:89:34: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarkListViewModelTests.swift:99:36: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/BookmarksMigrationTests.swift:136:52: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/ContentBlockingUpdatingTests.swift:255:56: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/DownloadManagerTests.swift:246:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/DownloadManagerTests.swift:270:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/FavoriteListViewModelTests.swift:66:32: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/FavoriteListViewModelTests.swift:80:35: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/FavoriteListViewModelTests.swift:82:38: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/NetworkProtectionVPNLocationViewModelTests.swift:144:55: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/NetworkProtectionVPNLocationViewModelTests.swift:600:59: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in DuckDuckGo: /DuckDuckGoTests/NetworkProtectionVPNLocationViewModelTests.swift:624:59: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /BrowserKit/Sources/Common/Theming/Gradient.swift:11:27: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /BrowserKit/Sources/SiteImageView/ImageProcessing/LetterImageGenerator.swift:67:53: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/ContentBlocker/ContentBlocker.swift:301:61: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/ContentBlocker/ContentBlocker.swift:320:57: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Extensions/UIViewController+Extension.swift:147:72: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+URLBarDelegate.swift:260:47: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift:2972:40: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/FaviconManager.swift:141:46: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/SearchEngines/SearchEngines.swift:115:58: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/StartAtHomeHelper.swift:82:31: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/StartAtHomeHelper.swift:84:31: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift:134:59: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift:160:40: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/Tabs/Legacy/LegacyTabTrayViewController.swift:155:51: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/Tabs/Views/TabTrayViewController.swift:104:64: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Extensions/DevicePickerViewController.swift:159:46: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Fakespot/Client/PartnerWebsites.swift:20:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/HomepageViewModel.swift:79:39: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/Wallpapers/v1/Interface/WallpaperManager.swift:187:55: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/Wallpapers/v1/Utilities/WallpaperStorageUtility.swift:151:21: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/Wallpapers/v1/Utilities/WallpaperStorageUtility.swift:152:22: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/Wallpapers/v1/Utilities/WallpaperStorageUtility.swift:153:18: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Onboarding/Models/OnboardingTelemetryUtility.swift:14:42: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Settings/ClearPrivateDataTableViewController.swift:54:36: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Settings/SearchSettingsTableViewController.swift:79:61: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/TabContentsScripts/FormAutofillHelper/FormAutofillHelper.swift:138:66: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/TabContentsScripts/FormAutofillHelper/FormAutofillHelper.swift:139:64: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/TabContentsScripts/FormAutofillHelper/FormAutofillHelper.swift:140:59: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Widgets/PhotonActionSheet/PhotonActionSheet.swift:399:26: warning: Prefer Key Path Violation: Use a key path argument instead of a closure with property access (prefer_key_path)
⚠️ Danger found 1327 violations with this PR. Due to GitHub's max issue comment size, the number shown has been truncated to 126.
17 Messages
📖 Linting Aerial with this PR took 0.83s vs 0.82s on main (1% slower)
📖 Linting Alamofire with this PR took 1.13s vs 1.14s on main (0% faster)
📖 Linting Brave with this PR took 6.63s vs 6.62s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 3.94s vs 3.95s on main (0% faster)
📖 Linting Firefox with this PR took 9.83s vs 9.81s on main (0% slower)
📖 Linting Kickstarter with this PR took 8.74s vs 8.84s on main (1% faster)
📖 Linting Moya with this PR took 0.48s vs 0.48s on main (0% slower)
📖 Linting NetNewsWire with this PR took 2.28s vs 2.26s on main (0% slower)
📖 Linting Nimble with this PR took 0.69s vs 0.68s on main (1% slower)
📖 Linting PocketCasts with this PR took 7.4s vs 7.55s on main (1% faster)
📖 Linting Quick with this PR took 0.39s vs 0.39s on main (0% slower)
📖 Linting Realm with this PR took 4.13s vs 4.19s on main (1% faster)
📖 Linting Sourcery with this PR took 2.11s vs 2.13s on main (0% faster)
📖 Linting Swift with this PR took 4.08s vs 4.05s on main (0% slower)
📖 Linting VLC with this PR took 1.12s vs 1.12s on main (0% slower)
📖 Linting Wire with this PR took 15.79s vs 15.88s on main (0% faster)
📖 Linting WordPress with this PR took 10.54s vs 10.62s on main (0% faster)

Generated by 🚫 Danger

@mildm8nnered
Copy link
Collaborator

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

@mildm8nnered
Copy link
Collaborator

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

So most hits look good, but I think this might be a false alarm (where there is no property named otherResults). I adapted this from proprietary code that I saw the hit with - I have not tested this specific example code here, but if it doesn't get a hit I can dig in deeper on my side.

            guard let otherResults = results as? [SomeClass] else {
                return
            }

            let dictionary = Dictionary(grouping: ↓otherResults) {
                $0.someProperty
            }

@SimplyDanny
Copy link
Collaborator Author

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

So most hits look good, but I think this might be a false alarm (where there is no property named otherResults). I adapted this from proprietary code that I saw the hit with - I have not tested this specific example code here, but if it doesn't get a hit I can dig in deeper on my side.

            guard let otherResults = results as? [SomeClass] else {
                return
            }

            let dictionary = Dictionary(grouping: ↓otherResults) {
                $0.someProperty
            }

The rule would complain about the closure and suggest Dictionary(grouping: otherResult, by: \.someProperty). Looks okay to me.

CHANGELOG.md Outdated Show resolved Hide resolved
@SimplyDanny SimplyDanny force-pushed the prefer-key-path-rule branch 2 times, most recently from 8dffce7 to 2927aa4 Compare July 6, 2024 20:10
@SimplyDanny
Copy link
Collaborator Author

I'm still unsure about this rule. As we see in the list of OSS findings, there will potentially be a lot of violations in existing code bases (78 in SwiftLint), most of them on .map, .filter, ... These are cases that could be and should be (due to their sheer amount) auto-fixed. However, fixing all findings will very likely lead to wrong code due to missing parameter names for custom functions accepting closures.

Fixing only a few (but most of the) findings seems to be against the philosophy of SwiftLint. Are there other rules that fix only partially? Would this be acceptable? Or do people expect a swiftlint lint --fix run to repair all violations?

@SimplyDanny SimplyDanny force-pushed the prefer-key-path-rule branch 3 times, most recently from fb1dae2 to 30c1cd7 Compare July 17, 2024 21:23
@SimplyDanny
Copy link
Collaborator Author

So I restricted the rule to known standard functions by default and there is an option to enable it for all other closures as well. I actually doubt that anyone would enable it to be honest, especially when it comes to auto-fixes.

@mildm8nnered: May I ask you again to run this rule with --fix on your project? In SwiftLint, there has only been one rewrite that caused a compile error afterwards - unfortunately one that SwiftLint cannot recognize as a false positive due to missing type information.

@SimplyDanny
Copy link
Collaborator Author

In SwiftLint, there has only been one rewrite that caused a compile error afterwards - unfortunately one that SwiftLint cannot recognize as a false positive due to missing type information.

Luckily, there is an accepted proposal that is going to make the auto-fixed code compile in this special case.

@SimplyDanny SimplyDanny force-pushed the prefer-key-path-rule branch 4 times, most recently from 617298f to eed7193 Compare July 28, 2024 18:01
@SimplyDanny
Copy link
Collaborator Author

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

Could you check again with the --fix argument, @mildm8nnered? I've so far applied auto-fixes to a few smaller code bases and at least the default case (restrict_to_standard_functions = true) worked except for the one issue mentioned in the previous comments.

@mildm8nnered
Copy link
Collaborator

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

Could you check again with the --fix argument, @mildm8nnered? I've so far applied auto-fixes to a few smaller code bases and at least the default case (restrict_to_standard_functions = true) worked except for the one issue mentioned in the previous comments.

So --fix worked for me - 167 files changed everything still compiles ...

@SimplyDanny
Copy link
Collaborator Author

SimplyDanny commented Jul 30, 2024

@mildm8nnered: I'd like to hear your opinion. The rule doesn't seem to be so involved. Any more cases or shortcomings you can think of?

Just trying it on my local codebase, which I expect to have a lot of violations, pretty much like the OSS findings.

Could you check again with the --fix argument, @mildm8nnered? I've so far applied auto-fixes to a few smaller code bases and at least the default case (restrict_to_standard_functions = true) worked except for the one issue mentioned in the previous comments.

So --fix worked for me - 167 files changed everything still compiles ...

Thank you! That's good news. So the PR can finally land. Would you like to give a vote, @mildm8nnered?

@SimplyDanny SimplyDanny merged commit a657438 into realm:main Aug 2, 2024
12 checks passed
@SimplyDanny SimplyDanny deleted the prefer-key-path-rule branch August 2, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants