Swift type checking using is #5561

ikelax · 2024-05-05T12:50:38Z

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.

…ilRule

SwiftLintBot · 2024-05-05T13:04:58Z

	26 Warnings
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift:2447:81: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift:459:41: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift:470:41: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift:459:46: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift:702:29: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift:271:24: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/HomepageViewController.swift:325:36: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Home/TopSites/DataManagement/TopSitesDataAdaptor.swift:183:19: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Library/HistoryPanel/HistoryPanel.swift:548:17: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Settings/SettingsTableViewController.swift:1012:20: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/TabContentsScripts/LoginsHelper.swift:326:35: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/TabContentsScripts/LoginsHelper.swift:327:37: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Widgets/SiteTableViewController.swift:112:41: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Firefox: /firefox-ios/Client/Frontend/Widgets/SiteTableViewController.swift:54:17: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Kickstarter: /Library/TestHelpers/MockStripeIntentService.swift:18:41: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Kickstarter: /Library/TestHelpers/MockStripeIntentService.swift:39:41: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Nimble: /Sources/Nimble/Matchers/MatchError.swift:62:24: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Realm: /RealmSwift/Impl/SchemaDiscovery.swift:159:27: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Realm: /RealmSwift/Realm.swift:1375:30: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/App/AppViewController.swift:79:36: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Swift: /stdlib/public/core/BridgeObjectiveC.swift:819:25: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Wire: /wire-ios-request-strategy/Tests/Sources/Payloads/Processing/ConversationEventPayloadProcessorTests.swift:1387:76: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in Wire: /wire-ios/Wire-iOS/Sources/UserInterface/Components/Views/MarkdownTextView+Recognizers.swift:41:61: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/My Site/MySiteViewController.swift:764:52: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Notifications/Tools/NotificationMediaDownloader.swift:98:88: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)
⚠️	This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Reader/ReaderCardsStreamViewController.swift:256:18: warning: Prefer Type Checking Violation: Prefer `a is X` to `a as? X != nil` (prefer_type_checking)

	17 Messages
📖	Linting Aerial with this PR took 0.84s vs 0.83s on main (1% slower)
📖	Linting Alamofire with this PR took 1.15s vs 1.16s on main (0% faster)
📖	Linting Brave with this PR took 6.98s vs 6.92s on main (0% slower)
📖	Linting DuckDuckGo with this PR took 4.31s vs 4.22s on main (2% slower)
📖	Linting Firefox with this PR took 9.98s vs 10.05s on main (0% faster)
📖	Linting Kickstarter with this PR took 9.2s vs 9.14s on main (0% slower)
📖	Linting Moya with this PR took 0.49s vs 0.49s on main (0% slower)
📖	Linting NetNewsWire with this PR took 2.38s vs 2.35s on main (1% slower)
📖	Linting Nimble with this PR took 0.7s vs 0.69s on main (1% slower)
📖	Linting PocketCasts with this PR took 8.0s vs 7.92s on main (1% slower)
📖	Linting Quick with this PR took 0.4s vs 0.4s on main (0% slower)
📖	Linting Realm with this PR took 4.4s vs 4.38s on main (0% slower)
📖	Linting Sourcery with this PR took 2.19s vs 2.16s on main (1% slower)
📖	Linting Swift with this PR took 4.14s vs 4.11s on main (0% slower)
📖	Linting VLC with this PR took 1.15s vs 1.12s on main (2% slower)
📖	Linting Wire with this PR took 16.69s vs 16.33s on main (2% slower)
📖	Linting WordPress with this PR took 10.82s vs 10.48s on main (3% slower)

Generated by 🚫 Danger

ikelax · 2024-05-29T21:15:43Z

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 · 2024-07-01T20:42:21Z

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 · 2024-07-03T13:39:00Z

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

ikelax · 2024-08-20T07:13:19Z

@mildm8nnered I stole a lot from #5649. What do you think about the current solution? Do you mind if I also mention you in the changelog?

mildm8nnered · 2024-08-20T16:38:08Z

@mildm8nnered I stole a lot from #5649. What do you think about the current solution? Do you mind if I also mention you in the changelog?

Hi @ikelax - absolutely fine to credit me, but make sure you put yourself first!

I'm on vacation right now, but back home at the end of this week, so I'll be able to review it then, but hopefully @SimplyDanny will take a look too, as he is the most stringent code reviewer.

CHANGELOG.md

Source/SwiftLintBuiltInRules/Rules/Idiomatic/PreferTypeCheckingRule.swift

SimplyDanny

Very well done! With a folded tree, this looks much simpler indeed.

With that, my stringent review is done and I'll hand over to @mildm8nnered to discover what I've missed. 😉

SimplyDanny · 2024-08-22T12:41:20Z

