[SR-7015] Fix CoreFoundation conditional downcast diagnostic message

[naru-jpn]

Fix error message about CoreFoundation conditional downcast diagnostic more helpful for user.

Resolves SR-7015.

[xwu]

This message is not grammatically correct. It also doesn't provide much clarity: check each CFTypeID...for what?

[naru-jpn]

Thanks to see my PR.

CFTypeIDs is written in SwiftObject.mm with message // FIXME: Actually compare CFTypeIDs, once they are available in the metadata.. I can't fix this FIXME yet.

When I wrote

_ = string as? CFArray

, error with message Conditional downcast to CoreFoundation type 'CFArray' will always succeed occurred. But actually that cast is invalid. So I think message should be modified more clear for user.

I'll try to replace other message representing correct status.

[naru-jpn]

I pushed new commit.
I think that will always succeed is wrong and correct message is ambiguous for now.

Before

Without import Foundation

// wrong
_ = string as? CFString // conditional downcast to CoreFoundation type 'CFString' will always succeed
// wrong
_ = string as? CFArray // conditional downcast to CoreFoundation type 'CFArray' will always succeed

With import Foundation

// actual
_ = string as? CFString // conditional cast from 'String' to 'CFString' always succeeds
// wrong
_ = string as? CFArray // conditional downcast to CoreFoundation type 'CFArray' will always succeed

After

Without import Foundation

// actual
_ = string as? CFString // conditional downcast to CoreFoundation type 'CFString' is ambiguous
// actual
_ = string as? CFArray // conditional downcast to CoreFoundation type 'CFArray' is ambiguous

With import Foundation

// actual
_ = string as? CFString // conditional cast from 'String' to 'CFString' always succeeds
// actual
_ = string as? CFArray // conditional downcast to CoreFoundation type 'CFArray' is ambiguous

[xwu]

Thanks for the continued effort. Writing diagnostic messages is very hard, so I don't envy your task. Jordan Rose wrote a very good guide about Swift diagnostic messages which I would recommend to you.

The original diagnostic message isn't, in fact, wrong. The Swift bug SR-7015 is pointing out that the message doesn't present any information about what the user should do about the problem. The challenge is to write something else very succinctly that will teach the user how to fix the problem correctly. It's not enough to say that something should be "checked"--you need to find a way, in very few words, to explain what should be checked or how.

[naru-jpn]

Thanks to give me information and I perhaps understood where my error message was wrong. Message before and after were same for user.

Now I think I want to try giving message usefully as same as in case of as or as!. There are more meaningful message in those cases than in case of as?. Thanks to watch my commits and sorry to take a lot of time.

[naru-jpn]

I pushed new commit.

I found some difficulties to check type directly. (For example, CSApply.swift, SwiftObject.mm)
I added a suggestion is using as instead of as?. At present, user can know that type to cast is not convertible if user replace as? to as. I wrote original error message because I'm not sure that new message is meaningful for user.

Before

// Conditional downcast to CoreFoundation type 'CFData' will always succeed
_ = string as? CFData

After

// Conditional downcast to CoreFoundation type 'CFData' will always succeed; did you mean to use 'as' to downcast?
_ = string as? CFData

// ↓ replace as? to as

// 'String' is not convertible to 'CFData'; did you mean to use 'as!' to force downcast?
_ = string as CFData

I think actually explanation of error message is some what wrong but user can know mismatch
of type to cast after replacing word.

[naru-jpn]

@xwu I'm glad if you see commit and write what do you think when you have a time!

[xwu]

If I understand correctly, your change causes the compiler to suggest a fix that doesn't work?

[naru-jpn]

@xwu Thank you for your message🙏🏻

Yes, if only once replacing.

Message from SR-7015 is the compiler produces a diagnostic which says "Conditional downcast to CoreFoundation type ... will always succeed". This does not offer any suggestions about what to do instead..
Originally user did not have a way to know actually each types are convertible. Suggestion cannot fix problem only once but user can know each types is not convertible after suggestion is accepted. I thought user can get more information than before.

There are many codes to fix this optional cast problem. So I think the way is implementing codes to compare CFType and others in case optional cast or escaping to already solved situations. I only can escaping way just now. Do you think should I implement complete suggestion?

[naru-jpn]

@xwu I decide to close this PR and I'll create more meaningful fix if possible! Thanks to spend of your time!!

[xwu]

Thanks for your effort. I'm not sure I understand what you're asking, but it's challenging to write good error messages. I'm still not sure, after this discussion, what the answer is to the question: when this error occurs, how do I fix my code? Your fix should answer that question.