Resolved: Insert "!" is a bad fixit

As somebody who spends his days teaching beginners to code, I can appreciate the desire to help newcomers learn. And it’s true that an incomprehensible, non-actionable error message can be a daunting barrier.

Autopilot is also a danger. Beginners have a strong tendency to throw code at the compiler until it runs.

The best teaching both prompts people to stop and think and gives them a path forward. I’d be in favor of:

  • the more helpful error message you proposed above (Elm is a model to follow here), and
  • providing multiple fixits (I’d argue for ??, if let, and !, in that order).
2 Likes

I believe Quincey Morris explains the issue better here:

Offering any fixit at the point of use instead of just the error message creates the problem for these users. Removing the fixit (outside of introducing !!) ensures the best outcome for both naive and experienced coders.

2 Likes

I agree with @rudkx that not providing any fixit here is poor form especially for new users. If they are so new to programming that they rely on mashing a button to get things to compile, then we ought to give them some least unreasonable form of that. The alternative is not that they’ll do a detailed analysis of their code, of which by construction they are not capable, but that they’ll give up.

1 Like

TBH, I was hoping someone would suggest a feasible fixit for the earlier site. There are a few identifiable cases:

— When url was declared as

  let url = y

where y is IUO. In that case, the best fixit is likely let url = y!.

— When url was declared as

  let url = someFunctionReturningNilToMeanNoResult()

