From dcf6153acb1b94da5dbc15809482b74543055d64 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Mon, 19 Sep 2022 14:03:01 -0400 Subject: [PATCH 1/6] Add test case --- .../smithy/lsp/ext/SmithyProjectTest.java | 17 +++++++++++++++++ .../lsp/ext/models/v2/apply-with-mixin.smithy | 13 +++++++++++++ .../lsp/ext/models/v2/mixin-imports.smithy | 10 ++++++++++ 3 files changed, 40 insertions(+) create mode 100644 src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy create mode 100644 src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy diff --git a/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java b/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java index 0c9a2ba..b66c147 100644 --- a/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java +++ b/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java @@ -82,6 +82,23 @@ public void respectingEmptyConfig() throws Exception { } } + // TODO Add test cases to the other tests that will check for this behavior too + @Test + public void handlesApplyWithMixins() throws Exception { + Path baseDir = Paths.get(SmithyProjectTest.class.getResource("models/v2").toURI()); + Path applyWithMixin = baseDir.resolve("apply-with-mixin.smithy"); + Path mixinImports = baseDir.resolve("mixin-imports.smithy"); + List modelFiles = ListUtils.of(applyWithMixin, mixinImports); + + try (Harness hs = Harness.create(SmithyBuildExtensions.builder().build(), modelFiles)) { + Map locationMap = hs.getProject().getLocations(); + + // An `apply` with a mixin is ignored, so the shape directly above the `apply` will potentially have a location + // that includes everything up to and including the `apply` + correctLocation(locationMap, "com.applywithmixin#SomeOpInput", 6, 0, 9, 1); + } + } + @Test public void definitionLocationsV1() throws Exception { Path baseDir = Paths.get(SmithyProjectTest.class.getResource("models/v1").toURI()); diff --git a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy new file mode 100644 index 0000000..3b95109 --- /dev/null +++ b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy @@ -0,0 +1,13 @@ +$version: "2.0" + +namespace com.applywithmixin + +use com.mixinimports#HasIsTestParam + +structure SomeOpInput with [HasIsTestParam] { + @required + body: String +} + +// An apply statement that targets a mixed in member from another namespace +apply SomeOpInput$isTest @documentation("Some documentation") diff --git a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy new file mode 100644 index 0000000..bf53a30 --- /dev/null +++ b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy @@ -0,0 +1,10 @@ +$version: "2.0" + +namespace com.mixinimports + +boolean IsTestInput + +@mixin +structure HasIsTestParam { + isTest: Boolean +} From 64cf28dd806d389ae560b64cfb5a679968fac064 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Mon, 19 Sep 2022 16:59:01 -0400 Subject: [PATCH 2/6] More robust tests and better file names --- .../smithy/lsp/ext/SmithyProjectTest.java | 32 ++++++++++++---- ...in-imports.smithy => apply-imports.smithy} | 2 +- .../lsp/ext/models/v2/apply-with-mixin.smithy | 13 ------- .../smithy/lsp/ext/models/v2/apply.smithy | 38 +++++++++++++++++++ 4 files changed, 63 insertions(+), 22 deletions(-) rename src/test/resources/software/amazon/smithy/lsp/ext/models/v2/{mixin-imports.smithy => apply-imports.smithy} (77%) delete mode 100644 src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy create mode 100644 src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply.smithy diff --git a/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java b/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java index b66c147..81b88b5 100644 --- a/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java +++ b/src/test/java/software/amazon/smithy/lsp/ext/SmithyProjectTest.java @@ -82,20 +82,36 @@ public void respectingEmptyConfig() throws Exception { } } - // TODO Add test cases to the other tests that will check for this behavior too @Test - public void handlesApplyWithMixins() throws Exception { + public void ignoresUnmodeledApplyStatements() throws Exception { Path baseDir = Paths.get(SmithyProjectTest.class.getResource("models/v2").toURI()); - Path applyWithMixin = baseDir.resolve("apply-with-mixin.smithy"); - Path mixinImports = baseDir.resolve("mixin-imports.smithy"); - List modelFiles = ListUtils.of(applyWithMixin, mixinImports); + Path main = baseDir.resolve("apply.smithy"); + Path imports = baseDir.resolve("apply-imports.smithy"); + List modelFiles = ListUtils.of(main, imports); try (Harness hs = Harness.create(SmithyBuildExtensions.builder().build(), modelFiles)) { Map locationMap = hs.getProject().getLocations(); - // An `apply` with a mixin is ignored, so the shape directly above the `apply` will potentially have a location - // that includes everything up to and including the `apply` - correctLocation(locationMap, "com.applywithmixin#SomeOpInput", 6, 0, 9, 1); + // Structure shape unchanged by apply + correctLocation(locationMap, "com.main#SomeOpInput", 12, 0, 15, 1); + + // Member is unchanged by apply + correctLocation(locationMap, "com.main#SomeOpInput$body", 14, 4, 14, 16); + + // The mixed in member should have an empty position + correctLocation(locationMap, "com.main#SomeOpInput$isTest", 12, 0, 12, 0); + + // Structure shape unchanged by apply + correctLocation(locationMap, "com.main#ArbitraryStructure", 25, 0, 27, 1); + + // Member is unchanged by apply + correctLocation(locationMap, "com.main#ArbitraryStructure$member", 26, 4, 26, 18); + + // Mixed-in member in another namespace unchanged by apply + correctLocation(locationMap, "com.imports#HasIsTestParam$isTest", 8, 4, 8, 19); + + // Structure in another namespace unchanged by apply + correctLocation(locationMap, "com.imports#HasIsTestParam", 7, 0, 9, 1); } } diff --git a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-imports.smithy similarity index 77% rename from src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy rename to src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-imports.smithy index bf53a30..240708a 100644 --- a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/mixin-imports.smithy +++ b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-imports.smithy @@ -1,6 +1,6 @@ $version: "2.0" -namespace com.mixinimports +namespace com.imports boolean IsTestInput diff --git a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy deleted file mode 100644 index 3b95109..0000000 --- a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply-with-mixin.smithy +++ /dev/null @@ -1,13 +0,0 @@ -$version: "2.0" - -namespace com.applywithmixin - -use com.mixinimports#HasIsTestParam - -structure SomeOpInput with [HasIsTestParam] { - @required - body: String -} - -// An apply statement that targets a mixed in member from another namespace -apply SomeOpInput$isTest @documentation("Some documentation") diff --git a/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply.smithy b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply.smithy new file mode 100644 index 0000000..3a63103 --- /dev/null +++ b/src/test/resources/software/amazon/smithy/lsp/ext/models/v2/apply.smithy @@ -0,0 +1,38 @@ +$version: "2.0" + +namespace com.main + +use com.imports#HasIsTestParam + +// Apply before shape definition +apply SomeOpInput @tags(["someTag"]) + +// Apply as first line in shapes section +apply ArbitraryStructure$member @tags(["someTag"]) + +structure SomeOpInput with [HasIsTestParam] { + @required + body: String +} + +/// Arbitrary doc comment + +// Arbitrary comment + +// Apply targeting a mixed in member from another namespace +apply SomeOpInput$isTest @documentation("Some documentation") + +// Structure to break up applys +structure ArbitraryStructure { + member: String +} + +// Multiple applys before first shape definition +apply ArbitraryStructure @documentation("Some documentation") + +// Apply targeting non-mixed in member +apply SomeOpInput$body @documentation("Some documentation") + + +// Apply targeting a shape from another namespace +apply HasIsTestParam @documentation("Some documentation") From c2af6f543a9e5084f9e62ba01e8e7b9940329d16 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Mon, 19 Sep 2022 17:47:50 -0400 Subject: [PATCH 3/6] Refactor shape location collection to ignore apply statements A new interface was created for getting shape locations, so we can add new methods for getting locations later more easily. Shape location collection was separated into its own class to separate and protect it from `SmithyProject`. The shape location collection was also refactored to get container locations in all files first, then easier to collect member locations. This makes handling multi-file models simpler. There was also a few minor clean ups in `SmithyProject`. --- .../smithy/lsp/ext/FileCachingCollector.java | 367 ++++++++++++++++++ .../lsp/ext/ShapeLocationCollector.java | 24 ++ .../amazon/smithy/lsp/ext/SmithyProject.java | 316 +-------------- 3 files changed, 410 insertions(+), 297 deletions(-) create mode 100644 src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java create mode 100644 src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java diff --git a/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java new file mode 100644 index 0000000..a0e3394 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java @@ -0,0 +1,367 @@ +package software.amazon.smithy.lsp.ext; + +import org.eclipse.lsp4j.Location; +import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.Range; +import software.amazon.smithy.lsp.Utils; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.InputTrait; +import software.amazon.smithy.model.traits.OutputTrait; +import software.amazon.smithy.model.traits.Trait; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * Creates a cache of {@link ModelFile} and uses it to collect the locations of container + * shapes in all files, then collects their members. + */ +final class FileCachingCollector implements ShapeLocationCollector { + + private Model model; + private Map locations; + private Map fileCache; + private Map> operationsWithInlineInputOutputMap; + private Map> containerMembersMap; + + @Override + public Map collectDefinitionLocations(Model model) { + this.locations = new HashMap<>(); + this.model = model; + this.fileCache = createModelFileCache(model); + this.operationsWithInlineInputOutputMap = new HashMap<>(); + this.containerMembersMap = new HashMap<>(); + + for (ModelFile modelFile : this.fileCache.values()) { + collectContainerShapeLocationsInModelFile(modelFile); + } + + operationsWithInlineInputOutputMap.forEach((this::collectInlineInputOutputLocations)); + containerMembersMap.forEach(this::collectMemberLocations); + return this.locations; + } + + private static Map createModelFileCache(Model model) { + Map fileCache = new HashMap<>(); + List modelFilenames = getAllFilenamesFromModel(model); + for (String filename : modelFilenames) { + List shapes = getReverseSortedShapesInFileFromModel(model, filename); + List lines = getFileLines(filename); + DocumentPreamble preamble = Document.detectPreamble(lines); + ModelFile modelFile = new ModelFile(filename, lines, preamble, shapes); + fileCache.put(filename, modelFile); + } + return fileCache; + } + + private void collectContainerShapeLocationsInModelFile(ModelFile modelFile) { + String filename = modelFile.filename; + int endMarker = getInitialEndMarker(modelFile.lines); + + for (Shape shape : modelFile.shapes) { + SourceLocation sourceLocation = shape.getSourceLocation(); + Position startPosition = getStartPosition(sourceLocation); + Position endPosition; + if (endMarker < sourceLocation.getLine()) { + endPosition = new Position(sourceLocation.getLine() - 1, sourceLocation.getColumn() - 1); + } else { + endPosition = getEndPosition(endMarker, modelFile.lines); + } + // If a shape belongs to an operation as an inlined input or output, collect a map of the operation + // with the reversed ordered list of inputs and outputs within that operation. Once the location of + // the containing operation has been determined, the map can be revisited to determine the locations of + // the inlined inputs and outputs. + Optional matchingOperation = getOperationForInlinedInputOrOutput(shape, modelFile); + + if (matchingOperation.isPresent()) { + operationsWithInlineInputOutputMap.computeIfAbsent(matchingOperation.get(), s -> + new ArrayList<>()).add(shape); + // Collect a map of container shapes and a list of member shapes, reverse ordered by source location + // in the model file. This map will be revisited after the location of the containing shape has been + // determined since it is needed to determine the locations of each member. + } else if (shape.isMemberShape()) { + MemberShape memberShape = shape.asMemberShape().get(); + ShapeId containerId = memberShape.getContainer(); + containerMembersMap.computeIfAbsent(containerId, s -> new ArrayList<>()).add(memberShape); + } else { + endMarker = advanceMarkerOnNonMemberShapes(startPosition, shape, modelFile); + locations.put(shape.getId(), createLocation(filename, startPosition, endPosition)); + } + } + } + + // Determine the location of inlined inputs and outputs can be determined using the containing operation. + private void collectInlineInputOutputLocations(OperationShape operation, List shapes) { + int operationEndMarker = locations.get(operation.getId()).getRange().getEnd().getLine(); + for (Shape shape : shapes) { + SourceLocation sourceLocation = shape.getSourceLocation(); + ModelFile modelFile = fileCache.get(sourceLocation.getFilename()); + Position startPosition = getStartPosition(sourceLocation); + Position endPosition = getEndPosition(operationEndMarker, modelFile.lines); + Location location = createLocation(modelFile.filename, startPosition, endPosition); + locations.put(shape.getId(), location); + operationEndMarker = sourceLocation.getLine() - 1; + } + } + + private void collectMemberLocations(ShapeId containerId, List members) { + + Location containerLocation = locations.get(containerId); + Range containerLocationRange = containerLocation.getRange(); + int memberEndMarker = containerLocationRange.getEnd().getLine(); + // Keep track of previous line to make sure that end marker has been advanced. + String previousLine = ""; + // The member shapes were reverse ordered by source location when assembling this list, so we can + // iterate through it as-is to work from bottom to top in the model file. + for (MemberShape memberShape : members) { + ModelFile modelFile = fileCache.get(memberShape.getSourceLocation().getFilename()); + int memberShapeSourceLocationLine = memberShape.getSourceLocation().getLine(); + + boolean isContainerInAnotherFile = !containerLocation.getUri().equals(getUri(modelFile.filename)); + // If the member's source location matches the container location's starting line (with offset), + // the member is inherited from a mixin and not present in the model file. + boolean isMemberMixedIn = memberShapeSourceLocationLine == containerLocationRange.getStart().getLine() + 1; + + if (isContainerInAnotherFile || isMemberMixedIn) { + locations.put(memberShape.getId(), createInheritedMemberLocation(containerLocation)); + // Otherwise, determine the correct location by trimming comments, empty lines and applied traits. + } else { + String currentLine = modelFile.lines.get(memberEndMarker - 1).trim(); + while (currentLine.startsWith("//") + || currentLine.equals("") + || currentLine.equals("}") + || currentLine.startsWith("@") + || currentLine.equals(previousLine) + ) { + memberEndMarker = memberEndMarker - 1; + currentLine = modelFile.lines.get(memberEndMarker - 1).trim(); + } + Position startPosition = getStartPosition(memberShape.getSourceLocation()); + Position endPosition = getEndPosition(memberEndMarker, modelFile.lines); + + // Advance the member end marker on any non-mixin traits on the current member, so that the next + // member location end is correct. Mixin traits will have been declared outside the + // containing shape and shouldn't impact determining the end location of the next member. + List traits = memberShape.getAllTraits().values().stream() + .filter(trait -> !trait.getSourceLocation().equals(SourceLocation.NONE)) + .filter(trait -> trait.getSourceLocation().getFilename().equals(modelFile.filename)) + .filter(trait -> !isFromMixin(containerLocationRange, trait)) + .collect(Collectors.toList()); + + if (!traits.isEmpty()) { + traits.sort(Comparator.comparing(Trait::getSourceLocation)); + memberEndMarker = traits.get(0).getSourceLocation().getLine(); + } + + locations.put(memberShape.getId(), createLocation(modelFile.filename, startPosition, endPosition)); + previousLine = currentLine; + } + } + } + + // Use an empty range at the container's start since inherited members are not present in the model file. + private static Location createInheritedMemberLocation(Location containerLocation) { + Position startPosition = containerLocation.getRange().getStart(); + Range memberRange = new Range(startPosition, startPosition); + return new Location(containerLocation.getUri(), memberRange); + } + + // If the trait was defined outside the container, it was mixed in. + private static boolean isFromMixin(Range containerRange, Trait trait) { + int traitLocationLine = trait.getSourceLocation().getLine(); + return traitLocationLine < containerRange.getStart().getLine() + || traitLocationLine > containerRange.getEnd().getLine(); + } + + // Get the operation that matches an inlined input or output structure. + private Optional getOperationForInlinedInputOrOutput(Shape shape, ModelFile modelFile) { + DocumentPreamble preamble = modelFile.preamble; + if (preamble.getIdlVersion().isPresent() + && preamble.getIdlVersion().get().startsWith("2") + && shape.isStructureShape() + && (shape.hasTrait(OutputTrait.class) || shape.hasTrait(InputTrait.class)) + ) { + String suffix = getOperationInputOrOutputSuffix(shape, preamble); + String shapeName = shape.getId().getName(); + String matchingOperationName = shapeName.substring(0, shapeName.length() - suffix.length()); + return model.shapes(OperationShape.class) + .filter(operationShape -> operationShape.getId().getName().equals(matchingOperationName)) + .findFirst() + .filter(operation -> shapeWasDefinedInline(operation, shape, modelFile)); + } + return Optional.empty(); + } + + private static String getOperationInputOrOutputSuffix(Shape shape, DocumentPreamble preamble) { + if (shape.hasTrait(InputTrait.class)) { + return preamble.getOperationInputSuffix().orElse("Input"); + } + if (shape.hasTrait(OutputTrait.class)) { + return preamble.getOperationOutputSuffix().orElse("Output"); + } + return ""; + } + + // Iterate through lines in reverse order from current shape start, to the beginning of the above shape, or the + // start of the operation. If the inline structure assignment operator is encountered, the current shape was + // defined inline. This check eliminates instances where an operation and its input or output matches the inline + // structure naming convention. + private Boolean shapeWasDefinedInline(OperationShape operation, Shape shape, ModelFile modelFile) { + int shapeStartLine = shape.getSourceLocation().getLine(); + int priorShapeLine = 0; + if (shape.hasTrait(InputTrait.class) && operation.getOutput().isPresent()) { + Shape output = model.expectShape(operation.getOutputShape().toShapeId()); + if (output.getSourceLocation().getLine() < shape.getSourceLocation().getLine()) { + priorShapeLine = output.getSourceLocation().getLine(); + } + } + if (shape.hasTrait(OutputTrait.class) && operation.getInput().isPresent()) { + Shape input = model.expectShape(operation.getInputShape().toShapeId()); + if (input.getSourceLocation().getLine() < shape.getSourceLocation().getLine()) { + priorShapeLine = input.getSourceLocation().getLine(); + } + } + int boundary = Math.max(priorShapeLine, operation.getSourceLocation().getLine()); + while (shapeStartLine >= boundary) { + String line = modelFile.lines.get(shapeStartLine); + if (line.contains(":=")) { + return true; + } + shapeStartLine--; + } + return false; + } + + private static Location createLocation(String file, Position startPosition, Position endPosition) { + return new Location(getUri(file), new Range(startPosition, endPosition)); + } + + private static int advanceMarkerOnNonMemberShapes(Position startPosition, Shape shape, ModelFile modelFile) { + // When handling non-member shapes, advance the end marker for traits and comments above the current + // shape, ignoring applied traits + int marker = startPosition.getLine(); + + List traits = shape.getAllTraits().values().stream() + .filter(trait -> !trait.getSourceLocation().equals(SourceLocation.NONE)) + .filter(trait -> trait.getSourceLocation().getLine() <= startPosition.getLine()) + .filter(trait -> trait.getSourceLocation().getFilename().equals(modelFile.filename)) + .filter(trait -> !modelFile.lines.get(trait.getSourceLocation().getLine()).trim().startsWith("apply")) + .collect(Collectors.toList()); + + // If the shape has traits, advance the end marker again. + if (!traits.isEmpty()) { + traits.sort(Comparator.comparing(Trait::getSourceLocation)); + marker = traits.get(0).getSourceLocation().getLine() - 1; + } + + // Move the end marker when encountering line comments or empty lines. + if (modelFile.lines.size() > marker) { + marker = getNextEndMarker(modelFile.lines, marker); + } + + return marker; + } + + private static int getInitialEndMarker(List lines) { + return getNextEndMarker(lines, lines.size()); + + } + + private static int getNextEndMarker(List lines, int currentEndMarker) { + if (lines.size() == 0) { + return currentEndMarker; + } + int endMarker = currentEndMarker; + while (endMarker > 0 && shouldIgnoreLine(lines.get(endMarker - 1))) { + endMarker--; + } + return endMarker; + } + + // Blank lines, comments, and apply statements are ignored because they are unmodeled + private static boolean shouldIgnoreLine(String line) { + String trimmed = line.trim(); + return trimmed.isEmpty() || trimmed.startsWith("//") || trimmed.startsWith("apply"); + } + + private static Position getStartPosition(SourceLocation sourceLocation) { + return new Position(sourceLocation.getLine() - 1, sourceLocation.getColumn() - 1); + } + + private static String getUri(String fileName) { + return Utils.isJarFile(fileName) + ? Utils.toSmithyJarFile(fileName) + : addFilePrefix(fileName); + } + + private static String addFilePrefix(String fileName) { + return !fileName.startsWith("file:") ? "file:" + fileName : fileName; + } + + private static List getAllFilenamesFromModel(Model model) { + return model.shapes() + .map(shape -> shape.getSourceLocation().getFilename()) + .distinct() + .collect(Collectors.toList()); + } + + private static List getReverseSortedShapesInFileFromModel(Model model, String filename) { + return model.shapes() + .filter(shape -> shape.getSourceLocation().getFilename().equals(filename)) + .sorted(Comparator.comparing(Shape::getSourceLocation).reversed()) + .collect(Collectors.toList()); + } + + private static List getFileLines(String file) { + try { + if (Utils.isSmithyJarFile(file) || Utils.isJarFile(file)) { + return Utils.jarFileContents(Utils.toSmithyJarFile(file)); + } else { + return Files.readAllLines(Paths.get(file)); + } + } catch (IOException e) { + LspLog.println("File " + file + " could not be loaded."); + } + return Collections.emptyList(); + } + + private static Position getEndPosition(int currentEndMarker, List fileLines) { + // Skip any blank lines, comments, or apply statements + int endLine = getNextEndMarker(fileLines, currentEndMarker); + + // Return end position of actual shape line if we have the lines, or set it to the start of the next line + if (fileLines.size() >= endLine) { + return new Position(endLine - 1, fileLines.get(endLine- 1).length()); + } + return new Position(endLine, 0); + } + + private static final class ModelFile { + private final String filename; + private final List lines; + private final DocumentPreamble preamble; + private final List shapes; + + private ModelFile(String filename, List lines, DocumentPreamble preamble, List shapes) { + this.filename = filename; + this.lines = lines; + this.preamble = preamble; + this.shapes = shapes; + } + } + +} diff --git a/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java new file mode 100644 index 0000000..2d7f0e1 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java @@ -0,0 +1,24 @@ +package software.amazon.smithy.lsp.ext; + +import org.eclipse.lsp4j.Location; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.utils.SmithyUnstableApi; + +import java.util.Map; + +/** + * Interface used to get the shape location information + * used by the language server. + */ +@SmithyUnstableApi +interface ShapeLocationCollector { + + /** + * Collects the definition locations of all shapes in the model. + * + * @param model Model to collect shape definition locations for + * @return Map of {@link ShapeId} to its definition {@link Location} + */ + Map collectDefinitionLocations(Model model); +} diff --git a/src/main/java/software/amazon/smithy/lsp/ext/SmithyProject.java b/src/main/java/software/amazon/smithy/lsp/ext/SmithyProject.java index f983bc9..f339010 100644 --- a/src/main/java/software/amazon/smithy/lsp/ext/SmithyProject.java +++ b/src/main/java/software/amazon/smithy/lsp/ext/SmithyProject.java @@ -24,7 +24,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -36,19 +35,11 @@ import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.messages.Either; import software.amazon.smithy.lsp.SmithyInterface; -import software.amazon.smithy.lsp.Utils; import software.amazon.smithy.lsp.ext.model.SmithyBuildExtensions; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.selector.Selector; -import software.amazon.smithy.model.shapes.MemberShape; -import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; -import software.amazon.smithy.model.shapes.ShapeType; -import software.amazon.smithy.model.traits.InputTrait; -import software.amazon.smithy.model.traits.OutputTrait; -import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidatedResultException; @@ -60,8 +51,13 @@ public final class SmithyProject { private final ValidatedResult model; private final File root; - SmithyProject(List imports, List smithyFiles, List externalJars, File root, - ValidatedResult model) { + SmithyProject( + List imports, + List smithyFiles, + List externalJars, + File root, + ValidatedResult model + ) { this.imports = imports; this.root = root; this.model = model; @@ -163,8 +159,12 @@ public Either> runSelector(String expression) { } } - private static Either load(List imports, List smithyFiles, - List externalJars, File root) { + private static Either load( + List imports, + List smithyFiles, + List externalJars, + File root + ) { Either> model = createModel(smithyFiles, externalJars); if (model.isLeft()) { @@ -175,8 +175,10 @@ private static Either load(List imports, List> createModel(List discoveredFiles, - List externalJars) { + private static Either> createModel( + List discoveredFiles, + List externalJars + ) { return SmithyInterface.readModel(discoveredFiles, externalJars); } @@ -185,241 +187,8 @@ public File getRoot() { } private static Map collectLocations(Model model) { - Map locations = new HashMap<>(); - List modelFiles = model.shapes() - .map(shape -> shape.getSourceLocation().getFilename()) - .distinct() - .collect(Collectors.toList()); - for (String modelFile : modelFiles) { - List lines = getFileLines(modelFile); - DocumentPreamble preamble = Document.detectPreamble(lines); - Map> operationsWithInlineInputOutputMap = new HashMap<>(); - Map> containerMembersMap = new HashMap<>(); - int endMarker = getInitialEndMarker(lines); - - // Get shapes reverse-sorted by source location to work from bottom of file to top. - List shapes = model.shapes() - .filter(shape -> shape.getSourceLocation().getFilename().equals(modelFile)) - .sorted(Comparator.comparing(Shape::getSourceLocation).reversed()) - .collect(Collectors.toList()); - - - for (Shape shape : shapes) { - SourceLocation sourceLocation = shape.getSourceLocation(); - Position startPosition = getStartPosition(sourceLocation); - Position endPosition; - if (endMarker < sourceLocation.getLine()) { - endPosition = new Position(sourceLocation.getLine() - 1, sourceLocation.getColumn() - 1); - } else { - endPosition = getEndPosition(endMarker, lines); - } - // If a shape belongs to an operation as an inlined input or output, collect a map of the operation - // with the reversed ordered list of inputs and outputs within that operation. Once the location of - // the containing operation has been determined, the map can be revisited to determine the locations of - // the inlined inputs and outputs. - Optional matchingOperation = getOperationForInlinedInputOrOutput(model, shape, - preamble, lines); - if (matchingOperation.isPresent()) { - operationsWithInlineInputOutputMap.computeIfAbsent(matchingOperation.get(), s -> new ArrayList<>()) - .add(shape); - // Collect a map of container shapes and a list of member shapes, reverse ordered by source location - // in the model file. This map will be revisited after the location of the containing shape has been - // determined since it is needed to determine the locations of each member. - } else if (shape.getType() == ShapeType.MEMBER) { - MemberShape memberShape = shape.asMemberShape().get(); - ShapeId containerId = memberShape.getContainer(); - containerMembersMap.computeIfAbsent(containerId, s -> new ArrayList<>()).add(memberShape); - } else { - endMarker = advanceMarkerOnNonMemberShapes(startPosition, shape, lines, modelFile); - locations.put(shape.getId(), createLocation(modelFile, startPosition, endPosition)); - } - } - - collectInlineInputOutputLocations(operationsWithInlineInputOutputMap, modelFile, lines, locations); - collectMemberLocations(containerMembersMap, modelFile, lines, locations); - } - return locations; - } - - // Determine the location of inlined inputs and outputs can be determined using the containing operation. - private static void collectInlineInputOutputLocations( - Map> operationsWithInlineInputOutputMap, - String modelFile, - List lines, - Map locations - ) { - for (Map.Entry> entry : operationsWithInlineInputOutputMap.entrySet()) { - OperationShape operation = entry.getKey(); - int operationEndMarker = locations.get(operation.getId()).getRange().getEnd().getLine(); - for (Shape shape : entry.getValue()) { - SourceLocation sourceLocation = shape.getSourceLocation(); - Position startPosition = getStartPosition(sourceLocation); - Position endPosition = getEndPosition(operationEndMarker, lines); - Location location = createLocation(modelFile, startPosition, endPosition); - locations.put(shape.getId(), location); - operationEndMarker = sourceLocation.getLine() - 1; - } - } - } - - // Determine locations of members using containing shape locations. - private static void collectMemberLocations( - Map> containerMembersMap, - String modelFile, List lines, - Map locations - ) { - for (Map.Entry> entry : containerMembersMap.entrySet()) { - Location containerLocation = locations.get(entry.getKey()); - Range containerLocationRange = containerLocation.getRange(); - int memberEndMarker = containerLocationRange.getEnd().getLine(); - // Keep track of previous line to make sure that end marker has been advanced. - String previousLine = ""; - // The member shapes were reverse ordered by source location when assembling this list, so we can - // iterate through it as-is to work from bottom to top in the model file. - for (MemberShape memberShape : entry.getValue()) { - int memberShapeSourceLocationLine = memberShape.getSourceLocation().getLine(); - // If the member's source location matches the container location's starting line (with offset), - // the member is mixed in and not present in the model file. - if (memberShapeSourceLocationLine == containerLocationRange.getStart().getLine() + 1) { - locations.put(memberShape.getId(), createElidedMemberLocation(containerLocation)); - // Otherwise, determine the correct location by trimming comments, empty lines and applied traits. - } else { - String currentLine = lines.get(memberEndMarker - 1).trim(); - while (currentLine.startsWith("//") || currentLine.equals("") || currentLine.equals("}") - || currentLine.startsWith("@") || currentLine.equals(previousLine)) { - memberEndMarker = memberEndMarker - 1; - currentLine = lines.get(memberEndMarker - 1).trim(); - } - Position startPosition = getStartPosition(memberShape.getSourceLocation()); - Position endPosition = getEndPosition(memberEndMarker, lines); - // Advance the member end marker on any non-mixin traits on the current member, so that the next - // member location end is correct. Mixin traits will have been declared outside the - // containing shape and shouldn't impact determining the end location of the next member. - List traits = memberShape.getAllTraits().values().stream() - .filter(trait -> !trait.getSourceLocation().equals(SourceLocation.NONE)) - .filter(trait -> trait.getSourceLocation().getFilename().equals(modelFile)) - .filter(trait -> !isFromMixin(containerLocationRange, trait)) - .collect(Collectors.toList()); - if (!traits.isEmpty()) { - traits.sort(Comparator.comparing(Trait::getSourceLocation)); - memberEndMarker = traits.get(0).getSourceLocation().getLine(); - } - locations.put(memberShape.getId(), createLocation(modelFile, startPosition, endPosition)); - previousLine = currentLine; - } - } - } - } - - // Use an empty range at the container's start since elided members are not present in the model file. - private static Location createElidedMemberLocation(Location containerLocation) { - Position startPosition = containerLocation.getRange().getStart(); - Range memberRange = new Range(startPosition, startPosition); - return new Location(containerLocation.getUri(), memberRange); - } - - // If the trait was defined outside the container, it was mixed in. - private static boolean isFromMixin(Range containerRange, Trait trait) { - int traitLocationLine = trait.getSourceLocation().getLine(); - return traitLocationLine < containerRange.getStart().getLine() - || traitLocationLine > containerRange.getEnd().getLine(); - } - - // Get the operation that matches an inlined input or output structure. - private static Optional getOperationForInlinedInputOrOutput( - Model model, - Shape shape, - DocumentPreamble preamble, - List lines - ) { - if (preamble.getIdlVersion().isPresent()) { - if (preamble.getIdlVersion().get().startsWith("2") && shape.isStructureShape() - && (shape.hasTrait(OutputTrait.class) || shape.hasTrait(InputTrait.class))) { - String suffix = null; - if (shape.hasTrait(InputTrait.class)) { - suffix = preamble.getOperationInputSuffix().orElse("Input"); - } - - if (shape.hasTrait(OutputTrait.class)) { - suffix = preamble.getOperationOutputSuffix().orElse("Output"); - } - - String shapeName = shape.getId().getName(); - String matchingOperationName = shapeName.substring(0, shapeName.length() - suffix.length()); - Optional operation = model.shapes(OperationShape.class) - .filter(operationShape -> operationShape.getId().getName().equals(matchingOperationName)) - .findFirst(); - if (shapeWasDefinedInline(operation, shape, model, lines)) { - return operation; - } - } - } - return Optional.empty(); - } - - // Iterate through lines in reverse order from current shape start, to the beginning of the above shape, or the - // start of the operation. If the inline structure assignment operator is encountered, the current shape was - // defined inline. This check eliminates instances where an operation and its input or output matches the inline - // structure naming convention. - private static boolean shapeWasDefinedInline(Optional optionalOperation, Shape shape, Model model, - List lines) { - if (!optionalOperation.isPresent()) { - return false; - } - int shapeStartLine = shape.getSourceLocation().getLine(); - int priorShapeLine = 0; - OperationShape operation = optionalOperation.get(); - if (shape.hasTrait(InputTrait.class) && operation.getOutput().isPresent()) { - Shape output = model.expectShape(operation.getOutputShape().toShapeId()); - if (output.getSourceLocation().getLine() < shape.getSourceLocation().getLine()) { - priorShapeLine = output.getSourceLocation().getLine(); - } - } - if (shape.hasTrait(OutputTrait.class) && operation.getInput().isPresent()) { - Shape input = model.expectShape(operation.getInputShape().toShapeId()); - if (input.getSourceLocation().getLine() < shape.getSourceLocation().getLine()) { - priorShapeLine = input.getSourceLocation().getLine(); - } - } - int boundary = Math.max(priorShapeLine, operation.getSourceLocation().getLine()); - while (shapeStartLine >= boundary) { - String line = lines.get(shapeStartLine); - if (line.contains(":=")) { - return true; - } - shapeStartLine--; - } - return false; - } - - private static Location createLocation(String file, Position startPosition, Position endPosition) { - return new Location(getUri(file), new Range(startPosition, endPosition)); - } - - private static int advanceMarkerOnNonMemberShapes(Position startPosition, Shape shape, List fileLines, - String modelFile) { - // When handling non-member shapes, advance the end marker for traits and comments above the current - // shape. - int marker = startPosition.getLine(); - // TODO: Handle traits being applied to shapes before shape definition in file. - List traits = shape.getAllTraits().values().stream() - .filter(trait -> !trait.getSourceLocation().equals(SourceLocation.NONE)) - .filter(trait -> trait.getSourceLocation().getFilename().equals(modelFile)) - .collect(Collectors.toList()); - // If the shape has traits, advance the end marker again. - if (!traits.isEmpty()) { - traits.sort(Comparator.comparing(Trait::getSourceLocation)); - marker = traits.get(0).getSourceLocation().getLine() - 1; - } - // Move the end marker when encountering line comments or empty lines. - if (fileLines.size() > marker) { - while (fileLines.get(marker - 1).trim().startsWith("//") - || fileLines.get(marker - 1).trim().equals("")) { - marker = marker - 1; - } - } - return marker; + ShapeLocationCollector collector = new FileCachingCollector(); + return collector.collectDefinitionLocations(model); } /** @@ -461,53 +230,6 @@ private boolean isPositionInRange(Range range, Position position) { return true; } - private static int getInitialEndMarker(List lines) { - int endMarker = lines.size(); - // Remove empty lines from the end of the file. - if (lines.size() > 0) { - while (lines.get(endMarker - 1).trim().equals("")) { - endMarker = endMarker - 1; - } - } - return endMarker; - } - - private static Position getStartPosition(SourceLocation sourceLocation) { - return new Position(sourceLocation.getLine() - 1, sourceLocation.getColumn() - 1); - } - - // If the lines of the model were successfully loaded, return the end position of the actual shape line, - // otherwise set it to the start of the next line. - private static Position getEndPosition(int endMarker, List lines) { - if (lines.size() >= endMarker) { - return new Position(endMarker - 1, lines.get(endMarker - 1).length()); - } - return new Position(endMarker, 0); - } - - private static List getFileLines(String file) { - try { - if (Utils.isSmithyJarFile(file) || Utils.isJarFile(file)) { - return Utils.jarFileContents(Utils.toSmithyJarFile(file)); - } else { - return Files.readAllLines(Paths.get(file)); - } - } catch (IOException e) { - LspLog.println("File " + file + " could not be loaded."); - } - return Collections.emptyList(); - } - - private static String getUri(String fileName) { - return Utils.isJarFile(fileName) - ? Utils.toSmithyJarFile(fileName) - : addFilePrefix(fileName); - } - - private static String addFilePrefix(String fileName) { - return !fileName.startsWith("file:") ? "file:" + fileName : fileName; - } - private static Boolean isValidSmithyFile(Path file) { String fName = file.getFileName().toString(); return fName.endsWith(Constants.SMITHY_EXTENSION); From de8a3b4bd8e5681203c57e35ef9bf72254a76e85 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Mon, 19 Sep 2022 18:38:33 -0400 Subject: [PATCH 4/6] Fix style --- .../smithy/lsp/ext/FileCachingCollector.java | 40 +++++++++++++------ .../lsp/ext/ShapeLocationCollector.java | 18 ++++++++- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java index a0e3394..259c5ef 100644 --- a/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java +++ b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java @@ -1,5 +1,31 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + package software.amazon.smithy.lsp.ext; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; import org.eclipse.lsp4j.Location; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; @@ -14,18 +40,6 @@ import software.amazon.smithy.model.traits.OutputTrait; import software.amazon.smithy.model.traits.Trait; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; - /** * Creates a cache of {@link ModelFile} and uses it to collect the locations of container * shapes in all files, then collects their members. @@ -345,7 +359,7 @@ private static Position getEndPosition(int currentEndMarker, List fileLi // Return end position of actual shape line if we have the lines, or set it to the start of the next line if (fileLines.size() >= endLine) { - return new Position(endLine - 1, fileLines.get(endLine- 1).length()); + return new Position(endLine - 1, fileLines.get(endLine - 1).length()); } return new Position(endLine, 0); } diff --git a/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java index 2d7f0e1..a278b16 100644 --- a/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java +++ b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java @@ -1,12 +1,26 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + package software.amazon.smithy.lsp.ext; +import java.util.Map; import org.eclipse.lsp4j.Location; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.utils.SmithyUnstableApi; -import java.util.Map; - /** * Interface used to get the shape location information * used by the language server. From 29f14cb1fb2e4476d2d8e88b8a22ab924603c5fa Mon Sep 17 00:00:00 2001 From: Miles Ziemer <45497130+milesziemer@users.noreply.github.com> Date: Tue, 20 Sep 2022 12:23:25 -0400 Subject: [PATCH 5/6] Update src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java Co-authored-by: Chase Coalwell <782571+srchase@users.noreply.github.com> --- .../software/amazon/smithy/lsp/ext/FileCachingCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java index 259c5ef..5c992f8 100644 --- a/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java +++ b/src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. From c4ab6e653453a7fe274acc8b668f466e2c2e8bf1 Mon Sep 17 00:00:00 2001 From: Miles Ziemer <45497130+milesziemer@users.noreply.github.com> Date: Tue, 20 Sep 2022 12:23:31 -0400 Subject: [PATCH 6/6] Update src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java Co-authored-by: Chase Coalwell <782571+srchase@users.noreply.github.com> --- .../software/amazon/smithy/lsp/ext/ShapeLocationCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java index a278b16..179a501 100644 --- a/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java +++ b/src/main/java/software/amazon/smithy/lsp/ext/ShapeLocationCollector.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License.