realm
Swift type checking using is
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.
| 17 Messages | |
|---|---|
| :book: | Linting Aerial with this PR took 0.84s vs 0.83s on main (1% slower) |
| :book: | Linting Alamofire with this PR took 1.15s vs 1.16s on main (0% faster) |
| :book: | Linting Brave with this PR took 6.98s vs 6.92s on main (0% slower) |
| :book: | Linting DuckDuckGo with this PR took 4.31s vs 4.22s on main (2% slower) |
| :book: | Linting Firefox with this PR took 9.98s vs 10.05s on main (0% faster) |
| :book: | Linting Kickstarter with this PR took 9.2s vs 9.14s on main (0% slower) |
| :book: | Linting Moya with this PR took 0.49s vs 0.49s on main (0% slower) |
| :book: | Linting NetNewsWire with this PR took 2.38s vs 2.35s on main (1% slower) |
| :book: | Linting Nimble with this PR took 0.7s vs 0.69s on main (1% slower) |
| :book: | Linting PocketCasts with this PR took 8.0s vs 7.92s on main (1% slower) |
| :book: | Linting Quick with this PR took 0.4s vs 0.4s on main (0% slower) |
| :book: | Linting Realm with this PR took 4.4s vs 4.38s on main (0% slower) |
| :book: | Linting Sourcery with this PR took 2.19s vs 2.16s on main (1% slower) |
| :book: | Linting Swift with this PR took 4.14s vs 4.11s on main (0% slower) |
| :book: | Linting VLC with this PR took 1.15s vs 1.12s on main (2% slower) |
| :book: | Linting Wire with this PR took 16.69s vs 16.33s on main (2% slower) |
| :book: | Linting WordPress with this PR took 10.82s vs 10.48s on main (3% slower) |
Generated by :no_entry_sign: Danger
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.
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
Thank you. I prefer your idea so far. As soon as I am free again, I will work on it.
@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 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.
@mildm8nnered FYI we've just updated SwiftLint and have disabled this rule globally because it is triggering on some examples which result in invalid code. The particular case we encountered looked like:
if let parseData(data) as MyType? != nil {
// ...
}
It appears this rule is triggering on as in addition to as? and when as is converted to is it's possible for the expression to lose type context that was necessary to compile the expression (since as provides static type coercion while is is only a runtime type check).
@mildm8nnered FYI we've just updated SwiftLint and have disabled this rule globally because it is triggering on some examples which result in invalid code. The particular case we encountered looked like:
if let parseData(data) as MyType? != nil { // ... }It appears this rule is triggering on
asin addition toas?and whenasis converted toisit's possible for the expression to lose type context that was necessary to compile the expression (sinceasprovides static type coercion whileisis only a runtime type check).
@jumhyn-browser - could you file this as a new issue, and I'll take a look when I get a chance