realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.46k stars 2.2k forks source link

Swift type checking using is #5561

Open ikelax opened 2 months ago

ikelax commented 2 months ago

Addresses #5295.

This new rule triggers on code like

if a as? Dog != nil { 
   doSomeThing()
}

which can be replaced by

if a is Dog  { 
   doSomeThing()
}

or

if let a = a as? Dog  { 
   a.run()
}

However, so far there is not a Rewriter for fixing this automatically. If I am not mistaken, a as? Dog != nil can always be replaced by a is Dog.

SwiftLintBot commented 2 months ago
10 Warnings
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift:459:46: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift:268:24: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/HomepageViewController.swift:320:36: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/TopSites/DataManagement/TopSitesDataAdaptor.swift:183:19: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Library/HistoryPanel/HistoryPanel.swift:548:17: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Widgets/SiteTableViewController.swift:54:17: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Nimble: /Sources/Nimble/Matchers/MatchError.swift:62:24: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/App/AppViewController.swift:79:36: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift:788:52: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
:warning: This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Reader/ReaderCardsStreamViewController.swift:245:18: error: No type casting and checking for nil Violation: Type casting and then checking for nil should be avoided (no_type_casting_and_checking_for_nil)
17 Messages
:book: Linting Aerial with this PR took 0.79s vs 0.8s on main (1% faster)
:book: Linting Alamofire with this PR took 1.13s vs 1.15s on main (1% faster)
:book: Linting Brave with this PR took 6.6s vs 6.66s on main (0% faster)
:book: Linting DuckDuckGo with this PR took 3.52s vs 3.47s on main (1% slower)
:book: Linting Firefox with this PR took 9.49s vs 9.37s on main (1% slower)
:book: Linting Kickstarter with this PR took 8.72s vs 8.46s on main (3% slower)
:book: Linting Moya with this PR took 0.49s vs 0.48s on main (2% slower)
:book: Linting NetNewsWire with this PR took 2.45s vs 2.37s on main (3% slower)
:book: Linting Nimble with this PR took 0.69s vs 0.68s on main (1% slower)
:book: Linting PocketCasts with this PR took 6.97s vs 7.08s on main (1% faster)
:book: Linting Quick with this PR took 0.33s vs 0.33s on main (0% slower)
:book: Linting Realm with this PR took 4.29s vs 4.23s on main (1% slower)
:book: Linting Sourcery with this PR took 2.12s vs 2.09s on main (1% slower)
:book: Linting Swift with this PR took 3.91s vs 3.97s on main (1% faster)
:book: Linting VLC with this PR took 1.12s vs 1.1s on main (1% slower)
:book: Linting Wire with this PR took 14.68s vs 14.54s on main (0% slower)
:book: Linting WordPress with this PR took 9.72s vs 9.86s on main (1% faster)

Generated by :no_entry_sign: Danger

ikelax commented 1 month ago

I added a Rewriter for fixing this automatically. Also, I checked the findings of SwiftLintBot. As far as I can tell, there is not anything that speaks against this rule.

mildm8nnered commented 3 days ago

So I wrote #5649 the other day because I had not spotted this PR. I've left mine open as a draft - please feel free to steal anything you like from it - I overrode visitPost(_ node: UnresolvedAsExprSyntax), and then look at the siblings of the parent ExprListSyntax to look for != nil, which might be a bit more efficient that looking at every ExprListSyntax (although I have not measured that).

I think the solutions are quite similar, but mine seem to pick up 26 OSS violations, versus 10 for this branch. When I looked at a couple, they did not seem to be false alarms. For example

https://github.com/wireapp/wire-ios/blob/ff4e0b2b87c7f78d638e3dc1791d587eaab8b6ed/wire-ios/Wire-iOS/Sources/UserInterface/Components/Views/MarkdownTextView+Recognizers.swift#L41:61

and

https://github.com/kickstarter/ios-oss/blob/eb2e07004a6ffb7be3764dfa6f23d518c0b38290/Library/TestHelpers/MockStripeIntentService.swift#L17:41

ikelax commented 1 day ago

Thank you. I prefer your idea so far. As soon as I am free again, I will work on it.