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

Update main branch from aws repo upstream #55

Closed
wants to merge 140 commits into from

Conversation

lewisjkl
Copy link

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.

Chase Coalwell and others added 30 commits March 15, 2022 08:10
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.
kubukoz and others added 28 commits August 21, 2023 10:39
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]>
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.
@CLAassistant
Copy link

CLAassistant commented Oct 10, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 7 committers have signed the CLA.

✅ daddykotex
✅ kubukoz
❌ RenFraser
❌ srchase
❌ smithy-automation
❌ milesziemer
❌ kstich
You have signed the CLA already but the status is still pending? Let us recheck it.

@lewisjkl lewisjkl closed this Oct 10, 2024
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.