Skip to content
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.

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

[SourceKit] Add typealias to doc structure - SR-4828 #11143

Merged
merged 1 commit into from
Jul 26, 2017

Conversation

johnfairh
Copy link
Contributor

This adds typealias declarations to the SourceKit document structure as included in the response from source.request.editor.open.{ interface/ interface.header/ interface.swiftsource/ interface.swifttype} and editor.replacetext. Update tests.

Resolves SR-4828.

One reason why it’s important for SourceKit clients to have all declarations included in doc structure is that it is the only interface that includes accessibility. There are 3-4 decl types missing/wrong right now.

This PR is pretty tentative: I can believe there are Xcode dependencies making it impractical for community changes to this part of the code, but I thought it was worth a shot. I built a toolchain and Xcode seems happy enough with it, not comprehensive ofc.

Code note: I added the new enum case alongside the other decl enum cases rather than at the end because the enum does not look to get persisted anywhere.

@CodaFi CodaFi requested a review from nkcsgexi July 24, 2017 21:43
@CodaFi
Copy link
Member

CodaFi commented Jul 24, 2017

This looks great, thank you for doing this ✨. Somebody that owns SourceKit should give you approval here, but in the meantime

@swift-ci please smoke test

@nkcsgexi
Copy link
Member

This is great improvement! Could you also add a lib/ide test in addition to the source kit test? You can find similar test in test/IDE/structure.swift.

@CodaFi
Copy link
Member

CodaFi commented Jul 25, 2017

@swift-ci please smoke test

@johnfairh
Copy link
Contributor Author

Added lib/ide test.

@nkcsgexi nkcsgexi merged commit 1f94eca into apple:master Jul 26, 2017
@nkcsgexi
Copy link
Member

Looks good. Merging.

@johnfairh
Copy link
Contributor Author

Great - thanks both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants