-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Swift type checking using is #5561
Swift type checking using is #5561
Conversation
I added a |
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 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 and |
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? |
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. |
Source/SwiftLintBuiltInRules/Rules/Idiomatic/PreferTypeCheckingRule.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. 😉
self.leftOperand.is(AsExprSyntax.self) | ||
&& self.operator.as(BinaryOperatorExprSyntax.self)?.operator.tokenKind == .binaryOperator("!=") | ||
&& self.rightOperand.is(NilLiteralExprSyntax.self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Congratulations on your first rule @ikelax ! |
Addresses #5295.
This new rule triggers on code like
which can be replaced by
or
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.