Source/SwiftLintBuiltInRules/Rules/Idiomatic/PreferTypeCheckingRule.swift

+        self.leftOperand.is(AsExprSyntax.self)
+        && self.operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind == .binaryOperator("!=")
+        && self.rightOperand.is(NilLiteralExprSyntax.self)


Suggested change

self.leftOperand.is(AsExprSyntax.self)

&& self.operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind == .binaryOperator("!=")

&& self.rightOperand.is(NilLiteralExprSyntax.self)

leftOperand.is(AsExprSyntax.self)

&& operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind == .binaryOperator("!=")

&& rightOperand.is(NilLiteralExprSyntax.self)

mildm8nnered

lgtm!

mildm8nnered · 2024-08-23T18:31:48Z

Congratulations on your first rule @ikelax !

ikelax and others added 4 commits May 1, 2024 20:46

implement check for violation for the rule

120713a

fix identifier, name and description for NoTypeCastingAndCheckingForN…

8540023

…ilRule

Merge branch 'realm:main' into issue-5295_swift_type_checking_using_is

ec0f989

fix file name for NoTypeCastingAndCheckingForNilRule

fa8f49c

mention changes done in issue 5295 or pr 5561 in CHANGELOG

57783e9

SimplyDanny mentioned this pull request May 17, 2024

Swift type checking using is #5295

Closed

ikelax and others added 4 commits May 29, 2024 19:44

Merge branch 'realm:main' into issue-5295_swift_type_checking_using_is

db18801

renaming the rule to type_checking_using_is

263761b

implement rewriter for type_checking_using_is

8768f3a

add more examples for type_checking_using_is rule

29f540b

fix lint issues

03f2bdf

mildm8nnered mentioned this pull request Jul 1, 2024

Added prefer_type_checking rule #5649

Closed

ikelax and others added 6 commits August 19, 2024 21:20

Merge branch 'realm:main' into issue-5295_swift_type_checking_using_is

1147a8b

copy approach of mildm8nnered

b7646c4

adjust rewriter to changes

0399ba3

adjust changelog to new rule name

d0ccb4f

refactor check to one method

1553f05

fix swiftlint

63b8613

mention also mildm8nnered in the changelog

0b6fa40

SimplyDanny linked an issue Aug 20, 2024 that may be closed by this pull request

Swift type checking using is #5295

Closed

SimplyDanny reviewed Aug 20, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Source/SwiftLintBuiltInRules/Rules/Idiomatic/PreferTypeCheckingRule.swift Outdated Show resolved Hide resolved

ikelax added 3 commits August 21, 2024 18:30

add folded example

2ef2b38

list names in changelog

e723eb4

rewrite the visitor with infixoperatorexprsyntax

62bccb2

ikelax added 2 commits August 21, 2024 21:14

rewrite rewriter and for infixoperatorexprsyntax

4a188e8

fix lint errors

36dfc81

ikelax requested a review from SimplyDanny August 21, 2024 20:09

SimplyDanny approved these changes Aug 22, 2024

View reviewed changes

SimplyDanny requested a review from mildm8nnered August 22, 2024 13:33

mildm8nnered approved these changes Aug 23, 2024

View reviewed changes

mildm8nnered merged commit 9f4cb92 into realm:main Aug 23, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swift type checking using is #5561

Swift type checking using is #5561

ikelax commented May 5, 2024 •

edited

Loading

SwiftLintBot commented May 5, 2024 •

edited

Loading

ikelax commented May 29, 2024 •

edited

Loading

mildm8nnered commented Jul 1, 2024

ikelax commented Jul 3, 2024

ikelax commented Aug 20, 2024 •

edited

Loading

mildm8nnered commented Aug 20, 2024

SimplyDanny left a comment

SimplyDanny Aug 22, 2024

mildm8nnered left a comment

mildm8nnered commented Aug 23, 2024

Swift type checking using is #5561

Swift type checking using is #5561

Conversation

ikelax commented May 5, 2024 • edited Loading

SwiftLintBot commented May 5, 2024 • edited Loading

ikelax commented May 29, 2024 • edited Loading

mildm8nnered commented Jul 1, 2024

ikelax commented Jul 3, 2024

ikelax commented Aug 20, 2024 • edited Loading

mildm8nnered commented Aug 20, 2024

SimplyDanny left a comment

Choose a reason for hiding this comment

SimplyDanny Aug 22, 2024

Choose a reason for hiding this comment

mildm8nnered left a comment

Choose a reason for hiding this comment

mildm8nnered commented Aug 23, 2024

ikelax commented May 5, 2024 •

edited

Loading

SwiftLintBot commented May 5, 2024 •

edited

Loading

ikelax commented May 29, 2024 •

edited

Loading

ikelax commented Aug 20, 2024 •

edited

Loading