[swift-evolution] Change default compiler fix for not-unwrapped Optional from ! To ?

[Rod Brown]

> On 15 Jul 2017, at 9:54 am, Robert Bennett via swift-evolution <swift-evolution at swift.org> wrote:
> 
> For expressions whose type is definitely non-optional, it makes sense to verbally suggest both ! and ?? (and/or !! – was that accepted?), and it’s probably best to insert ! when the fixit is accepted. For expressions whose type is undetermined, but which need some form of Optional handling, it would be best for the compiler to assume the type is meant to be optional and insert a ?.

I think its fair that where the context can be optional, the default fixit should be optional. I’ve seen this issue myself from junior swift devs where they apply a fixit.

e.g.:

let optionalReturnedValue = item.optionalValue.optionalValue2?.item

And the fixit Xcode recommends on optionalValue is “!"…

let optionalReturnedValue = item.optionalValue!.optionalValue2?.item

It seems wiser in these cases if the compiler is smart enough for the default fixit to be “?”. I’m not sure if the compiler is able to do that or is in scope, that’s for smarter people than I to say.

In cases where the compiler, due to the context, requires the value to be non-optional, for a “!”, “??”, or the proposed “!!” makes sense.

“If let”, “guard let” etc all very much butcher the code at the callsite. They fundamentally change the control flow, and I could imagine many of the times its suggestions really won’t make sense for the given situation. How smart should we make the compiler before we’re practically making it rewrite all your code anyway? In this case, I’d rather there be no fixit at all, and just let the developer actually understand and restructure their code to correctly handle the optional case.

- Rod


> 
> On Jul 14, 2017, at 2:41 PM, Erica Sadun via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
> 
>> 
>>> On Jul 14, 2017, at 2:11 AM, Víctor Pimentel via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>> 
>>>> On 14 Jul 2017, at 08:05, Rod Brown via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On 14 Jul 2017, at 2:36 pm, Robert Bennett via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>>>> 
>>>>> When writing Swift code, it is not uncommon to forget to unwrap optionals. The compiler will offer a fixit, telling you you must insert either a ? or a !. However, when you accept the fixit, ! is inserted (at least, it is for me in Xcode 8.3).
>>>> When you treat an optional as non-optional, the compiler has no way to do a fixit that would appropriately handle the optional. Felix made a good example. The only direct fixit would be a force unwrap. If you used “?” then your non-optional use would turn into an optional and your parameter would generally be non-optional. The fixit is just explicitly doing what you implicitly expected it to do.
>>>> 
>>>>> 
>>>>> Ideally the fixit would default to ? because this is the preferred option; ! is often a sign of non-Swifty code and does not interact well with idiomatic Swift constructs such as if-let-(as?), guard-let-(as?)-else, etc. Also I think it’s safe to say that fixits should not err on the side of crashing at runtime.
>>>> 
>>>> " ! is often a sign of non-Swifty code “
>>>> I would strongly challenge this assumption. Many core team members have commented about appropriate uses of the ! operator. It shouldn’t be used lightly, but it’s there for a reason and it most definitely isn’t “non-swifty”.
>>> 
>>> I think it's not a farfetched assumption to imply that if the compiler encounters that code, the programmer didn't realize or remember that they were dealing with optionals.
>>> 
>>> If the compiler suggests to force unwrap that expression, it is also fair to assume that most inexperience programmers will just apply that fix it. A more experience programmer can decide whether to force unwrap it or to handle the nil case in any other way, depending on the context and code style.
>>> 
>>> Personally, I'd prefer that the compiler didn't encourage so much to use force unwrapping, not because I think that it has no use, but because I think that newbies should not learn that pattern first.
>>> 
>>> Surely, if I were new to the language and the compiler kept doing that fix it, I would think that it's "the way" to deal with optionals.
>>> 
>>> What would I prefer? At least, for the fix it to provide several options, more or less in this order:
>>> 
>>> - checked unwrap (if applies)
>>> - if let
>>> - guard let
>>> - force unwrap
>> 
>> Unfortunately, I think `!` *is* the proper fix-it here. The mechanisms to insert conditional binding, for example, are too unwieldy a tool at the many arbitrary places where an optional is used in place of a non-optional. I believe introducing an unwrap-or-die operator in parens (see https://github.com/apple/swift-evolution/pull/729 <https://github.com/apple/swift-evolution/pull/729>) would provide a better fixit:
>> 
>> * A sophisticated developer is unlikely to use fix-its as a blunt instrument to make code work. Their use of  `!` can be appropriate and, in their hands, unlikely to be fixit-driven.
>> 
>> * The unsophisticated developer is better served with `!!`, which self-documents the safety issue and can be further evaluated for refactoring, supporting safer learner patterns. Unlike the other approaches for conditional binding, `!!` is fix-it friendly. 
>> 
>> -- E
>> 
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170715/7e6db54e/attachment.html>