Skip to content

[SourceKit] Force print '!' for optional method call in code completion. #16910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 31, 2018
Merged

[SourceKit] Force print '!' for optional method call in code completion. #16910

merged 1 commit into from
May 31, 2018

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented May 30, 2018 •
edited
Loading

Calling @objc optional func requires ? or ! after its name. When completing method calls for them, key.sourcetext should have ! whereas key.name shouldn't.

Previously,

@objc protocol Foo {
    @objc optional func bar()
}

func foo(_ foo: Foo) {
    foo.<HERE>
}

completing at <HERE> used to complete bar()(without !).

rdar://problem/37904574

@rintaro
Copy link
Member Author

rintaro commented May 30, 2018

@swift-ci Please smoke test

@rintaro rintaro changed the title [SourceKit] For print '!' for optional method call in code completion. [SourceKit] Force print '!' for optional method call in code completion. May 30, 2018
@rintaro
Copy link
Member Author

rintaro commented May 30, 2018

@benlangmuir Could you take a look? I'm not 100% sure this fix is OK.
Also, I'm wondering it should be defaulted to ? instead of !. If yes key.typename should be optional?

benlangmuir
benlangmuir requested changes May 30, 2018
View reviewed changes
Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think making it optional (with optional typename) would be better.

test/SourceKit/CodeComplete/complete_optionalmethod.swift Outdated
@@ -0,0 +1,13 @@
import Foundation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really necessary in a test? I don't think most of our tests with @objc are importing Foundation.

tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp Outdated
}
if (!C.isAnnotation() && C.hasText()) {
case ChunkKind::OptionalMethodCallTail:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the chunk not an annotation? Then it would be handled by the simple case below. Generally annotation chunks should be skipped.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it isAnnotation() to prevent key.name to have ! in it. I'll try to modify key.name generation instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would be better. It wouldn't be the end of the world if the ! or ? ended up in the name either.

@rintaro rintaro force-pushed the sourcekit-complete-optionalmethod branch 2 times, most recently from a8cbd3e to 7fdfe7a Compare May 31, 2018 18:12
@rintaro
Copy link
Member Author

rintaro commented May 31, 2018

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented May 31, 2018 •
edited
Loading

We decided not to make it optional typename. It's consistent with the behavior for optional chain.

@rintaro
[SourceKit] Force print '?' for optional method call in code completion.
e159a06 
Calling '@objc optional func' requires '?' or '!' after its name. When
completing method calls for them, 'key.sourcetext' should have '?'
whereas 'key.name' shouldn't.
Note that we deliberately do not use optional type name for
'key.typename'. This is consistent with optional chain '?.<propertyName>'
behavior.

rdar://problem/37904574
@rintaro rintaro force-pushed the sourcekit-complete-optionalmethod branch from 7fdfe7a to e159a06 Compare May 31, 2018 20:06
@rintaro
Copy link
Member Author

rintaro commented May 31, 2018

@swift-ci Please smoke test

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented May 31, 2018

@swift-ci Please smoke test

@rintaro rintaro merged commit 66c71d1 into swiftlang:master May 31, 2018
@rintaro rintaro deleted the sourcekit-complete-optionalmethod branch May 31, 2018 23:06
kitasuke pushed a commit to kitasuke/swift that referenced this pull request Jun 9, 2018
@rintaro @kitasuke
[SourceKit] Force print '?' for optional method call in code completi…
cfe1e35 
…on. (swiftlang#16910)

Calling '@objc optional func' requires '?' or '!' after its name. When
completing method calls for them, 'key.sourcetext' should have '?'
whereas 'key.name' shouldn't.
Note that we deliberately do not use optional type name for
'key.typename'. This is consistent with optional chain '?.<propertyName>'
behavior.

rdar://problem/37904574
@AnnaZaks AnnaZaks mannequin mentioned this pull request Jul 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benlangmuir benlangmuir benlangmuir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rintaro @benlangmuir