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.
#168 started
tracking build file changes via lifecycle methods, didOpen, etc. But it
didn't make a distinction between what was a build file, and what was a
Smithy file. There are two paths didOpen can take - the first is when
the file being opened is known to be part of a project. In this case,
the file is already tracked by its owning Project, so its basically a
no-op. The second path is when the file does not belong to any project,
in which case we created a "detached" project, which is a project with
no build files and just a single Smithy file. So if you opened a build
file that wasn't known to be part of a Project, the language server
tried to make a detached project containing the build file as a smithy
file. This is obviously wrong, but wasn't observable to clients AFAICT
because clients weren't set up to send requests to the server for build
files (specifically, you wouldn't get diagnostics or anything, only for
.smithy files). However, recent commits, including
#188, now want
to provide language support for smithy-build.json. In testing these new
commits with local changes to have VSCode send requests for
smithy-build.json, the issue could be observed. Specifically, the issue
happens when you open a new smithy-build.json before we receive the
didChangeWatchedFiles notification that tells us a new build file was
created. didChangeWatchedFiles is the way we actually updated the state
of projects to include new build files, or create new Projects. Since we
can receive didOpen for a build file before didChangeWatchedFiles, we
needed to do something with the build file on didOpen.
This commit addresses the problem by adding a new Project type,
UNRESOLVED
, which is a project containing a single build file that noexisting projects are aware of. We do this by modifying the didOpen path
when the file isn't known to any project, checking if it is a build file
using a PathMatcher, and if it is, creating an unresolved project for
it. Then, when we load projects following a didChangeWatchedFiles, we
just drop any unresolved projects with the same path as any of the build
files in the newly loaded projects (see ServerState::resolveProjects).
I also made some (upgrades?) to FilePatterns to better handle the
discrepancy between matching behavior of PathMatchers and clients
(see #191).
Now there are (private)
*PatternOptions
enums that FilePatterns uses toconfigure the pattern for different use cases. For example, the new
FilePatterns::getSmithyFileWatchPathMatchers provides a list of
PathMatchers which should match the same paths as the watcher patterns
we send back to clients, which is useful for testing.
I also fixed an issue where parsing an empty build file would cause an NPE
when trying to map validation events to ranges. Document::rangeBetween
would return
null
if the document was empty, but I wasn't checking for thatin ToSmithyNode (which creates parse events).
The reason the range is
null
is because Document.lineOfIndex returnsoob for an index of 0 into an empty document. Makes sense, as an empty
document has no lines. I updated a DocumentTest to clarify this
behavior.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.