[SR-4637] Add fix-it for removing '@escaping' from optional closure parameter

[swift-ci]

Previous ID	SR-4637
Radar	None
Original Reporter	garricn (JIRA User)
Type	Bug
Status	Resolved
Resolution	Done

Attachment: Download

Environment

Xcode 8.3.1, Swift 3.1

Additional Detail from JIRA

Votes	2
Component/s	Compiler
Labels	Bug, StarterBug
Assignee	ismetanin (JIRA)
Priority	Medium

md5: ea3ce9103e3b95d06baa97059732e723

Issue Description:

As of SR-2444, optional closures as function parameters are already escaping escaping by default. Currently, when adding the @escaping attribute to an optional closure parameter, the compiler throws the following error:

@escaping attribute only applies to function types

Since this type is already escaping escaping by default, it would be helpful to add / amend the error as follows:

Optional closures are already @escaping.

And add the following Fix-it:

Fix-it Delete "@escaping"

[swift-ci]

Comment by Garric Nahapetian (JIRA)

Thank you @belkadan for updating this SR. It's my fist so I'm just getting the hang of things. Any feedback/help is greatly appreciated 🙂

[milseman]

I'm not sure "by default" is the correct phrasing here, as I don't think there's any way to make them non-escaping. "Optional closures are already escaping" could be a different error message, or alternatively a note. But the fixit would be very helpful!

garricn (JIRA User) ask if you have any questions

[swift-ci]

Comment by Garric Nahapetian (JIRA)

I certainly have/will have questions.

1) I plan to start working on it this weekend. I don't need to feel rushed, correct?
2) To be sure, am I to replace the error "@escaping attribute only applies to function types" with the error "Optional closures are already @escaping"? Or can / should I supplement the existing error with the new error containing the Fix-it?
3) I plan to do some logical investigating to try to find out how to solve this on my own. I keep hearing Ayaka's voice in my head "What else does this same or similar thing? Nevertheless, help is greatly appreciated!
4) I found this file: https://github.com/apple/swift/blob/30dd1a90474032717332f26aca8acbfda427d8bb/test/attr/attr_escaping.swift which looks like it has a lot to do with this except that its a test file, I think, and its a Swift file. I have a feeling I need to get into a .cpp file.
5) I also have this: #4905 to refer to which sounds related.

Anyhoo, thank you and good night! 🙂

[milseman]

1) Correct.
2) The existing error is still important, you can't have an argument that is @escaping Int, for instance. Keep the current behavior in the general case, but you're checking for the special case of optional closure. "If this is an optional closure, then emit the more specific and helpful diagnostic". And I think the message should be "Optional closures are already escaping".
3) Find the diag code for the current error message, then grep the source base for uses of that diag to see where to put your special case checking logic. You can then search that section of the code for other things that might check for e.g. optionality.
4) That is a test file (hence being under the test directory). You can do a TDD-like development flow by adding the test cases where you think you should have a better diagnostic and fixit around, then make those tests pass
5) That PR does AST printing, which should be orthogonal to this effort. You'll probably be modifying files with "Sema..." or "TypeCheck..." in their names, as this diagnostic is emitted as part of type checking.

[swift-ci]

Comment by Garric Nahapetian (JIRA)

@milseman Excellent feedback. Thank you!

[swift-ci]

Comment by Kaden Wilkinson (JIRA)

Error is currently pretty cryptic in Swift 4 

[swift-ci]

Comment by Michael Critz (JIRA)

#tryswift

[swift-ci]

Comment by Garric Nahapetian (JIRA)

I thought we already fixed this: #8947

I wonder what happened...

[swift-ci]

Comment by Michael Critz (JIRA)

garricn (JIRA User) I was able to reproduce the original bug. The note didn’t appear in the fixit UI. The work is mostly done. I’d like to add some minimal logic to the fix-it to be more informative and accurate. Basically, add the suggested text from the April 2017 comments.

[swift-ci]

Comment by Garric Nahapetian (JIRA)

Great!

[belkadan]

critz (JIRA User), are you still working on this?

(I'm checking in on all the StarterBugs that haven't been touched in over a month; it's totally fine if you just haven't had time but still want to keep it.)

[swift-ci]

Comment by Michael Critz (JIRA)

@belkadan not actively. Feel free to bump me off this.

[belkadan]

Done. You're welcome to return to it if you get the time!

[swift-ci]

Comment by Ivan Smetanin (JIRA)

Fixed in #30746