Pull upstream changes #53
Merged
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
Co-authored-by: Smithy Automation <github-aws-smithy-automation@amazon.com>
* Refactor for performance improvements Re-writes almost everything in the language server to improve performance, and lay the ground work for further progress and features. Given the scope of these changes, this should be considered a WIP as I may have broken some things. Overview of performance improvements: - Per-file model updates on change events - Model validation only run on save - Async execution of model building and validation with cancellation - Async execution of some language features like completion and document symbol with cancellation - Incremental file change updates - Reduced file reads from disk and string copies From the end user's perspective, only one major change has been made what the language server can do. It now uses a smithy-build.json in the root of the workspace as the source of truth for what is part of the project. Previously, the server loaded all Smithy files it found in any subdir of the root. This doesn't scale well with multi-root workspaces, and leads to an issue where Smithy files in the build directory are added to the project, duplicating sources. The new process looks for a smithy-build.json and uses only its `sources` and `imports` as files to load into the model (maven deps are still supported, this is just referring to project files). For backward compatibility, the old SmithyBuildExtensions `build/smithy-dependencies.json` and `.smithy.json` are still supported. A new file, `.smithy-project.json`, is being developed which allows projects that are configured outside of a smithy-build.json (such as a Gradle project) to specify their project files _and_ dependencies. Right now these dependencies are local, paths to JARs, but it may make sense to support Maven dependencies in there as well. More to come on how `.smithy-project.json` works in documentation updates. To support using the language server without a smithy-build.json, a future update is in progress to allow a 'detached' project which loads whatever files you open. Other updates: - Use smithy-syntax formatter - Report progress to client on load - Add configuration option for the minimum severity of validation events - Update dependencies * Add support for non-project files This makes the language server work on files that aren't connected (or attached) to a smithy-build.json/other build file. This works by loading said files as they are opened in their own, single-file projects with no dependencies, which are removed when the file is closed. A diagnostic was also added to indicate when a file is 'detached' from a project, and appears on the first line of the file. I could have made all detached files part of their own special project, could be more convenient when doing something quick with multiple files without a smithy-build.json. The smithy cli can work this way, although you still have to specify the files to build in the command, so we could change this in the future. The difference is I don't think we'd have a way of opting out of the single project without some config that would end up being more work to set up than a smithy-build.json. * Normalize paths in the project loader Fixes an issue where the server can't find project files when smithy-build.json has unnormalized paths. * remove metadata when reloading single files The performance refactor caused a regression where making changes to a file with metadata would cause that metadata key to be duplicated. Since the server used to reload all files on a change, we never had to worry about this, but since we're trying to now only reload single files, we need to remove any metadata in that file from the model before reloading. This works similarly to how we collect per-file shapes, but is slightly more complex because array metadata with the same key get merged into a single array (as opposed to non-arrays, which cause an error when there are the same keys). * Various fixes to project file management This commit makes updates to how we manage files that are opened, and what happens when you add/remove files. Previously, if you moved a detached file into your project, the server would load that file from disk, rather than using the in-memory Document. More test cases were added around adding/moving detached files. Also made it so that diagnostics are re-reported back to the client for open files after a reload. * Properly load detached files with invalid syntax Fixes an issue where basically any time you created a file not attached to a project, it wouldn't ever be loaded properly and any updates wouldn't register. This happened because the server wasn't creating a SmithyFile for a detached file if it had no shapes in it. The SmithyFile is created only when the project is initialized, so subsequent changes wouldn't fix it. * Refactor Project to have its ProjectConfig Also adds some test cases for moving around detached and/or broken files. * Fix applying traits in partial load The partial loading strategy, which is meant to reduce the amount of unnecessary re-parsing of unchanged files on every file change, works by removing shapes defined in the file that has changed. But this wasn't taking into account traits applied using `apply` in other files. When a shape is removed, all the traits are removed, and the `apply` isn't persisted. So we need to keep track of all trait applications that aren't in the same file as the shape def. Additionally, list traits have their values merged, so we actually need to either partially remove a trait (i.e. only certain elements), or just reload all the files that contain part of the trait's value. This implementation does the latter, which also requires creating a sort of dependency graph of which files need other files to be loaded with them. There's likely room for optimization here (potentially switching to the first approach), but we will have to guage the performance. This commit also consolidates the project updating logic for adding, removing, and changing files into a single method of Project, and adds a bunch of tests around different situations of `apply`. * Keep existing project on load failure Fixes an issue where if you did the following: 1. Created a valid project (ok smithy-build.json, loaded model) 2. Made the smithy-build.json invalid (e.g by adding an invalid dep) 3. Fixed the smithy-build.json The project would not recover, and any open project files would be lost to the server. * Support direct use completions, absolute shape ids This commit makes it so you can manually type out a use statement, and get completions for the absolute shape id. It also adds support for completion/definition/hover for absolute shape ids in general. * Fix Document, UriAdapter, and tests for windows Previously, Document used System.lineSeparator() for figuring out where line starts would be (index of linesep + 1). But if the file was created (and, say, packaged in a jar) on another OS, it would have different lineseps. This change makes use of a simple fact I overlooked in the initial implementation, which was that '\n' is still the last character on each line, so we don't need to break on System.lineSeparator(), just on newline (unless there's still some OS using '\r' only line breaks). UriAdapter was updated to handle windows URIs, which would be made into invalid paths with a leading '/' after removing 'file://'. A bunch of test cases were also updated, which essentially all had one or both of the above problems.
Quick pre-req for moving release process to jreleaser
|
|
|
Happy to merge it, but why is macos Java 8 failing ? |
|
Not sure, I'll look into it. It was failing in my bootleg releases too, so jdk8 and windows were disabled there. |
|
For future reference, |
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.
Pulls in the gigantic performance refactor in smithy-lang#146, as well as formatter changes!
The LSP will now use the formatter from smithy-syntax.