-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Diagnostics] Offer 'is' replacement for unused 'if let' expression with optional operand #37897
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
[Diagnostics] Offer 'is' replacement for unused 'if let' expression with optional operand #37897
Conversation
… operand is optional
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.
Thank you for the contribution! Code makes sense to me, I have left a couple of minor comments inline.
One more thing - please use clang-format
to format the code, you can do that via git-clang-format
as well after git add
ing the files, that tool could be found in clang/tools/clang-format
at your llvm-project
checkout.
lib/Sema/MiscDiagnostics.cpp
Outdated
if (auto oeExpr = | ||
dyn_cast<OptionalEvaluationExpr>(initExpr)) { | ||
if (auto ccExpr = dyn_cast<ConditionalCheckedCastExpr>( | ||
oeExpr->getSubExpr())) { |
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.
There might be parens involved so you'd need to use getSubExpr->getSemanticsProvidingExpr()
here otherwise it would fallback to != nil
diagnostics again in case like (bb) as? int
. Also please add that to https://github.com/apple/swift/pull/37897/files#diff-cb1dfa36aab299d0abafe04242dbebf8a555ad1a20041b47c1299f4687935f41R2885.
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.
Thanks for the comment :) I modified the code to use getValueProvidingExpr
to cover try expressions as well, and added necessary tests!
Thanks! I applied |
Co-authored-by: Luciano Almeida <[email protected]>
@swift-ci please smoke test |
@swift-ci please clean smoke test macOS platform |
2 similar comments
@swift-ci please clean smoke test macOS platform |
@swift-ci please clean smoke test macOS platform |
@swift-ci please smoke test macOS platform |
1 similar comment
@swift-ci please smoke test macOS platform |
Currently, the above code does not produce the better fix-it for replacing the expression with
is
because the expression was wrapped insideOptionalEvaluationExpr
.This PR enables optional typed values to produce
is
replacement fixit.Resolves SR-14646