Resolved: Insert "!" is a bad fixit

Evolution Pitches
83

Interesting. Can you elaborate on what do you see as the proper place for optional chaining then?

84

All compiler errors should suggest possible fixes, and if possible, those suggestions should be written in a way that is useful to people who may not understand the error at all.

My logic is that we don’t design errors to protect developers who choose to mash buttons without understanding from themselves. If they aren’t ready to learn about a concept now, it’s not the role of the compiler to force them to do so right this second before they can make a compiling program. Either they will circle back later and learn the concept, or they won’t. But if they’re mashing buttons, they aren’t ready to learn about it now.

1 Like
85

I would appreciate feedback on this updated proposal: [Replaced] Introducing the Unwrap-or-Die Operator (please see new PR) by erica · Pull Request #729 · apple/swift-evolution · GitHub

1 Like
86

How about starting a new thread for this?

87

I see it as useful to propagate optional intermediate results until the point they need to be unwrapped. It seems like this is precisely the use that you feel should be disallowed.

88

Oh, of course. The obvious and preferred handling of an optional chain (along with any other optional) is checked unwrapping with if let/guard let.

I was focusing on valid cases where a simple fix-it of '?' or '??' would be enough to get a valid and reasonable result without force unwrapping or checked unwrapping (the preferred normal way to handle the optionals)

Back to the thread subject. Everybody agrees that a lone Insert '!' is not a good fix-it for novice developers. I don't think we should remove the fix-it altogether but improve it in multiple ways:

  1. Provide a distinct diagnostic for the last part of a chain, instead of using the same diagnostic that is used in the middle of the chain.
  2. Provide a novice friendly message for the diagnostic.
  3. Provide '?? <#default value#>' as the first suggested fix-it.
  4. Expand '!' fix-it message to remind what '!' asserts.
  5. (Much Later) Add static analysis to backtrack '!' assertions to their originating operation and warn the user to move the assertion there or to perform a checked unwrap at that location to reduce unintended propagation of optional types.
2 Likes
89

I couldn't agree more. For some reason, this patronizing behavior of trying to save clueless programmers that have no idea what they are doing, keeps coming up.

And beyond that, there are absolutely cases where inserting "!" is a very appropriate fix it. For example when trying out stuff in Playgrounds and more importantly when using Swift as a scripting language.

90

Given how much of this thread appears within the proposal, I believe this is the right place for further discussion.

91

Please read the updated proposal and let me know if any parts are unduly patronizing. I will try to amend. I try to make clear the differentiation between language learners, those with experience (both in Swift and other Languages), and those who can do exactly as they need because they understand the consequence.

You bring up a larger, more important, point as well: not all Swift development is production code. Scripting, playgrounds, learning, and utility code with shorter life and more flexible safety requirements properly fall under the language umbrella.

92

I’ve been there myself. That fixit is a trap card waiting for novice users.

When that fixit was sprung on me, I only introduced crashes, and I didn’t know why until much, much later.

5 Likes
93

I’m sorry, but I don’t know what proposal are you guys discussing. Is it the “unwrap or die” operator’s proposal?

94

Here's a direct link (you can see it also by going through the pull request, clicking Files Changed, clicking View: https://github.com/erica/swift-evolution/blob/bfdf9e245f862de6302a0d9810ba5c1d322df821/proposals/XXXX-unwrap-or-die.md

1 Like
95

I understand now. I like the proposal! +1

Edit: and I think this criticism sums up the fixit issue very well:

Fixits should not be used as bandaids to make things compile

1 Like
96

Ok! I am going to argue the complete opposite of the thread title and say that RIGHT NOW, the main problem is that "!" is not offered as a fixit ENOUGH and that optional chaining with "?" is offered too often, and that this is the cause of the poor Apple dev forum / Stack Overflow example code.

This sort of horror, for instance cellDescriptors[(parentCellIndexPath?.section)!], as discussed already, is the result of optimistically offering optional chaining the first time, and only during the second compile when it is already there does the compiler realize that it still needs a force unwrap and offers ( )!. This code would be a big step better if it was just cellDescriptors[parentCellIndexPath!.section]. It is much more obvious what the actual optional thing is that needs to be handled, instead of sticking the ! unreadably and un-understandably far away.

So I did that. [Sema] More correct optional chaining vs. force unwrap fixits by gregomni · Pull Request #15321 · apple/swift · GitHub

This should improve the fixit choices and make things compile correctly after the first round, and is, I think, an improvement over the status quo. It is also a necessary first step before trying to make smarter fixits at the point of the declaration rather than at each use, because it makes clear (to the programmer and to the compiler) the declaration that is being force unwrapped each time.

97

I’m not a fan of the “unwrap or die” operator in isolation, but it doesn’t offend me to the extent of trying to prevent it entering the language. However, I find the motivation in the proposal troubling, and would like to explore the logic of that a bit.

The point about “die” is that we only want to do that in the case of what we call “programmer error”. Let’s start looking at cases:

