Resolved: Insert "!" is a bad fixit

Evolution Pitches
42

I think it's worth saying (or at least throwing out for dispute) that the fixit offering to add ! isn't of much value to anyone. For a beginner, it's often the wrong fix. For everyone, when it's actually the correct solution, applying the fixit is rarely faster than typing the single !character.

(There are exceptions, of course. With a heavily parenthesized expression, for example, it can be awkward to see exactly where to put the !.)

In terms of a better fixit, it seems at least possible that the compiler might distinguish between cases where the user explicitly called for an optional type:

let z = 1 as Int? // ok because of explicit cast to optional
let z: Int? = Int (exactly: 1) // ok because of type annotation

and cases where the optionality wasn't overt in the source code:

let x = 2 as? Int // implied cast to Int?, but not obviously to a newbie
let x = Int (exactly: 3) // return type is Int?
// assuming: let y: Int! = 4
let x = y // IUO implicitly becomes Int?
let x = z // already Int?, but unobviously due to lack of context

In these cases, if the variable is used where a non-optional is needed, it would be reasonable for the compiler to offer to add ! to the end of the RHS expression, or to wrap the declaration in guard let or if let. At the very least, the compiler should suggest changing the declaration to make x non-optional, even if it can't provide a fixit.

Secondarily, it seems like it should be possible for the compiler to determine that a variable is always used with ? or !, and then provide a warning (and maybe a fixit) to suggest that the variable be made non-optional (thus cleaning up code that's already littered with ! and ?). Ideally, there would be some flow analysis to see whether a nil value is ever used (rather than being optional-chained or force-unwrapped out of existence).

This would be similar to the current message suggesting that an unmutated var variable be changed to let.

2 Likes
43

Yes, this is the real root of the problem, not ! fix it.

Compiler needs to start warning when a function returns an optional and when you start propagating optional types through type inference like this. It needs a deeper static analysis, but I think compiler should encourage you to deal with optionals as you receive them from functions and ask for an explicit type cast or type annotation when your code looks like it is unintentionally propagating optional type all over the place by ignoring that some function returns an optional result. It is a much better point to help novices learn Swift faster and better.

Don't get me wrong, I do agree that we should add additional fixit options and prioritize safer options first in the case of !.

44

Isn't one of the huge benefits of optionals that you can safely propagate them until a later point when you absolutely need to deal with them? Encouraging people to unwrap immediately kind of defeats the purpose, doesn't it?

45

That is why I mentioned it needs a deeper static analysis to try to identify the intent correctly. For example if you know let x = Int (exactly: 3) is guaranteed to always have value, why not assert it as let x = Int (exactly: 3)! instead of asserting it with ! several levels down at the point of use?

46

Examples with statically known values are easy to make but don't generalize. If you have let x = Int(exactly: 3), then the compiler should instead suggest let x: Int? = 3 and let x = 3 as alternatives.

But we're talking about diagnostics that are meant to be useful in a wide range of cases, and I don't see how you intend to give the compiler the smarts to infer intent.

1 Like
47

I think the answer to this is — counterintuitively — no, not usually.

There are (semantically) two kinds of optionals. One is when a result is "genuinely" optional, meaning you don't necessarily need it. In that case, you want to carry the optional around, kind of like traveling with a suitcase which may or may not be empty. (If necessary, you can buy or borrow socks later.)

The other is where a nil result means you don't have something you need. If you can't get socks en route, you'd better check the suitcase before you depart.

The counterintuitive part is that #2 occurs in the wild much more often than #1, especially in Cocoa programming, where a nil return from SDK APIs is often a kind of mild failure.

It can't infer intent from the declaration, but it can make a more informed guess if the code refers to an optional without unwrapping, especially if the code does that more than once.

48

But it's not up to the library to determine which of the semantic scenarios arises; it's up to the end user. The same method can be "genuinely optional" to me and not to you, and Swift's design is meant to support this usage--first and foremost, I would argue.

That's equivalent to inferring that the "genuinely optional" case is essentially never intentional. I disagree.

50

I think it's more of a human problem, and it's a bad problem because it has 2 levels:

  1. It turns out that masses of users have no clue that a declared variable is optional, until the compiler tells them at a point of use.

  2. It turns out that masses of users don't realize that they can change a declared variable to a non-optional, by unwrapping its initial value somehow (! or guard let or if let).

Instead, they think they're condemned to using the optional variable as-is, and so to fixitting the errors wherever they occur. As a result, they think that optionals are one of Swift's dumbest features, not one of its best. (To them, the compiler knows how to fix the problem, so why doesn't it just do it?)**

Clearly, the problem would be solved by educating these users, but how to do that? Currently, the compiler reinforces their ignorance by offering ? or ! via a fixit, since the existence of a fixit tends to suggest that it is the "correct" fix, at least to these same uneducated users.

I would say that this thread is about how to better educate uninformed users, without interfering too much with what experienced users want to do. Would you be prepared to put up with the occasional unnecessary warning in your expertly-written code if a lot of inexpert users could be helped thereby?

** A couple of years back, someone posted in the Apple developer forums that the "problem" of handling ignorable whitespace around lexical tokens had been solved 40 years ago by Pascal, and Swift's requirement of equal whitespace surrounding binary operators was just ineptitude. Many people just don't "get" things until they learn what they're for, and sometimes the compiler is the only teacher they have.

1 Like
51

Many people and organizations treat warnings as errors. It would not be acceptable to have the compiler give false-positive warnings that can't be silenced without refactoring code that is correctly written. Doing so here would be tantamount to prohibiting propagation of Optional.none--quite the nonstarter.

52

Indeed. I agree 100%.

We're talking about two scenarios here, though. One is when an optionally-type variable is used where a non-optional value is required. In that case, there's an error that must be fixed. It does no harm to offer a fixit that rewrites the declaration and/or refactors code, as well as or instead of a fixit that inserts a ? or !, if judged appropriate. The user is free to fix the error a different way.

The other scenario is when there is no error, but the compiler can find a rewrite that preserves the same semantics, and the rewrite is judged unobjectionable. For example, if x is a variable of optional type, and the optionality wasn't very obvious at the declaration site, and the only references to x are of these forms:

  x!
  (x?.someNonOptionalProperty)!

then it seems harmless to offer to turn x into a IUO, or to be otherwise unwrapped at the call site.

I see this as analogous to the warning about unmutated var variables. A user with a policy of declaring everything var would be unable to clear such warnings, but there is a pragmatic decision in the current compiler not to cater to a user of these tastes.

I think this second scenario only fell into the discussion by accident, and can be ignored if unpalatable. The important question is what fixits are offered when an error occurs.

53

If it's a local declaration, then a fixit that offers the alternative of changing that declaration is totally fine by me. In fact, I think that's pretty great.

No fixit should offer to change a type's API, though (for example, changing an instance property's type), and fixits in general shouldn't change code to the point that one would call it "refactoring." Those are much too invasive to be made available by mashing a button.

I find that to be pretty unpalatable, myself. It would be subtly different to assert non-nil at every call site versus unwrapping initially in scenarios, for example, where multithreading is involved. [Edit: although, if it were purely a local binding, and the wrapped version is never passed to another function...hmm...] I'm sure there are other scenarios I haven't thought of. If restricted to let bindings, then I'd imagine it'd be feasible, but I haven't thought it through. In any case, I agree this is beyond the scope of the fixit issue that's the main topic of this thread.

54

Without commenting on the merits of “!!”, I don’t think changing the fixit to that improves anything, especially not for the newer programmers being discussed in this thread.

For them, when they mash the fixit to remove the error,

!! "Just compile already!"

becomes nothing more than an extra-verbose way of spelling “!”.

• • •

I think it is better to have the fixit offer multiple options:

  1. If a default value is applicable, insert “?? <default>”
  2. If the optional cannot be nil, insert “!”
  3. Consider unwrapping with “if let” or “guard let”

That way the developer can take their pick, and inexperienced developers will at least be exposed to multiple approaches for handling optionals.

2 Likes
55

I was continuing with @QuinceyMorris's example. Let me try to further clarify what I mean.

Assume a programmer writes this:

let x = optionalReturningFunc(/*args*/)
// Do some stuff
let y = optionalReturningFunc2(/*args*/)
// Do some more stuff
let z = optionalReturningFunc3(/*args*/)
// Do the rest of the stuff

Then the programmer types this, and compiler offers to insert ! for x, y and z:

let t = function(x,y,z)

The programmer accepts compiler suggestion and it becomes:

let t = function(x!,y!,z!)

Then the programmer runs the code, and it stops with a runtime error on the above line. A novice programmer will have a hard time figuring out which one of x, y, z or function is causing the error.

Now imagine if the compiler went further and after the above fix-it was applied, generated a new warning suggesting the programmer move "I know it is not nil" assertion where this assertion actually materializes here:

let x =  optionalReturningFunc(/*args*/) // Warning: ...

The new warning could read something like this:

Warning: x is asserted not-nil at use site and is not checked or manipulated after this point.

With the following "fix it" suggestions:

  1. Put ! after the function call to assert the function result can't be nil.
  2. Put explicit optional type to silence this warning.
    (It could go even further and suggest handling nil result with if let or guard let)

If the programmer accepts the first option, the code ultimately becomes:

let x = optionalReturningFunc(/*args*/)!
// Do some stuff
let y = optionalReturningFunc2(/*args*/)!
// Do some more stuff
let z = optionalReturningFunc3(/*args*/)!
// Do the rest of the stuff
let t = function(x,y,z)

This time, when the programmer runs the code, it fails on the declaration site of either x, y, or z and our imaginary programmer will have a better idea what went wrong and probably add a check to see if the function actually returned a value.

We could go further and make things even more intelligent, but I hope you get the basic idea of putting assertions that programmer makes where the assertion actually materializes.

1 Like
56

As I wrote in reply to @QuinceyMorris, I agree that for local bindings this is a valid fixit option.

As he correctly points out, though, discussion veered towards much more expansive transformations that go beyond this example--it's in those transformations where I've said that the compiler would overstep the mark in trying to deduce intent.

57

If the fixit is something like !! <# "Explanation why the left hand value cannot be nil." #> then there's a pretty good user-supporting hint that will counter a lot of "Value of optional type not unwrapped" confusion. Someone who puts in "Just compile already" over that kind of placeholder isn't fixable.

2 Likes
58

Should we add as! and try! to this discussion? There are some similiarities. Should developers describe why type conversation always succeeds or why error never occurs? Yes, this cases more ambiguous then unary !. But if we get !! then as! and try! can become next candidates for similiar operator.

59

+1 from me, this fixit has just taught a lot of people I've worked with to not actually understand their own code and click "fix" until it compiles.

1 Like
60

Where is !! described?

EDIT: odd, after I posted this, a great many more responses showed up, and I was able to search for more discussion about !!.

61

See @Erica_Sadun's old thread that proposed it.

1 Like
62

…and has just documented the level of thought they put into choosing the force unwrap, which is fantastic!

A lone ! might have been thoughtless fixit-mashing, or might have been based on careful analysis of the whole system’s invariants. Code that makes clear its own thoughtlessness, while not ideal, is still more maintainable than code whose thoughtlessness must be determined through painstaking analysis.

2 Likes