Resolved: Insert "!" is a bad fixit

Evolution Pitches
1

Over time, I have introduced proposals to make the use of force unwraps more readable, intentional, and maintainable. My proposed solutions included the unwrap or die operator and throwable nil-coalescing. Whether these solutions are suitable for Swift is orthogonal to a more fundamental issue of !-insertion fixits.

My gut feeling is that the fixit for Insert "!" forms one of the most dangerous enticements for language users, especially those new to Swift.

56%20AM
56%20AM988Ă—156 17.3 KB

Eliminating this fixit entirely could lead new users to better solutions while allowing experienced developers to use forced-unwrap in a considered and intentional manner. Before I file a bug report, I thought I'd bring this to the forums for feedback and insights. Thank you in advance.

30 Likes
2

I completely agree. In my opinion, force-unwrapping should be used extremely sparingly, and encouraging its use leads to poorly-designed code.

6 Likes
3

How does eliminating the fixit lead to better solutions?

I agree that encouraging people to force unwrap is not ideal, but we really want to produce actionable diagnostics, and just pointing out that the optional is not unwrapped without giving any indication of how to fix the problem is not ideal either.

Do you have a specific suggestion for what we should produce here instead?

(BTW, I have my own problem with that particular diagnostic in that we often suggest '?' when it's absolutely not applicable).

2 Likes
4

I believe the better or "correct" solutions cannot be automatically implemented such as using guard or if-let, often with secondary declarations of local variables.

In this case, the compiler should catch and warn. The experienced user will know to add "!" (which is already in the feedback). The inexperienced user should not be given a dangerous "remedy", just to provide a "Fix".

5 Likes
5

I know three "macro-like" ways to fix the provided sample code:

// Won't compile: url is optional
let resourceData = try String(contentsOf: url, encoding: .utf8)
... // remaining code until end of scope

One is wrapping everything in an if let:

if let url = url {
    let resourceData = try String(contentsOf: url, encoding: .utf8)
    ... // remaining code
}

One is guard:

guard let url = url else {
    <#placeholder for exiting code#>
}
let resourceData = try String(contentsOf: url, encoding: .utf8)
... // remaining code

(But the guard won't work unless url can be shadowed.)

A last one is Optional.map/flatMap depending on the return type:

let resourceData = try url.map { url in String(contentsOf: url, encoding: .utf8) }
... // remaining code

I don't know if fix-its are able of those.

And anyway, none of them scale well when you have several optionals to "fix".

:-/ Difficult problem :-)

2 Likes
6

we could insert ?? <# default value #> instead? Maybe the fixit could offer multiple options with the ! one at the bottom of the list to discourage it's use

9 Likes
7

I just had that thought as well. IIRC we already produce this for the Optional-to-Any assignment warning.

8

We do! For that case, we suggest 3 things, !, ?? and as Any

Seems like we should at the very least do both ! and ?? for the case in question

1 Like
9

The drawback to this is inexperienced users (especially people brand new to the language who are forced to confront Optional pretty quickly) getting stuck. Some will think to do a web search for substrings of the message to "unstuck" themselves, but not everyone. Not everyone who does that search will immediately understand the posts they find explaining what's going on, either.

10

Strong support for everything Erica said. Stack Overflow questions (and answers) are littered with the unfortunately results of this fixit.

Mark Lacey asked:

How does eliminating the fixit lead to better solutions?

The correct way to hand an optional is to stop and think. What does “none” mean in this context? In what circumstances can that thing happen? Is there a reasonable recovery path? Is proceeding with bad assumptions more dangerous than killing the process? Or is bringing everything down more destructive than a soft failure?

The answers to these questions are completely different if you’re implementing, say, a database vs. a progress bar.

Optional handling is far situational for any one fixit to be the correct default.

3 Likes
11

This is why !! is such a clean solution. Not only does it document why an unwrap should never fail, it provides a proper fixit for exactly this solution:

!! <# reason why this value cannot be nil #>

1 Like
12

If !! becomes a thing, it should be one of the options suggested by the fixit, of course.

13

The core team has made it clear that !! is a non-starter.

1 Like
14

Perhaps it's time to resurrect that proposal and see if we can garner more support for it this time.

1 Like
15

Having the fixit does not stop experienced users from asking all of those questions and doing the right thing based on the answers.

Not having any fixit or some actionable diagnostic does stop new users from being able to progress in learning if it means they cannot get code to compile.

You may say, "Great, they shouldn't learn bad habits, so let them be stuck until they digest exactly what Optional is and exactly the right way to answer all of these questions", but that's really just an invitation to frustrate new users.

That's why I am asking for alternatives. If you want to suggest a message that says "use if let or guard to safely unwrap the optional before attempting to use it here", that may very well be fine - it doesn't have to be an automatically applied fixit to be actionable and helpful. Simply saying, "Value of optional type URL? not unwrapped" is not helpful for those users.

3 Likes
16

I disagree that providing ! as an option is bad.

It is true that some users may opt for it when they really should be considering the nil case. But it is also true that some users, new and experienced alike, may forget to insert ! precisely because in their mental model of the scenario they have correctly reasoned that the value cannot be nil.

Some users have adopted the position that ! should be used almost never, but that is not the consensus. (As an aside, these users seem not to use preconditions or assertions.) Compiler diagnostics are not the place to litigate opinions of style. Force unwrapping is correctly used when any other assertion is correctly used, and in some cases that may be very frequently indeed but in others it may not be advisable.

5 Likes
17

we could insert ?? <# default value #> instead?

This is the best idea so far. It could be added to the ! fixit (remember, fixits can suggest several options).

We have something equivalent in string interpolation, which works pretty well : "\(optional)".

18

It's the easy-fixit that's the problem, not the compiler feedback. The IDE already points to the error item, and it's a single keystroke to add the !. This is not litigating opinions of style and forced unwraps play an important role in mature code. This is avoiding a moral hazard with an absolute minimum effect on experienced users.

19

I do agree that it being the only fixit presented is a moral hazard. We should definitely add something like ?? <# default value #> and maybe make that the top suggestion.

But if you're saying that adding ! is one keystroke anyway, then--as long as it's not the only suggestion--pressing a fixit button is no easier, and on that basis showing it isn't the problem and hiding it won't make any problem go away.

1 Like
20

The problem with the fixit (for inexperienced Swift programmers) is that this is almost certainly not the point at which things went wrong. Almost always (for the inexperienced) the problem is that url is optional by accident, and the correct fix is to add a ! to the RHS of the assignment from which url's type was inferred — or, at least, to handle the optional there.

This is somewhat analogous to when you try to mutate a let variable. In that case, you get a fixit offering to change its declaration to var, which is often the right thing to do.

There are many, many inexperienced Swift programmers who ignore optionality, then simply use fix-its to have the compiler insert enough ! and ? operators to make the errors go away. You can easily see this in the code fragments posted over at forums.developer.apple.com.

6 Likes