(#1) We unwrap an optional that “cannot” be nil.

For example, the unwrap may be in a line immediately following a test for nil that leaves the scope if the optional’s value is nil. Since the error “can never happen”, there is no point in using !! instead of !.

(#2) But of course, programmers can make errors



so there are cases where we would actually "die" if there were a programmer error. The proposed solution seeks to “improve” this by providing an error message that “explains” what went wrong. That’s better than just crashing, isn’t it?

Well, no. If you can explain what went wrong, that means you know what the error was. If you know what the error was, the correct procedure is to go correct the error, not to code an error message and leave the error fatally lurking in the code.

An example of this scenario might be when you have a URL string that you want to turn into a URL object, and this string has already been validated as correctly-formed. If you can’t form a URL from the string, then something has gone horribly and inexplicably wrong somewhere.

But there’s really no point in using “unwrap or die” with a message like “A valid URL string failed to initialize a URL”, because you can generally craft a solution to the problem that doesn’t involve dying. For example, if URL initialization fails, it may be proper to treat the valid URL string as an invalid string, which is a class of non-fatal errors that already has an existing escape hatch (say via throw). Or you can invent a new escape hatch.

(#3) Actually, I went a bit too far in that previous paragraph. An escape hatch may not be feasible in practice.

Now you have an optional that’s nil, and it shouldn’t be, and you have no idea why, and you have no practical recovery mechanism. In this case, “die” is actually the only solution, but now the "unwrap or die" operator has a problem with the error message.

The message can’t explain anything (because you don’t know the explanation — if you did, this problem would eliminable under #2 above). What you’re left with is to use what is basically an arbitrary, unique string. All it's good for is to locate the source line if the string ever shows up in a console log just before a crash.

That’s the trouble I have with the motivation for the “unwrap or die” operator. It purports to explain an error, but it's only useful in a situation where no real explanation is available.

I have, of course, overstated my case in order to keep this post from being longer than it is. Things are never that simple, and the “unwrap or die” operator might be something we want in the Swift language anyway. But as a mitigation of the “bad” fixit problem, I don’t think it’s a strongly appealing candidate.

(Also, apologies for all the "air quotes". Stylistically bad, I know.)

5 Likes
98

I strongly agree that inserting “!” Is a bad fixit. This was the most confusing thing about learning Swift for me when it was released.

At the same time, I think it’s a shame that adding yet another operator is the focus of the discussion here. Reading the evolution proposal I was struck by a couple of things that I found very confusing:

let something = array.last !! “Reason why Array isn’t empty”

And

let cast = xyz as? AnotherType !! “Here’s some text”

I find both cases confusing, especially from the perspective of a beginner. Where is the information as to what is optional in the array example? (What’d be worse is if this was not a standard library type where most experienced devs are aware what is optional). And where is the information as to why the second example should crash given the normally safe “?” in the second?

While I think this may be a “nifty” addition for people who know what’s going on here, in certain situations (not the ones above though!), this is just more noise and confusion amongst the sea of !s and ?s that seemingly motivated this proposal in the first place.

There have been some excellent suggestions above regarding improving the compiler diagnostics. One that seemed particularly helpful outlined:

  1. The name of the variable that has the optional type of Type?. If it’s not a variable, spelling out which function call or initialiser returns an optional result.
  2. Mentioning explicitly that its value can be nil and therefore needs to be unwrapped (possibly when it was created)
  3. Suggesting using if let, guard let, OR, asserting the value can never be nil by inserting ! either here or (in the case of a variable) when it was created.
  4. Alternatively adding ?? <> to provide a fallback value.

This is a very worthy topic but I really don’t think adding even more surface to the language does anything to help new users understand what’s happening here. Also as an experienced developer I can see situations (just two mentioned above) where !! significantly reduces code readability.

3 Likes
99

+1. I think that the proposal very well motivated and balanced. I like some sort of "Design by Contract" inspired solutions in Swift (guards, preconditions, etc.) and !! will improve this direction.

100

I also like that !! covers as? and try? cases.

101

I agree that in cocoa nil is a mild failure but in my pure Swift libraries and many that I see outside of the legacy platforms #1 is far more prevalent.

102

I just wanted to follow up on this thread. Due to the discussion and suggestions here (thank you all!) and some work over the last month or so by @Douglas_Gregor and I and related general type check fix cleanup by @xedin and @rudkx, a lot of these issues are improved. The force unwrap fixit still exists, but it is now never the only or preferred fixit offered, and hopefully the explanations of the errors are a lot more beginner-friendly now.

  • For the original motivating example:

    10%20PM
    10%20PM1252×206 47.5 KB

  • And for member access with optional chaining:
    54%20PM

  • The compiler is now smarter about optional chaining in general, and won't even offer force-unwrap as a fix where the contextually required type is optional or if the member itself returns optional (because you'd have to deal with that member's result via force-unwrap or a default value in any case).

    00%20PM
    00%20PM1508×166 40.2 KB

    14%20PM
    14%20PM1572×306 82 KB

  • If you do choose to force-unwrap with '!' in an expression that contains an optional chain, the compiler fixit now changes the chain into a force instead of wrapping the whole mess in parentheses. I.e. if you choose the '!' fix for object?.url it will now become object!.url instead of the former especially lovely (object?.url)!.

  • Finally, if a local variable is inferred to be optional, but then used only in an expression that needs to be non-optional, the compiler can now offer to fix this where the variable is declared instead of only where it is used. In addition, that declaration can be converted via fixit into a guard statement.

    35%20PM
    35%20PM1574×308 72.8 KB

    becomes:
    04%20PM
    04%20PM1602×176 32.8 KB

More details in:
https://github.com/apple/swift/pull/17921
https://github.com/apple/swift/pull/18094
https://github.com/apple/swift/pull/18324

35 Likes