[WIP] Swift semantic diagnostics first pass #763
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds support for Swift semantic completion. It adds a new subclass of `Completer` which is a thin client for SwiftySwiftVim, the semantic backend. SwiftySwiftVim is an HTTP interface to semantic facilities in the Swift Compiler, tailored to the needs of a text editor. Usage: After you've met all of the system dependencies, then simply run build.py with --swift-completer. This will checkout and build SwiftySwiftVim from source in `third_party/swiftyswiftvim`. Build Summary: To build SwiftySwiftVim from source, you need a few `system` dependencies: - OSX ( for Xcode ) - Xcode ( for prebuilt swift binary ) - Boost See https://github.com/jerrymarino/swiftyswiftvim for detailed information.
This patch cleans up a few things in preperation for upstreaming. I added tests for debug info and completions, so we can make sure it is working end to end. I also addressed other comments from the code review including: - Code formatting and Flake 8 issues - Valloric's other comments Comments around backend dependencies should be resolved here: jerrymarino/swiftyswiftvim@abde37e Now OSX and Xcode options are used by default rather than hardcoded. This should make it easier for people to use this with other platforms in the future. Additionally, I improved query classification. It was previously returning cached results in circumstances that didn't make any sense. Mostly, I think this is in a good state. There are still a few remaining tasks that need to be addressed: - Add and test "flag" support - Write more documentation - Sort out the CI situation for OSX only
- Use 127.0.0.1 for an IP since that is conventional in YCMD. - Use minimum Xcode Version for SourceKit. In my testing, the travis images xcode8, xcode8.1, would not work. - Disable SwiftCompleter when swift is not present
Implement the clang compilation database standard for swift files. The format is specified for Clang: https://clang.llvm.org/docs/JSONCompilationDatabase.html I think this is a good file format to use for swift. The implementation includes a "CompilationDatabase" to read in the files and a high level API to get flags, "Flags". It uses logic based on the `cpp` version of `Flags`. I originally hoped to reusing the `cpp` version of this but there 2 show stoppers: - Flags.py logic is highly specialized to clang based languages. I don't like the idea of a complicated implementation that works for both swift and clang. - Swift users should not have to download the massive payload that clang is. The class simply takes a compilation invocation, and converts it to completion flags. Additionally, I am caching database loads based on the file timestamp. Flags are also cached, and I think the invalidation will need more work in the future. Based on my own testing, the logic seems sufficient for a first pass. There is tangentially related work around writing programs to generate these files for standard build systems: - SwiftPM / SwiftC. It'd be nice to geneate these comp dbs from the output of these programs. Perhaps a proposal can be made to get SwiftPM generating these. - Bazel: it doesn't support comp dbs for any language by default, but it is easy to add. - Xcode: Perhaps we can modify Xcpretty to and similar programs to get this working with Xcode. The standard method for comp dbs in `xcodebuild` is to parse the output and dump a DB based on that. A few OSS solutions exist for this but are limited to `clang` for now. I opened an issue here: xcpretty/xcpretty#281 I included some assertions that I wrote while developing this, and I'll need to get it into the YCMD test suite before merging.
This patch moves the compilation DB unit tests into the testing system. I added a fixture directory for this including: - a compilation database template to write absolute paths into the database during tests. - ViewController.swift which depends on AppDelegate.swift I also added integration tests for the comp db and improved completion tests in general. These test somewhat into SourceKit land, but it is good end to end test to make sure we aren't messing up. - Makes sure that reasonable completions are returned. - Recursive include test to make sure the flags are compatible: loading transitive headers in the SDK. The other assertion makes sure that we can correctly load dependencies via the `primary-file` flag. iOS is a good baseline since it's a massive swift userbase. The 2 OSX specific tests are wrapped to only execute on Darwin.
The integration test only works on the new Xcode and it will not work if the compile commands includes flags with missing build artifacts. For example, if the comp db includes flags that reference output from a build that is no longer around ( or on a different machine in this case)
This patch removes completions for operators and addresses review feedback. We were originally returning completions for operator by hacking the completion offset. This may have been intentional at one point ( or an off by 1); after using this a lot and @puremournings feedback I think its best to remove it. This enables expressing the ' ' trigger within YCMD and cleans up a lot. Additionally: - Add an example in the test code of docs showing in the UI - Improve comprehension of uncommented completions - Add lock on startup and shutdown - Add Flag logging - Remove completion type customization - Address manual formatting consistency - Remove snippets by default. This needs extra support code in vimscript and is probably out of scope for a V1 to integrate this end to end or opensource my plugin. With the types rendering along with function signatures in the menu and preview window it's seems fine for now. I am quite happy with it now, overall, it feels way nicer!
After some manual testing and studying the code, I've came up with a few improvements to the feel of completion. - Improve completion triggering. ' ' is poor trigger. I used a regex to match on word characters. This works way better and is more aligned with Xcode triggering mechanisms. - Disable should use inner override. This isn't really necessary with all of the changes that have been made in the last few commits. - Improve menu text. Based on recommendation, having the 'name' and 'description' in the menu was redundant. Now, it only shows the 'description', which for methods, is the signature and types.
Wait for initial Swifty Swift Vim Boot. Tasks in the backend's startup process ( like dynamic linking and setting up the HTTP stack ) make it impossible for the backend to respond to requests immediately after launching the process. If the process isn't ready, YCMD will fail. It simply waits until the process writes to stdout, which in practice is the loading message. Profiling: Runs were made on a 2014 MBA ( 1.4 GHz Intel Core i5 ). I observed booting to be reasonably quick. Right after starting the machine, it took less than 100ms. After starting the service once it was dramatically reduced. I used a 5 second timeout because I don't see a good to make it much smaller and I don't have access to lower perf hardware for developing this. Stability: I assume this is why the CI was failing here and there. I also tested adding a sleep before the tests yielded stability on the CI after several runs ( see jerrymarino/ycmd Travis build history of jmarino_health_check_completion_tests ). @puremourning also reported he was seeing timeouts. I was also able to consistently see timeouts in vim and fail the test suite immediately after starting the test machine. Reproduction Steps: - run the test suite immediately after system start - open Vim immediately after system start Additionally: Increase shutdown test timeout. Now that we wait for the swift server to start, it slows down this test tried it with 5 seconds and then 10.
Wait for the swift server to boot
Update the semantic server to HEAD since it now treats warnings as errors. ( jerrymarino/swiftyswiftvim#5 )
This patch makes errors and warnings work as you type :) Relates to #762 It simply adds a network request to the backend which in turn delegates to SourceKit. Fixits are available, but not hooked up yet. I plan to add this in a follow up. There are 2 related changes: - a change needed for flags which is implemented on the backend. This patch also updates the server. - a change in the YouCompleteMe client. I'll submit this seperately: +++ b/python/ycm/youcompleteme.py @@ -101,7 +101,7 @@ CORE_OUTDATED_MESSAGE = ( 'script. See the documentation for more details.' ) SERVER_IDLE_SUICIDE_SECONDS = 10800 # 3 hours DIAGNOSTIC_UI_FILETYPES = set( [ 'cpp', 'cs', 'c', 'objc', 'objcpp', - 'typescript' ] ) + 'typescript', 'swift' ] ) Additionally: Refactor the completer to follow consistent patterns around the response -> model -> YCMD datatype workflow: diagnostics and completions follow a similar pattern of creating lightweight models which are parsed from JSON, and can yield YCMD reps of the data types.
|
|
4 tasks
Codecov Report
@@ Coverage Diff @@
## master #763 +/- ##
========================================
+ Coverage 93.89% 94.5% +0.6%
========================================
Files 79 81 +2
Lines 5276 5439 +163
Branches 158 157 -1
========================================
+ Hits 4954 5140 +186
+ Misses 279 257 -22
+ Partials 43 42 -1 |
|
This looks like it depends on #487, can we mark it as WIP? |
|
@vheon sure, sounds good 👍 |
jerrymarino
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch makes errors and warnings work as you type 💥
It builds on Fundamental Swift Support. It adds a network request to the backend which in turn delegates to SourceKit.
Fixits are available, but not hooked up yet. I plan to add this in a follow up.
There are 2 related changes:
a change needed for flags which is implemented on the backend. This
patch also updates the server.
a change in the YouCompleteMe client. I can submit this seperately:
Additionally:
Refactor the completer to follow consistent patterns around the response -> model -> YCMD datatype workflow: diagnostics and completions follow a similar pattern of creating lightweight models which are parsed from SON, and can yield YCMD representations.
Relates to #762
This change is