Skip to content

[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

Merged
merged 3 commits into from
Jun 22, 2021

Conversation

mininny
Copy link
Contributor

@mininny mininny commented Jun 13, 2021

let foo: Any? = 42 as Int

// current fix-it: Replace 'let bar = foo as? Int' with '(foo as? Int) != nil'
// better fix-it: Replace 'let bar = foo as? Int' with 'foo is Int'

if let bar = foo as? Int { 
    print("'foo' is of type 'Int'")
}

Currently, the above code does not produce the better fix-it for replacing the expression with is because the expression was wrapped inside OptionalEvaluationExpr.

This PR enables optional typed values to produce is replacement fixit.

Resolves SR-14646

@varungandhi-apple varungandhi-apple requested a review from xedin June 14, 2021 03:28
Copy link
Contributor

@xedin xedin left a 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 adding the files, that tool could be found in clang/tools/clang-format at your llvm-project checkout.

if (auto oeExpr =
dyn_cast<OptionalEvaluationExpr>(initExpr)) {
if (auto ccExpr = dyn_cast<ConditionalCheckedCastExpr>(
oeExpr->getSubExpr())) {
Copy link
Contributor

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.

Copy link
Contributor Author

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 getValueProvidingExprto cover try expressions as well, and added necessary tests!

@mininny
Copy link
Contributor Author

mininny commented Jun 17, 2021

One more thing - please use clang-format to format the code, you can do that via git-clang-format as well after git adding the files, that tool could be found in clang/tools/clang-format at your llvm-project checkout.

Thanks! I applied clang-format to format the code :)

@xedin
Copy link
Contributor

xedin commented Jun 17, 2021

@swift-ci please smoke test

@xedin
Copy link
Contributor

xedin commented Jun 17, 2021

@swift-ci please clean smoke test macOS platform

2 similar comments
@xedin
Copy link
Contributor

xedin commented Jun 17, 2021

@swift-ci please clean smoke test macOS platform

@LucianoPAlmeida
Copy link
Contributor

@swift-ci please clean smoke test macOS platform

@xedin
Copy link
Contributor

xedin commented Jun 21, 2021

@swift-ci please smoke test macOS platform

1 similar comment
@xedin
Copy link
Contributor

xedin commented Jun 22, 2021

@swift-ci please smoke test macOS platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants