-
Notifications
You must be signed in to change notification settings - Fork 22
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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`.
- Loading branch information
1 parent
c9193ad
commit 62413b1
Showing
9 changed files
with
681 additions
and
117 deletions.
There are no files selected for viewing
234 changes: 149 additions & 85 deletions
234
src/main/java/software/amazon/smithy/lsp/project/Project.java
Large diffs are not rendered by default.
Oops, something went wrong.
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 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 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
124 changes: 124 additions & 0 deletions
124
src/main/java/software/amazon/smithy/lsp/project/SmithyFileDependenciesIndex.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package software.amazon.smithy.lsp.project; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import software.amazon.smithy.model.Model; | ||
| import software.amazon.smithy.model.node.Node; | ||
| import software.amazon.smithy.model.shapes.Shape; | ||
| import software.amazon.smithy.model.shapes.ShapeId; | ||
| import software.amazon.smithy.model.shapes.ToShapeId; | ||
| import software.amazon.smithy.model.traits.Trait; | ||
| import software.amazon.smithy.model.validation.ValidatedResult; | ||
|
|
||
| /** | ||
| * An index that caches rebuild dependency relationships between Smithy files, | ||
| * shapes, and traits. | ||
| * | ||
| * <p>This is specifically for the following scenarios: | ||
| * <ol> | ||
| * <li>A file applies traits to shapes in other files. If that file changes, the | ||
| * applied traits need to be removed before the file is reloaded, so there | ||
| * aren't duplicate traits.</li> | ||
| * <li>A file has shapes with traits applied in other files. If that file changes, | ||
| * the traits need to be re-applied when the model is re-assembled, so they | ||
| * aren't lost.</li> | ||
| * <li>Either 1 or 2, but specifically with list traits, which are merged via | ||
| * <a href="https://smithy.io/2.0/spec/model.html#trait-conflict-resolution"> | ||
| * trait conflict resolution | ||
| * </a>. For these traits, all files that contain parts of the list trait must | ||
| * be fully reloaded, since we can only remove the whole trait, not parts of it. | ||
| * </li> | ||
| * </ol> | ||
| */ | ||
| final class SmithyFileDependenciesIndex { | ||
| static final SmithyFileDependenciesIndex EMPTY = new SmithyFileDependenciesIndex( | ||
| new HashMap<>(0), new HashMap<>(0), new HashMap<>(0), new HashMap<>(0)); | ||
|
|
||
| private final Map<String, Set<String>> filesToDependentFiles; | ||
| private final Map<ShapeId, Set<String>> shapeIdsToDependenciesFiles; | ||
| private final Map<String, Map<ShapeId, List<Trait>>> filesToTraitsTheyApply; | ||
| private final Map<ShapeId, List<Trait>> shapesToAppliedTraitsInOtherFiles; | ||
|
|
||
| private SmithyFileDependenciesIndex( | ||
| Map<String, Set<String>> filesToDependentFiles, | ||
| Map<ShapeId, Set<String>> shapeIdsToDependenciesFiles, | ||
| Map<String, Map<ShapeId, List<Trait>>> filesToTraitsTheyApply, | ||
| Map<ShapeId, List<Trait>> shapesToAppliedTraitsInOtherFiles | ||
| ) { | ||
| this.filesToDependentFiles = filesToDependentFiles; | ||
| this.shapeIdsToDependenciesFiles = shapeIdsToDependenciesFiles; | ||
| this.filesToTraitsTheyApply = filesToTraitsTheyApply; | ||
| this.shapesToAppliedTraitsInOtherFiles = shapesToAppliedTraitsInOtherFiles; | ||
| } | ||
|
|
||
| Set<String> getDependentFiles(String path) { | ||
| return filesToDependentFiles.getOrDefault(path, Collections.emptySet()); | ||
| } | ||
|
|
||
| Set<String> getDependenciesFiles(ToShapeId toShapeId) { | ||
| return shapeIdsToDependenciesFiles.getOrDefault(toShapeId.toShapeId(), Collections.emptySet()); | ||
| } | ||
|
|
||
| Map<ShapeId, List<Trait>> getAppliedTraitsInFile(String path) { | ||
| return filesToTraitsTheyApply.getOrDefault(path, Collections.emptyMap()); | ||
| } | ||
|
|
||
| List<Trait> getTraitsAppliedInOtherFiles(ToShapeId toShapeId) { | ||
| return shapesToAppliedTraitsInOtherFiles.getOrDefault(toShapeId.toShapeId(), Collections.emptyList()); | ||
| } | ||
|
|
||
| // TODO: Make this take care of metadata too | ||
| static SmithyFileDependenciesIndex compute(ValidatedResult<Model> modelResult) { | ||
| if (!modelResult.getResult().isPresent()) { | ||
| return EMPTY; | ||
| } | ||
|
|
||
| SmithyFileDependenciesIndex index = new SmithyFileDependenciesIndex( | ||
| new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>()); | ||
|
|
||
| Model model = modelResult.getResult().get(); | ||
| for (Shape shape : model.toSet()) { | ||
| String shapeSourceFilename = shape.getSourceLocation().getFilename(); | ||
| for (Trait traitApplication : shape.getAllTraits().values()) { | ||
| // We only care about trait applications in the source files | ||
| if (traitApplication.isSynthetic()) { | ||
| continue; | ||
| } | ||
|
|
||
| Node traitNode = traitApplication.toNode(); | ||
| if (traitNode.isArrayNode()) { | ||
| for (Node element : traitNode.expectArrayNode()) { | ||
| String elementSourceFilename = element.getSourceLocation().getFilename(); | ||
| if (!elementSourceFilename.equals(shapeSourceFilename)) { | ||
| index.filesToDependentFiles.computeIfAbsent(elementSourceFilename, (k) -> new HashSet<>()) | ||
| .add(shapeSourceFilename); | ||
| index.shapeIdsToDependenciesFiles.computeIfAbsent(shape.getId(), (k) -> new HashSet<>()) | ||
| .add(elementSourceFilename); | ||
| } | ||
| } | ||
| } else { | ||
| String traitSourceFilename = traitApplication.getSourceLocation().getFilename(); | ||
| if (!traitSourceFilename.equals(shapeSourceFilename)) { | ||
| index.shapesToAppliedTraitsInOtherFiles.computeIfAbsent(shape.getId(), (k) -> new ArrayList<>()) | ||
| .add(traitApplication); | ||
| index.filesToTraitsTheyApply.computeIfAbsent(traitSourceFilename, (k) -> new HashMap<>()) | ||
| .computeIfAbsent(shape.getId(), (k) -> new ArrayList<>()) | ||
| .add(traitApplication); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return index; | ||
| } | ||
| } |
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 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
33 changes: 33 additions & 0 deletions
33
src/test/java/software/amazon/smithy/lsp/UtilMatchers.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package software.amazon.smithy.lsp; | ||
|
|
||
| import java.util.Optional; | ||
| import org.hamcrest.CustomTypeSafeMatcher; | ||
| import org.hamcrest.Description; | ||
| import org.hamcrest.Matcher; | ||
|
|
||
| public final class UtilMatchers { | ||
| private UtilMatchers() {} | ||
|
|
||
| public static <T> Matcher<Optional<T>> anOptionalOf(Matcher<T> matcher) { | ||
| return new CustomTypeSafeMatcher<Optional<T>>("An optional that is present with value " + matcher.toString()) { | ||
| @Override | ||
| protected boolean matchesSafely(Optional<T> item) { | ||
| return item.isPresent() && matcher.matches(item.get()); | ||
| } | ||
|
|
||
| @Override | ||
| public void describeMismatchSafely(Optional<T> item, Description description) { | ||
| if (!item.isPresent()) { | ||
| description.appendText("was an empty optional"); | ||
| } else { | ||
| matcher.describeMismatch(item.get(), description); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| } |
Oops, something went wrong.