forked from smithy-lang/smithy-language-server
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update main branch from aws repo upstream #55
Closed
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
The goal of this is to have access to a published version of the LSP to maven central to facilitate consumption in IDEs. Publishing occurs upon merges to the dss branch, or upon tagged release. * Adds a mill build (used only for the sake of publishing) * Adds dynver-like versioning logic * Adds a publish script * Adds necessary CI setup Publishing is setup correctly, as proven by [this build step](https://github.com/disneystreaming/smithy-language-server/runs/4273043861?check_suite_focus=true). By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Co-authored-by: Smithy Automation <[email protected]>
Co-authored-by: Smithy Automation <[email protected]>
Co-authored-by: Smithy Automation <[email protected]>
* Add a test for input/output fix ``` java.lang.RuntimeException: Exception while collecting container shape locations in model file: /var/folders/zv/7_jglb8d4tz6_zs72sfkq9v40000gq/T/hs7886717560855699356/main.smithy at software.amazon.smithy.lsp.ext.FileCachingCollector.collectDefinitionLocations(FileCachingCollector.java:70) at software.amazon.smithy.lsp.ext.SmithyProject.collectLocations(SmithyProject.java:205) at software.amazon.smithy.lsp.ext.SmithyProject.lambda$new$0(SmithyProject.java:73) at java.base/java.util.Optional.ifPresent(Optional.java:178) at software.amazon.smithy.lsp.ext.SmithyProject.<init>(SmithyProject.java:73) at software.amazon.smithy.lsp.ext.SmithyProject.load(SmithyProject.java:184) at software.amazon.smithy.lsp.ext.SmithyProject.load(SmithyProject.java:149) at software.amazon.smithy.lsp.ext.Harness.loadHarness(Harness.java:127) at software.amazon.smithy.lsp.ext.Harness.create(Harness.java:117) at software.amazon.smithy.lsp.ext.SmithyProjectTest.shapeIdFromLocationV2(SmithyProjectTest.java:425) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38) at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at jdk.proxy1/jdk.proxy1.$Proxy2.processTestClass(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) Caused by: java.lang.StringIndexOutOfBoundsException: begin 0, end -3, length 6 at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4601) at java.base/java.lang.String.substring(String.java:2704) at software.amazon.smithy.lsp.ext.FileCachingCollector.getOperationForInlinedInputOrOutput(FileCachingCollector.java:243) at software.amazon.smithy.lsp.ext.FileCachingCollector.collectContainerShapeLocationsInModelFile(FileCachingCollector.java:111) at software.amazon.smithy.lsp.ext.FileCachingCollector.collectDefinitionLocations(FileCachingCollector.java:67) ... 52 more ``` * Avoid substring if not suffixed
Co-authored-by: Smithy Automation <[email protected]>
Co-authored-by: Smithy Automation <[email protected]>
This reverts commit 8f00ef2.
Co-authored-by: Smithy Automation <[email protected]>
A test depended on the ordering of validation events, which 1.44.0 changes.
Co-authored-by: Smithy Automation <[email protected]>
Co-authored-by: Smithy Automation <[email protected]>
* 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
Co-authored-by: Smithy Automation <[email protected]>
* Upgrade dependencies and jdk to 21 A few minor code changes were made for the upgrade, including removing LspLog (which should have already been removed). I didn't make any larger changes to make use of java 21 features - that can be done later. The primary motivation of this commit is to update our dependency on lsp4j, which outdated and depended on a version of guava with two CVEs out: CVE-2023-2976, CVE-2020-8908. In order to do so, a jdk upgrade was required since newer versions of lsp4j are past jdk 8. We were going to upgrade the jdk soon anways. Also got rid of an extra signing block in the publishing task, which is taken care of by jreleaser.
This version includes the same features as 0.3.0, but includes the JDK upgrade and lsp4j dependency upgrade from #157.
* Add support for multiple workspaces Previously, the language server only knew about a single workspace root, so if your editor was using [WorkspaceFolders](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_workspaceFolders) the server would just pick the first one, and not load any others. This commit allows the server to load multiple workspaces. The primary challenge was handling state changes to individual workspaces independently. We use client-side file watchers and the `didChangeWatchedFiles` notification to make sure projects are up to date with new and deleted Smithy files, and any changes to build files (i.e. smithy-build.json). `didChangeWatchedFiles` sends a flat list of file events - not partitioned by workspace - so we have to figure out which projects each change applies to. This is done by creating a `PathMatcher` for each project's smithy files and build files, then matching on each file event. This way, we can apply changes to each individual project, rather than reloading everything. Selectors were also updated to select from all available projects. * Fix file patterns for windows The file patterns we were using for telling the client which files to watch, and to match file events in `didChangeWatchedFiles` to projects, were not working properly on windows because they didn't use the correct file separator.
Refactors various things to make use of newer java features, including record classes, text blocks, and new APIs.
|
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.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.