diff --git a/src/main/java/software/amazon/smithy/lsp/project/Project.java b/src/main/java/software/amazon/smithy/lsp/project/Project.java index d0dd97f7..01535f7f 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/Project.java +++ b/src/main/java/software/amazon/smithy/lsp/project/Project.java @@ -7,7 +7,6 @@ import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -21,9 +20,12 @@ import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.protocol.UriAdapter; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.loader.ModelAssembler; import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.AbstractShapeBuilder; import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.utils.IoUtils; @@ -39,17 +41,19 @@ public final class Project { private final Map smithyFiles; private final Supplier assemblerFactory; private ValidatedResult modelResult; - // TODO: Probably should move this into SmithyFile + // TODO: Move this into SmithyFileDependenciesIndex private Map> perFileMetadata; + private SmithyFileDependenciesIndex smithyFileDependenciesIndex; private Project(Builder builder) { this.root = Objects.requireNonNull(builder.root); - this.config = Objects.requireNonNull(builder.config); + this.config = builder.config; this.dependencies = builder.dependencies; this.smithyFiles = builder.smithyFiles; this.modelResult = builder.modelResult; this.assemblerFactory = builder.assemblerFactory; this.perFileMetadata = builder.perFileMetadata; + this.smithyFileDependenciesIndex = builder.smithyFileDependenciesIndex; } /** @@ -148,8 +152,7 @@ public SmithyFile getSmithyFile(String uri) { * @param uri The URI of the Smithy file to update */ public void updateModelWithoutValidating(String uri) { - Document document = getDocument(uri); - updateModel(uri, document, false); + updateFiles(Collections.emptySet(), Collections.emptySet(), Collections.singleton(uri), false); } /** @@ -158,119 +161,90 @@ public void updateModelWithoutValidating(String uri) { * @param uri The URI of the Smithy file to update */ public void updateAndValidateModel(String uri) { - Document document = getDocument(uri); - updateModel(uri, document, true); + updateFiles(Collections.emptySet(), Collections.emptySet(), Collections.singleton(uri), true); } - // TODO: This is a little all over the place /** - * Update the model with the contents of the given {@code document}, optionally - * running validation. + * Updates this project by adding and removing files. Runs model validation. * - * @param uri The URI of the Smithy file to update - * @param document The {@link Document} with updated contents - * @param validate Whether to run validation + *

Added files are assumed to not be managed by the client, and are loaded from disk. + * + * @param addUris URIs of files to add + * @param removeUris URIs of files to remove */ - public void updateModel(String uri, Document document, boolean validate) { - if (document == null || !modelResult.getResult().isPresent()) { - // TODO: At one point in testing, the server got stuck with a certain validation event - // always being present, and no other features working. I haven't been able to reproduce - // it, but I added these logs to check for it. - if (document == null) { - LOGGER.info("No document loaded for " + uri + ", skipping model load."); - } - if (!modelResult.getResult().isPresent()) { - LOGGER.info("No model loaded, skipping updating model with " + uri); - } - // TODO: If there's no model, we didn't collect the smithy files (so no document), so I'm thinking - // maybe we do nothing here. But we could also still update the document, and - // just compute the shapes later? - return; - } - - String path = UriAdapter.toPath(uri); - - SmithyFile previous = smithyFiles.get(path); - Model currentModel = modelResult.getResult().get(); // unwrap would throw if the model is broken - - Model.Builder builder = prepBuilderForReload(currentModel); - for (Shape shape : previous.getShapes()) { - builder.removeShape(shape.getId()); - } - for (Map.Entry> e : this.perFileMetadata.entrySet()) { - if (!e.getKey().equals(path)) { - e.getValue().forEach(builder::putMetadataProperty); - } - } - Model rest = builder.build(); - - ModelAssembler assembler = assemblerFactory.get() - .addModel(rest) - .addUnparsedModel(path, document.copyText()); - - if (!validate) { - assembler.disableValidation(); - } - - this.modelResult = assembler.assemble(); - - Set updatedShapes = getFileShapes(path, previous.getShapes()); - // TODO: Could cache validation events - SmithyFile updated = ProjectLoader.buildSmithyFile(path, document, updatedShapes).build(); - this.perFileMetadata = ProjectLoader.computePerFileMetadata(this.modelResult); - this.smithyFiles.put(path, updated); + public void updateFiles(Set addUris, Set removeUris) { + updateFiles(addUris, removeUris, Collections.emptySet(), true); } /** - * Updates this project by adding and removing files. Also runs model validation. + * Updates this project by adding, removing, and changing files. Can optionally run validation. + * + *

Added files are assumed to not be managed by the client, and are loaded from disk. * * @param addUris URIs of files to add * @param removeUris URIs of files to remove + * @param changeUris URIs of files that changed + * @param validate Whether to run model validation. */ - public void updateFiles(Collection addUris, Collection removeUris) { + public void updateFiles(Set addUris, Set removeUris, Set changeUris, boolean validate) { if (!modelResult.getResult().isPresent()) { - LOGGER.severe("Attempted to update files in project with no model: " + addUris + " " + removeUris); + // TODO: If there's no model, we didn't collect the smithy files (so no document), so I'm thinking + // maybe we do nothing here. But we could also still update the document, and + // just compute the shapes later? + LOGGER.severe("Attempted to update files in project with no model: " + + addUris + " " + removeUris + " " + changeUris); return; } - if (addUris.isEmpty() && removeUris.isEmpty()) { + if (addUris.isEmpty() && removeUris.isEmpty() && changeUris.isEmpty()) { LOGGER.info("No files provided to update"); return; } - Model currentModel = modelResult.getResult().get(); + Model currentModel = modelResult.getResult().get(); // unwrap would throw if the model is broken ModelAssembler assembler = assemblerFactory.get(); - if (!removeUris.isEmpty()) { - // Slightly strange way to do this, but we need to remove all model metadata, then - // re-add only metadata for remaining files. - Set remainingFilesWithMetadata = new HashSet<>(perFileMetadata.keySet()); + // So we don't have to recompute the paths later + Set removedPaths = new HashSet<>(removeUris.size()); + Set changedPaths = new HashSet<>(changeUris.size()); + + Set visited = new HashSet<>(); + + if (!removeUris.isEmpty() || !changeUris.isEmpty()) { Model.Builder builder = prepBuilderForReload(currentModel); for (String uri : removeUris) { String path = UriAdapter.toPath(uri); + removedPaths.add(path); - remainingFilesWithMetadata.remove(path); + removeFileForReload(assembler, builder, path, visited); + removeDependentsForReload(assembler, builder, path, visited); // Note: no need to remove anything from sources/imports, since they're // based on what's in the build files. - SmithyFile smithyFile = smithyFiles.remove(path); + smithyFiles.remove(path); + } - if (smithyFile == null) { - LOGGER.severe("Attempted to remove file not in project: " + uri); - continue; - } - for (Shape shape : smithyFile.getShapes()) { - builder.removeShape(shape.getId()); - } + for (String uri : changeUris) { + String path = UriAdapter.toPath(uri); + changedPaths.add(path); + + removeFileForReload(assembler, builder, path, visited); + removeDependentsForReload(assembler, builder, path, visited); } - for (String remainingFileWithMetadata : remainingFilesWithMetadata) { - Map fileMetadata = perFileMetadata.get(remainingFileWithMetadata); - for (Map.Entry fileMetadataEntry : fileMetadata.entrySet()) { - builder.putMetadataProperty(fileMetadataEntry.getKey(), fileMetadataEntry.getValue()); + + // visited will be a superset of removePaths + addRemainingMetadataForReload(builder, visited); + + assembler.addModel(builder.build()); + + for (String visitedPath : visited) { + // Only add back stuff we aren't trying to remove. + // Only removed paths will have had their SmithyFile removed. + if (!removedPaths.contains(visitedPath)) { + assembler.addUnparsedModel(visitedPath, smithyFiles.get(visitedPath).getDocument().copyText()); } } - assembler.addModel(builder.build()); } else { assembler.addModel(currentModel); } @@ -279,8 +253,28 @@ public void updateFiles(Collection addUris, Collection removeUri assembler.addImport(UriAdapter.toPath(uri)); } + if (!validate) { + assembler.disableValidation(); + } + this.modelResult = assembler.assemble(); this.perFileMetadata = ProjectLoader.computePerFileMetadata(this.modelResult); + this.smithyFileDependenciesIndex = SmithyFileDependenciesIndex.compute(this.modelResult); + + for (String visitedPath : visited) { + if (!removedPaths.contains(visitedPath)) { + SmithyFile current = smithyFiles.get(visitedPath); + Set updatedShapes = getFileShapes(visitedPath, smithyFiles.get(visitedPath).getShapes()); + // Only recompute the rest of the smithy file if it changed + if (changedPaths.contains(visitedPath)) { + // TODO: Could cache validation events + this.smithyFiles.put(visitedPath, + ProjectLoader.buildSmithyFile(visitedPath, current.getDocument(), updatedShapes).build()); + } else { + current.setShapes(updatedShapes); + } + } + } for (String uri : addUris) { String path = UriAdapter.toPath(uri); @@ -301,6 +295,70 @@ private Model.Builder prepBuilderForReload(Model model) { .clearMetadata(); } + private void removeFileForReload( + ModelAssembler assembler, + Model.Builder builder, + String path, + Set visited + ) { + if (path == null || visited.contains(path) || path.equals(SourceLocation.none().getFilename())) { + return; + } + + visited.add(path); + + for (Shape shape : smithyFiles.get(path).getShapes()) { + builder.removeShape(shape.getId()); + + // This shape may have traits applied to it in other files, + // so simply removing the shape loses the information about + // those traits. + + // This shape's dependencies files will be removed and re-loaded + smithyFileDependenciesIndex.getDependenciesFiles(shape).forEach((depPath) -> + removeFileForReload(assembler, builder, depPath, visited)); + + // Traits applied in other files are re-added to the assembler so if/when the shape + // is reloaded, it will have those traits + smithyFileDependenciesIndex.getTraitsAppliedInOtherFiles(shape).forEach((trait) -> + assembler.addTrait(shape.getId(), trait)); + } + } + + private void removeDependentsForReload( + ModelAssembler assembler, + Model.Builder builder, + String path, + Set visited + ) { + // This file may apply traits to shapes in other files. Normally, letting the assembler simply reparse + // the file would be fine because it would ignore the duplicated trait application coming from the same + // source location. But if the apply statement is changed/removed, the old application isn't removed, so we + // could get a duplicate trait, or a merged array trait. + smithyFileDependenciesIndex.getDependentFiles(path).forEach((depPath) -> { + removeFileForReload(assembler, builder, depPath, visited); + }); + smithyFileDependenciesIndex.getAppliedTraitsInFile(path).forEach((shapeId, traits) -> { + Shape shape = builder.getCurrentShapes().get(shapeId); + if (shape != null) { + builder.removeShape(shapeId); + AbstractShapeBuilder b = Shape.shapeToBuilder(shape); + for (Trait trait : traits) { + b.removeTrait(trait.toShapeId()); + } + builder.addShape(b.build()); + } + }); + } + + private void addRemainingMetadataForReload(Model.Builder builder, Set filesToSkip) { + for (Map.Entry> e : this.perFileMetadata.entrySet()) { + if (!filesToSkip.contains(e.getKey())) { + e.getValue().forEach(builder::putMetadataProperty); + } + } + } + private Set getFileShapes(String path, Set orDefault) { return this.modelResult.getResult() .map(model -> model.shapes() @@ -315,12 +373,13 @@ static Builder builder() { static final class Builder { private Path root; - private ProjectConfig config; + private ProjectConfig config = ProjectConfig.empty(); private final List dependencies = new ArrayList<>(); private final Map smithyFiles = new HashMap<>(); private ValidatedResult modelResult; private Supplier assemblerFactory = Model::assembler; private Map> perFileMetadata = new HashMap<>(); + private SmithyFileDependenciesIndex smithyFileDependenciesIndex = SmithyFileDependenciesIndex.EMPTY; private Builder() { } @@ -367,6 +426,11 @@ public Builder perFileMetadata(Map> perFileMetadata) { return this; } + public Builder smithyFileDependenciesIndex(SmithyFileDependenciesIndex smithyFileDependenciesIndex) { + this.smithyFileDependenciesIndex = smithyFileDependenciesIndex; + return this; + } + public Project build() { return new Project(this); } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java index f9d03c4e..11bb77a5 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java @@ -29,6 +29,10 @@ private ProjectConfig(Builder builder) { this.mavenConfig = builder.mavenConfig; } + static ProjectConfig empty() { + return builder().build(); + } + static Builder builder() { return new Builder(); } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java index df712900..625a3f77 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java @@ -205,6 +205,7 @@ public static Result> load( return Result.ok(projectBuilder.smithyFiles(smithyFiles) .perFileMetadata(computePerFileMetadata(modelResult)) + .smithyFileDependenciesIndex(SmithyFileDependenciesIndex.compute(modelResult)) .build()); } diff --git a/src/main/java/software/amazon/smithy/lsp/project/SmithyFile.java b/src/main/java/software/amazon/smithy/lsp/project/SmithyFile.java index ac748d17..912e9075 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/SmithyFile.java +++ b/src/main/java/software/amazon/smithy/lsp/project/SmithyFile.java @@ -27,7 +27,9 @@ public final class SmithyFile { private final String path; private final Document document; - private final Set shapes; + // TODO: If we have more complex use-cases for partially updating SmithyFile, we + // could use a toBuilder() + private Set shapes; private final DocumentNamespace namespace; private final DocumentImports imports; private final Map documentShapes; @@ -64,6 +66,10 @@ public Set getShapes() { return shapes; } + void setShapes(Set shapes) { + this.shapes = shapes; + } + /** * @return This Smithy file's imports, if they exist */ diff --git a/src/main/java/software/amazon/smithy/lsp/project/SmithyFileDependenciesIndex.java b/src/main/java/software/amazon/smithy/lsp/project/SmithyFileDependenciesIndex.java new file mode 100644 index 00000000..5fea7bb8 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/project/SmithyFileDependenciesIndex.java @@ -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. + * + *

This is specifically for the following scenarios: + *

    + *
  1. 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.
    2. + *
  2. 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.
    3. + *
  3. Either 1 or 2, but specifically with list traits, which are merged via + * + * trait conflict resolution + * . 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. + *
    4. + *
+ */ +final class SmithyFileDependenciesIndex { + static final SmithyFileDependenciesIndex EMPTY = new SmithyFileDependenciesIndex( + new HashMap<>(0), new HashMap<>(0), new HashMap<>(0), new HashMap<>(0)); + + private final Map> filesToDependentFiles; + private final Map> shapeIdsToDependenciesFiles; + private final Map>> filesToTraitsTheyApply; + private final Map> shapesToAppliedTraitsInOtherFiles; + + private SmithyFileDependenciesIndex( + Map> filesToDependentFiles, + Map> shapeIdsToDependenciesFiles, + Map>> filesToTraitsTheyApply, + Map> shapesToAppliedTraitsInOtherFiles + ) { + this.filesToDependentFiles = filesToDependentFiles; + this.shapeIdsToDependenciesFiles = shapeIdsToDependenciesFiles; + this.filesToTraitsTheyApply = filesToTraitsTheyApply; + this.shapesToAppliedTraitsInOtherFiles = shapesToAppliedTraitsInOtherFiles; + } + + Set getDependentFiles(String path) { + return filesToDependentFiles.getOrDefault(path, Collections.emptySet()); + } + + Set getDependenciesFiles(ToShapeId toShapeId) { + return shapeIdsToDependenciesFiles.getOrDefault(toShapeId.toShapeId(), Collections.emptySet()); + } + + Map> getAppliedTraitsInFile(String path) { + return filesToTraitsTheyApply.getOrDefault(path, Collections.emptyMap()); + } + + List getTraitsAppliedInOtherFiles(ToShapeId toShapeId) { + return shapesToAppliedTraitsInOtherFiles.getOrDefault(toShapeId.toShapeId(), Collections.emptyList()); + } + + // TODO: Make this take care of metadata too + static SmithyFileDependenciesIndex compute(ValidatedResult 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; + } +} diff --git a/src/test/java/software/amazon/smithy/lsp/SmithyLanguageServerTest.java b/src/test/java/software/amazon/smithy/lsp/SmithyLanguageServerTest.java index bf718328..22f52113 100644 --- a/src/test/java/software/amazon/smithy/lsp/SmithyLanguageServerTest.java +++ b/src/test/java/software/amazon/smithy/lsp/SmithyLanguageServerTest.java @@ -22,8 +22,8 @@ import static software.amazon.smithy.lsp.LspMatchers.makesEditedDocument; import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; import static software.amazon.smithy.lsp.SmithyMatchers.hasShapeWithId; -import static software.amazon.smithy.lsp.SmithyMatchers.hasShapeWithIdAndTraits; import static software.amazon.smithy.lsp.SmithyMatchers.hasValue; +import static software.amazon.smithy.lsp.UtilMatchers.anOptionalOf; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; @@ -58,13 +58,15 @@ import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.eclipse.lsp4j.services.LanguageClient; import org.hamcrest.Matcher; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.protocol.RangeAdapter; import software.amazon.smithy.model.node.ArrayNode; 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.traits.LengthTrait; public class SmithyLanguageServerTest { @Test @@ -953,7 +955,6 @@ public void addingDetachedFile() { .text(modelText) .build()); server.didChangeWatchedFiles(RequestBuilders.didChangeWatchedFiles() - .event(uri, FileChangeType.Deleted) .event(movedUri, FileChangeType.Created) .build()); @@ -1452,8 +1453,7 @@ public void addingDetachedFileWithInvalidSyntax() throws Exception { } @Test - @Disabled - public void todoCheckApplysInPartialLoad() throws Exception { + public void appliedTraitsAreMaintainedInPartialLoad() throws Exception { String modelText1 = "$version: \"2\"\n" + "namespace com.foo\n" + "string Foo\n"; @@ -1472,16 +1472,16 @@ public void todoCheckApplysInPartialLoad() throws Exception { .build()); server.didChange(RequestBuilders.didChange() .uri(uri2) - .range(RangeAdapter.point(3, 23)) + .range(RangeAdapter.of(3, 23, 3, 24)) .text("2") .build()); server.getLifecycleManager().waitForAllTasks(); - assertThat(server.getProject().getModelResult().getResult().isPresent(), is(true)); assertThat(server.getProject().getModelResult(), hasValue(hasShapeWithId("com.foo#Bar"))); - assertThat(server.getProject().getModelResult(), hasValue( - hasShapeWithIdAndTraits("com.foo#Foo", "smithy.api#length"))); + Shape foo = server.getProject().getModelResult().getResult().get().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.getIntroducedTraits().keySet(), containsInAnyOrder(LengthTrait.ID)); + assertThat(foo.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(2L))); String uri1 = workspace.getUri("model-0.smithy"); @@ -1497,12 +1497,11 @@ public void todoCheckApplysInPartialLoad() throws Exception { server.getLifecycleManager().waitForAllTasks(); - assertThat(server.getProject().getModelResult().getResult().isPresent(), is(true)); assertThat(server.getProject().getModelResult(), hasValue(hasShapeWithId("com.foo#Bar"))); - assertThat(server.getProject().getModelResult(), hasValue(hasShapeWithId("com.foo#Baz"))); assertThat(server.getProject().getModelResult(), hasValue(hasShapeWithId("com.foo#Another"))); - assertThat(server.getProject().getModelResult(), hasValue( - hasShapeWithIdAndTraits("com.foo#Foo", "smithy.api#length"))); + foo = server.getProject().getModelResult().getResult().get().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.getIntroducedTraits().keySet(), containsInAnyOrder(LengthTrait.ID)); + assertThat(foo.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(2L))); } public static SmithyLanguageServer initFromWorkspace(TestWorkspace workspace) { diff --git a/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java b/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java index b4fa2b54..72f05d03 100644 --- a/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java +++ b/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java @@ -21,11 +21,20 @@ public final class SmithyMatchers { private SmithyMatchers() {} public static Matcher> hasValue(Matcher matcher) { - return new CustomTypeSafeMatcher>("a validated result with a value") { + return new CustomTypeSafeMatcher>("A validated result with value " + matcher.toString()) { @Override protected boolean matchesSafely(ValidatedResult item) { return item.getResult().isPresent() && matcher.matches(item.getResult().get()); } + + @Override + public void describeMismatchSafely(ValidatedResult item, Description description) { + if (item.getResult().isPresent()) { + matcher.describeMismatch(item.getResult().get(), description); + } else { + description.appendText("Expected a value but result was empty."); + } + } }; } @@ -46,23 +55,6 @@ public void describeMismatchSafely(Model model, Description description) { }; } - public static Matcher hasShapeWithIdAndTraits(String id, String... traitIds) { - return new CustomTypeSafeMatcher("a model with a shape with id " + id + " that has traits: " + String.join(",", traitIds)) { - @Override - protected boolean matchesSafely(Model item) { - if (!item.getShape(ShapeId.from(id)).isPresent()) { - return false; - } - Shape shape = item.expectShape(ShapeId.from(id)); - boolean hasTraits = true; - for (String traitId : traitIds) { - hasTraits = hasTraits && shape.hasTrait(traitId); - } - return hasTraits; - } - }; - } - public static Matcher eventWithMessage(Matcher message) { return new CustomTypeSafeMatcher("has matching message") { @Override diff --git a/src/test/java/software/amazon/smithy/lsp/UtilMatchers.java b/src/test/java/software/amazon/smithy/lsp/UtilMatchers.java new file mode 100644 index 00000000..d5e8a8e0 --- /dev/null +++ b/src/test/java/software/amazon/smithy/lsp/UtilMatchers.java @@ -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 Matcher> anOptionalOf(Matcher matcher) { + return new CustomTypeSafeMatcher>("An optional that is present with value " + matcher.toString()) { + @Override + protected boolean matchesSafely(Optional item) { + return item.isPresent() && matcher.matches(item.get()); + } + + @Override + public void describeMismatchSafely(Optional item, Description description) { + if (!item.isPresent()) { + description.appendText("was an empty optional"); + } else { + matcher.describeMismatch(item.get(), description); + } + } + }; + } +} diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java index 9b9d1959..48857821 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java @@ -18,6 +18,7 @@ import static org.hamcrest.Matchers.hasSize; import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; import static software.amazon.smithy.lsp.SmithyMatchers.hasShapeWithId; +import static software.amazon.smithy.lsp.UtilMatchers.anOptionalOf; import static software.amazon.smithy.lsp.document.DocumentTest.string; import java.nio.file.Path; @@ -25,11 +26,17 @@ import java.util.List; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; +import software.amazon.smithy.lsp.TestWorkspace; +import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.lsp.protocol.RangeAdapter; import software.amazon.smithy.lsp.protocol.UriAdapter; import software.amazon.smithy.lsp.util.Result; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.LengthTrait; +import software.amazon.smithy.model.traits.PatternTrait; +import software.amazon.smithy.model.traits.TagsTrait; import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidationEvent; @@ -251,4 +258,338 @@ public void loadsProjectWithUnNormalizedDirs() { containsString(root.resolve("smithy-test-traits.jar") + "!/META-INF/smithy/smithy.test.json"))); assertThat(project.getDependencies(), hasItem(root.resolve("smithy-test-traits.jar"))); } + + @Test + public void changeFileApplyingSimpleTrait() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n" + + "apply Bar @length(min: 1)\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + } + + @Test + public void changeFileApplyingListTrait() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n" + + "apply Bar @tags([\"foo\"])\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + } + + @Test + public void changeFileApplyingListTraitWithUnrelatedDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n" + + "apply Bar @tags([\"foo\"])\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n" + + "string Baz\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Baz @length(min: 1)\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + Shape baz = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Baz")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(baz.hasTrait("length"), is(true)); + assertThat(baz.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + baz = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Baz")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(baz.hasTrait("length"), is(true)); + assertThat(baz.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + } + + @Test + public void changingFileApplyingListTraitWithRelatedDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n" + + "apply Bar @tags([\"foo\"])\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @length(min: 1)\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + } + + @Test + public void changingFileApplyingListTraitWithRelatedArrayTraitDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n" + + "apply Bar @tags([\"foo\"])\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @tags([\"bar\"])\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo", "bar")); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo", "bar")); + } + + @Test + public void changingFileWithDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n" + + "apply Foo @length(min: 1)\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("length"), is(true)); + assertThat(foo.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("length"), is(true)); + assertThat(foo.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + } + + @Test + public void changingFileWithArrayDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n" + + "apply Foo @tags([\"foo\"])\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + } + + @Test + public void changingFileWithMixedArrayDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "@tags([\"foo\"])\n" + + "string Foo\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n" + + "apply Foo @tags([\"foo\"])\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo", "foo")); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo", "foo")); + } + + @Test + public void changingFileWithArrayDependenciesWithDependencies() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Foo\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n" + + "apply Foo @tags([\"foo\"])\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @length(min: 1)\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.point(document.end()), "\n"); + + project.updateModelWithoutValidating(uri); + + foo = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(foo.hasTrait("tags"), is(true)); + assertThat(foo.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + } + + @Test + public void removingSimpleApply() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @length(min: 1)\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @pattern(\"a\")\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("pattern"), is(true)); + assertThat(bar.expectTrait(PatternTrait.class).getPattern().pattern(), equalTo("a")); + assertThat(bar.hasTrait("length"), is(true)); + assertThat(bar.expectTrait(LengthTrait.class).getMin(), anOptionalOf(equalTo(1L))); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.lineSpan(2, 0, document.lineEnd(2)), ""); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("pattern"), is(true)); + assertThat(bar.expectTrait(PatternTrait.class).getPattern().pattern(), equalTo("a")); + assertThat(bar.hasTrait("length"), is(false)); + } + + @Test + public void removingArrayApply() { + String m1 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @tags([\"foo\"])\n"; + String m2 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "string Bar\n"; + String m3 = "$version: \"2\"\n" + + "namespace com.foo\n" + + "apply Bar @tags([\"bar\"])\n"; + TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); + Project project = ProjectLoader.load(workspace.getRoot()).unwrap(); + + Shape bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("foo", "bar")); + + String uri = workspace.getUri("model-0.smithy"); + Document document = project.getDocument(uri); + document.applyEdit(RangeAdapter.lineSpan(2, 0, document.lineEnd(2)), ""); + + project.updateModelWithoutValidating(uri); + + bar = project.getModelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); + assertThat(bar.hasTrait("tags"), is(true)); + assertThat(bar.expectTrait(TagsTrait.class).getTags(), containsInAnyOrder("bar")); + } }