In that case, the best fixit might be

  guard let url = someFunctionReturningNilToMeanNoResult() else { /* handle the nil result */

intentionally triggering a second error because the guard doesn’t exit the scope, forcing the user to handle the error.

— But sometimes, an if let is what is wanted, which could be fixed as

  if let url = someFunctionReturningNilToMeanNoResult() {
    … // the rest of the existing scope
  }

The only problem is, that’s a pretty intrusive fix.

Still, it might be worth playing with fixit options like these, if they’re implementable in the compiler.

2 Likes

One of the greatest things about Swift is how easy it is to get started. Back when I learned in the 90s you needed to know stuff about compiler directives just to run “Hello World!”. While I agree that in my own code the fewer exclamation marks the better (I love being able to search a file for ‘!’ and getting zero hits) we really don’t want to make a language where your first experience is getting stuck. Novices should be able to make code run. If there’s a better fix, great, if not this fix is better than no fix.

2 Likes

Why is this dangerous for newcomers? So their app will crash in the future, that’s a great learning opportunity for a newcomer! Why would crashing be dangerous? Do you envision these newcomers to be writing Google Maps or the Falcon Heavy guidance system?

2 Likes

Yes, when I was teaching Swift this fixit would cause more problems than it would solve…

3 Likes

Here are a couple of actual examples from the Apple developer forums:

    func sliderDidChangeValue(_ newSliderValue: Float, parentCell: CustomCell) {  
  
        let parentCellIndexPath = tblExpandable.indexPath(for: parentCell)  
        getIndicesOfVisibleRows()  
        let indexOfChangedRow = visibleRowsPerSection[(parentCellIndexPath?.section)!][(parentCellIndexPath?.row)!]  
        let newStep = roundf(newSliderValue / 0.1)  
        let stepped = String.localizedStringWithFormat("%.1f %@", newStep * 0.1 , "") 
        ((cellDescriptors[(parentCellIndexPath?.section)!])[(indexOfChangedRow)]).updateValue(stepped, forKey: "secondaryTitle")  
        ((cellDescriptors[(parentCellIndexPath?.section)!])[(indexOfChangedRow)-1]).updateValue(stepped, forKey: "primaryTitle")  
        tblExpandable.reloadSections(IndexSet(integer: ((parentCellIndexPath?.section)!)), with: UITableViewRowAnimation.fade)  
    }  

This user seems to have learned to resolve optionality issues stepwise, following the initial fixit to insert a ?, then a second fixit to insert a !. It doesn’t crash, and it’s perfectly well-formed both syntactically and semantically.

    @objc func scrapePDF() {  
        let documentURL = self.documentDisplayWebView!.url!  
        let document = PDFDocument(url: documentURL)  
        let numberOfPages = document!.pageCount  
        DispatchQueue.global().async {  
            for pageNumber in 1...numberOfPages {  
               print(document?.page(at: pageNumber)!.string!)  
            }  
        }  
    }  

This user knew about using ! on document at the first reference, but apparently followed fixits the second time. Also, the user added ! to the documentURL declaration, but never thought of doing the exact same thing on the next line.

New users don’t learn anything from this. They just conclude that Swift is a stupid language.

I’ve seen much worse, much more convoluted, examples than these.

9 Likes

I see bad trend. Some companies just forbid force unwrapping without any exceptions. In this case “!” usage become error with special tools. Fix-it feature for “!” looks very puzzling in this context.
For some reasons “!” has undeserved bad “background”. This situation leads to gap between Swift capabilities and real usage on practice. May be we need some “!” subject restart in guides and documentation to promote good style force unwrapping usage. But it is not enough.
"!" usage should become more fundamental (may be more hard to use) but not just fix-it feature for unexperienced users.

In cases of custom objects/classes you cannot simply add the ?? for default values.

Adding default values are not necessarily a good thing, because in certain cases the logic might require unwrapping the optional instead.

my 2 cents.

If a change to this is being contemplated, then rather than simply remove the ! to unwrap it a good idea would be to suggest using some form of unwraping or the optional requires some work.

There is a lot of code that is badly written due to the suggestions made by the compiler and beginner developers.

so I would vote for better compiler messages and for better unwrapping optionals fix-it code.

I would love to be offered the option of guard let and if let by Xcode’s fix-its (assuming it’s capable of offering more than one suggested fix?).

Both of those are much safer, more “correct” ways to unwrap an optional value.

1 Like

In many cases, a forced unwrap, simply using !, is the right way to unwrap an optional value. The problem isn’t with ! existing or being used in code, especially when it’s established that an optional cannot be nil at the point of ! use. The problem is that the compiler is guiding newer users down a path of using ! in an extremely unsafe way because it cannot contextualize the ! with regards to developer intent.

New users, who may be unfamiliar with guard let and if let, which provide exit patterns and conditional binding of unwrapped values, are stuck in the mode of “what do I need to do to make this code compile?” Stack Overflow, the dev forums, IRC, various slacks, etc are overflowing with examples of terrible ! use. Those of us who mentor and teach see reflexive ! insertion “just to make things work” propagate because of the fixit availability.

This is not a reflection of ! being an antipattern. This is due to a problem with compiler tooling and specifically the “moral hazard” of its fixit.

I personally want to introduce !! the unwrap or die operator, which asserts that an item cannot be nil and then provides a succinct in-code reason why the optional must be .some. Using !! creates better in-context documentation at the point of use both for experienced and naive users.

In addition experienced users can always use ! because it is and will continue to be an important part of the Swift programming language.

I don’t want this thread to be a referendum on “Should I eschew ! in my code?”. You shouldn’t. The points I’m posing are these:

  • The Swift compiler lacks a holistic understanding of developer intent.
  • The ! fixit should not be used as a bandaid to make things compile.
  • Experienced developers can distinguish whether an unwrapped value reflects an overlooked unwrap or it is guaranteed to never contain nil.
  • Inexperienced developers, unless they’re moving from a language with similar constructs, usually will not. They’re focused on getting past a compilation barrier, without realizing the hazard of nil values at the point of use.
  • The fixit is a modest courtesy for experienced developers.
  • The fixit is a moral hazard for inexperienced developers.

Given Swift in its current form, the choices that should be considered are:

  1. Removing the fixit entirely but retaining the information in the error message.
  2. Removing the fixit and retooling the error message to offer a link to optional best-use tutorials.
  3. Offering a better fixit by adopting !!.

I don’t think “overhauling the compiler to guess user intent” is on the table nor “remove ! from the programming language”.

5 Likes

Excellent analysis. I’d add the ?? fixit option because it also helps education, and I’d love the !! or any “(stuff) or die with extra clear error message” alternative.

I also feel that preconditionFailure is a better support for !! than fatalError. But there is a long-standing Swift issue, SR-905, which prevents precondition messages to be present in the release builds, with the adverse consequence that precondition failure messages are lost when crashes happen on released software :’( Of course we could stick to fatalError. But it would be better if SR-905 was fixed. Because what’s the purpose of a clear die message if crash reports don’t show it?

2 Likes

Erica, maybe this “Insert ! is a bad fix-it” issue needs a revival of a !! proposal first? Since the core team said that !! was a non-starter, we could look for an acceptable way to provide the same feature?

This revival of !! would also be a better place to bring SR-905 on the table, because I think that this bug really needs support: library developers have to switch from preconditions to fatal errors, losing the benefits of unsafe builds optimizations.

I agree and have spoken to @Ben_Cohen about the matter. I’ll be revising it next week and would appreciate feedback as I retool it.

2 Likes

I’ll be glad to please!

It would be of enormous help to collect examples of “naive” unwrap use from the dev forums, Stack Overflow, and other locations to provide concrete code that demonstrates the “fixit to compile” issue. If you have examples, please post a link to them, add them here (please include provenance), or email me.

Thank you.

Just in case you missed it: @QuinceyMorris did provide two nice examples above, for inclusion in your collection of relevant code snippets: Resolved: Insert "!" is a bad fixit

They are not easy to read, but they have an interesting “real life” quality: this is no code written by a total newbie developer, but instead code written by developers that already have skills, and just have not yet been enlightened by Swift’s Optional.

I found that the magic is to search for “unexpectedly found nil” instead of trying to find !).

3 Likes