From 01da1ff3ee16ad2990fc0f439073fa836eac2b2b Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Fri, 17 Jan 2025 10:20:29 -0500 Subject: [PATCH 1/9] Report diagnostics for build files This commit makes the language server send diagnostics back to build files, i.e. smithy-build.json. Previously, any issues in build files would result in the project failing to load, and those errors would be reported to the client's window. With this change, issues are recomputed on change, and sent back as diagnostics so you get squigglies. Much better. To accomplish this, a number of changes were needed: 1. Reparse build files on change. Previously, we just updated the document. 2. Have a way to run some sort of validation on build files that can tolerate errors. This is split into two parts: 1. A regular validation stage that takes the parsed build file and tries to map it to its specific java representation, collecting any errors that occur. For example, smithy-build.json is turned into SmithyBuildConfig. 2. A resolution stage that takes the java representation and tries to resolve maven dependencies, and recursively find all model paths from sources and imports. 3. Keep track of events emitted from this validation so they can be sent back to the client. 2 is the most complicated part. SmithyBuildConfig does some extra work under the hood when it is deserialized from a Node, like environment variable replacement. I wanted to make sure there wasn't any drift between the language server and other Smithy tools, so I kept using SmithyBuildConfig::fromNode, but now any exception thrown from this will be mapped to a validation event. Each of the other build files work the same way. I also kept the same merging logic for aggregating config from multiple build files. Next is the resolution part. Maven resolution can fail in multiple ways. We have to try to map any exceptions back to a source location, because we don't have access to the original source locations. For finding source/import files, I wanted to be able to report when files aren't found (this also helps to make sure assembling the model doesn't fail due to files not being found), so we have to do the same thing (that is, map back to a source location). Resolution in general is expensive, as it could be hitting maven central, but doing this mapping could also be expensive, so we don't perform the resolution step when build files change - only when a project is actually loaded. We will have to see how this validation feels, and make improvements where necessary. --- .../amazon/smithy/lsp/FilePatterns.java | 4 +- .../amazon/smithy/lsp/ProjectRootVisitor.java | 4 +- .../amazon/smithy/lsp/ServerState.java | 22 +- .../smithy/lsp/SmithyLanguageServer.java | 44 +- .../lsp/diagnostics/SmithyDiagnostics.java | 142 ++-- .../amazon/smithy/lsp/project/BuildFile.java | 49 +- .../smithy/lsp/project/BuildFileType.java | 38 + .../amazon/smithy/lsp/project/BuildFiles.java | 91 +++ .../amazon/smithy/lsp/project/Project.java | 28 +- .../smithy/lsp/project/ProjectConfig.java | 208 +++--- .../lsp/project/ProjectConfigLoader.java | 648 ++++++++++++++---- .../smithy/lsp/project/ProjectDependency.java | 25 - .../project/ProjectDependencyResolver.java | 113 --- .../smithy/lsp/project/ProjectFile.java | 5 + .../smithy/lsp/project/ProjectLoader.java | 176 +---- .../smithy/lsp/project/SmithyProjectJson.java | 64 ++ .../smithy/lsp/project/ToSmithyNode.java | 97 +++ .../smithy/lsp/protocol/LspAdapter.java | 30 +- .../amazon/smithy/lsp/util/Result.java | 163 ----- .../amazon/smithy/lsp/FilePatternsTest.java | 6 +- .../lsp/FileWatcherRegistrationsTest.java | 4 +- .../amazon/smithy/lsp/SmithyMatchers.java | 10 + .../lsp/language/CompletionHandlerTest.java | 5 +- .../lsp/language/DefinitionHandlerTest.java | 5 +- .../lsp/language/DocumentSymbolTest.java | 5 +- .../smithy/lsp/language/HoverHandlerTest.java | 5 +- .../lsp/project/ProjectConfigLoaderTest.java | 85 --- .../smithy/lsp/project/ProjectConfigTest.java | 272 ++++++++ .../smithy/lsp/project/ProjectLoaderTest.java | 230 +++++++ .../smithy/lsp/project/ProjectTest.java | 228 +----- 30 files changed, 1679 insertions(+), 1127 deletions(-) create mode 100644 src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java create mode 100644 src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java delete mode 100644 src/main/java/software/amazon/smithy/lsp/project/ProjectDependency.java delete mode 100644 src/main/java/software/amazon/smithy/lsp/project/ProjectDependencyResolver.java create mode 100644 src/main/java/software/amazon/smithy/lsp/project/SmithyProjectJson.java create mode 100644 src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java delete mode 100644 src/main/java/software/amazon/smithy/lsp/util/Result.java delete mode 100644 src/test/java/software/amazon/smithy/lsp/project/ProjectConfigLoaderTest.java create mode 100644 src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java create mode 100644 src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java diff --git a/src/main/java/software/amazon/smithy/lsp/FilePatterns.java b/src/main/java/software/amazon/smithy/lsp/FilePatterns.java index 23bab11f..447e0fe6 100644 --- a/src/main/java/software/amazon/smithy/lsp/FilePatterns.java +++ b/src/main/java/software/amazon/smithy/lsp/FilePatterns.java @@ -12,8 +12,8 @@ import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; +import software.amazon.smithy.lsp.project.BuildFileType; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectConfigLoader; /** * Utility methods for computing glob patterns that match against Smithy files @@ -87,7 +87,7 @@ private static String getBuildFilesPattern(Path root, boolean isWorkspacePattern rootString += "**" + File.separator; } - return escapeBackslashes(rootString + "{" + String.join(",", ProjectConfigLoader.PROJECT_BUILD_FILES) + "}"); + return escapeBackslashes(rootString + "{" + String.join(",", BuildFileType.ALL_FILENAMES) + "}"); } // When computing the pattern used for telling the client which files to watch, we want diff --git a/src/main/java/software/amazon/smithy/lsp/ProjectRootVisitor.java b/src/main/java/software/amazon/smithy/lsp/ProjectRootVisitor.java index a1016b9d..448e3da0 100644 --- a/src/main/java/software/amazon/smithy/lsp/ProjectRootVisitor.java +++ b/src/main/java/software/amazon/smithy/lsp/ProjectRootVisitor.java @@ -17,14 +17,14 @@ import java.util.ArrayList; import java.util.EnumSet; import java.util.List; -import software.amazon.smithy.lsp.project.ProjectConfigLoader; +import software.amazon.smithy.lsp.project.BuildFileType; /** * Finds Project roots based on the location of smithy-build.json and .smithy-project.json. */ final class ProjectRootVisitor extends SimpleFileVisitor { private static final PathMatcher PROJECT_ROOT_MATCHER = FileSystems.getDefault().getPathMatcher( - "glob:{" + ProjectConfigLoader.SMITHY_BUILD + "," + ProjectConfigLoader.SMITHY_PROJECT + "}"); + "glob:{" + BuildFileType.SMITHY_BUILD.filename() + "," + BuildFileType.SMITHY_PROJECT.filename() + "}"); private static final int MAX_VISIT_DEPTH = 10; private final List roots = new ArrayList<>(); diff --git a/src/main/java/software/amazon/smithy/lsp/ServerState.java b/src/main/java/software/amazon/smithy/lsp/ServerState.java index c0de6d52..9760c20d 100644 --- a/src/main/java/software/amazon/smithy/lsp/ServerState.java +++ b/src/main/java/software/amazon/smithy/lsp/ServerState.java @@ -26,7 +26,6 @@ import software.amazon.smithy.lsp.project.ProjectFile; import software.amazon.smithy.lsp.project.ProjectLoader; import software.amazon.smithy.lsp.protocol.LspAdapter; -import software.amazon.smithy.lsp.util.Result; /** * Keeps track of the state of the server. @@ -143,10 +142,9 @@ List tryInitProject(Path root) { LOGGER.finest("Initializing project at " + root); lifecycleTasks.cancelAllTasks(); - Result> loadResult = ProjectLoader.load(root, this); String projectName = root.toString(); - if (loadResult.isOk()) { - Project updatedProject = loadResult.unwrap(); + try { + Project updatedProject = ProjectLoader.load(root, this); if (updatedProject.type() == Project.Type.EMPTY) { removeProjectAndResolveDetached(projectName); @@ -157,17 +155,15 @@ List tryInitProject(Path root) { LOGGER.finest("Initialized project at " + root); return List.of(); - } + } catch (Exception e) { + LOGGER.severe("Failed to load project at " + root); - LOGGER.severe("Init project failed"); + // If we overwrite an existing project with an empty one, we lose track of the state of tracked + // files. Instead, we will just keep the original project before the reload failure. + projects.computeIfAbsent(projectName, ignored -> Project.empty(root)); - // TODO: Maybe we just start with this anyways by default, and then add to it - // if we find a smithy-build.json, etc. - // If we overwrite an existing project with an empty one, we lose track of the state of tracked - // files. Instead, we will just keep the original project before the reload failure. - projects.computeIfAbsent(projectName, ignored -> Project.empty(root)); - - return loadResult.unwrapErr(); + return List.of(e); + } } void loadWorkspace(WorkspaceFolder workspaceFolder) { diff --git a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java index 1cc512b0..7fd92701 100644 --- a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java +++ b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java @@ -447,22 +447,31 @@ public void didChange(DidChangeTextDocumentParams params) { } } - // Don't reload or update the project on build file changes, only on save - if (!(projectAndFile.file() instanceof SmithyFile smithyFile)) { - return; - } + projectAndFile.file().reparse(); - smithyFile.reparse(); - if (!this.serverOptions.getOnlyReloadOnSave()) { - Project project = projectAndFile.project(); + Project project = projectAndFile.project(); + switch (projectAndFile.file()) { + case SmithyFile ignored -> { + if (this.serverOptions.getOnlyReloadOnSave()) { + return; + } + + // TODO: A consequence of this is that any existing validation events are cleared, which + // is kinda annoying. + // Report any parse/shape/trait loading errors + CompletableFuture future = CompletableFuture + .runAsync(() -> project.updateModelWithoutValidating(uri)) + .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile)); + + state.lifecycleTasks().putTask(uri, future); + } + case BuildFile ignored -> { + CompletableFuture future = CompletableFuture + .runAsync(project::validateConfig) + .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile)); - // TODO: A consequence of this is that any existing validation events are cleared, which - // is kinda annoying. - // Report any parse/shape/trait loading errors - CompletableFuture future = CompletableFuture - .runAsync(() -> project.updateModelWithoutValidating(uri)) - .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile)); - state.lifecycleTasks().putTask(uri, future); + state.lifecycleTasks().putTask(uri, future); + } } } @@ -656,4 +665,11 @@ private CompletableFuture sendFileDiagnostics(ProjectAndFile projectAndFil client.publishDiagnostics(publishDiagnosticsParams); }); } + + private CompletableFuture sendFileDiagnostics(String uri, List diagnostics) { + return CompletableFuture.runAsync(() -> { + var params = new PublishDiagnosticsParams(uri, diagnostics); + client.publishDiagnostics(params); + }); + } } diff --git a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java index 2edc2034..a3e73be3 100644 --- a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java +++ b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java @@ -15,6 +15,7 @@ import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; import software.amazon.smithy.lsp.document.DocumentParser; +import software.amazon.smithy.lsp.project.BuildFile; import software.amazon.smithy.lsp.project.IdlFile; import software.amazon.smithy.lsp.project.Project; import software.amazon.smithy.lsp.project.ProjectAndFile; @@ -51,82 +52,121 @@ public static List getFileDiagnostics(ProjectAndFile projectAndFile, return List.of(); } - if (!(projectAndFile.file() instanceof SmithyFile smithyFile)) { - return List.of(); - } + Diagnose diagnose = switch (projectAndFile.file()) { + case SmithyFile smithyFile -> new DiagnoseSmithy(smithyFile, projectAndFile.project()); + case BuildFile buildFile -> new DiagnoseBuild(buildFile, projectAndFile.project()); + }; - Project project = projectAndFile.project(); String path = projectAndFile.file().path(); + EventToDiagnostic eventToDiagnostic = diagnose.getEventToDiagnostic(); - EventToDiagnostic eventToDiagnostic = eventToDiagnostic(smithyFile); - - List diagnostics = project.modelResult().getValidationEvents().stream() + List diagnostics = diagnose.getValidationEvents().stream() .filter(event -> event.getSeverity().compareTo(minimumSeverity) >= 0 && event.getSourceLocation().getFilename().equals(path)) .map(eventToDiagnostic::toDiagnostic) .collect(Collectors.toCollection(ArrayList::new)); - Diagnostic versionDiagnostic = versionDiagnostic(smithyFile); - if (versionDiagnostic != null) { - diagnostics.add(versionDiagnostic); - } - - if (projectAndFile.project().type() == Project.Type.DETACHED) { - diagnostics.add(detachedDiagnostic(smithyFile)); - } + diagnose.addExtraDiagnostics(diagnostics); return diagnostics; } - private static Diagnostic versionDiagnostic(SmithyFile smithyFile) { - if (!(smithyFile instanceof IdlFile idlFile)) { - return null; + private sealed interface Diagnose { + List getValidationEvents(); + + EventToDiagnostic getEventToDiagnostic(); + + void addExtraDiagnostics(List diagnostics); + } + + private record DiagnoseSmithy(SmithyFile smithyFile, Project project) implements Diagnose { + @Override + public List getValidationEvents() { + return project.modelResult().getValidationEvents(); } - Syntax.IdlParseResult syntaxInfo = idlFile.getParse(); - if (syntaxInfo.version().version().startsWith("2")) { - return null; - } else if (!LspAdapter.isEmpty(syntaxInfo.version().range())) { - var diagnostic = createDiagnostic( - syntaxInfo.version().range(), "You can upgrade to idl version 2.", UPDATE_VERSION); - diagnostic.setCodeDescription(UPDATE_VERSION_DESCRIPTION); - return diagnostic; - } else { - int end = smithyFile.document().lineEnd(0); - Range range = LspAdapter.lineSpan(0, 0, end); - return createDiagnostic(range, "You should define a version for your Smithy file", DEFINE_VERSION); + @Override + public EventToDiagnostic getEventToDiagnostic() { + if (!(smithyFile instanceof IdlFile idlFile)) { + return new Simple(); + } + + var idlParse = idlFile.getParse(); + var view = StatementView.createAtStart(idlParse).orElse(null); + if (view == null) { + return new Simple(); + } else { + var documentParser = DocumentParser.forStatements( + smithyFile.document(), view.parseResult().statements()); + return new Idl(view, documentParser); + } } - } - private static Diagnostic detachedDiagnostic(SmithyFile smithyFile) { - Range range; - if (smithyFile.document() == null) { - range = LspAdapter.origin(); - } else { - int end = smithyFile.document().lineEnd(0); - range = LspAdapter.lineSpan(0, 0, end); + @Override + public void addExtraDiagnostics(List diagnostics) { + Diagnostic versionDiagnostic = versionDiagnostic(smithyFile); + if (versionDiagnostic != null) { + diagnostics.add(versionDiagnostic); + } + + if (project.type() == Project.Type.DETACHED) { + diagnostics.add(detachedDiagnostic(smithyFile)); + } } - return createDiagnostic(range, "This file isn't attached to a project", DETACHED_FILE); - } - private static Diagnostic createDiagnostic(Range range, String title, String code) { - return new Diagnostic(range, title, DiagnosticSeverity.Warning, "smithy-language-server", code); + private static Diagnostic versionDiagnostic(SmithyFile smithyFile) { + if (!(smithyFile instanceof IdlFile idlFile)) { + return null; + } + + Syntax.IdlParseResult syntaxInfo = idlFile.getParse(); + if (syntaxInfo.version().version().startsWith("2")) { + return null; + } else if (!LspAdapter.isEmpty(syntaxInfo.version().range())) { + var diagnostic = createDiagnostic( + syntaxInfo.version().range(), "You can upgrade to idl version 2.", UPDATE_VERSION); + diagnostic.setCodeDescription(UPDATE_VERSION_DESCRIPTION); + return diagnostic; + } else { + int end = smithyFile.document().lineEnd(0); + Range range = LspAdapter.lineSpan(0, 0, end); + return createDiagnostic(range, "You should define a version for your Smithy file", DEFINE_VERSION); + } + } + + private static Diagnostic detachedDiagnostic(SmithyFile smithyFile) { + Range range; + if (smithyFile.document() == null) { + range = LspAdapter.origin(); + } else { + int end = smithyFile.document().lineEnd(0); + range = LspAdapter.lineSpan(0, 0, end); + } + + return createDiagnostic(range, "This file isn't attached to a project", DETACHED_FILE); + } } - private static EventToDiagnostic eventToDiagnostic(SmithyFile smithyFile) { - if (!(smithyFile instanceof IdlFile idlFile)) { - return new Simple(); + private record DiagnoseBuild(BuildFile buildFile, Project project) implements Diagnose { + @Override + public List getValidationEvents() { + return project().configEvents(); } - var idlParse = idlFile.getParse(); - var view = StatementView.createAtStart(idlParse).orElse(null); - if (view == null) { + @Override + public EventToDiagnostic getEventToDiagnostic() { return new Simple(); - } else { - var documentParser = DocumentParser.forStatements(smithyFile.document(), view.parseResult().statements()); - return new Idl(view, documentParser); } + + @Override + public void addExtraDiagnostics(List diagnostics) { + // TODO: Add warning diagnostic to build ext files + } + } + + private static Diagnostic createDiagnostic(Range range, String title, String code) { + return new Diagnostic(range, title, DiagnosticSeverity.Warning, "smithy-language-server", code); } private sealed interface EventToDiagnostic { diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java index d2374302..2eb67e3c 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java @@ -5,7 +5,9 @@ package software.amazon.smithy.lsp.project; +import java.util.concurrent.locks.ReentrantLock; import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.lsp.syntax.Syntax; /** * The language server's representation of a smithy-build.json @@ -14,10 +16,25 @@ public final class BuildFile implements ProjectFile { private final String path; private final Document document; + private final BuildFileType type; + private final ReentrantLock parseLock = new ReentrantLock(); + private Syntax.NodeParseResult parseResult; - BuildFile(String path, Document document) { + private BuildFile( + String path, + Document document, + BuildFileType type, + Syntax.NodeParseResult parseResult + ) { this.path = path; this.document = document; + this.type = type; + this.parseResult = parseResult; + } + + static BuildFile create(String path, Document document, BuildFileType type) { + Syntax.NodeParseResult parseResult = Syntax.parseNode(document); + return new BuildFile(path, document, type, parseResult); } @Override @@ -29,4 +46,34 @@ public String path() { public Document document() { return document; } + + @Override + public void reparse() { + Syntax.NodeParseResult updatedParse = Syntax.parseNode(document()); + + parseLock.lock(); + try { + this.parseResult = updatedParse; + } finally { + parseLock.unlock(); + } + } + + public BuildFileType type() { + return type; + } + + /** + * @return The latest computed {@link Syntax.NodeParseResult} of this build file + * @apiNote Don't call this method over and over. {@link Syntax.NodeParseResult} + * is immutable so just call this once and use the returned value. + */ + public Syntax.NodeParseResult getParse() { + parseLock.lock(); + try { + return parseResult; + } finally { + parseLock.unlock(); + } + } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java new file mode 100644 index 00000000..c8f570ae --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java @@ -0,0 +1,38 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.lsp.project; + +import java.io.File; +import java.util.Arrays; +import java.util.List; + +public enum BuildFileType { + SMITHY_BUILD("smithy-build.json"), + SMITHY_BUILD_EXT_0("build" + File.separator + "smithy-projectDependencies.json"), + SMITHY_BUILD_EXT_1(".smithy.json"), + SMITHY_PROJECT(".smithy-project.json"),; + + public static final List ALL_FILENAMES = Arrays.stream(BuildFileType.values()) + .map(BuildFileType::filename) + .toList(); + + private final String filename; + + BuildFileType(String filename) { + this.filename = filename; + } + + public String filename() { + return filename; + } + + boolean supportsMavenConfiguration() { + return switch (this) { + case SMITHY_BUILD, SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> true; + default -> false; + }; + } +} diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java new file mode 100644 index 00000000..f8075ed1 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java @@ -0,0 +1,91 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.lsp.project; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import software.amazon.smithy.lsp.ManagedFiles; +import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.lsp.protocol.LspAdapter; +import software.amazon.smithy.utils.IoUtils; + +final class BuildFiles implements Iterable { + private final Map buildFiles; + + private BuildFiles(Map buildFiles) { + this.buildFiles = buildFiles; + } + + @Override + public Iterator iterator() { + return buildFiles.values().iterator(); + } + + BuildFile getByPath(String path) { + return buildFiles.get(path); + } + + BuildFile getByType(BuildFileType type) { + for (BuildFile buildFile : buildFiles.values()) { + if (buildFile.type() == type) { + return buildFile; + } + } + return null; + } + + boolean isEmpty() { + return buildFiles.isEmpty(); + } + + static BuildFiles empty() { + return new BuildFiles(new HashMap<>()); + } + + static BuildFiles of(Collection buildFiles) { + Map buildFileMap = new HashMap<>(buildFiles.size()); + for (BuildFile buildFile : buildFiles) { + buildFileMap.put(buildFile.path(), buildFile); + } + return new BuildFiles(buildFileMap); + } + + static BuildFiles load(Path root, ManagedFiles managedFiles) { + Map buildFiles = new HashMap<>(BuildFileType.values().length); + for (BuildFileType type : BuildFileType.values()) { + BuildFile buildFile = readBuildFile(type, root, managedFiles); + if (buildFile != null) { + buildFiles.put(buildFile.path(), buildFile); + } + } + return new BuildFiles(buildFiles); + } + + private static BuildFile readBuildFile( + BuildFileType type, + Path workspaceRoot, + ManagedFiles managedFiles + ) { + Path buildFilePath = workspaceRoot.resolve(type.filename()); + if (!Files.isRegularFile(buildFilePath)) { + return null; + } + + String pathString = buildFilePath.toString(); + String uri = LspAdapter.toUri(pathString); + Document document = managedFiles.getManagedDocument(uri); + if (document == null) { + // Note: This shouldn't fail since we checked for the file's existence + document = Document.of(IoUtils.readUtf8File(pathString)); + } + + return BuildFile.create(pathString, document, type); + } +} 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 e70287d9..f5a0e720 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/Project.java +++ b/src/main/java/software/amazon/smithy/lsp/project/Project.java @@ -30,6 +30,7 @@ import software.amazon.smithy.model.shapes.ToShapeId; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.validation.ValidatedResult; +import software.amazon.smithy.model.validation.ValidationEvent; import software.amazon.smithy.utils.IoUtils; /** @@ -41,7 +42,6 @@ public final class Project { private final Path root; private final ProjectConfig config; - private final List dependencies; private final Map smithyFiles; private final Supplier assemblerFactory; private final Type type; @@ -51,7 +51,6 @@ public final class Project { Project( Path root, ProjectConfig config, - List dependencies, Map smithyFiles, Supplier assemblerFactory, Type type, @@ -60,7 +59,6 @@ public final class Project { ) { this.root = root; this.config = config; - this.dependencies = dependencies; this.smithyFiles = smithyFiles; this.assemblerFactory = assemblerFactory; this.type = type; @@ -97,7 +95,6 @@ public enum Type { public static Project empty(Path root) { return new Project(root, ProjectConfig.empty(), - List.of(), new HashMap<>(), Model::assembler, Type.EMPTY, @@ -112,10 +109,14 @@ public Path root() { return root; } - public ProjectConfig config() { + ProjectConfig config() { return config; } + public List configEvents() { + return config.events(); + } + /** * @return The paths of all Smithy sources specified * in this project's smithy build configuration files, @@ -140,13 +141,6 @@ public List imports() { .collect(Collectors.toList()); } - /** - * @return The paths of all resolved dependencies - */ - public List dependencies() { - return dependencies; - } - /** * @return The paths of all Smithy files loaded in the project. */ @@ -184,7 +178,11 @@ public ProjectFile getProjectFile(String uri) { return smithyFile; } - return config.buildFiles().get(path); + return config.buildFiles().getByPath(path); + } + + public void validateConfig() { + this.config.validate(); } /** @@ -215,6 +213,8 @@ public void updateAndValidateModel(String uri) { */ public void updateFiles(Set addUris, Set removeUris) { updateFiles(addUris, removeUris, Collections.emptySet(), true); + // Config has to be re-validated because it may be reporting missing files + validateConfig(); } /** @@ -227,7 +227,7 @@ public void updateFiles(Set addUris, Set removeUris) { * @param changeUris URIs of files that changed * @param validate Whether to run model validation. */ - public void updateFiles(Set addUris, Set removeUris, Set changeUris, boolean validate) { + private void updateFiles(Set addUris, Set removeUris, Set changeUris, boolean validate) { if (modelResult.getResult().isEmpty()) { // 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 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 7a07f6eb..c3bc7a59 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java @@ -5,165 +5,123 @@ package software.amazon.smithy.lsp.project; -import java.util.ArrayList; -import java.util.HashMap; +import java.net.URL; +import java.nio.file.Path; import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; +import java.util.concurrent.locks.ReentrantLock; import software.amazon.smithy.build.model.MavenConfig; -import software.amazon.smithy.model.node.Node; -import software.amazon.smithy.model.node.ObjectNode; -import software.amazon.smithy.model.node.StringNode; +import software.amazon.smithy.model.validation.ValidationEvent; /** * A complete view of all a project's configuration that is needed to load it, * merged from all configuration sources. */ -public final class ProjectConfig { +final class ProjectConfig { + private static final MavenConfig DEFAULT_MAVEN = MavenConfig.builder().build(); + private final List sources; private final List imports; - private final String outputDirectory; - private final List dependencies; - private final MavenConfig mavenConfig; - private final Map buildFiles; - - private ProjectConfig(Builder builder) { - this.sources = builder.sources; - this.imports = builder.imports; - this.outputDirectory = builder.outputDirectory; - this.dependencies = builder.dependencies; - this.mavenConfig = builder.mavenConfig; - this.buildFiles = builder.buildFiles; + private final List projectDependencies; + private final MavenConfig maven; + private final BuildFiles buildFiles; + private final List modelPaths; + private final List resolvedDependencies; + private final ReentrantLock eventsLock = new ReentrantLock(); + private List events; + + ProjectConfig( + List sources, + List imports, + List projectDependencies, + MavenConfig maven, + BuildFiles buildFiles + ) { + this.sources = sources; + this.imports = imports; + this.projectDependencies = projectDependencies; + this.maven = maven == null ? DEFAULT_MAVEN : maven; + this.buildFiles = buildFiles; + this.modelPaths = List.of(); + this.resolvedDependencies = List.of(); + this.events = List.of(); + } + + ProjectConfig( + ProjectConfig config, + List modelPaths, + List resolvedDependencies, + List events + ) { + this.sources = config.sources; + this.imports = config.imports; + this.projectDependencies = config.projectDependencies; + this.maven = config.maven; + this.buildFiles = config.buildFiles; + this.modelPaths = modelPaths; + this.resolvedDependencies = resolvedDependencies; + this.events = events; } static ProjectConfig empty() { - return builder().build(); + return new ProjectConfig( + List.of(), List.of(), List.of(), DEFAULT_MAVEN, BuildFiles.empty() + ); } - static Builder builder() { - return new Builder(); + static ProjectConfig detachedConfig(Path modelPath) { + return new ProjectConfig( + empty(), + List.of(modelPath), + List.of(), + List.of() + ); } - /** - * @return All explicitly configured sources - */ - public List sources() { + List sources() { return sources; } - /** - * @return All explicitly configured imports - */ - public List imports() { + List imports() { return imports; } - /** - * @return The configured output directory, if one is present - */ - public Optional outputDirectory() { - return Optional.ofNullable(outputDirectory); + List projectDependencies() { + return projectDependencies; } - /** - * @return All configured external (non-maven) dependencies - */ - public List dependencies() { - return dependencies; + MavenConfig maven() { + return maven; } - /** - * @return The Maven configuration, if present - */ - public Optional maven() { - return Optional.ofNullable(mavenConfig); - } - - /** - * @return Map of path to each {@link BuildFile} loaded in the project - */ - public Map buildFiles() { + BuildFiles buildFiles() { return buildFiles; } - static final class Builder { - final List sources = new ArrayList<>(); - final List imports = new ArrayList<>(); - String outputDirectory; - final List dependencies = new ArrayList<>(); - MavenConfig mavenConfig; - private final Map buildFiles = new HashMap<>(); - - private Builder() { - } - - static Builder load(BuildFile buildFile) { - Node node = Node.parseJsonWithComments(buildFile.document().copyText(), buildFile.path()); - ObjectNode objectNode = node.expectObjectNode(); - ProjectConfig.Builder projectConfigBuilder = ProjectConfig.builder(); - objectNode.getArrayMember("sources").ifPresent(arrayNode -> - projectConfigBuilder.sources(arrayNode.getElementsAs(StringNode.class).stream() - .map(StringNode::getValue) - .collect(Collectors.toList()))); - objectNode.getArrayMember("imports").ifPresent(arrayNode -> - projectConfigBuilder.imports(arrayNode.getElementsAs(StringNode.class).stream() - .map(StringNode::getValue) - .collect(Collectors.toList()))); - objectNode.getStringMember("outputDirectory").ifPresent(stringNode -> - projectConfigBuilder.outputDirectory(stringNode.getValue())); - objectNode.getArrayMember("dependencies").ifPresent(arrayNode -> - projectConfigBuilder.dependencies(arrayNode.getElements().stream() - .map(ProjectDependency::fromNode) - .collect(Collectors.toList()))); - return projectConfigBuilder; - } - - public Builder sources(List sources) { - this.sources.clear(); - this.sources.addAll(sources); - return this; - } - - public Builder addSources(List sources) { - this.sources.addAll(sources); - return this; - } - - public Builder imports(List imports) { - this.imports.clear(); - this.imports.addAll(imports); - return this; - } - - public Builder addImports(List imports) { - this.imports.addAll(imports); - return this; - } - - public Builder outputDirectory(String outputDirectory) { - this.outputDirectory = outputDirectory; - return this; - } + List modelPaths() { + return modelPaths; + } - public Builder dependencies(List dependencies) { - this.dependencies.clear(); - this.dependencies.addAll(dependencies); - return this; - } + List resolvedDependencies() { + return resolvedDependencies; + } - public Builder mavenConfig(MavenConfig mavenConfig) { - this.mavenConfig = mavenConfig; - return this; + List events() { + eventsLock.lock(); + try { + return events; + } finally { + eventsLock.unlock(); } + } - public Builder buildFiles(Map buildFiles) { - this.buildFiles.putAll(buildFiles); - return this; - } + void validate() { + var updatedEvents = ProjectConfigLoader.validateBuildFiles(buildFiles); - public ProjectConfig build() { - return new ProjectConfig(this); + eventsLock.lock(); + try { + this.events = updatedEvents; + } finally { + eventsLock.unlock(); } } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java index 05e0aeda..008d9811 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java @@ -6,172 +6,552 @@ package software.amazon.smithy.lsp.project; import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.logging.Logger; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Supplier; +import java.util.stream.Stream; +import software.amazon.smithy.build.model.MavenConfig; +import software.amazon.smithy.build.model.MavenRepository; import software.amazon.smithy.build.model.SmithyBuildConfig; -import software.amazon.smithy.lsp.ManagedFiles; -import software.amazon.smithy.lsp.document.Document; -import software.amazon.smithy.lsp.protocol.LspAdapter; -import software.amazon.smithy.lsp.util.Result; +import software.amazon.smithy.cli.EnvironmentVariable; +import software.amazon.smithy.cli.dependencies.DependencyResolver; +import software.amazon.smithy.cli.dependencies.DependencyResolverException; +import software.amazon.smithy.cli.dependencies.MavenDependencyResolver; +import software.amazon.smithy.cli.dependencies.ResolvedArtifact; +import software.amazon.smithy.model.SourceException; +import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.loader.ModelDiscovery; +import software.amazon.smithy.model.node.ArrayNode; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.node.ObjectNode; -import software.amazon.smithy.utils.IoUtils; +import software.amazon.smithy.model.node.StringNode; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidationEvent; /** - * Loads {@link ProjectConfig}s from a given root directory - * - *

This aggregates configuration from multiple sources, including - * {@link ProjectConfigLoader#SMITHY_BUILD}, - * {@link ProjectConfigLoader#SMITHY_BUILD_EXTS}, and - * {@link ProjectConfigLoader#SMITHY_PROJECT}. Each of these are looked - * for in the project root directory. If none are found, an empty smithy-build - * is assumed. Any exceptions that occur are aggregated and will fail the load. - * - *

Aggregation is done as follows: - *

    - *
  1. - * Start with an empty {@link SmithyBuildConfig.Builder}. This will - * aggregate {@link SmithyBuildConfig} and {@link SmithyBuildExtensions} - *
    2. - *
  2. - * If a smithy-build.json exists, try to load it. If one doesn't exist, - * use an empty {@link SmithyBuildConfig} (with version "1"). Merge the result - * into the builder - *
    3. - *
  3. - * If any {@link ProjectConfigLoader#SMITHY_BUILD_EXTS} exist, try to load - * and merge them into a single {@link SmithyBuildExtensions.Builder} - *
    4. - *
  4. - * If a {@link ProjectConfigLoader#SMITHY_PROJECT} exists, try to load it. - * Otherwise use an empty {@link ProjectConfig.Builder}. This will be the - * result of the load - *
    5. - *
  5. - * Merge any {@link ProjectConfigLoader#SMITHY_BUILD_EXTS} into the original - * {@link SmithyBuildConfig.Builder} and build it - *
    6. - *
  6. - * Add all sources, imports, and MavenConfig from the {@link SmithyBuildConfig} - * to the {@link ProjectConfig.Builder} - *
    7. - *
  7. - * If the {@link ProjectConfig.Builder} doesn't specify an outputDirectory, - * use the one in {@link SmithyBuildConfig}, if present - *
    8. - *
+ * Loads and validates {@link ProjectConfig}s from {@link BuildFiles}. */ -public final class ProjectConfigLoader { - public static final String SMITHY_BUILD = "smithy-build.json"; - public static final String[] SMITHY_BUILD_EXTS = { - "build" + File.separator + "smithy-dependencies.json", ".smithy.json"}; - public static final String SMITHY_PROJECT = ".smithy-project.json"; - public static final List PROJECT_BUILD_FILES = new ArrayList<>(2 + SMITHY_BUILD_EXTS.length); - - private static final Logger LOGGER = Logger.getLogger(ProjectConfigLoader.class.getName()); - private static final SmithyBuildConfig DEFAULT_SMITHY_BUILD = SmithyBuildConfig.builder().version("1").build(); - private static final NodeMapper NODE_MAPPER = new NodeMapper(); +final class ProjectConfigLoader { + private interface BuildFileValidator { + T validate(BuildFile source, Node node, Consumer eventConsumer); + } + + private record SmithyBuildJsonValidator() implements BuildFileValidator { + @Override + public SmithyBuildConfig validate(BuildFile source, Node node, Consumer eventConsumer) { + try { + return SmithyBuildConfig.fromNode(node); + } catch (Exception e) { + eventConsumer.accept(toEvent(e, source)); + } + return null; + } + } + + private record SmithyBuildExtValidator() implements BuildFileValidator { + @Override + public SmithyBuildExtensions validate(BuildFile source, Node node, Consumer eventConsumer) { + try { + var config = NODE_MAPPER.deserialize(node, SmithyBuildExtensions.class); + config.mergeMavenFromSmithyBuildConfig(SmithyBuildConfig.fromNode(node)); + return config; + } catch (Exception e) { + eventConsumer.accept(toEvent(e, source)); + } + return null; + } + } - static { - PROJECT_BUILD_FILES.add(SMITHY_BUILD); - PROJECT_BUILD_FILES.add(SMITHY_PROJECT); - PROJECT_BUILD_FILES.addAll(Arrays.asList(SMITHY_BUILD_EXTS)); + private record SmithyProjectJsonValidator() implements BuildFileValidator { + @Override + public SmithyProjectJson validate(BuildFile source, Node node, Consumer eventConsumer) { + try { + return SmithyProjectJson.fromNode(node); + } catch (Exception e) { + eventConsumer.accept(toEvent(e, source)); + } + return null; + } } - private ProjectConfigLoader() { + private static final NodeMapper NODE_MAPPER = new NodeMapper(); + private static final BuildFileType[] EXTS = {BuildFileType.SMITHY_BUILD_EXT_0, BuildFileType.SMITHY_BUILD_EXT_1}; + // Taken from smithy-cli ConfigurationUtils + private static final Supplier CENTRAL = () -> MavenRepository.builder() + .url("https://repo.maven.apache.org/maven2") + .build(); + private static final Supplier DEFAULT_RESOLVER_FACTORY = () -> + new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); + + private final Path root; + private final BuildFiles buildFiles; + private final List events = new ArrayList<>(); + private final Map smithyNodes = new HashMap<>(BuildFileType.values().length); + private final Supplier dependencyResolverFactory; + + ProjectConfigLoader(Path root, BuildFiles buildFiles, Supplier dependencyResolverFactory) { + this.root = root; + this.buildFiles = buildFiles; + this.dependencyResolverFactory = dependencyResolverFactory; } - static Result> loadFromRoot(Path workspaceRoot, ManagedFiles managedFiles) { - SmithyBuildConfig.Builder builder = SmithyBuildConfig.builder(); - List exceptions = new ArrayList<>(); - Map buildFiles = new HashMap<>(); + ProjectConfigLoader(Path root, BuildFiles buildFiles) { + this(root, buildFiles, DEFAULT_RESOLVER_FACTORY); + } - Path smithyBuildPath = workspaceRoot.resolve(SMITHY_BUILD); - if (Files.isRegularFile(smithyBuildPath)) { - LOGGER.info("Loading smithy-build.json from " + smithyBuildPath); - Result result = Result.ofFallible(() -> { - BuildFile buildFile = addBuildFile(buildFiles, smithyBuildPath, managedFiles); - return SmithyBuildConfig.fromNode( - Node.parseJsonWithComments(buildFile.document().copyText(), buildFile.path())); + static List validateBuildFiles(BuildFiles buildFiles) { + List events = new ArrayList<>(); + for (BuildFile buildFile : buildFiles) { + BuildFileValidator validator = switch (buildFile.type()) { + case SMITHY_BUILD -> new SmithyBuildJsonValidator(); + case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> new SmithyBuildExtValidator(); + case SMITHY_PROJECT -> new SmithyProjectJsonValidator(); + }; + loadFile(buildFile, validator, events::add, (ignored) -> { }); - result.get().ifPresent(builder::merge); - result.getErr().ifPresent(exceptions::add); + } + return events; + } + + ProjectConfig load() { + SmithyBuildConfig smithyBuildConfig = loadFile(BuildFileType.SMITHY_BUILD, new SmithyBuildJsonValidator()); + + SmithyBuildExtensions.Builder extBuilder = null; + for (BuildFileType extType : EXTS) { + SmithyBuildExtensions ext = loadFile(extType, new SmithyBuildExtValidator()); + if (ext != null) { + if (extBuilder == null) { + extBuilder = SmithyBuildExtensions.builder(); + } + extBuilder.merge(ext); + } + } + + SmithyBuildConfig merged = mergeSmithyBuildConfig(smithyBuildConfig, extBuilder); + SmithyProjectJson smithyProjectJson = loadFile(BuildFileType.SMITHY_PROJECT, new SmithyProjectJsonValidator()); + + ProjectConfig partial = createConfig(merged, smithyProjectJson); + var resolveResult = resolve(partial); + + return new ProjectConfig( + partial, + resolveResult.modelPaths(), + resolveResult.resolvedDependencies(), + resolveResult.events() + ); + } + + private T loadFile(BuildFileType type, BuildFileValidator validator) { + var buildFile = buildFiles.getByType(type); + if (buildFile != null) { + return loadFile(buildFile, validator, events::add, (node) -> smithyNodes.put(type, node)); + } + return null; + } + + private static T loadFile( + BuildFile buildFile, + BuildFileValidator validator, + Consumer eventConsumer, + Consumer nodeConsumer + ) { + var nodeResult = ToSmithyNode.toSmithyNode(buildFile); + nodeResult.getValidationEvents().forEach(eventConsumer); + var smithyNode = nodeResult.getResult().orElse(null); + if (smithyNode != null) { + nodeConsumer.accept(smithyNode); + return validator.validate(buildFile, smithyNode, eventConsumer); + } + return null; + } + + private SmithyBuildConfig mergeSmithyBuildConfig( + SmithyBuildConfig smithyBuildConfig, + SmithyBuildExtensions.Builder extBuilder + ) { + if (smithyBuildConfig == null && extBuilder == null) { + return null; + } else if (extBuilder == null) { + return smithyBuildConfig; + } else if (smithyBuildConfig == null) { + try { + return extBuilder.build().asSmithyBuildConfig(); + } catch (Exception e) { + // Add the event to any ext file + for (BuildFileType ext : EXTS) { + BuildFile buildFile = buildFiles.getByType(ext); + if (buildFile != null) { + events.add(toEvent(e, buildFile)); + break; + } + } + } } else { - LOGGER.info("No smithy-build.json found at " + smithyBuildPath + ", defaulting to empty config."); - builder.merge(DEFAULT_SMITHY_BUILD); - } - - SmithyBuildExtensions.Builder extensionsBuilder = SmithyBuildExtensions.builder(); - for (String ext : SMITHY_BUILD_EXTS) { - Path extPath = workspaceRoot.resolve(ext); - if (Files.isRegularFile(extPath)) { - Result result = Result.ofFallible(() -> { - BuildFile buildFile = addBuildFile(buildFiles, extPath, managedFiles); - return loadSmithyBuildExtensions(buildFile); - }); - result.get().ifPresent(extensionsBuilder::merge); - result.getErr().ifPresent(exceptions::add); - } - } - - ProjectConfig.Builder finalConfigBuilder = ProjectConfig.builder(); - Path smithyProjectPath = workspaceRoot.resolve(SMITHY_PROJECT); - if (Files.isRegularFile(smithyProjectPath)) { - LOGGER.info("Loading .smithy-project.json from " + smithyProjectPath); - Result result = Result.ofFallible(() -> { - BuildFile buildFile = addBuildFile(buildFiles, smithyProjectPath, managedFiles); - return ProjectConfig.Builder.load(buildFile); - }); - if (result.isOk()) { - finalConfigBuilder = result.unwrap(); + try { + var extConfig = extBuilder.build().asSmithyBuildConfig(); + return smithyBuildConfig.toBuilder().merge(extConfig).build(); + } catch (Exception e) { + // Add the event to either smithy-build.json, or an ext file + for (BuildFile buildFile : buildFiles) { + if (buildFile.type().supportsMavenConfiguration()) { + events.add(toEvent(e, buildFile)); + break; + } + } + } + } + + return null; + } + + private static ValidationEvent toEvent(Exception e, BuildFile fallbackBuildFile) { + SourceException asSourceException = null; + if (e instanceof SourceException sourceException) { + asSourceException = sourceException; + } else if (e.getCause() instanceof SourceException sourceException) { + asSourceException = sourceException; + } + + if (asSourceException != null && !SourceLocation.NONE.equals(asSourceException.getSourceLocation())) { + return ValidationEvent.fromSourceException(asSourceException); + } + + return ValidationEvent.builder() + .id("SmithyBuildConfig") + .severity(Severity.ERROR) + .message(e.getMessage()) + .sourceLocation(new SourceLocation(fallbackBuildFile.path(), 1, 1)) + .build(); + } + + private ProjectConfig createConfig(SmithyBuildConfig smithyBuildConfig, SmithyProjectJson smithyProjectJson) { + // TODO: Make this more efficient with right-sized lists + List sources = new ArrayList<>(); + List imports = new ArrayList<>(); + List projectDependencies = new ArrayList<>(); + MavenConfig mavenConfig = null; + + if (smithyBuildConfig != null) { + sources.addAll(smithyBuildConfig.getSources()); + imports.addAll(smithyBuildConfig.getImports()); + var mavenOpt = smithyBuildConfig.getMaven(); + if (mavenOpt.isPresent()) { + mavenConfig = mavenOpt.get(); + } + } + + if (smithyProjectJson != null) { + sources.addAll(smithyProjectJson.sources()); + imports.addAll(smithyProjectJson.imports()); + projectDependencies.addAll(smithyProjectJson.dependencies()); + } + + return new ProjectConfig( + sources, + imports, + projectDependencies, + mavenConfig, + buildFiles + ); + } + + private ResolveResult resolve(ProjectConfig config) { + Set mavenDependencies = resolveMaven(config.maven()); + Set projectDependencies = resolveProjectDependencies(config.projectDependencies()); + + List resolvedDependencies = new ArrayList<>(); + try { + for (var dep : mavenDependencies) { + resolvedDependencies.add(dep.toUri().toURL()); + } + for (var dep : projectDependencies) { + resolvedDependencies.add(dep.toUri().toURL()); + } + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + + Set uniqueModelPaths = collectAllSmithyFilePaths(config.sources(), config.imports()); + List modelPaths = new ArrayList<>(uniqueModelPaths); + + return new ResolveResult(modelPaths, resolvedDependencies, events); + } + + private Set resolveMaven(MavenConfig maven) { + if (maven.getRepositories().isEmpty() && maven.getDependencies().isEmpty()) { + return Set.of(); + } + + List exceptions = new ArrayList<>(); + DependencyResolver resolver = dependencyResolverFactory.get(); + + Set repositories = getConfiguredMavenRepos(maven); + for (MavenRepository repo : repositories) { + try { + resolver.addRepository(repo); + } catch (DependencyResolverException e) { + exceptions.add(e); + } + } + + for (String dependency : maven.getDependencies()) { + try { + resolver.addDependency(dependency); + } catch (DependencyResolverException e) { + exceptions.add(e); + } + } + + List resolvedArtifacts; + try { + resolvedArtifacts = resolver.resolve(); + } catch (DependencyResolverException e) { + exceptions.add(e); + resolvedArtifacts = List.of(); + } + + handleDependencyResolverExceptions(exceptions); + + Set dependencyPaths = new HashSet<>(resolvedArtifacts.size()); + for (ResolvedArtifact resolvedArtifact : resolvedArtifacts) { + Path path = resolvedArtifact.getPath().toAbsolutePath(); + if (!Files.exists(path)) { + throw new RuntimeException( + "Dependency was resolved (" + resolvedArtifact + "), but it was not found on disk at " + path); + } + dependencyPaths.add(path); + } + + return dependencyPaths; + } + + private void handleDependencyResolverExceptions(List exceptions) { + if (exceptions.isEmpty()) { + return; + } + + StringBuilder builder = new StringBuilder(); + for (DependencyResolverException exception : exceptions) { + builder.append(exception.getMessage()).append("\n"); + } + String message = builder.toString(); + + for (Node smithyNode : smithyNodes.values()) { + if (!(smithyNode instanceof ObjectNode objectNode)) { + continue; + } + + for (StringNode memberNameNode : objectNode.getMembers().keySet()) { + String memberName = memberNameNode.getValue(); + if ("maven".equals(memberName)) { + events.add(ValidationEvent.builder() + .id("DependencyResolver") + .severity(Severity.ERROR) + .message("Dependency resolution failed: " + message) + .sourceLocation(memberNameNode) + .build()); + break; + } + } + } + } + + private Set resolveProjectDependencies(List projectDependencies) { + Set notFoundDependencies = new HashSet<>(); + Set dependencies = new HashSet<>(); + + for (var dependency : projectDependencies) { + Path path = root.resolve(dependency.path()).normalize(); + if (!Files.exists(path)) { + notFoundDependencies.add(dependency.path()); } else { - exceptions.add(result.unwrapErr()); + dependencies.add(path); } } - if (!exceptions.isEmpty()) { - return Result.err(exceptions); + handleNotFoundProjectDependencies(notFoundDependencies); + + return dependencies; + } + + private void handleNotFoundProjectDependencies(Set notFound) { + if (notFound.isEmpty()) { + return; } - builder.merge(extensionsBuilder.build().asSmithyBuildConfig()); - SmithyBuildConfig config = builder.build(); - finalConfigBuilder.addSources(config.getSources()).addImports(config.getImports()); - config.getMaven().ifPresent(finalConfigBuilder::mavenConfig); - if (finalConfigBuilder.outputDirectory == null) { - config.getOutputDirectory().ifPresent(finalConfigBuilder::outputDirectory); + if (!(smithyNodes.get(BuildFileType.SMITHY_PROJECT) instanceof ObjectNode objectNode)) { + return; + } + + if (objectNode.getMember("dependencies").orElse(null) instanceof ArrayNode arrayNode) { + for (Node elem : arrayNode) { + if (elem instanceof ObjectNode depNode + && depNode.getMember("path").orElse(null) instanceof StringNode depPathNode + && notFound.contains(depPathNode.getValue())) { + events.add(ValidationEvent.builder() + .id("FileNotFound") + .severity(Severity.ERROR) + .message("File not found") + .sourceLocation(depPathNode) + .build()); + } + } } - finalConfigBuilder.buildFiles(buildFiles); - return Result.ok(finalConfigBuilder.build()); } - private static BuildFile addBuildFile(Map buildFiles, Path path, ManagedFiles managedFiles) { - String pathString = path.toString(); - String uri = LspAdapter.toUri(pathString); - Document managed = managedFiles.getManagedDocument(uri); - BuildFile buildFile; - if (managed != null) { - buildFile = new BuildFile(pathString, managed); - } else { - Document document = Document.of(IoUtils.readUtf8File(path)); - buildFile = new BuildFile(pathString, document); + // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos + private static Set getConfiguredMavenRepos(MavenConfig config) { + Set repositories = new LinkedHashSet<>(); + + String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); + if (envRepos != null) { + for (String repo : envRepos.split("\\|")) { + repositories.add(MavenRepository.builder().url(repo.trim()).build()); + } + } + + Set configuredRepos = config.getRepositories(); + + if (!configuredRepos.isEmpty()) { + repositories.addAll(configuredRepos); + } else if (envRepos == null) { + repositories.add(CENTRAL.get()); + } + return repositories; + } + + // sources and imports can contain directories or files, relative or absolute. + // Note: The model assembler can handle loading all smithy files in a directory, so there's some potential + // here for inconsistent behavior. + private Set collectAllSmithyFilePaths(List sources, List imports) { + Set notFound = new HashSet<>(); + Set paths = new HashSet<>(); + + collectFilePaths(paths, sources, notFound); + collectFilePaths(paths, imports, notFound); + + handleNotFoundSourcesAndImports(notFound); + + return paths; + } + + private void collectFilePaths(Set accumulator, List paths, Set notFound) { + for (String file : paths) { + Path path = root.resolve(file).normalize(); + if (!Files.exists(path)) { + notFound.add(path.toString()); + } else { + collectDirectory(accumulator, root, path); + } + } + } + + private void handleNotFoundSourcesAndImports(Set notFound) { + for (Node smithyNode : smithyNodes.values()) { + if (!(smithyNode instanceof ObjectNode objectNode)) { + continue; + } + + if (objectNode.getMember("sources").orElse(null) instanceof ArrayNode sourcesNode) { + addNotFoundEvents(sourcesNode, notFound); + } + + if (objectNode.getMember("imports").orElse(null) instanceof ArrayNode importsNode) { + addNotFoundEvents(importsNode, notFound); + } } - buildFiles.put(buildFile.path(), buildFile); - return buildFile; } - private static SmithyBuildExtensions loadSmithyBuildExtensions(BuildFile buildFile) { - // NOTE: This is the legacy way we loaded build extensions. It used to throw a checked exception. - ObjectNode node = Node.parseJsonWithComments( - buildFile.document().copyText(), buildFile.path()).expectObjectNode(); - SmithyBuildExtensions config = NODE_MAPPER.deserialize(node, SmithyBuildExtensions.class); - config.mergeMavenFromSmithyBuildConfig(SmithyBuildConfig.fromNode(node)); - return config; + private void addNotFoundEvents(ArrayNode searchNode, Set notFound) { + for (Node elem : searchNode) { + if (elem instanceof StringNode stringNode) { + String fullPath = root.resolve(stringNode.getValue()).normalize().toString(); + if (notFound.contains(fullPath)) { + events.add(ValidationEvent.builder() + .id("FileNotFound") + .severity(Severity.ERROR) + .message("File not found: " + fullPath) + .sourceLocation(stringNode) + .build()); + } + } + } } + + // All of this copied from smithy-build SourcesPlugin, except I changed the `accumulator` to + // be a Collection instead of a list. + private static void collectDirectory(Collection accumulator, Path root, Path current) { + try { + if (Files.isDirectory(current)) { + try (Stream paths = Files.list(current)) { + paths.filter(p -> !p.equals(current)) + .filter(p -> Files.isDirectory(p) || Files.isRegularFile(p)) + .forEach(p -> collectDirectory(accumulator, root, p)); + } + } else if (Files.isRegularFile(current)) { + if (current.toString().endsWith(".jar")) { + String jarRoot = root.equals(current) + ? current.toString() + : (current + File.separator); + collectJar(accumulator, jarRoot, current); + } else { + collectFile(accumulator, current); + } + } + } catch (IOException ignored) { + // For now just ignore this - the assembler would have run into the same issues + } + } + + private static void collectJar(Collection accumulator, String jarRoot, Path jarPath) { + URL manifestUrl = ModelDiscovery.createSmithyJarManifestUrl(jarPath.toString()); + + String prefix = computeJarFilePrefix(jarRoot, jarPath); + for (URL model : ModelDiscovery.findModels(manifestUrl)) { + String name = ModelDiscovery.getSmithyModelPathFromJarUrl(model); + Path target = Paths.get(prefix + name); + collectFile(accumulator, target); + } + } + + private static String computeJarFilePrefix(String jarRoot, Path jarPath) { + Path jarFilenamePath = jarPath.getFileName(); + + if (jarFilenamePath == null) { + return jarRoot; + } + + String jarFilename = jarFilenamePath.toString(); + return jarRoot + jarFilename.substring(0, jarFilename.length() - ".jar".length()) + File.separator; + } + + private static void collectFile(Collection accumulator, Path target) { + if (target == null) { + return; + } + String filename = target.toString(); + if (filename.endsWith(".smithy") || filename.endsWith(".json")) { + accumulator.add(target); + } + } + + private record ResolveResult( + List modelPaths, + List resolvedDependencies, + List events + ) {} } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectDependency.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectDependency.java deleted file mode 100644 index 0c5fd650..00000000 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectDependency.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package software.amazon.smithy.lsp.project; - -import software.amazon.smithy.model.node.Node; -import software.amazon.smithy.model.node.ObjectNode; - -/** - * An arbitrary project dependency, used to specify non-maven dependencies - * that exist locally. - * - * @param name The name of the dependency - * @param path The path of the dependency - */ -record ProjectDependency(String name, String path) { - static ProjectDependency fromNode(Node node) { - ObjectNode objectNode = node.expectObjectNode(); - String name = objectNode.expectStringMember("name").getValue(); - String path = objectNode.expectStringMember("path").getValue(); - return new ProjectDependency(name, path); - } -} diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectDependencyResolver.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectDependencyResolver.java deleted file mode 100644 index eca2ecd8..00000000 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectDependencyResolver.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package software.amazon.smithy.lsp.project; - -import java.io.File; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; -import java.util.function.Supplier; -import java.util.stream.Collectors; -import software.amazon.smithy.build.SmithyBuild; -import software.amazon.smithy.build.model.MavenConfig; -import software.amazon.smithy.build.model.MavenRepository; -import software.amazon.smithy.cli.EnvironmentVariable; -import software.amazon.smithy.cli.dependencies.DependencyResolver; -import software.amazon.smithy.cli.dependencies.MavenDependencyResolver; -import software.amazon.smithy.cli.dependencies.ResolvedArtifact; -import software.amazon.smithy.lsp.util.Result; - -/** - * Resolves all Maven dependencies and {@link ProjectDependency} for a project. - * - *

Resolving a {@link ProjectDependency} is as simple as getting its path - * relative to the project root, but is done here in order to be loaded the - * same way as Maven dependencies. - * TODO: There are some things in here taken from smithy-cli. Should figure out - * if we can expose them through smithy-cli instead of duplicating them here to - * avoid drift. - */ -final class ProjectDependencyResolver { - // Taken from smithy-cli ConfigurationUtils - private static final Supplier CENTRAL = () -> MavenRepository.builder() - .url("https://repo.maven.apache.org/maven2") - .build(); - - private ProjectDependencyResolver() { - } - - static Result, Exception> resolveDependencies(Path root, ProjectConfig config) { - return Result.ofFallible(() -> { - List deps = ProjectDependencyResolver.create(config).resolve() - .stream() - .map(ResolvedArtifact::getPath) - .collect(Collectors.toCollection(ArrayList::new)); - config.dependencies().forEach((projectDependency) -> { - // TODO: Not sure if this needs to check for existence - Path path = root.resolve(projectDependency.path()).normalize(); - deps.add(path); - }); - return deps; - }); - } - - // Taken (roughly) from smithy-cli ClasspathAction::resolveDependencies - private static DependencyResolver create(ProjectConfig config) { - // TODO: Seeing what happens if we just don't use the file cache. When we do, at least for testing, the - // server writes a classpath.json to build/smithy/ which is used by all tests, messing everything up. - DependencyResolver resolver = new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); - - Set configuredRepositories = getConfiguredMavenRepos(config); - configuredRepositories.forEach(resolver::addRepository); - - // TODO: Support lock file ? - config.maven().ifPresent(maven -> maven.getDependencies().forEach(resolver::addDependency)); - - return resolver; - } - - // TODO: If this cache file is necessary for the server's use cases, we may - // want to keep an in memory version of it so we don't write stuff to - // people's build dirs. Right now, we just don't use it at all. - // Taken (roughly) from smithy-cli ClasspathAction::getCacheFile - private static File getCacheFile(ProjectConfig config) { - return getOutputDirectory(config).resolve("classpath.json").toFile(); - } - - // Taken from smithy-cli BuildOptions::resolveOutput - private static Path getOutputDirectory(ProjectConfig config) { - return config.outputDirectory() - .map(Paths::get) - .orElseGet(SmithyBuild::getDefaultOutputDirectory); - } - - // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos - private static Set getConfiguredMavenRepos(ProjectConfig config) { - Set repositories = new LinkedHashSet<>(); - - String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); - if (envRepos != null) { - for (String repo : envRepos.split("\\|")) { - repositories.add(MavenRepository.builder().url(repo.trim()).build()); - } - } - - Set configuredRepos = config.maven() - .map(MavenConfig::getRepositories) - .orElse(Collections.emptySet()); - - if (!configuredRepos.isEmpty()) { - repositories.addAll(configuredRepos); - } else if (envRepos == null) { - repositories.add(CENTRAL.get()); - } - return repositories; - } -} diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java index 55f88ea5..5948a748 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java @@ -21,4 +21,9 @@ public sealed interface ProjectFile permits SmithyFile, BuildFile { * @return The underlying document of the file */ Document document(); + + /** + * Reparse the underlying document + */ + void reparse(); } 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 77f1d26e..461bf67c 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java @@ -5,42 +5,28 @@ package software.amazon.smithy.lsp.project; -import java.io.File; -import java.io.IOException; -import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Supplier; -import java.util.logging.Logger; -import java.util.stream.Stream; import software.amazon.smithy.lsp.ManagedFiles; import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.protocol.LspAdapter; -import software.amazon.smithy.lsp.util.Result; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.loader.ModelAssembler; -import software.amazon.smithy.model.loader.ModelDiscovery; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.utils.IoUtils; import software.amazon.smithy.utils.TriConsumer; /** * Loads {@link Project}s. - * - * TODO: There's a lot of duplicated logic and redundant code here to refactor. */ public final class ProjectLoader { - private static final Logger LOGGER = Logger.getLogger(ProjectLoader.class.getName()); - private ProjectLoader() { } @@ -56,12 +42,6 @@ private ProjectLoader() { * @return The loaded project */ public static Project loadDetached(String uri, String text) { - LOGGER.info("Loading detachedProjects project at " + uri); - - String asPath = LspAdapter.toPath(uri); - Path path = Paths.get(asPath); - List allSmithyFilePaths = List.of(path); - Document document = Document.of(text); ManagedFiles managedFiles = (fileUri) -> { if (uri.equals(fileUri)) { @@ -70,21 +50,13 @@ public static Project loadDetached(String uri, String text) { return null; }; - List dependencies = List.of(); - - LoadModelResult result; - try { - result = doLoad(managedFiles, dependencies, allSmithyFilePaths); - } catch (IOException e) { - // Note: This can't happen because we aren't doing any fallible IO, - // as only the prelude will be read from disk - throw new RuntimeException(e); - } + Path path = Paths.get(LspAdapter.toPath(uri)); + ProjectConfig config = ProjectConfig.detachedConfig(path); + LoadModelResult result = doLoad(managedFiles, config); return new Project( path, - ProjectConfig.empty(), - dependencies, + config, result.smithyFiles(), result.assemblerFactory(), Project.Type.DETACHED, @@ -109,45 +81,24 @@ public static Project loadDetached(String uri, String text) { * @param managedFiles Files managed by the server * @return Result of loading the project */ - public static Result> load(Path root, ManagedFiles managedFiles) { - Result> configResult = ProjectConfigLoader.loadFromRoot(root, managedFiles); - if (configResult.isErr()) { - return Result.err(configResult.unwrapErr()); + public static Project load(Path root, ManagedFiles managedFiles) throws Exception { + var buildFiles = BuildFiles.load(root, managedFiles); + if (buildFiles.isEmpty()) { + return Project.empty(root); } - ProjectConfig config = configResult.unwrap(); - - if (config.buildFiles().isEmpty()) { - return Result.ok(Project.empty(root)); - } - - Result, Exception> resolveResult = ProjectDependencyResolver.resolveDependencies(root, config); - if (resolveResult.isErr()) { - return Result.err(Collections.singletonList(resolveResult.unwrapErr())); - } - - List dependencies = resolveResult.unwrap(); - // Note: The model assembler can handle loading all smithy files in a directory, so there's some potential - // here for inconsistent behavior. - List allSmithyFilePaths = collectAllSmithyPaths(root, config.sources(), config.imports()); + ProjectConfig config = new ProjectConfigLoader(root, buildFiles).load(); + LoadModelResult result = doLoad(managedFiles, config); - LoadModelResult result; - try { - result = doLoad(managedFiles, dependencies, allSmithyFilePaths); - } catch (Exception e) { - return Result.err(Collections.singletonList(e)); - } - - return Result.ok(new Project( + return new Project( root, config, - dependencies, result.smithyFiles(), result.assemblerFactory(), Project.Type.NORMAL, result.modelResult(), result.rebuildIndex() - )); + ); } private record LoadModelResult( @@ -158,24 +109,15 @@ private record LoadModelResult( ) { } - private static LoadModelResult doLoad( - ManagedFiles managedFiles, - List dependencies, - List allSmithyFilePaths - ) throws IOException { + private static LoadModelResult doLoad(ManagedFiles managedFiles, ProjectConfig config) { // The model assembler factory is used to get assemblers that already have the correct // dependencies resolved for future loads - Supplier assemblerFactory = createModelAssemblerFactory(dependencies); + Supplier assemblerFactory = createModelAssemblerFactory(config.resolvedDependencies()); - Map smithyFiles = new HashMap<>(allSmithyFilePaths.size()); + Map smithyFiles = new HashMap<>(config.modelPaths().size()); - // TODO: Assembler can fail if a file is not found. We can be more intelligent about - // handling this case to allow partially loading the project, but we will need to - // collect and report the errors somehow. For now, using collectAllSmithyPaths skips - // any files that don't exist, so we're essentially side-stepping the issue by - // coincidence. ModelAssembler assembler = assemblerFactory.get(); - ValidatedResult modelResult = loadModel(managedFiles, allSmithyFilePaths, assembler, smithyFiles); + ValidatedResult modelResult = loadModel(managedFiles, config.modelPaths(), assembler, smithyFiles); Project.RebuildIndex rebuildIndex = Project.RebuildIndex.create(modelResult); addDependencySmithyFiles(managedFiles, rebuildIndex.filesToDefinedShapes().keySet(), smithyFiles); @@ -256,8 +198,7 @@ private static void findOrReadDocument( consumer.accept(filePath, text, document); } - private static Supplier createModelAssemblerFactory(List dependencies) - throws MalformedURLException { + private static Supplier createModelAssemblerFactory(List dependencies) { // We don't want the model to be broken when there are unknown traits, // because that will essentially disable language server features, so // we need to allow unknown traits for each factory. @@ -266,89 +207,10 @@ private static Supplier createModelAssemblerFactory(List d return () -> Model.assembler().putProperty(ModelAssembler.ALLOW_UNKNOWN_TRAITS, true); } - URLClassLoader classLoader = createDependenciesClassLoader(dependencies); + URL[] urls = dependencies.toArray(new URL[0]); + URLClassLoader classLoader = new URLClassLoader(urls); return () -> Model.assembler(classLoader) .discoverModels(classLoader) .putProperty(ModelAssembler.ALLOW_UNKNOWN_TRAITS, true); } - - private static URLClassLoader createDependenciesClassLoader(List dependencies) throws MalformedURLException { - // Taken (roughly) from smithy-ci IsolatedRunnable - URL[] urls = new URL[dependencies.size()]; - int i = 0; - for (Path dependency : dependencies) { - urls[i++] = dependency.toUri().toURL(); - } - return new URLClassLoader(urls); - } - - // sources and imports can contain directories or files, relative or absolute - private static List collectAllSmithyPaths(Path root, List sources, List imports) { - List paths = new ArrayList<>(); - for (String file : sources) { - Path path = root.resolve(file).normalize(); - collectDirectory(paths, root, path); - } - for (String file : imports) { - Path path = root.resolve(file).normalize(); - collectDirectory(paths, root, path); - } - return paths; - } - - // All of this copied from smithy-build SourcesPlugin - private static void collectDirectory(List accumulator, Path root, Path current) { - try { - if (Files.isDirectory(current)) { - try (Stream paths = Files.list(current)) { - paths.filter(p -> !p.equals(current)) - .filter(p -> Files.isDirectory(p) || Files.isRegularFile(p)) - .forEach(p -> collectDirectory(accumulator, root, p)); - } - } else if (Files.isRegularFile(current)) { - if (current.toString().endsWith(".jar")) { - String jarRoot = root.equals(current) - ? current.toString() - : (current + File.separator); - collectJar(accumulator, jarRoot, current); - } else { - collectFile(accumulator, current); - } - } - } catch (IOException ignored) { - // For now just ignore this - the assembler would have run into the same issues - } - } - - private static void collectJar(List accumulator, String jarRoot, Path jarPath) { - URL manifestUrl = ModelDiscovery.createSmithyJarManifestUrl(jarPath.toString()); - - String prefix = computeJarFilePrefix(jarRoot, jarPath); - for (URL model : ModelDiscovery.findModels(manifestUrl)) { - String name = ModelDiscovery.getSmithyModelPathFromJarUrl(model); - Path target = Paths.get(prefix + name); - collectFile(accumulator, target); - } - } - - private static String computeJarFilePrefix(String jarRoot, Path jarPath) { - Path jarFilenamePath = jarPath.getFileName(); - - if (jarFilenamePath == null) { - return jarRoot; - } - - String jarFilename = jarFilenamePath.toString(); - return jarRoot + jarFilename.substring(0, jarFilename.length() - ".jar".length()) + File.separator; - } - - private static void collectFile(List accumulator, Path target) { - if (target == null) { - return; - } - String filename = target.toString(); - if (filename.endsWith(".smithy") || filename.endsWith(".json")) { - accumulator.add(target); - } - } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/SmithyProjectJson.java b/src/main/java/software/amazon/smithy/lsp/project/SmithyProjectJson.java new file mode 100644 index 00000000..3474f7b8 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/project/SmithyProjectJson.java @@ -0,0 +1,64 @@ +/* + * 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.List; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.node.StringNode; + +record SmithyProjectJson( + List sources, + List imports, + List dependencies, + String outputDirectory +) { + static SmithyProjectJson empty() { + return new SmithyProjectJson(List.of(), List.of(), List.of(), null); + } + + static SmithyProjectJson fromNode(Node node) { + ObjectNode objectNode = node.expectObjectNode(); + + List sources = objectNode.getArrayMember("sources") + .map(arrayNode -> arrayNode.getElementsAs(StringNode.class).stream() + .map(StringNode::getValue) + .toList()) + .orElse(List.of()); + + List imports = objectNode.getArrayMember("imports") + .map(arrayNode -> arrayNode.getElementsAs(StringNode.class).stream() + .map(StringNode::getValue) + .toList()) + .orElse(List.of()); + + List dependencies = objectNode.getArrayMember("dependencies") + .map(arrayNode -> arrayNode.getElements().stream() + .map(ProjectDependency::fromNode) + .toList()) + .orElse(List.of()); + + String outputDirectory = objectNode.getStringMemberOrDefault("outputDirectory", null); + + return new SmithyProjectJson(sources, imports, dependencies, outputDirectory); + } + + /** + * An arbitrary project dependency, used to specify non-maven projectDependencies + * that exist locally. + * + * @param name The name of the dependency + * @param path The path of the dependency + */ + record ProjectDependency(String name, String path) { + static ProjectDependency fromNode(Node node) { + ObjectNode objectNode = node.expectObjectNode(); + String name = objectNode.expectStringMember("name").getValue(); + String path = objectNode.expectStringMember("path").getValue(); + return new ProjectDependency(name, path); + } + } +} diff --git a/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java new file mode 100644 index 00000000..f711dd64 --- /dev/null +++ b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java @@ -0,0 +1,97 @@ +/* + * 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.List; +import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.lsp.protocol.LspAdapter; +import software.amazon.smithy.lsp.syntax.Syntax; +import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.node.ArrayNode; +import software.amazon.smithy.model.node.BooleanNode; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.NullNode; +import software.amazon.smithy.model.node.NumberNode; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.node.StringNode; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidatedResult; +import software.amazon.smithy.model.validation.ValidationEvent; + +final class ToSmithyNode { + private final String path; + private final Document document; + private final List events; + + private ToSmithyNode(String path, Document document) { + this.path = path; + this.document = document; + this.events = new ArrayList<>(); + } + + static ValidatedResult toSmithyNode(BuildFile buildFile) { + var toSmithyNode = new ToSmithyNode(buildFile.path(), buildFile.document()); + var smithyNode = toSmithyNode.toSmithyNode(buildFile.getParse().value()); + return new ValidatedResult<>(smithyNode, toSmithyNode.events); + } + + private Node toSmithyNode(Syntax.Node syntaxNode) { + if (syntaxNode == null) { + return null; + } + + SourceLocation sourceLocation = nodeStartSourceLocation(syntaxNode); + return switch (syntaxNode) { + case Syntax.Node.Obj obj -> { + ObjectNode.Builder builder = ObjectNode.builder().sourceLocation(sourceLocation); + for (Syntax.Node.Kvp kvp : obj.kvps().kvps()) { + String keyValue = kvp.key().stringValue(); + StringNode key = new StringNode(keyValue, nodeStartSourceLocation(kvp.key())); + Node value = toSmithyNode(kvp.value()); + if (value != null) { + builder.withMember(key, value); + } + } + yield builder.build(); + } + case Syntax.Node.Arr arr -> { + ArrayNode.Builder builder = ArrayNode.builder().sourceLocation(sourceLocation); + for (Syntax.Node elem : arr.elements()) { + Node elemNode = toSmithyNode(elem); + if (elemNode != null) { + builder.withValue(elemNode); + } + } + yield builder.build(); + } + case Syntax.Ident ident -> { + String value = ident.stringValue(); + yield switch (value) { + case "true", "false" -> new BooleanNode(Boolean.parseBoolean(value), sourceLocation); + case "null" -> new NullNode(sourceLocation); + default -> null; + }; + } + case Syntax.Node.Str str -> new StringNode(str.stringValue(), sourceLocation); + case Syntax.Node.Num num -> new NumberNode(num.value(), sourceLocation); + case Syntax.Node.Err err -> { + events.add(ValidationEvent.builder() + .id("ParseError") + .severity(Severity.ERROR) + .message(err.message()) + .sourceLocation(sourceLocation) + .build()); + yield null; + } + default -> null; + }; + } + + private SourceLocation nodeStartSourceLocation(Syntax.Node node) { + return LspAdapter.toSourceLocation(path, document.rangeBetween(node.start(), node.end())); + } +} diff --git a/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java b/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java index 1bd9e540..8335786e 100644 --- a/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java +++ b/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java @@ -163,8 +163,34 @@ public static Position toPosition(SourceLocation sourceLocation) { */ public static Location toLocation(FromSourceLocation fromSourceLocation) { SourceLocation sourceLocation = fromSourceLocation.getSourceLocation(); - return new Location(toUri(sourceLocation.getFilename()), point( - new Position(sourceLocation.getLine() - 1, sourceLocation.getColumn() - 1))); + return new Location(toUri(sourceLocation.getFilename()), point(toPosition(sourceLocation))); + } + + /** + * Get a {@link SourceLocation} with the given path, at the start of the given + * range. + * + * @param path The path of the source location + * @param range The range of the source location + * @return The source location + */ + public static SourceLocation toSourceLocation(String path, Range range) { + return toSourceLocation(path, range.getStart()); + } + + /** + * Get a {@link SourceLocation} with the given path, at the given position. + * + * @param path The path of the source location + * @param position The position of the source location + * @return The source location + */ + public static SourceLocation toSourceLocation(String path, Position position) { + return new SourceLocation( + path, + position.getLine() + 1, + position.getCharacter() + 1 + ); } /** diff --git a/src/main/java/software/amazon/smithy/lsp/util/Result.java b/src/main/java/software/amazon/smithy/lsp/util/Result.java deleted file mode 100644 index d4c4112c..00000000 --- a/src/main/java/software/amazon/smithy/lsp/util/Result.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package software.amazon.smithy.lsp.util; - -import java.util.Optional; -import java.util.function.Function; -import java.util.function.Supplier; - -/** - * Type representing the result of an operation that could be successful - * or fail. - * - * @param Type of successful result - * @param Type of failed result - */ -public final class Result { - private final T value; - private final E error; - - private Result(T value, E error) { - this.value = value; - this.error = error; - } - - /** - * @param value The success value - * @param Type of successful result - * @param Type of failed result - * @return The successful result - */ - public static Result ok(T value) { - return new Result<>(value, null); - } - - /** - * @param error The failed value - * @param Type of successful result - * @param Type of failed result - * @return The failed result - */ - public static Result err(E error) { - return new Result<>(null, error); - } - - /** - * @param fallible A function that may fail - * @param Type of successful result - * @return A result containing the result of calling {@code fallible} - */ - public static Result ofFallible(Supplier fallible) { - try { - return Result.ok(fallible.get()); - } catch (Exception e) { - return Result.err(e); - } - } - - /** - * @param throwing A function that may throw - * @param Type of successful result - * @return A result containing the result of calling {@code throwing} - */ - public static Result ofThrowing(ThrowingSupplier throwing) { - try { - return Result.ok(throwing.get()); - } catch (Exception e) { - return Result.err(e); - } - } - - /** - * @return Whether this result is successful - */ - public boolean isOk() { - return this.value != null; - } - - /** - * @return Whether this result is failed - */ - public boolean isErr() { - return this.error != null; - } - - /** - * @return The successful value, or throw an exception if this Result is failed - */ - public T unwrap() { - if (get().isEmpty()) { - throw new RuntimeException("Called unwrap on an Err Result: " + getErr().get()); - } - return get().get(); - } - - /** - * @return The failed value, or throw an exception if this Result is successful - */ - public E unwrapErr() { - if (getErr().isEmpty()) { - throw new RuntimeException("Called unwrapErr on an Ok Result: " + get().get()); - } - return getErr().get(); - } - - /** - * @return Get the successful value if present - */ - public Optional get() { - return Optional.ofNullable(value); - } - - /** - * @return Get the failed value if present - */ - public Optional getErr() { - return Optional.ofNullable(error); - } - - /** - * Transforms the successful value of this Result, if present. - * - * @param mapper Function to apply to the successful value of this result - * @param The type to map to - * @return A new result with {@code mapper} applied, if this result is a - * successful one - */ - public Result map(Function mapper) { - if (isOk()) { - return Result.ok(mapper.apply(unwrap())); - } - return Result.err(unwrapErr()); - } - - /** - * Transforms the failed value of this Result, if present. - * - * @param mapper Function to apply to the failed value of this result - * @param The type to map to - * @return A new result with {@code mapper} applied, if this result is a - * failed one - */ - public Result mapErr(Function mapper) { - if (isErr()) { - return Result.err(mapper.apply(unwrapErr())); - } - return Result.ok(unwrap()); - } - - - /** - * A supplier that throws a checked exception. - * - * @param The output of the supplier - * @param The exception type that can be thrown - */ - @FunctionalInterface - public interface ThrowingSupplier { - T get() throws E; - } -} diff --git a/src/test/java/software/amazon/smithy/lsp/FilePatternsTest.java b/src/test/java/software/amazon/smithy/lsp/FilePatternsTest.java index 9ac13497..d74b4901 100644 --- a/src/test/java/software/amazon/smithy/lsp/FilePatternsTest.java +++ b/src/test/java/software/amazon/smithy/lsp/FilePatternsTest.java @@ -15,7 +15,7 @@ import org.junit.jupiter.api.Test; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.utils.ListUtils; public class FilePatternsTest { @@ -39,7 +39,7 @@ public void createsProjectPathMatchers() { .build()) .build(); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); PathMatcher smithyMatcher = FilePatterns.getSmithyFilesPathMatcher(project); PathMatcher buildMatcher = FilePatterns.getProjectBuildFilesPathMatcher(project); @@ -65,7 +65,7 @@ public void createsWorkspacePathMatchers() throws IOException { workspaceRoot.resolve("bar").toFile().mkdir(); workspaceRoot.resolve("bar/smithy-build.json").toFile().createNewFile(); - Project fooProject = ProjectLoader.load(fooWorkspace.getRoot(), new ServerState()).unwrap(); + Project fooProject = ProjectTest.load(fooWorkspace.getRoot()); PathMatcher fooBuildMatcher = FilePatterns.getProjectBuildFilesPathMatcher(fooProject); PathMatcher workspaceBuildMatcher = FilePatterns.getWorkspaceBuildFilesPathMatcher(workspaceRoot); diff --git a/src/test/java/software/amazon/smithy/lsp/FileWatcherRegistrationsTest.java b/src/test/java/software/amazon/smithy/lsp/FileWatcherRegistrationsTest.java index 88c710b8..26637e6d 100644 --- a/src/test/java/software/amazon/smithy/lsp/FileWatcherRegistrationsTest.java +++ b/src/test/java/software/amazon/smithy/lsp/FileWatcherRegistrationsTest.java @@ -16,7 +16,7 @@ import org.junit.jupiter.api.Test; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.utils.ListUtils; public class FileWatcherRegistrationsTest { @@ -40,7 +40,7 @@ public void createsCorrectRegistrations() { .build()) .build(); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); List matchers = FileWatcherRegistrations.getSmithyFileWatcherRegistrations(List.of(project)) .stream() .map(Registration::getRegisterOptions) diff --git a/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java b/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java index 6dd38cfd..cb60abc7 100644 --- a/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java +++ b/src/test/java/software/amazon/smithy/lsp/SmithyMatchers.java @@ -11,6 +11,7 @@ import org.hamcrest.Description; import org.hamcrest.Matcher; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.loader.Prelude; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; @@ -72,6 +73,15 @@ public void describeMismatchSafely(ValidationEvent event, Description descriptio }; } + public static Matcher eventWithSourceLocation(Matcher sourceLocationMatcher) { + return new CustomTypeSafeMatcher<>("has source location " + sourceLocationMatcher.toString()) { + @Override + protected boolean matchesSafely(ValidationEvent item) { + return sourceLocationMatcher.matches(item.getSourceLocation()); + } + }; + } + public static Matcher eventWithId(Matcher id) { return new CustomTypeSafeMatcher<>("has id matching " + id.toString()) { @Override diff --git a/src/test/java/software/amazon/smithy/lsp/language/CompletionHandlerTest.java b/src/test/java/software/amazon/smithy/lsp/language/CompletionHandlerTest.java index 1982dec6..aaac2cd6 100644 --- a/src/test/java/software/amazon/smithy/lsp/language/CompletionHandlerTest.java +++ b/src/test/java/software/amazon/smithy/lsp/language/CompletionHandlerTest.java @@ -25,13 +25,12 @@ import org.junit.jupiter.api.Test; import software.amazon.smithy.lsp.LspMatchers; import software.amazon.smithy.lsp.RequestBuilders; -import software.amazon.smithy.lsp.ServerState; import software.amazon.smithy.lsp.TestWorkspace; import software.amazon.smithy.lsp.TextWithPositions; import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.project.IdlFile; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.lsp.project.SmithyFile; public class CompletionHandlerTest { @@ -1086,7 +1085,7 @@ private static List getCompLabels(String text, Position... positions) { private static List getCompItems(String text, Position... positions) { TestWorkspace workspace = TestWorkspace.singleModel(text); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); String uri = workspace.getUri("main.smithy"); IdlFile smithyFile = (IdlFile) (SmithyFile) project.getProjectFile(uri); diff --git a/src/test/java/software/amazon/smithy/lsp/language/DefinitionHandlerTest.java b/src/test/java/software/amazon/smithy/lsp/language/DefinitionHandlerTest.java index 8f680cfa..cd16243f 100644 --- a/src/test/java/software/amazon/smithy/lsp/language/DefinitionHandlerTest.java +++ b/src/test/java/software/amazon/smithy/lsp/language/DefinitionHandlerTest.java @@ -22,12 +22,11 @@ import org.eclipse.lsp4j.Position; import org.junit.jupiter.api.Test; import software.amazon.smithy.lsp.RequestBuilders; -import software.amazon.smithy.lsp.ServerState; import software.amazon.smithy.lsp.TestWorkspace; import software.amazon.smithy.lsp.TextWithPositions; import software.amazon.smithy.lsp.project.IdlFile; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.lsp.project.SmithyFile; import software.amazon.smithy.lsp.syntax.StatementView; import software.amazon.smithy.lsp.syntax.Syntax; @@ -366,7 +365,7 @@ private static GetLocationsResult getLocations(TextWithPositions textWithPositio private static GetLocationsResult getLocations(String text, Position... positions) { TestWorkspace workspace = TestWorkspace.singleModel(text); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); String uri = workspace.getUri("main.smithy"); SmithyFile smithyFile = (SmithyFile) project.getProjectFile(uri); diff --git a/src/test/java/software/amazon/smithy/lsp/language/DocumentSymbolTest.java b/src/test/java/software/amazon/smithy/lsp/language/DocumentSymbolTest.java index 0242eab8..434c8612 100644 --- a/src/test/java/software/amazon/smithy/lsp/language/DocumentSymbolTest.java +++ b/src/test/java/software/amazon/smithy/lsp/language/DocumentSymbolTest.java @@ -12,11 +12,10 @@ import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; -import software.amazon.smithy.lsp.ServerState; import software.amazon.smithy.lsp.TestWorkspace; import software.amazon.smithy.lsp.project.IdlFile; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.lsp.project.SmithyFile; public class DocumentSymbolTest { @@ -49,7 +48,7 @@ public void documentSymbols() { private static List getDocumentSymbolNames(String text) { TestWorkspace workspace = TestWorkspace.singleModel(text); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); String uri = workspace.getUri("main.smithy"); IdlFile idlFile = (IdlFile) (SmithyFile) project.getProjectFile(uri); diff --git a/src/test/java/software/amazon/smithy/lsp/language/HoverHandlerTest.java b/src/test/java/software/amazon/smithy/lsp/language/HoverHandlerTest.java index 7a22bc66..8fec8f52 100644 --- a/src/test/java/software/amazon/smithy/lsp/language/HoverHandlerTest.java +++ b/src/test/java/software/amazon/smithy/lsp/language/HoverHandlerTest.java @@ -16,12 +16,11 @@ import org.eclipse.lsp4j.Position; import org.junit.jupiter.api.Test; import software.amazon.smithy.lsp.RequestBuilders; -import software.amazon.smithy.lsp.ServerState; import software.amazon.smithy.lsp.TestWorkspace; import software.amazon.smithy.lsp.TextWithPositions; import software.amazon.smithy.lsp.project.IdlFile; import software.amazon.smithy.lsp.project.Project; -import software.amazon.smithy.lsp.project.ProjectLoader; +import software.amazon.smithy.lsp.project.ProjectTest; import software.amazon.smithy.lsp.project.SmithyFile; import software.amazon.smithy.model.validation.Severity; @@ -154,7 +153,7 @@ private static List getHovers(TextWithPositions text) { private static List getHovers(String text, Position... positions) { TestWorkspace workspace = TestWorkspace.singleModel(text); - Project project = ProjectLoader.load(workspace.getRoot(), new ServerState()).unwrap(); + Project project = ProjectTest.load(workspace.getRoot()); String uri = workspace.getUri("main.smithy"); SmithyFile smithyFile = (SmithyFile) project.getProjectFile(uri); diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigLoaderTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigLoaderTest.java deleted file mode 100644 index 6b4c0bb5..00000000 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigLoaderTest.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package software.amazon.smithy.lsp.project; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasSize; -import static software.amazon.smithy.lsp.project.ProjectTest.toPath; - -import java.nio.file.Path; -import java.util.List; -import java.util.stream.Collectors; -import org.junit.jupiter.api.Test; -import software.amazon.smithy.build.model.MavenConfig; -import software.amazon.smithy.build.model.MavenRepository; -import software.amazon.smithy.lsp.ServerState; -import software.amazon.smithy.lsp.util.Result; - -public class ProjectConfigLoaderTest { - @Test - public void loadsConfigWithEnvVariable() { - System.setProperty("FOO", "bar"); - Path root = toPath(getClass().getResource("env-config")); - Result> result = load(root); - - assertThat(result.isOk(), is(true)); - ProjectConfig config = result.unwrap(); - assertThat(config.maven().isPresent(), is(true)); - MavenConfig mavenConfig = config.maven().get(); - assertThat(mavenConfig.getRepositories(), hasSize(1)); - MavenRepository repository = mavenConfig.getRepositories().stream().findFirst().get(); - assertThat(repository.getUrl(), containsString("example.com/maven/my_repo")); - assertThat(repository.getHttpCredentials().isPresent(), is(true)); - assertThat(repository.getHttpCredentials().get(), containsString("my_user:bar")); - } - - @Test - public void loadsLegacyConfig() { - Path root = toPath(getClass().getResource("legacy-config")); - Result> result = load(root); - - assertThat(result.isOk(), is(true)); - ProjectConfig config = result.unwrap(); - assertThat(config.maven().isPresent(), is(true)); - MavenConfig mavenConfig = config.maven().get(); - assertThat(mavenConfig.getDependencies(), containsInAnyOrder("baz")); - assertThat(mavenConfig.getRepositories().stream() - .map(MavenRepository::getUrl) - .collect(Collectors.toList()), containsInAnyOrder("foo", "bar")); - } - - @Test - public void prefersNonLegacyConfig() { - Path root = toPath(getClass().getResource("legacy-config-with-conflicts")); - Result> result = load(root); - - assertThat(result.isOk(), is(true)); - ProjectConfig config = result.unwrap(); - assertThat(config.maven().isPresent(), is(true)); - MavenConfig mavenConfig = config.maven().get(); - assertThat(mavenConfig.getDependencies(), containsInAnyOrder("dep1", "dep2")); - assertThat(mavenConfig.getRepositories().stream() - .map(MavenRepository::getUrl) - .collect(Collectors.toList()), containsInAnyOrder("url1", "url2")); - } - - @Test - public void mergesBuildExts() { - Path root = toPath(getClass().getResource("build-exts")); - Result> result = load(root); - - assertThat(result.isOk(), is(true)); - ProjectConfig config = result.unwrap(); - assertThat(config.imports(), containsInAnyOrder(containsString("main.smithy"), containsString("other.smithy"))); - } - - private static Result> load(Path root) { - return ProjectConfigLoader.loadFromRoot(root, new ServerState()); - } -} diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java new file mode 100644 index 00000000..f1f11ff1 --- /dev/null +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java @@ -0,0 +1,272 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.lsp.project; + +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithId; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithSourceLocation; +import static software.amazon.smithy.lsp.project.ProjectTest.toPath; +import static software.amazon.smithy.lsp.protocol.LspAdapter.toSourceLocation; + +import java.nio.file.Path; +import java.util.List; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.build.model.MavenRepository; +import software.amazon.smithy.cli.dependencies.DependencyResolver; +import software.amazon.smithy.cli.dependencies.DependencyResolverException; +import software.amazon.smithy.cli.dependencies.ResolvedArtifact; +import software.amazon.smithy.lsp.ServerState; +import software.amazon.smithy.lsp.TextWithPositions; +import software.amazon.smithy.lsp.document.Document; + +public class ProjectConfigTest { + @Test + public void loadsConfigWithEnvVariable() { + System.setProperty("FOO", "bar"); + Path root = toPath(getClass().getResource("env-config")); + ProjectConfig config = load(root); + + assertThat(config.maven().getRepositories(), hasSize(1)); + MavenRepository repository = config.maven().getRepositories().stream().findFirst().get(); + assertThat(repository.getUrl(), containsString("example.com/maven/my_repo")); + assertThat(repository.getHttpCredentials().isPresent(), is(true)); + assertThat(repository.getHttpCredentials().get(), containsString("my_user:bar")); + } + + @Test + public void loadsLegacyConfig() { + Path root = toPath(getClass().getResource("legacy-config")); + ProjectConfig config = load(root); + + assertThat(config.maven().getDependencies(), containsInAnyOrder("baz")); + assertThat(config.maven().getRepositories().stream() + .map(MavenRepository::getUrl) + .collect(Collectors.toList()), containsInAnyOrder("foo", "bar")); + } + + @Test + public void prefersNonLegacyConfig() { + Path root = toPath(getClass().getResource("legacy-config-with-conflicts")); + ProjectConfig config = load(root); + + assertThat(config.maven().getDependencies(), containsInAnyOrder("dep1", "dep2")); + assertThat(config.maven().getRepositories().stream() + .map(MavenRepository::getUrl) + .collect(Collectors.toList()), containsInAnyOrder("url1", "url2")); + } + + @Test + public void mergesBuildExts() { + Path root = toPath(getClass().getResource("build-exts")); + ProjectConfig config = load(root); + + assertThat(config.imports(), containsInAnyOrder(containsString("main.smithy"), containsString("other.smithy"))); + } + + @Test + public void validatesSmithyBuildJson() { + var text = TextWithPositions.from(""" + { + "version" : %1, // Should be a string + "sources": ["foo"] + } + """); + var eventPosition = text.positions()[0]; + var config = load(Path.of("test"), BuildFileType.SMITHY_BUILD, text.text()); + + var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + assertThat(buildFile, notNullValue()); + assertThat(config.events(), containsInAnyOrder( + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) + )); + assertThat(config.sources(), empty()); + } + + @Test + public void validatesSmithyProjectJson() { + var text = TextWithPositions.from(""" + { + "sources": ["foo"], + "dependencies": [ + %"foo" // Should be an object + ] + } + """); + var eventPosition = text.positions()[0]; + var config = load(Path.of("test"), BuildFileType.SMITHY_PROJECT, text.text()); + + var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_PROJECT); + assertThat(buildFile, notNullValue()); + assertThat(config.events(), containsInAnyOrder( + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) + )); + assertThat(config.sources(), empty()); + } + + @Test + public void validatesMavenConfig() { + // "httpCredentials" is invalid, but we don't get the source location in the exception + var text = TextWithPositions.from(""" + %{ + "version" : "1", + "sources": ["foo"], + "maven": { + "repositories": [ + { + "url": "foo", + "httpCredentials": "bar" + } + ] + } + } + """); + var eventPosition = text.positions()[0]; + var config = load(Path.of("test"), BuildFileType.SMITHY_BUILD, text.text()); + + var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + assertThat(buildFile, notNullValue()); + assertThat(config.events(), containsInAnyOrder(allOf( + eventWithMessage(containsString("httpCredentials")), + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) + ))); + assertThat(config.sources(), empty()); + } + + @Test + public void resolveValidatesFilesExist() { + var text = TextWithPositions.from(""" + { + "sources": [%"foo"], + "imports": [%"bar"], + "dependencies": [ + { + "name": "baz", + "path": %"baz" + } + ] + } + """); + var notFoundSourcePosition = text.positions()[0]; + var notFoundImportPosition = text.positions()[1]; + var notFoundDepPosition = text.positions()[2]; + var root = Path.of("test"); + var config = load(root, BuildFileType.SMITHY_PROJECT, text.text()); + + var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_PROJECT); + assertThat(buildFile, notNullValue()); + assertThat(config.events(), containsInAnyOrder( + allOf( + eventWithId(equalTo("FileNotFound")), + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), notFoundSourcePosition))) + ), + allOf( + eventWithId(equalTo("FileNotFound")), + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), notFoundImportPosition))) + ), + allOf( + eventWithId(equalTo("FileNotFound")), + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), notFoundDepPosition))) + ) + )); + assertThat(config.sources(), containsInAnyOrder(equalTo("foo"))); + assertThat(config.imports(), containsInAnyOrder(equalTo("bar"))); + assertThat(config.projectDependencies().getFirst().path(), equalTo("baz")); + } + + @Test + public void resolveValidatesMavenDependencies() { + var text = TextWithPositions.from(""" + { + "version": "1", + %"maven": { + "dependencies": ["foo"], + "repositories": [ + { + "url": "bar" + } + ] + } + } + """); + var eventPosition = text.positions()[0]; + var root = Path.of("test"); + Supplier resolverFactory = () -> new DependencyResolver() { + @Override + public void addRepository(MavenRepository mavenRepository) { + throw new DependencyResolverException("repo " + mavenRepository.getUrl()); + } + + @Override + public void addDependency(String s) { + throw new DependencyResolverException("dep " + s); + } + + @Override + public List resolve() { + throw new DependencyResolverException("call resolve"); + } + }; + var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); + var config = new ProjectConfigLoader(root, buildFiles, resolverFactory).load(); + + var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + assertThat(buildFile, notNullValue()); + assertThat(config.events(), containsInAnyOrder( + allOf( + eventWithId(equalTo("DependencyResolver")), + eventWithMessage(allOf( + containsString("repo bar"), + containsString("dep foo"), + containsString("call resolve") + )), + eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) + ) + )); + } + + private record NoOpResolver() implements DependencyResolver { + @Override + public void addRepository(MavenRepository mavenRepository) { + } + + @Override + public void addDependency(String s) { + } + + @Override + public List resolve() { + return List.of(); + } + } + + private static BuildFiles createBuildFiles(Path root, BuildFileType type, String content) { + var buildFile = BuildFile.create(root.resolve(type.filename()).toString(), Document.of(content), type); + return BuildFiles.of(List.of(buildFile)); + } + + private static ProjectConfig load(Path root, BuildFileType type, String content) { + var buildFiles = createBuildFiles(root, type, content); + var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); + return loader.load(); + } + + private static ProjectConfig load(Path root) { + var buildFiles = BuildFiles.load(root, new ServerState()); + var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); + return loader.load(); + } +} diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java new file mode 100644 index 00000000..5d015598 --- /dev/null +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java @@ -0,0 +1,230 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.lsp.project; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.hasSize; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithId; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; +import static software.amazon.smithy.lsp.SmithyMatchers.hasShapeWithId; +import static software.amazon.smithy.lsp.SmithyMatchers.hasValue; + +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.lsp.protocol.LspAdapter; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.validation.Severity; + +public class ProjectLoaderTest { + @Test + public void loadsFlatProject() { + Path root = ProjectTest.toPath(getClass().getResource("flat")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); + assertThat(project.config().imports(), empty()); + assertThat(project.config().resolvedDependencies(), empty()); + assertThat(project.modelResult().isBroken(), is(false)); + assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); + } + + @Test + public void loadsProjectWithMavenDep() { + Path root = ProjectTest.toPath(getClass().getResource("maven-dep")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); + assertThat(project.config().imports(), empty()); + assertThat(project.config().resolvedDependencies(), hasSize(3)); + assertThat(project.modelResult().isBroken(), is(false)); + assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); + } + + @Test + public void loadsProjectWithSubdir() { + Path root = ProjectTest.toPath(getClass().getResource("subdirs")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.config().sources(), hasItems( + endsWith("model"), + endsWith("model2"))); + assertThat(project.getAllSmithyFilePaths(), hasItems( + equalTo(root.resolve("model/main.smithy").toString()), + equalTo(root.resolve("model/subdir/sub.smithy").toString()), + equalTo(root.resolve("model2/subdir2/sub2.smithy").toString()), + equalTo(root.resolve("model2/subdir2/subsubdir/subsub.smithy").toString()))); + assertThat(project.modelResult().isBroken(), is(false)); + assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); + assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Bar")); + assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Baz")); + } + + @Test + public void loadsModelWithUnknownTrait() { + Path root = ProjectTest.toPath(getClass().getResource("unknown-trait")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); + assertThat(project.modelResult().isBroken(), is(false)); // unknown traits don't break it + assertThat(project.modelResult().getValidationEvents(), hasItem(eventWithId(containsString("UnresolvedTrait")))); + assertThat(project.modelResult().getResult().isPresent(), is(true)); + assertThat(project.modelResult().getResult().get(), hasShapeWithId("com.foo#Foo")); + } + + @Test + public void loadsWhenModelHasInvalidSyntax() { + Path root = ProjectTest.toPath(getClass().getResource("invalid-syntax")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); + assertThat(project.modelResult().isBroken(), is(true)); + assertThat(project.modelResult().getValidationEvents(), hasItem(eventWithId(equalTo("Model")))); + assertThat(project.modelResult(), hasValue(allOf( + hasShapeWithId("com.foo#Foo"), + hasShapeWithId("com.foo#Foo$bar")))); + assertThat(project.getAllSmithyFilePaths(), hasItem(containsString("main.smithy"))); + } + + @Test + public void loadsProjectWithMultipleNamespaces() { + Path root = ProjectTest.toPath(getClass().getResource("multiple-namespaces")); + Project project = ProjectTest.load(root); + + assertThat(project.config().sources(), hasItem(endsWith("model"))); + assertThat(project.modelResult().getValidationEvents(), empty()); + assertThat(project.getAllSmithyFilePaths(), hasItems(containsString("a.smithy"), containsString("b.smithy"))); + + assertThat(project.modelResult(), hasValue(allOf( + hasShapeWithId("a#Hello"), + hasShapeWithId("a#HelloInput"), + hasShapeWithId("a#HelloOutput"), + hasShapeWithId("b#Hello"), + hasShapeWithId("b#HelloInput"), + hasShapeWithId("b#HelloOutput")))); + } + + @Test + public void loadsProjectWithExternalJars() { + Path root = ProjectTest.toPath(getClass().getResource("external-jars")); + Project project = ProjectTest.load(root); + + assertThat(project.config().sources(), containsInAnyOrder( + endsWith("test-traits.smithy"), + endsWith("test-validators.smithy"))); + assertThat(project.getAllSmithyFilePaths(), hasItems( + containsString("test-traits.smithy"), + containsString("test-validators.smithy"), + // Note: Depending on the order of how jar dependencies are added to the model assembler, + // this may or may not be present. This is because we're relying on the shapes loaded in + // the model to determine all Smithy files, and this file re-defines a shape, so the shape + // definition is super-seeded. + // containsString("smithy-test-traits.jar!/META-INF/smithy/smithy.test.json"), + containsString("alloy-core.jar!/META-INF/smithy/uuid.smithy"))); + + assertThat(project.modelResult().isBroken(), is(true)); + assertThat(project.modelResult().getValidationEvents(Severity.ERROR), hasItem(eventWithMessage(containsString("Proto index 1")))); + + assertThat(project.modelResult().getResult().isPresent(), is(true)); + Model model = project.modelResult().getResult().get(); + assertThat(model, hasShapeWithId("smithy.test#test")); + assertThat(model, hasShapeWithId("ns.test#Weather")); + assertThat(model.expectShape(ShapeId.from("ns.test#Weather")).hasTrait("smithy.test#test"), is(true)); + } + + @Test + public void loadsProjectWithInvalidSmithyBuildJson() { + Path root = ProjectTest.toPath(getClass().getResource("broken/missing-version")); + Project project = ProjectTest.load(root); + + assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); + assertThat(project.config().events(), hasItem(eventWithMessage(containsString("version")))); + assertThat(project.modelResult().isBroken(), is(false)); + } + + @Test + public void loadsProjectWithUnparseableSmithyBuildJson() { + Path root = ProjectTest.toPath(getClass().getResource("broken/parse-failure")); + Project project = ProjectTest.load(root); + + assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); + assertThat(project.config().events().isEmpty(), is(false)); + assertThat(project.modelResult().isBroken(), is(false)); + } + + @Test + public void loadsProjectWithNonExistingSource() { + Path root = ProjectTest.toPath(getClass().getResource("broken/source-doesnt-exist")); + Project project = ProjectTest.load(root); + + assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); + assertThat(project.config().events(), hasItem(eventWithId(equalTo("FileNotFound")))); + assertThat(project.modelResult().isBroken(), is(false)); + assertThat(project.getAllSmithyFiles().size(), equalTo(1)); // still have the prelude + } + + @Test + public void loadsProjectWithUnresolvableMavenDependency() { + Path root = ProjectTest.toPath(getClass().getResource("broken/unresolvable-maven-dependency")); + Project project = ProjectTest.load(root); + + assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); + assertThat(project.config().events(), hasItem(eventWithId(equalTo("DependencyResolver")))); + assertThat(project.modelResult().isBroken(), is(false)); + } + + @Test + public void loadsProjectWithUnresolvableProjectDependency() { + Path root = ProjectTest.toPath(getClass().getResource("broken/unresolvable-project-dependency")); + Project project = ProjectTest.load(root); + + assertHasBuildFile(project, BuildFileType.SMITHY_PROJECT); + assertThat(project.config().events(), hasItem(eventWithId(equalTo("FileNotFound")))); + assertThat(project.modelResult().isBroken(), is(false)); + } + + @Test + public void loadsProjectWithUnNormalizedDirs() throws Exception { + Path root = ProjectTest.toPath(getClass().getResource("unnormalized-dirs")); + Project project = ProjectTest.load(root); + + assertThat(project.root(), equalTo(root)); + assertThat(project.sources(), hasItems( + root.resolve("model"), + root.resolve("model2"))); + assertThat(project.imports(), hasItem(root.resolve("model3"))); + assertThat(project.getAllSmithyFilePaths(), hasItems( + equalTo(root.resolve("model/test-traits.smithy").toString()), + equalTo(root.resolve("model/one.smithy").toString()), + equalTo(root.resolve("model2/two.smithy").toString()), + equalTo(root.resolve("model3/three.smithy").toString()), + containsString("smithy-test-traits.jar!/META-INF/smithy/smithy.test.json"))); + assertThat(project.config().resolvedDependencies(), hasItem( + root.resolve("smithy-test-traits.jar").toUri().toURL())); + } + + private static void assertHasBuildFile(Project project, BuildFileType expectedType) { + String uri = LspAdapter.toUri(project.root().resolve(expectedType.filename()).toString()); + var file = project.getProjectFile(uri); + assertThat(file, instanceOf(BuildFile.class)); + assertThat(((BuildFile) file).type(), is(expectedType)); + } +} 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 8d030bcb..2b8c2ccb 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectTest.java @@ -5,21 +5,10 @@ package software.amazon.smithy.lsp.project; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.hasSize; -import static software.amazon.smithy.lsp.SmithyMatchers.eventWithId; -import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; -import static software.amazon.smithy.lsp.SmithyMatchers.hasShapeWithId; -import static software.amazon.smithy.lsp.SmithyMatchers.hasValue; import static software.amazon.smithy.lsp.UtilMatchers.anOptionalOf; import java.net.URISyntaxException; @@ -27,201 +16,18 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.List; import org.junit.jupiter.api.Test; import software.amazon.smithy.lsp.ServerState; import software.amazon.smithy.lsp.TestWorkspace; import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.protocol.LspAdapter; -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; public class ProjectTest { - @Test - public void loadsFlatProject() { - Path root = toPath(getClass().getResource("flat")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); - assertThat(project.config().imports(), empty()); - assertThat(project.dependencies(), empty()); - assertThat(project.modelResult().isBroken(), is(false)); - assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); - } - - @Test - public void loadsProjectWithMavenDep() { - Path root = toPath(getClass().getResource("maven-dep")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); - assertThat(project.config().imports(), empty()); - assertThat(project.dependencies(), hasSize(3)); - assertThat(project.modelResult().isBroken(), is(false)); - assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); - } - - @Test - public void loadsProjectWithSubdir() { - Path root = toPath(getClass().getResource("subdirs")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.config().sources(), hasItems( - endsWith("model"), - endsWith("model2"))); - assertThat(project.getAllSmithyFilePaths(), hasItems( - equalTo(root.resolve("model/main.smithy").toString()), - equalTo(root.resolve("model/subdir/sub.smithy").toString()), - equalTo(root.resolve("model2/subdir2/sub2.smithy").toString()), - equalTo(root.resolve("model2/subdir2/subsubdir/subsub.smithy").toString()))); - assertThat(project.modelResult().isBroken(), is(false)); - assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Foo")); - assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Bar")); - assertThat(project.modelResult().unwrap(), hasShapeWithId("com.foo#Baz")); - } - - @Test - public void loadsModelWithUnknownTrait() { - Path root = toPath(getClass().getResource("unknown-trait")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); - assertThat(project.modelResult().isBroken(), is(false)); // unknown traits don't break it - assertThat(project.modelResult().getValidationEvents(), hasItem(eventWithId(containsString("UnresolvedTrait")))); - assertThat(project.modelResult().getResult().isPresent(), is(true)); - assertThat(project.modelResult().getResult().get(), hasShapeWithId("com.foo#Foo")); - } - - @Test - public void loadsWhenModelHasInvalidSyntax() { - Path root = toPath(getClass().getResource("invalid-syntax")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.config().sources(), hasItem(endsWith("main.smithy"))); - assertThat(project.modelResult().isBroken(), is(true)); - assertThat(project.modelResult().getValidationEvents(), hasItem(eventWithId(equalTo("Model")))); - assertThat(project.modelResult(), hasValue(allOf( - hasShapeWithId("com.foo#Foo"), - hasShapeWithId("com.foo#Foo$bar")))); - assertThat(project.getAllSmithyFilePaths(), hasItem(containsString("main.smithy"))); - } - - @Test - public void loadsProjectWithMultipleNamespaces() { - Path root = toPath(getClass().getResource("multiple-namespaces")); - Project project = load(root).unwrap(); - - assertThat(project.config().sources(), hasItem(endsWith("model"))); - assertThat(project.modelResult().getValidationEvents(), empty()); - assertThat(project.getAllSmithyFilePaths(), hasItems(containsString("a.smithy"), containsString("b.smithy"))); - - assertThat(project.modelResult(), hasValue(allOf( - hasShapeWithId("a#Hello"), - hasShapeWithId("a#HelloInput"), - hasShapeWithId("a#HelloOutput"), - hasShapeWithId("b#Hello"), - hasShapeWithId("b#HelloInput"), - hasShapeWithId("b#HelloOutput")))); - } - - @Test - public void loadsProjectWithExternalJars() { - Path root = toPath(getClass().getResource("external-jars")); - Result> result = load(root); - - assertThat(result.isOk(), is(true)); - Project project = result.unwrap(); - assertThat(project.config().sources(), containsInAnyOrder( - endsWith("test-traits.smithy"), - endsWith("test-validators.smithy"))); - assertThat(project.getAllSmithyFilePaths(), hasItems( - containsString("test-traits.smithy"), - containsString("test-validators.smithy"), - containsString("smithy-test-traits.jar!/META-INF/smithy/smithy.test.json"), - containsString("alloy-core.jar!/META-INF/smithy/uuid.smithy"))); - - assertThat(project.modelResult().isBroken(), is(true)); - assertThat(project.modelResult().getValidationEvents(Severity.ERROR), hasItem(eventWithMessage(containsString("Proto index 1")))); - - assertThat(project.modelResult().getResult().isPresent(), is(true)); - Model model = project.modelResult().getResult().get(); - assertThat(model, hasShapeWithId("smithy.test#test")); - assertThat(model, hasShapeWithId("ns.test#Weather")); - assertThat(model.expectShape(ShapeId.from("ns.test#Weather")).hasTrait("smithy.test#test"), is(true)); - } - - @Test - public void failsLoadingInvalidSmithyBuildJson() { - Path root = toPath(getClass().getResource("broken/missing-version")); - Result> result = load(root); - - assertThat(result.isErr(), is(true)); - } - - @Test - public void failsLoadingUnparseableSmithyBuildJson() { - Path root = toPath(getClass().getResource("broken/parse-failure")); - Result> result = load(root); - - assertThat(result.isErr(), is(true)); - } - - @Test - public void doesntFailLoadingProjectWithNonExistingSource() { - Path root = toPath(getClass().getResource("broken/source-doesnt-exist")); - Result> result = load(root); - - assertThat(result.isErr(), is(false)); - assertThat(result.unwrap().getAllSmithyFiles().size(), equalTo(1)); // still have the prelude - } - - - @Test - public void failsLoadingUnresolvableMavenDependency() { - Path root = toPath(getClass().getResource("broken/unresolvable-maven-dependency")); - Result> result = load(root); - - assertThat(result.isErr(), is(true)); - } - - @Test - public void failsLoadingUnresolvableProjectDependency() { - Path root = toPath(getClass().getResource("broken/unresolvable-maven-dependency")); - Result> result = load(root); - - assertThat(result.isErr(), is(true)); - } - - @Test - public void loadsProjectWithUnNormalizedDirs() { - Path root = toPath(getClass().getResource("unnormalized-dirs")); - Project project = load(root).unwrap(); - - assertThat(project.root(), equalTo(root)); - assertThat(project.sources(), hasItems( - root.resolve("model"), - root.resolve("model2"))); - assertThat(project.imports(), hasItem(root.resolve("model3"))); - assertThat(project.getAllSmithyFilePaths(), hasItems( - equalTo(root.resolve("model/test-traits.smithy").toString()), - equalTo(root.resolve("model/one.smithy").toString()), - equalTo(root.resolve("model2/two.smithy").toString()), - equalTo(root.resolve("model3/three.smithy").toString()), - containsString("smithy-test-traits.jar!/META-INF/smithy/smithy.test.json"))); - assertThat(project.dependencies(), hasItem(root.resolve("smithy-test-traits.jar"))); - } - @Test public void changeFileApplyingSimpleTrait() { String m1 = """ @@ -236,7 +42,7 @@ public void changeFileApplyingSimpleTrait() { string Bar """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("length"), is(true)); @@ -267,7 +73,7 @@ public void changeFileApplyingListTrait() { string Bar """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("tags"), is(true)); @@ -304,7 +110,7 @@ public void changeFileApplyingListTraitWithUnrelatedDependencies() { apply Baz @length(min: 1) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); Shape baz = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Baz")); @@ -346,7 +152,7 @@ public void changingFileApplyingListTraitWithRelatedDependencies() { apply Bar @length(min: 1) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("tags"), is(true)); @@ -386,7 +192,7 @@ public void changingFileApplyingListTraitWithRelatedArrayTraitDependencies() { apply Bar @tags(["bar"]) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("tags"), is(true)); @@ -417,7 +223,7 @@ public void changingFileWithDependencies() { apply Foo @length(min: 1) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape foo = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); assertThat(foo.hasTrait("length"), is(true)); @@ -448,7 +254,7 @@ public void changingFileWithArrayDependencies() { apply Foo @tags(["foo"]) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape foo = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); assertThat(foo.hasTrait("tags"), is(true)); @@ -480,7 +286,7 @@ public void changingFileWithMixedArrayDependencies() { apply Foo @tags(["foo"]) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape foo = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); assertThat(foo.hasTrait("tags"), is(true)); @@ -516,7 +322,7 @@ public void changingFileWithArrayDependenciesWithDependencies() { apply Bar @length(min: 1) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape foo = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Foo")); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); @@ -557,7 +363,7 @@ public void removingSimpleApply() { apply Bar @pattern("a") """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("pattern"), is(true)); @@ -595,7 +401,7 @@ public void removingArrayApply() { apply Bar @tags(["bar"]) """; TestWorkspace workspace = TestWorkspace.multipleModels(m1, m2, m3); - Project project = load(workspace.getRoot()).unwrap(); + Project project = load(workspace.getRoot()); Shape bar = project.modelResult().unwrap().expectShape(ShapeId.from("com.foo#Bar")); assertThat(bar.hasTrait("tags"), is(true)); @@ -615,15 +421,19 @@ public void removingArrayApply() { @Test public void loadsEmptyProjectWhenThereAreNoConfigFiles() throws Exception { Path root = Files.createTempDirectory("foo"); - Project project = load(root).unwrap(); + Project project = load(root); assertThat(project.type(), equalTo(Project.Type.EMPTY)); } - private static Result> load(Path root) { - return ProjectLoader.load(root, new ServerState()); + public static Project load(Path root) { + try { + return ProjectLoader.load(root, new ServerState()); + } catch (Exception e) { + throw new RuntimeException(e); + } } - + public static Path toPath(URL url) { try { return Paths.get(url.toURI()); From 1330fdb7aca11eba11cc90680de52d0f8e46097e Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Sun, 19 Jan 2025 20:41:19 -0500 Subject: [PATCH 2/9] Refactoring Moved some things around. ProjectConfig no longer stores BuildFiles or validation events. Those are stored on the Project that owns them. --- .../amazon/smithy/lsp/project/BuildFiles.java | 2 +- .../amazon/smithy/lsp/project/Project.java | 24 +- .../smithy/lsp/project/ProjectConfig.java | 69 +---- .../lsp/project/ProjectConfigLoader.java | 243 +++++++++--------- .../smithy/lsp/project/ProjectLoader.java | 14 +- .../smithy/lsp/project/ProjectConfigTest.java | 62 +++-- .../smithy/lsp/project/ProjectLoaderTest.java | 10 +- 7 files changed, 196 insertions(+), 228 deletions(-) diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java index f8075ed1..01e420fe 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java @@ -46,7 +46,7 @@ boolean isEmpty() { } static BuildFiles empty() { - return new BuildFiles(new HashMap<>()); + return new BuildFiles(new HashMap<>(0)); } static BuildFiles of(Collection buildFiles) { 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 f5a0e720..1a8f1325 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/Project.java +++ b/src/main/java/software/amazon/smithy/lsp/project/Project.java @@ -42,28 +42,34 @@ public final class Project { private final Path root; private final ProjectConfig config; + private final BuildFiles buildFiles; private final Map smithyFiles; private final Supplier assemblerFactory; private final Type type; - private ValidatedResult modelResult; - private RebuildIndex rebuildIndex; + private volatile ValidatedResult modelResult; + private volatile RebuildIndex rebuildIndex; + private volatile List configEvents; Project( Path root, ProjectConfig config, + BuildFiles buildFiles, Map smithyFiles, Supplier assemblerFactory, Type type, ValidatedResult modelResult, - RebuildIndex rebuildIndex + RebuildIndex rebuildIndex, + List configEvents ) { this.root = root; this.config = config; + this.buildFiles = buildFiles; this.smithyFiles = smithyFiles; this.assemblerFactory = assemblerFactory; this.type = type; this.modelResult = modelResult; this.rebuildIndex = rebuildIndex; + this.configEvents = configEvents; } /** @@ -95,11 +101,13 @@ public enum Type { public static Project empty(Path root) { return new Project(root, ProjectConfig.empty(), + BuildFiles.empty(), new HashMap<>(), Model::assembler, Type.EMPTY, ValidatedResult.empty(), - new RebuildIndex()); + new RebuildIndex(), + List.of()); } /** @@ -114,7 +122,7 @@ ProjectConfig config() { } public List configEvents() { - return config.events(); + return configEvents; } /** @@ -178,11 +186,11 @@ public ProjectFile getProjectFile(String uri) { return smithyFile; } - return config.buildFiles().getByPath(path); + return buildFiles.getByPath(path); } - public void validateConfig() { - this.config.validate(); + public synchronized void validateConfig() { + this.configEvents = ProjectConfigLoader.validateBuildFiles(buildFiles); } /** 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 c3bc7a59..3f05e0a0 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfig.java @@ -8,9 +8,7 @@ import java.net.URL; import java.nio.file.Path; import java.util.List; -import java.util.concurrent.locks.ReentrantLock; import software.amazon.smithy.build.model.MavenConfig; -import software.amazon.smithy.model.validation.ValidationEvent; /** * A complete view of all a project's configuration that is needed to load it, @@ -23,58 +21,39 @@ final class ProjectConfig { private final List imports; private final List projectDependencies; private final MavenConfig maven; - private final BuildFiles buildFiles; private final List modelPaths; private final List resolvedDependencies; - private final ReentrantLock eventsLock = new ReentrantLock(); - private List events; ProjectConfig( List sources, List imports, List projectDependencies, MavenConfig maven, - BuildFiles buildFiles + List modelPaths, + List resolvedDependencies ) { this.sources = sources; this.imports = imports; this.projectDependencies = projectDependencies; this.maven = maven == null ? DEFAULT_MAVEN : maven; - this.buildFiles = buildFiles; - this.modelPaths = List.of(); - this.resolvedDependencies = List.of(); - this.events = List.of(); - } - - ProjectConfig( - ProjectConfig config, - List modelPaths, - List resolvedDependencies, - List events - ) { - this.sources = config.sources; - this.imports = config.imports; - this.projectDependencies = config.projectDependencies; - this.maven = config.maven; - this.buildFiles = config.buildFiles; this.modelPaths = modelPaths; this.resolvedDependencies = resolvedDependencies; - this.events = events; + } + + private ProjectConfig() { + this(List.of(), List.of(), List.of(), DEFAULT_MAVEN, List.of(), List.of()); + } + + private ProjectConfig(Path modelPath) { + this(List.of(), List.of(), List.of(), DEFAULT_MAVEN, List.of(modelPath), List.of()); } static ProjectConfig empty() { - return new ProjectConfig( - List.of(), List.of(), List.of(), DEFAULT_MAVEN, BuildFiles.empty() - ); + return new ProjectConfig(); } static ProjectConfig detachedConfig(Path modelPath) { - return new ProjectConfig( - empty(), - List.of(modelPath), - List.of(), - List.of() - ); + return new ProjectConfig(modelPath); } List sources() { @@ -93,10 +72,6 @@ MavenConfig maven() { return maven; } - BuildFiles buildFiles() { - return buildFiles; - } - List modelPaths() { return modelPaths; } @@ -104,24 +79,4 @@ List modelPaths() { List resolvedDependencies() { return resolvedDependencies; } - - List events() { - eventsLock.lock(); - try { - return events; - } finally { - eventsLock.unlock(); - } - } - - void validate() { - var updatedEvents = ProjectConfigLoader.validateBuildFiles(buildFiles); - - eventsLock.lock(); - try { - this.events = updatedEvents; - } finally { - eventsLock.unlock(); - } - } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java index 008d9811..a99add0a 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java @@ -46,11 +46,36 @@ * Loads and validates {@link ProjectConfig}s from {@link BuildFiles}. */ final class ProjectConfigLoader { - private interface BuildFileValidator { + private static final NodeMapper NODE_MAPPER = new NodeMapper(); + private static final BuildFileType[] EXTS = {BuildFileType.SMITHY_BUILD_EXT_0, BuildFileType.SMITHY_BUILD_EXT_1}; + // Taken from smithy-cli ConfigurationUtils + private static final Supplier CENTRAL = () -> MavenRepository.builder() + .url("https://repo.maven.apache.org/maven2") + .build(); + private static final Supplier DEFAULT_RESOLVER_FACTORY = () -> + new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); + + private final Path root; + private final BuildFiles buildFiles; + private final List events = new ArrayList<>(); + private final Map smithyNodes = new HashMap<>(BuildFileType.values().length); + private final Supplier dependencyResolverFactory; + + ProjectConfigLoader(Path root, BuildFiles buildFiles, Supplier dependencyResolverFactory) { + this.root = root; + this.buildFiles = buildFiles; + this.dependencyResolverFactory = dependencyResolverFactory; + } + + ProjectConfigLoader(Path root, BuildFiles buildFiles) { + this(root, buildFiles, DEFAULT_RESOLVER_FACTORY); + } + + private interface BuildFileLoader { T validate(BuildFile source, Node node, Consumer eventConsumer); } - private record SmithyBuildJsonValidator() implements BuildFileValidator { + private record SmithyBuildJsonLoader() implements BuildFileLoader { @Override public SmithyBuildConfig validate(BuildFile source, Node node, Consumer eventConsumer) { try { @@ -62,7 +87,7 @@ public SmithyBuildConfig validate(BuildFile source, Node node, Consumer { + private record SmithyBuildExtLoader() implements BuildFileLoader { @Override public SmithyBuildExtensions validate(BuildFile source, Node node, Consumer eventConsumer) { try { @@ -76,7 +101,7 @@ public SmithyBuildExtensions validate(BuildFile source, Node node, Consumer { + private record SmithyProjectJsonLoader() implements BuildFileLoader { @Override public SmithyProjectJson validate(BuildFile source, Node node, Consumer eventConsumer) { try { @@ -88,84 +113,65 @@ public SmithyProjectJson validate(BuildFile source, Node node, Consumer CENTRAL = () -> MavenRepository.builder() - .url("https://repo.maven.apache.org/maven2") - .build(); - private static final Supplier DEFAULT_RESOLVER_FACTORY = () -> - new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); - - private final Path root; - private final BuildFiles buildFiles; - private final List events = new ArrayList<>(); - private final Map smithyNodes = new HashMap<>(BuildFileType.values().length); - private final Supplier dependencyResolverFactory; - - ProjectConfigLoader(Path root, BuildFiles buildFiles, Supplier dependencyResolverFactory) { - this.root = root; - this.buildFiles = buildFiles; - this.dependencyResolverFactory = dependencyResolverFactory; - } - - ProjectConfigLoader(Path root, BuildFiles buildFiles) { - this(root, buildFiles, DEFAULT_RESOLVER_FACTORY); - } - static List validateBuildFiles(BuildFiles buildFiles) { List events = new ArrayList<>(); for (BuildFile buildFile : buildFiles) { - BuildFileValidator validator = switch (buildFile.type()) { - case SMITHY_BUILD -> new SmithyBuildJsonValidator(); - case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> new SmithyBuildExtValidator(); - case SMITHY_PROJECT -> new SmithyProjectJsonValidator(); + BuildFileLoader loader = switch (buildFile.type()) { + case SMITHY_BUILD -> new SmithyBuildJsonLoader(); + case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> new SmithyBuildExtLoader(); + case SMITHY_PROJECT -> new SmithyProjectJsonLoader(); }; - loadFile(buildFile, validator, events::add, (ignored) -> { + + loadFile(buildFile, loader, events::add, (ignored) -> { }); } return events; } - ProjectConfig load() { - SmithyBuildConfig smithyBuildConfig = loadFile(BuildFileType.SMITHY_BUILD, new SmithyBuildJsonValidator()); + record Result(ProjectConfig config, List events) {} - SmithyBuildExtensions.Builder extBuilder = null; - for (BuildFileType extType : EXTS) { - SmithyBuildExtensions ext = loadFile(extType, new SmithyBuildExtValidator()); - if (ext != null) { - if (extBuilder == null) { - extBuilder = SmithyBuildExtensions.builder(); - } - extBuilder.merge(ext); + Result load() { + SmithyBuildConfig smithyBuildConfig = loadFile(BuildFileType.SMITHY_BUILD, new SmithyBuildJsonLoader()); + SmithyBuildExtensions.Builder extBuilder = loadExts(); + SmithyBuildConfig merged = mergeSmithyBuildConfig(smithyBuildConfig, extBuilder); + SmithyProjectJson smithyProjectJson = loadFile(BuildFileType.SMITHY_PROJECT, new SmithyProjectJsonLoader()); + + List sources = new ArrayList<>(); + List imports = new ArrayList<>(); + List projectDependencies = new ArrayList<>(); + MavenConfig mavenConfig = null; + + if (merged != null) { + sources.addAll(merged.getSources()); + imports.addAll(merged.getImports()); + var mavenOpt = merged.getMaven(); + if (mavenOpt.isPresent()) { + mavenConfig = mavenOpt.get(); } } - SmithyBuildConfig merged = mergeSmithyBuildConfig(smithyBuildConfig, extBuilder); - SmithyProjectJson smithyProjectJson = loadFile(BuildFileType.SMITHY_PROJECT, new SmithyProjectJsonValidator()); + if (smithyProjectJson != null) { + sources.addAll(smithyProjectJson.sources()); + imports.addAll(smithyProjectJson.imports()); + projectDependencies.addAll(smithyProjectJson.dependencies()); + } - ProjectConfig partial = createConfig(merged, smithyProjectJson); - var resolveResult = resolve(partial); + ProjectConfig resolved = resolve(sources, imports, mavenConfig, projectDependencies); - return new ProjectConfig( - partial, - resolveResult.modelPaths(), - resolveResult.resolvedDependencies(), - resolveResult.events() - ); + return new Result(resolved, events); } - private T loadFile(BuildFileType type, BuildFileValidator validator) { + private T loadFile(BuildFileType type, BuildFileLoader loader) { var buildFile = buildFiles.getByType(type); if (buildFile != null) { - return loadFile(buildFile, validator, events::add, (node) -> smithyNodes.put(type, node)); + return loadFile(buildFile, loader, events::add, (node) -> smithyNodes.put(type, node)); } return null; } private static T loadFile( BuildFile buildFile, - BuildFileValidator validator, + BuildFileLoader loader, Consumer eventConsumer, Consumer nodeConsumer ) { @@ -174,11 +180,25 @@ private static T loadFile( var smithyNode = nodeResult.getResult().orElse(null); if (smithyNode != null) { nodeConsumer.accept(smithyNode); - return validator.validate(buildFile, smithyNode, eventConsumer); + return loader.validate(buildFile, smithyNode, eventConsumer); } return null; } + private SmithyBuildExtensions.Builder loadExts() { + SmithyBuildExtensions.Builder extBuilder = null; + for (BuildFileType extType : EXTS) { + SmithyBuildExtensions ext = loadFile(extType, new SmithyBuildExtLoader()); + if (ext != null) { + if (extBuilder == null) { + extBuilder = SmithyBuildExtensions.builder(); + } + extBuilder.merge(ext); + } + } + return extBuilder; + } + private SmithyBuildConfig mergeSmithyBuildConfig( SmithyBuildConfig smithyBuildConfig, SmithyBuildExtensions.Builder extBuilder @@ -238,61 +258,42 @@ private static ValidationEvent toEvent(Exception e, BuildFile fallbackBuildFile) .build(); } - private ProjectConfig createConfig(SmithyBuildConfig smithyBuildConfig, SmithyProjectJson smithyProjectJson) { - // TODO: Make this more efficient with right-sized lists - List sources = new ArrayList<>(); - List imports = new ArrayList<>(); - List projectDependencies = new ArrayList<>(); - MavenConfig mavenConfig = null; - - if (smithyBuildConfig != null) { - sources.addAll(smithyBuildConfig.getSources()); - imports.addAll(smithyBuildConfig.getImports()); - var mavenOpt = smithyBuildConfig.getMaven(); - if (mavenOpt.isPresent()) { - mavenConfig = mavenOpt.get(); - } - } - - if (smithyProjectJson != null) { - sources.addAll(smithyProjectJson.sources()); - imports.addAll(smithyProjectJson.imports()); - projectDependencies.addAll(smithyProjectJson.dependencies()); - } - - return new ProjectConfig( - sources, - imports, - projectDependencies, - mavenConfig, - buildFiles - ); - } - - private ResolveResult resolve(ProjectConfig config) { - Set mavenDependencies = resolveMaven(config.maven()); - Set projectDependencies = resolveProjectDependencies(config.projectDependencies()); + private ProjectConfig resolve( + List sources, + List imports, + MavenConfig mavenConfig, + List projectDependencies + ) { + Set resolvedMaven = resolveMaven(mavenConfig); + Set resolveProjectDependencies = resolveProjectDependencies(projectDependencies); List resolvedDependencies = new ArrayList<>(); try { - for (var dep : mavenDependencies) { + for (var dep : resolvedMaven) { resolvedDependencies.add(dep.toUri().toURL()); } - for (var dep : projectDependencies) { + for (var dep : resolveProjectDependencies) { resolvedDependencies.add(dep.toUri().toURL()); } } catch (MalformedURLException e) { throw new RuntimeException(e); } - Set uniqueModelPaths = collectAllSmithyFilePaths(config.sources(), config.imports()); + Set uniqueModelPaths = collectAllSmithyFilePaths(sources, imports); List modelPaths = new ArrayList<>(uniqueModelPaths); - return new ResolveResult(modelPaths, resolvedDependencies, events); + return new ProjectConfig( + sources, + imports, + projectDependencies, + mavenConfig, + modelPaths, + resolvedDependencies + ); } private Set resolveMaven(MavenConfig maven) { - if (maven.getRepositories().isEmpty() && maven.getDependencies().isEmpty()) { + if (maven == null || (maven.getRepositories().isEmpty() && maven.getDependencies().isEmpty())) { return Set.of(); } @@ -339,6 +340,27 @@ private Set resolveMaven(MavenConfig maven) { return dependencyPaths; } + // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos + private static Set getConfiguredMavenRepos(MavenConfig config) { + Set repositories = new LinkedHashSet<>(); + + String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); + if (envRepos != null) { + for (String repo : envRepos.split("\\|")) { + repositories.add(MavenRepository.builder().url(repo.trim()).build()); + } + } + + Set configuredRepos = config.getRepositories(); + + if (!configuredRepos.isEmpty()) { + repositories.addAll(configuredRepos); + } else if (envRepos == null) { + repositories.add(CENTRAL.get()); + } + return repositories; + } + private void handleDependencyResolverExceptions(List exceptions) { if (exceptions.isEmpty()) { return; @@ -413,27 +435,6 @@ private void handleNotFoundProjectDependencies(Set notFound) { } } - // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos - private static Set getConfiguredMavenRepos(MavenConfig config) { - Set repositories = new LinkedHashSet<>(); - - String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); - if (envRepos != null) { - for (String repo : envRepos.split("\\|")) { - repositories.add(MavenRepository.builder().url(repo.trim()).build()); - } - } - - Set configuredRepos = config.getRepositories(); - - if (!configuredRepos.isEmpty()) { - repositories.addAll(configuredRepos); - } else if (envRepos == null) { - repositories.add(CENTRAL.get()); - } - return repositories; - } - // sources and imports can contain directories or files, relative or absolute. // Note: The model assembler can handle loading all smithy files in a directory, so there's some potential // here for inconsistent behavior. @@ -548,10 +549,4 @@ private static void collectFile(Collection accumulator, Path target) { accumulator.add(target); } } - - private record ResolveResult( - List modelPaths, - List resolvedDependencies, - List events - ) {} } 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 461bf67c..6e1823f0 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java @@ -57,11 +57,13 @@ public static Project loadDetached(String uri, String text) { return new Project( path, config, + BuildFiles.empty(), result.smithyFiles(), result.assemblerFactory(), Project.Type.DETACHED, result.modelResult(), - result.rebuildIndex() + result.rebuildIndex(), + List.of() ); } @@ -87,17 +89,19 @@ public static Project load(Path root, ManagedFiles managedFiles) throws Exceptio return Project.empty(root); } - ProjectConfig config = new ProjectConfigLoader(root, buildFiles).load(); - LoadModelResult result = doLoad(managedFiles, config); + ProjectConfigLoader.Result configResult = new ProjectConfigLoader(root, buildFiles).load(); + LoadModelResult result = doLoad(managedFiles, configResult.config()); return new Project( root, - config, + configResult.config(), + buildFiles, result.smithyFiles(), result.assemblerFactory(), Project.Type.NORMAL, result.modelResult(), - result.rebuildIndex() + result.rebuildIndex(), + configResult.events() ); } diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java index f1f11ff1..422e87f0 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java @@ -38,7 +38,7 @@ public class ProjectConfigTest { public void loadsConfigWithEnvVariable() { System.setProperty("FOO", "bar"); Path root = toPath(getClass().getResource("env-config")); - ProjectConfig config = load(root); + ProjectConfig config = load(root).config(); assertThat(config.maven().getRepositories(), hasSize(1)); MavenRepository repository = config.maven().getRepositories().stream().findFirst().get(); @@ -50,7 +50,7 @@ public void loadsConfigWithEnvVariable() { @Test public void loadsLegacyConfig() { Path root = toPath(getClass().getResource("legacy-config")); - ProjectConfig config = load(root); + ProjectConfig config = load(root).config(); assertThat(config.maven().getDependencies(), containsInAnyOrder("baz")); assertThat(config.maven().getRepositories().stream() @@ -61,7 +61,7 @@ public void loadsLegacyConfig() { @Test public void prefersNonLegacyConfig() { Path root = toPath(getClass().getResource("legacy-config-with-conflicts")); - ProjectConfig config = load(root); + ProjectConfig config = load(root).config(); assertThat(config.maven().getDependencies(), containsInAnyOrder("dep1", "dep2")); assertThat(config.maven().getRepositories().stream() @@ -72,7 +72,7 @@ public void prefersNonLegacyConfig() { @Test public void mergesBuildExts() { Path root = toPath(getClass().getResource("build-exts")); - ProjectConfig config = load(root); + ProjectConfig config = load(root).config(); assertThat(config.imports(), containsInAnyOrder(containsString("main.smithy"), containsString("other.smithy"))); } @@ -86,14 +86,16 @@ public void validatesSmithyBuildJson() { } """); var eventPosition = text.positions()[0]; - var config = load(Path.of("test"), BuildFileType.SMITHY_BUILD, text.text()); + Path root = Path.of("test"); + var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); + var result = load(root, buildFiles); - var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + var buildFile = buildFiles.getByType(BuildFileType.SMITHY_BUILD); assertThat(buildFile, notNullValue()); - assertThat(config.events(), containsInAnyOrder( + assertThat(result.events(), containsInAnyOrder( eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) )); - assertThat(config.sources(), empty()); + assertThat(result.config().sources(), empty()); } @Test @@ -107,14 +109,16 @@ public void validatesSmithyProjectJson() { } """); var eventPosition = text.positions()[0]; - var config = load(Path.of("test"), BuildFileType.SMITHY_PROJECT, text.text()); + Path root = Path.of("test"); + var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_PROJECT, text.text()); + var result = load(root, buildFiles); - var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_PROJECT); + var buildFile = buildFiles.getByType(BuildFileType.SMITHY_PROJECT); assertThat(buildFile, notNullValue()); - assertThat(config.events(), containsInAnyOrder( + assertThat(result.events(), containsInAnyOrder( eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) )); - assertThat(config.sources(), empty()); + assertThat(result.config().sources(), empty()); } @Test @@ -135,15 +139,17 @@ public void validatesMavenConfig() { } """); var eventPosition = text.positions()[0]; - var config = load(Path.of("test"), BuildFileType.SMITHY_BUILD, text.text()); + Path root = Path.of("test"); + var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); + var result = load(root, buildFiles); - var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + var buildFile = buildFiles.getByType(BuildFileType.SMITHY_BUILD); assertThat(buildFile, notNullValue()); - assertThat(config.events(), containsInAnyOrder(allOf( + assertThat(result.events(), containsInAnyOrder(allOf( eventWithMessage(containsString("httpCredentials")), eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), eventPosition))) ))); - assertThat(config.sources(), empty()); + assertThat(result.config().sources(), empty()); } @Test @@ -164,11 +170,12 @@ public void resolveValidatesFilesExist() { var notFoundImportPosition = text.positions()[1]; var notFoundDepPosition = text.positions()[2]; var root = Path.of("test"); - var config = load(root, BuildFileType.SMITHY_PROJECT, text.text()); + var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_PROJECT, text.text()); + var result = load(root, buildFiles); - var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_PROJECT); + var buildFile = buildFiles.getByType(BuildFileType.SMITHY_PROJECT); assertThat(buildFile, notNullValue()); - assertThat(config.events(), containsInAnyOrder( + assertThat(result.events(), containsInAnyOrder( allOf( eventWithId(equalTo("FileNotFound")), eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), notFoundSourcePosition))) @@ -182,9 +189,9 @@ public void resolveValidatesFilesExist() { eventWithSourceLocation(equalTo(toSourceLocation(buildFile.path(), notFoundDepPosition))) ) )); - assertThat(config.sources(), containsInAnyOrder(equalTo("foo"))); - assertThat(config.imports(), containsInAnyOrder(equalTo("bar"))); - assertThat(config.projectDependencies().getFirst().path(), equalTo("baz")); + assertThat(result.config().sources(), containsInAnyOrder(equalTo("foo"))); + assertThat(result.config().imports(), containsInAnyOrder(equalTo("bar"))); + assertThat(result.config().projectDependencies().getFirst().path(), equalTo("baz")); } @Test @@ -221,11 +228,11 @@ public List resolve() { } }; var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); - var config = new ProjectConfigLoader(root, buildFiles, resolverFactory).load(); + var result = new ProjectConfigLoader(root, buildFiles, resolverFactory).load(); - var buildFile = config.buildFiles().getByType(BuildFileType.SMITHY_BUILD); + var buildFile = buildFiles.getByType(BuildFileType.SMITHY_BUILD); assertThat(buildFile, notNullValue()); - assertThat(config.events(), containsInAnyOrder( + assertThat(result.events(), containsInAnyOrder( allOf( eventWithId(equalTo("DependencyResolver")), eventWithMessage(allOf( @@ -258,13 +265,12 @@ private static BuildFiles createBuildFiles(Path root, BuildFileType type, String return BuildFiles.of(List.of(buildFile)); } - private static ProjectConfig load(Path root, BuildFileType type, String content) { - var buildFiles = createBuildFiles(root, type, content); + private static ProjectConfigLoader.Result load(Path root, BuildFiles buildFiles) { var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); return loader.load(); } - private static ProjectConfig load(Path root) { + private static ProjectConfigLoader.Result load(Path root) { var buildFiles = BuildFiles.load(root, new ServerState()); var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); return loader.load(); diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java index 5d015598..0e240790 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectLoaderTest.java @@ -156,7 +156,7 @@ public void loadsProjectWithInvalidSmithyBuildJson() { Project project = ProjectTest.load(root); assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); - assertThat(project.config().events(), hasItem(eventWithMessage(containsString("version")))); + assertThat(project.configEvents(), hasItem(eventWithMessage(containsString("version")))); assertThat(project.modelResult().isBroken(), is(false)); } @@ -166,7 +166,7 @@ public void loadsProjectWithUnparseableSmithyBuildJson() { Project project = ProjectTest.load(root); assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); - assertThat(project.config().events().isEmpty(), is(false)); + assertThat(project.configEvents().isEmpty(), is(false)); assertThat(project.modelResult().isBroken(), is(false)); } @@ -176,7 +176,7 @@ public void loadsProjectWithNonExistingSource() { Project project = ProjectTest.load(root); assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); - assertThat(project.config().events(), hasItem(eventWithId(equalTo("FileNotFound")))); + assertThat(project.configEvents(), hasItem(eventWithId(equalTo("FileNotFound")))); assertThat(project.modelResult().isBroken(), is(false)); assertThat(project.getAllSmithyFiles().size(), equalTo(1)); // still have the prelude } @@ -187,7 +187,7 @@ public void loadsProjectWithUnresolvableMavenDependency() { Project project = ProjectTest.load(root); assertHasBuildFile(project, BuildFileType.SMITHY_BUILD); - assertThat(project.config().events(), hasItem(eventWithId(equalTo("DependencyResolver")))); + assertThat(project.configEvents(), hasItem(eventWithId(equalTo("DependencyResolver")))); assertThat(project.modelResult().isBroken(), is(false)); } @@ -197,7 +197,7 @@ public void loadsProjectWithUnresolvableProjectDependency() { Project project = ProjectTest.load(root); assertHasBuildFile(project, BuildFileType.SMITHY_PROJECT); - assertThat(project.config().events(), hasItem(eventWithId(equalTo("FileNotFound")))); + assertThat(project.configEvents(), hasItem(eventWithId(equalTo("FileNotFound")))); assertThat(project.modelResult().isBroken(), is(false)); } From e4c93b083044e77e5e7cbd3d407995b339465146 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Mon, 20 Jan 2025 13:07:46 -0500 Subject: [PATCH 3/9] Add 'use-smithy-build' diagnostic --- .../lsp/diagnostics/SmithyDiagnostics.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java index a3e73be3..0413dac2 100644 --- a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java +++ b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java @@ -33,9 +33,12 @@ public final class SmithyDiagnostics { public static final String UPDATE_VERSION = "migrating-idl-1-to-2"; public static final String DEFINE_VERSION = "define-idl-version"; public static final String DETACHED_FILE = "detached-file"; + public static final String USE_SMITHY_BUILD = "use-smithy-build"; private static final DiagnosticCodeDescription UPDATE_VERSION_DESCRIPTION = new DiagnosticCodeDescription("https://smithy.io/2.0/guides/migrating-idl-1-to-2.html"); + private static final DiagnosticCodeDescription USE_SMITHY_BUILD_DESCRIPTION = + new DiagnosticCodeDescription("https://smithy.io/2.0/guides/smithy-build-json.html#using-smithy-build-json"); private SmithyDiagnostics() { } @@ -161,7 +164,24 @@ public EventToDiagnostic getEventToDiagnostic() { @Override public void addExtraDiagnostics(List diagnostics) { - // TODO: Add warning diagnostic to build ext files + switch (buildFile.type()) { + case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> diagnostics.add(useSmithyBuild()); + } + } + + private Diagnostic useSmithyBuild() { + Range range = LspAdapter.origin(); + Diagnostic diagnostic = createDiagnostic( + range, + String.format(""" + You should use smithy-build.json as your build configuration file for Smithy. + The %s file is not supported by Smithy, and support from the language server + will be removed in a later version. + """, buildFile.type().filename()), + USE_SMITHY_BUILD + ); + diagnostic.setCodeDescription(USE_SMITHY_BUILD_DESCRIPTION); + return diagnostic; } } From f0cd8cec32269ebd2ac663467722dac0670c8d56 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 21 Jan 2025 11:29:41 -0500 Subject: [PATCH 4/9] Checkstyle --- .../amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java | 2 ++ .../java/software/amazon/smithy/lsp/project/ProjectFile.java | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java index 0413dac2..ce9c7652 100644 --- a/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java +++ b/src/main/java/software/amazon/smithy/lsp/diagnostics/SmithyDiagnostics.java @@ -166,6 +166,8 @@ public EventToDiagnostic getEventToDiagnostic() { public void addExtraDiagnostics(List diagnostics) { switch (buildFile.type()) { case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> diagnostics.add(useSmithyBuild()); + default -> { + } } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java index 5948a748..0ddfe67f 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectFile.java @@ -23,7 +23,7 @@ public sealed interface ProjectFile permits SmithyFile, BuildFile { Document document(); /** - * Reparse the underlying document + * Reparse the underlying document. */ void reparse(); } From 8a43047c5ec2ebc353beabba39b8c9a19c99d45f Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 21 Jan 2025 11:54:17 -0500 Subject: [PATCH 5/9] Fix gitignored test resources --- .gitignore | 3 ++- .../software/amazon/smithy/lsp/project/BuildFileType.java | 2 +- .../software/amazon/smithy/lsp/project/ProjectConfigTest.java | 1 + .../lsp/project/build-exts/build/smithy-dependencies.json | 4 ++++ 4 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json diff --git a/.gitignore b/.gitignore index 5ae1ecbd..ebfbd49d 100644 --- a/.gitignore +++ b/.gitignore @@ -13,7 +13,8 @@ project/project .gradle # Ignore Gradle build output directory -build +# Note: Only ignore the top-level build dir, tests use dirs named 'build' which we don't want to ignore +/build bin diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java index c8f570ae..b007fb43 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java @@ -11,7 +11,7 @@ public enum BuildFileType { SMITHY_BUILD("smithy-build.json"), - SMITHY_BUILD_EXT_0("build" + File.separator + "smithy-projectDependencies.json"), + SMITHY_BUILD_EXT_0("build" + File.separator + "smithy-dependencies.json"), SMITHY_BUILD_EXT_1(".smithy.json"), SMITHY_PROJECT(".smithy-project.json"),; diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java index 422e87f0..03984eb3 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java @@ -75,6 +75,7 @@ public void mergesBuildExts() { ProjectConfig config = load(root).config(); assertThat(config.imports(), containsInAnyOrder(containsString("main.smithy"), containsString("other.smithy"))); + assertThat(config.maven().getDependencies(), containsInAnyOrder("foo")); } @Test diff --git a/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json b/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json new file mode 100644 index 00000000..1f9a1005 --- /dev/null +++ b/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json @@ -0,0 +1,4 @@ +{ + "version": "1", + "mavenDependencies": ["foo"] +} \ No newline at end of file From 034dbb522c4122e10276eeec138825782c7c03a4 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Wed, 22 Jan 2025 10:14:08 -0500 Subject: [PATCH 6/9] Refactoring, add docs --- .../amazon/smithy/lsp/project/BuildFile.java | 3 + .../smithy/lsp/project/BuildFileType.java | 41 ++ .../amazon/smithy/lsp/project/BuildFiles.java | 8 +- .../amazon/smithy/lsp/project/Project.java | 2 +- .../lsp/project/ProjectConfigLoader.java | 689 ++++++++++-------- .../smithy/lsp/project/ProjectLoader.java | 4 +- .../smithy/lsp/project/ToSmithyNode.java | 8 + .../smithy/lsp/project/ProjectConfigTest.java | 8 +- .../smithy/lsp/project/ToSmithyNodeTest.java | 78 ++ 9 files changed, 516 insertions(+), 325 deletions(-) create mode 100644 src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java index 2eb67e3c..97d55555 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFile.java @@ -59,6 +59,9 @@ public void reparse() { } } + /** + * @return The type of this build file + */ public BuildFileType type() { return type; } diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java index b007fb43..3e2d5082 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFileType.java @@ -9,12 +9,50 @@ import java.util.Arrays; import java.util.List; +/** + * The type of build file. + * + *

The language server supports loading project config from multiple kinds + * of files. + */ public enum BuildFileType { + /** + * Primary smithy-build configuration file used by most projects. + * + * @see software.amazon.smithy.build.model.SmithyBuildConfig + * @see smithy-build.json + */ SMITHY_BUILD("smithy-build.json"), + + /** + * A config file used specifically for the language server from before + * maven deps from smithy-build.json were supported. + * + * @see SmithyBuildExtensions + */ SMITHY_BUILD_EXT_0("build" + File.separator + "smithy-dependencies.json"), + + /** + * A config file used specifically for the language server from before + * maven deps from smithy-build.json were supported. + * + * @see SmithyBuildExtensions + */ SMITHY_BUILD_EXT_1(".smithy.json"), + + /** + * A config file used specifically for the language server to specify + * project config for a project that isn't specifying sources and + * dependencies in smithy-build.json, typically some external build + * system is being used. + * + * @see SmithyProjectJson + */ SMITHY_PROJECT(".smithy-project.json"),; + /** + * The filenames of all {@link BuildFileType}s. + */ public static final List ALL_FILENAMES = Arrays.stream(BuildFileType.values()) .map(BuildFileType::filename) .toList(); @@ -25,6 +63,9 @@ public enum BuildFileType { this.filename = filename; } + /** + * @return The filename that denotes this {@link BuildFileType}. + */ public String filename() { return filename; } diff --git a/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java index 01e420fe..c9123b84 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java +++ b/src/main/java/software/amazon/smithy/lsp/project/BuildFiles.java @@ -16,6 +16,10 @@ import software.amazon.smithy.lsp.protocol.LspAdapter; import software.amazon.smithy.utils.IoUtils; +/** + * Immutable container for multiple {@link BuildFile}s, with accessors by path + * and {@link BuildFileType}. + */ final class BuildFiles implements Iterable { private final Map buildFiles; @@ -45,10 +49,6 @@ boolean isEmpty() { return buildFiles.isEmpty(); } - static BuildFiles empty() { - return new BuildFiles(new HashMap<>(0)); - } - static BuildFiles of(Collection buildFiles) { Map buildFileMap = new HashMap<>(buildFiles.size()); for (BuildFile buildFile : buildFiles) { 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 1a8f1325..b2922b5d 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/Project.java +++ b/src/main/java/software/amazon/smithy/lsp/project/Project.java @@ -101,7 +101,7 @@ public enum Type { public static Project empty(Path root) { return new Project(root, ProjectConfig.empty(), - BuildFiles.empty(), + BuildFiles.of(List.of()), new HashMap<>(), Model::assembler, Type.EMPTY, diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java index a99add0a..c99897e1 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectConfigLoader.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Stream; @@ -40,106 +41,89 @@ import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.node.StringNode; import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; /** - * Loads and validates {@link ProjectConfig}s from {@link BuildFiles}. + * Loads {@link ProjectConfig}s from {@link BuildFiles}. */ final class ProjectConfigLoader { private static final NodeMapper NODE_MAPPER = new NodeMapper(); private static final BuildFileType[] EXTS = {BuildFileType.SMITHY_BUILD_EXT_0, BuildFileType.SMITHY_BUILD_EXT_1}; - // Taken from smithy-cli ConfigurationUtils - private static final Supplier CENTRAL = () -> MavenRepository.builder() - .url("https://repo.maven.apache.org/maven2") - .build(); - private static final Supplier DEFAULT_RESOLVER_FACTORY = () -> - new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); - - private final Path root; + private final BuildFiles buildFiles; private final List events = new ArrayList<>(); private final Map smithyNodes = new HashMap<>(BuildFileType.values().length); - private final Supplier dependencyResolverFactory; - ProjectConfigLoader(Path root, BuildFiles buildFiles, Supplier dependencyResolverFactory) { - this.root = root; + private ProjectConfigLoader(BuildFiles buildFiles) { this.buildFiles = buildFiles; - this.dependencyResolverFactory = dependencyResolverFactory; - } - - ProjectConfigLoader(Path root, BuildFiles buildFiles) { - this(root, buildFiles, DEFAULT_RESOLVER_FACTORY); - } - - private interface BuildFileLoader { - T validate(BuildFile source, Node node, Consumer eventConsumer); - } - - private record SmithyBuildJsonLoader() implements BuildFileLoader { - @Override - public SmithyBuildConfig validate(BuildFile source, Node node, Consumer eventConsumer) { - try { - return SmithyBuildConfig.fromNode(node); - } catch (Exception e) { - eventConsumer.accept(toEvent(e, source)); - } - return null; - } - } - - private record SmithyBuildExtLoader() implements BuildFileLoader { - @Override - public SmithyBuildExtensions validate(BuildFile source, Node node, Consumer eventConsumer) { - try { - var config = NODE_MAPPER.deserialize(node, SmithyBuildExtensions.class); - config.mergeMavenFromSmithyBuildConfig(SmithyBuildConfig.fromNode(node)); - return config; - } catch (Exception e) { - eventConsumer.accept(toEvent(e, source)); - } - return null; - } - } - - private record SmithyProjectJsonLoader() implements BuildFileLoader { - @Override - public SmithyProjectJson validate(BuildFile source, Node node, Consumer eventConsumer) { - try { - return SmithyProjectJson.fromNode(node); - } catch (Exception e) { - eventConsumer.accept(toEvent(e, source)); - } - return null; - } } + /** + * Runs structural validation on each of the given {@link BuildFiles}, + * without performing dependency resolution or constructing a new + * {@link ProjectConfig}. + * + * @param buildFiles The build files to validate + * @return The list of validation events + */ static List validateBuildFiles(BuildFiles buildFiles) { List events = new ArrayList<>(); for (BuildFile buildFile : buildFiles) { - BuildFileLoader loader = switch (buildFile.type()) { - case SMITHY_BUILD -> new SmithyBuildJsonLoader(); - case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> new SmithyBuildExtLoader(); - case SMITHY_PROJECT -> new SmithyProjectJsonLoader(); + LoadBuildFile loader = switch (buildFile.type()) { + case SMITHY_BUILD -> LoadBuildFile.LOAD_SMITHY_BUILD; + case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> LoadBuildFile.LOAD_BUILD_EXT; + case SMITHY_PROJECT -> LoadBuildFile.LOAD_SMITHY_PROJECT; }; - loadFile(buildFile, loader, events::add, (ignored) -> { + loadFile(buildFile, loader, events::add, (type, node) -> { }); } return events; } + /** + * Result of loading the config. Used in place of {@link ValidatedResult} + * because its value may not be present, which we don't want here. + * + * @param config The loaded config, non-nullable + * @param events The events that occurred during loading, non-nullable + */ record Result(ProjectConfig config, List events) {} - Result load() { - SmithyBuildConfig smithyBuildConfig = loadFile(BuildFileType.SMITHY_BUILD, new SmithyBuildJsonLoader()); - SmithyBuildExtensions.Builder extBuilder = loadExts(); - SmithyBuildConfig merged = mergeSmithyBuildConfig(smithyBuildConfig, extBuilder); - SmithyProjectJson smithyProjectJson = loadFile(BuildFileType.SMITHY_PROJECT, new SmithyProjectJsonLoader()); + /** + * Loads a project's config from the given {@link BuildFiles}, resolving + * dependencies using the default Maven dependency resolver. + * + * @param root The root of the project whose config is being loaded + * @param buildFiles The build files to load config from + * @return The result of loading the config + */ + static Result load(Path root, BuildFiles buildFiles) { + return load(root, buildFiles, Resolver.DEFAULT_RESOLVER_FACTORY); + } + + /** + * Loads a project's config from the given {@link BuildFiles}, resolving + * dependencies using the given factory. + * + * @param root The root of the project whose config is being loaded + * @param buildFiles The build files to load config from + * @param dependencyResolverFactory A factory to get the Maven dependency + * resolver to use + * @return The result of loading the config + */ + static Result load(Path root, BuildFiles buildFiles, Supplier dependencyResolverFactory) { + var loader = new ProjectConfigLoader(buildFiles); + SmithyBuildConfig smithyBuildConfig = loader.loadSmithyBuild(); + SmithyBuildExtensions.Builder extBuilder = loader.loadExts(); + SmithyBuildConfig merged = loader.mergeSmithyBuildConfig(smithyBuildConfig, extBuilder); + SmithyProjectJson smithyProjectJson = loader.loadSmithyProject(); List sources = new ArrayList<>(); List imports = new ArrayList<>(); - List projectDependencies = new ArrayList<>(); MavenConfig mavenConfig = null; + List projectDependencies = new ArrayList<>(); if (merged != null) { sources.addAll(merged.getSources()); @@ -156,39 +140,39 @@ Result load() { projectDependencies.addAll(smithyProjectJson.dependencies()); } - ProjectConfig resolved = resolve(sources, imports, mavenConfig, projectDependencies); + var resolver = new Resolver(root, loader.events, loader.smithyNodes, dependencyResolverFactory); + ProjectConfig resolved = resolver.resolve(sources, imports, mavenConfig, projectDependencies); - return new Result(resolved, events); + return new Result(resolved, resolver.events()); } - private T loadFile(BuildFileType type, BuildFileLoader loader) { - var buildFile = buildFiles.getByType(type); - if (buildFile != null) { - return loadFile(buildFile, loader, events::add, (node) -> smithyNodes.put(type, node)); - } - return null; + private SmithyBuildConfig loadSmithyBuild() { + return loadFile( + buildFiles.getByType(BuildFileType.SMITHY_BUILD), + LoadBuildFile.LOAD_SMITHY_BUILD, + events::add, + smithyNodes::put + ); } - private static T loadFile( - BuildFile buildFile, - BuildFileLoader loader, - Consumer eventConsumer, - Consumer nodeConsumer - ) { - var nodeResult = ToSmithyNode.toSmithyNode(buildFile); - nodeResult.getValidationEvents().forEach(eventConsumer); - var smithyNode = nodeResult.getResult().orElse(null); - if (smithyNode != null) { - nodeConsumer.accept(smithyNode); - return loader.validate(buildFile, smithyNode, eventConsumer); - } - return null; + private SmithyProjectJson loadSmithyProject() { + return loadFile( + buildFiles.getByType(BuildFileType.SMITHY_PROJECT), + LoadBuildFile.LOAD_SMITHY_PROJECT, + events::add, + smithyNodes::put + ); } private SmithyBuildExtensions.Builder loadExts() { SmithyBuildExtensions.Builder extBuilder = null; for (BuildFileType extType : EXTS) { - SmithyBuildExtensions ext = loadFile(extType, new SmithyBuildExtLoader()); + SmithyBuildExtensions ext = loadFile( + buildFiles.getByType(extType), + LoadBuildFile.LOAD_BUILD_EXT, + events::add, + smithyNodes::put + ); if (ext != null) { if (extBuilder == null) { extBuilder = SmithyBuildExtensions.builder(); @@ -199,6 +183,31 @@ private SmithyBuildExtensions.Builder loadExts() { return extBuilder; } + private static T loadFile( + BuildFile buildFile, + LoadBuildFile loadBuildFile, + Consumer eventConsumer, + BiConsumer nodeConsumer + ) { + if (buildFile == null) { + return null; + } + + ValidatedResult nodeResult = ToSmithyNode.toSmithyNode(buildFile); + nodeResult.getValidationEvents().forEach(eventConsumer); + Node smithyNode = nodeResult.getResult().orElse(null); + if (smithyNode != null) { + nodeConsumer.accept(buildFile.type(), smithyNode); + try { + return loadBuildFile.load(smithyNode); + } catch (Exception e) { + eventConsumer.accept(toEvent(e, buildFile)); + } + } + + return null; + } + private SmithyBuildConfig mergeSmithyBuildConfig( SmithyBuildConfig smithyBuildConfig, SmithyBuildExtensions.Builder extBuilder @@ -239,6 +248,9 @@ private SmithyBuildConfig mergeSmithyBuildConfig( } private static ValidationEvent toEvent(Exception e, BuildFile fallbackBuildFile) { + // Most exceptions thrown will be from structural validation, i.e. is the Node in the expected format for the + // build file. These exceptions will be SourceExceptions most likely, which are easy to map to a source + // location. SourceException asSourceException = null; if (e instanceof SourceException sourceException) { asSourceException = sourceException; @@ -246,10 +258,13 @@ private static ValidationEvent toEvent(Exception e, BuildFile fallbackBuildFile) asSourceException = sourceException; } + // If the source location is NONE, the filename won't map to any actual file so you won't see the error if (asSourceException != null && !SourceLocation.NONE.equals(asSourceException.getSourceLocation())) { return ValidationEvent.fromSourceException(asSourceException); } + // Worst case, just put the error at the top of the file. If this happens enough that it is a problem, we + // can revisit how this validation works, or manually map the specific cases. return ValidationEvent.builder() .id("SmithyBuildConfig") .severity(Severity.ERROR) @@ -258,295 +273,343 @@ private static ValidationEvent toEvent(Exception e, BuildFile fallbackBuildFile) .build(); } - private ProjectConfig resolve( - List sources, - List imports, - MavenConfig mavenConfig, - List projectDependencies - ) { - Set resolvedMaven = resolveMaven(mavenConfig); - Set resolveProjectDependencies = resolveProjectDependencies(projectDependencies); + /** + * Strategy for deserializing a {@link Node} into a {@code T}, differently + * depending on {@link BuildFileType}. + * + * @param The deserialized type + */ + private interface LoadBuildFile { + LoadBuildFile LOAD_SMITHY_BUILD = SmithyBuildConfig::fromNode; - List resolvedDependencies = new ArrayList<>(); - try { - for (var dep : resolvedMaven) { - resolvedDependencies.add(dep.toUri().toURL()); - } - for (var dep : resolveProjectDependencies) { - resolvedDependencies.add(dep.toUri().toURL()); - } - } catch (MalformedURLException e) { - throw new RuntimeException(e); - } + LoadBuildFile LOAD_BUILD_EXT = (node) -> { + var config = NODE_MAPPER.deserialize(node, SmithyBuildExtensions.class); + config.mergeMavenFromSmithyBuildConfig(SmithyBuildConfig.fromNode(node)); + return config; + }; - Set uniqueModelPaths = collectAllSmithyFilePaths(sources, imports); - List modelPaths = new ArrayList<>(uniqueModelPaths); + LoadBuildFile LOAD_SMITHY_PROJECT = SmithyProjectJson::fromNode; - return new ProjectConfig( - sources, - imports, - projectDependencies, - mavenConfig, - modelPaths, - resolvedDependencies - ); + T load(Node node); } - private Set resolveMaven(MavenConfig maven) { - if (maven == null || (maven.getRepositories().isEmpty() && maven.getDependencies().isEmpty())) { - return Set.of(); + /** + * Handles resolving dependencies, and finding all model paths that will be + * loaded in the project. It also keeps track of any errors that occur in + * this process, and tries to map them back to a specific location in a + * build file so we can show a diagnostic. + * + * @param root The root of the project, used to resolve model paths + * @param events The list to add any events that occur to + * @param smithyNodes The loaded smithy nodes for each build file type, + * used to map errors to a specific location + * @param dependencyResolverFactory Provides the Maven dependency resolver + * implementation to use + */ + private record Resolver( + Path root, + List events, + Map smithyNodes, + Supplier dependencyResolverFactory + ) { + // Taken from smithy-cli ConfigurationUtils + private static final Supplier CENTRAL = () -> MavenRepository.builder() + .url("https://repo.maven.apache.org/maven2") + .build(); + private static final Supplier DEFAULT_RESOLVER_FACTORY = () -> + new MavenDependencyResolver(EnvironmentVariable.SMITHY_MAVEN_CACHE.get()); + + private ProjectConfig resolve( + List sources, + List imports, + MavenConfig mavenConfig, + List projectDependencies + ) { + Set resolvedMaven = resolveMaven(mavenConfig); + Set resolveProjectDependencies = resolveProjectDependencies(projectDependencies); + + List resolvedDependencies = new ArrayList<>(); + try { + for (var dep : resolvedMaven) { + resolvedDependencies.add(dep.toUri().toURL()); + } + for (var dep : resolveProjectDependencies) { + resolvedDependencies.add(dep.toUri().toURL()); + } + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + + Set uniqueModelPaths = collectAllSmithyFilePaths(sources, imports); + List modelPaths = new ArrayList<>(uniqueModelPaths); + + return new ProjectConfig( + sources, + imports, + projectDependencies, + mavenConfig, + modelPaths, + resolvedDependencies + ); } - List exceptions = new ArrayList<>(); - DependencyResolver resolver = dependencyResolverFactory.get(); + private Set resolveMaven(MavenConfig maven) { + if (maven == null || (maven.getRepositories().isEmpty() && maven.getDependencies().isEmpty())) { + return Set.of(); + } - Set repositories = getConfiguredMavenRepos(maven); - for (MavenRepository repo : repositories) { - try { - resolver.addRepository(repo); - } catch (DependencyResolverException e) { - exceptions.add(e); + List exceptions = new ArrayList<>(); + DependencyResolver resolver = dependencyResolverFactory.get(); + + Set repositories = getConfiguredMavenRepos(maven); + for (MavenRepository repo : repositories) { + try { + resolver.addRepository(repo); + } catch (DependencyResolverException e) { + exceptions.add(e); + } + } + + for (String dependency : maven.getDependencies()) { + try { + resolver.addDependency(dependency); + } catch (DependencyResolverException e) { + exceptions.add(e); + } } - } - for (String dependency : maven.getDependencies()) { + List resolvedArtifacts; try { - resolver.addDependency(dependency); + resolvedArtifacts = resolver.resolve(); } catch (DependencyResolverException e) { exceptions.add(e); + resolvedArtifacts = List.of(); } - } - - List resolvedArtifacts; - try { - resolvedArtifacts = resolver.resolve(); - } catch (DependencyResolverException e) { - exceptions.add(e); - resolvedArtifacts = List.of(); - } - handleDependencyResolverExceptions(exceptions); + handleDependencyResolverExceptions(exceptions); - Set dependencyPaths = new HashSet<>(resolvedArtifacts.size()); - for (ResolvedArtifact resolvedArtifact : resolvedArtifacts) { - Path path = resolvedArtifact.getPath().toAbsolutePath(); - if (!Files.exists(path)) { - throw new RuntimeException( - "Dependency was resolved (" + resolvedArtifact + "), but it was not found on disk at " + path); + Set dependencyPaths = new HashSet<>(resolvedArtifacts.size()); + for (ResolvedArtifact resolvedArtifact : resolvedArtifacts) { + Path path = resolvedArtifact.getPath().toAbsolutePath(); + if (!Files.exists(path)) { + throw new RuntimeException(String.format( + "Dependency was resolved (%s), but it was not found on disk at %s", + resolvedArtifact, path)); + } + dependencyPaths.add(path); } - dependencyPaths.add(path); - } - return dependencyPaths; - } + return dependencyPaths; + } - // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos - private static Set getConfiguredMavenRepos(MavenConfig config) { - Set repositories = new LinkedHashSet<>(); + // Taken from smithy-cli ConfigurationUtils::getConfiguredMavenRepos + private static Set getConfiguredMavenRepos(MavenConfig config) { + Set repositories = new LinkedHashSet<>(); - String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); - if (envRepos != null) { - for (String repo : envRepos.split("\\|")) { - repositories.add(MavenRepository.builder().url(repo.trim()).build()); + String envRepos = EnvironmentVariable.SMITHY_MAVEN_REPOS.get(); + if (envRepos != null) { + for (String repo : envRepos.split("\\|")) { + repositories.add(MavenRepository.builder().url(repo.trim()).build()); + } } - } - - Set configuredRepos = config.getRepositories(); - if (!configuredRepos.isEmpty()) { - repositories.addAll(configuredRepos); - } else if (envRepos == null) { - repositories.add(CENTRAL.get()); - } - return repositories; - } + Set configuredRepos = config.getRepositories(); - private void handleDependencyResolverExceptions(List exceptions) { - if (exceptions.isEmpty()) { - return; + if (!configuredRepos.isEmpty()) { + repositories.addAll(configuredRepos); + } else if (envRepos == null) { + repositories.add(CENTRAL.get()); + } + return repositories; } - StringBuilder builder = new StringBuilder(); - for (DependencyResolverException exception : exceptions) { - builder.append(exception.getMessage()).append("\n"); - } - String message = builder.toString(); + private void handleDependencyResolverExceptions(List exceptions) { + if (exceptions.isEmpty()) { + return; + } - for (Node smithyNode : smithyNodes.values()) { - if (!(smithyNode instanceof ObjectNode objectNode)) { - continue; + StringBuilder builder = new StringBuilder(); + for (DependencyResolverException exception : exceptions) { + builder.append(exception.getMessage()).append("\n"); } + String message = builder.toString(); - for (StringNode memberNameNode : objectNode.getMembers().keySet()) { - String memberName = memberNameNode.getValue(); - if ("maven".equals(memberName)) { - events.add(ValidationEvent.builder() - .id("DependencyResolver") - .severity(Severity.ERROR) - .message("Dependency resolution failed: " + message) - .sourceLocation(memberNameNode) - .build()); - break; + for (Node smithyNode : smithyNodes.values()) { + if (!(smithyNode instanceof ObjectNode objectNode)) { + continue; + } + + for (StringNode memberNameNode : objectNode.getMembers().keySet()) { + String memberName = memberNameNode.getValue(); + if ("maven".equals(memberName)) { + events.add(ValidationEvent.builder() + .id("DependencyResolver") + .severity(Severity.ERROR) + .message("Dependency resolution failed: " + message) + .sourceLocation(memberNameNode) + .build()); + break; + } } } } - } - private Set resolveProjectDependencies(List projectDependencies) { - Set notFoundDependencies = new HashSet<>(); - Set dependencies = new HashSet<>(); + private Set resolveProjectDependencies(List projectDependencies) { + Set notFoundDependencies = new HashSet<>(); + Set dependencies = new HashSet<>(); - for (var dependency : projectDependencies) { - Path path = root.resolve(dependency.path()).normalize(); - if (!Files.exists(path)) { - notFoundDependencies.add(dependency.path()); - } else { - dependencies.add(path); + for (var dependency : projectDependencies) { + Path path = root.resolve(dependency.path()).normalize(); + if (!Files.exists(path)) { + notFoundDependencies.add(dependency.path()); + } else { + dependencies.add(path); + } } - } - - handleNotFoundProjectDependencies(notFoundDependencies); - return dependencies; - } + handleNotFoundProjectDependencies(notFoundDependencies); - private void handleNotFoundProjectDependencies(Set notFound) { - if (notFound.isEmpty()) { - return; + return dependencies; } - if (!(smithyNodes.get(BuildFileType.SMITHY_PROJECT) instanceof ObjectNode objectNode)) { - return; - } + private void handleNotFoundProjectDependencies(Set notFound) { + if (notFound.isEmpty()) { + return; + } + + if (!(smithyNodes.get(BuildFileType.SMITHY_PROJECT) instanceof ObjectNode objectNode)) { + return; + } - if (objectNode.getMember("dependencies").orElse(null) instanceof ArrayNode arrayNode) { - for (Node elem : arrayNode) { - if (elem instanceof ObjectNode depNode - && depNode.getMember("path").orElse(null) instanceof StringNode depPathNode - && notFound.contains(depPathNode.getValue())) { - events.add(ValidationEvent.builder() - .id("FileNotFound") - .severity(Severity.ERROR) - .message("File not found") - .sourceLocation(depPathNode) - .build()); + if (objectNode.getMember("dependencies").orElse(null) instanceof ArrayNode arrayNode) { + for (Node elem : arrayNode) { + if (elem instanceof ObjectNode depNode + && depNode.getMember("path").orElse(null) instanceof StringNode depPathNode + && notFound.contains(depPathNode.getValue())) { + events.add(ValidationEvent.builder() + .id("FileNotFound") + .severity(Severity.ERROR) + .message("File not found") + .sourceLocation(depPathNode) + .build()); + } } } } - } - // sources and imports can contain directories or files, relative or absolute. - // Note: The model assembler can handle loading all smithy files in a directory, so there's some potential - // here for inconsistent behavior. - private Set collectAllSmithyFilePaths(List sources, List imports) { - Set notFound = new HashSet<>(); - Set paths = new HashSet<>(); + // sources and imports can contain directories or files, relative or absolute. + // Note: The model assembler can handle loading all smithy files in a directory, so there's some potential + // here for inconsistent behavior. + private Set collectAllSmithyFilePaths(List sources, List imports) { + Set notFound = new HashSet<>(); + Set paths = new HashSet<>(); - collectFilePaths(paths, sources, notFound); - collectFilePaths(paths, imports, notFound); + collectFilePaths(paths, sources, notFound); + collectFilePaths(paths, imports, notFound); - handleNotFoundSourcesAndImports(notFound); + handleNotFoundSourcesAndImports(notFound); - return paths; - } + return paths; + } - private void collectFilePaths(Set accumulator, List paths, Set notFound) { - for (String file : paths) { - Path path = root.resolve(file).normalize(); - if (!Files.exists(path)) { - notFound.add(path.toString()); - } else { - collectDirectory(accumulator, root, path); + private void collectFilePaths(Set accumulator, List paths, Set notFound) { + for (String file : paths) { + Path path = root.resolve(file).normalize(); + if (!Files.exists(path)) { + notFound.add(path.toString()); + } else { + collectDirectory(accumulator, root, path); + } } } - } - private void handleNotFoundSourcesAndImports(Set notFound) { - for (Node smithyNode : smithyNodes.values()) { - if (!(smithyNode instanceof ObjectNode objectNode)) { - continue; - } + private void handleNotFoundSourcesAndImports(Set notFound) { + for (Node smithyNode : smithyNodes.values()) { + if (!(smithyNode instanceof ObjectNode objectNode)) { + continue; + } - if (objectNode.getMember("sources").orElse(null) instanceof ArrayNode sourcesNode) { - addNotFoundEvents(sourcesNode, notFound); - } + if (objectNode.getMember("sources").orElse(null) instanceof ArrayNode sourcesNode) { + addNotFoundEvents(sourcesNode, notFound); + } - if (objectNode.getMember("imports").orElse(null) instanceof ArrayNode importsNode) { - addNotFoundEvents(importsNode, notFound); + if (objectNode.getMember("imports").orElse(null) instanceof ArrayNode importsNode) { + addNotFoundEvents(importsNode, notFound); + } } } - } - private void addNotFoundEvents(ArrayNode searchNode, Set notFound) { - for (Node elem : searchNode) { - if (elem instanceof StringNode stringNode) { - String fullPath = root.resolve(stringNode.getValue()).normalize().toString(); - if (notFound.contains(fullPath)) { - events.add(ValidationEvent.builder() - .id("FileNotFound") - .severity(Severity.ERROR) - .message("File not found: " + fullPath) - .sourceLocation(stringNode) - .build()); + private void addNotFoundEvents(ArrayNode searchNode, Set notFound) { + for (Node elem : searchNode) { + if (elem instanceof StringNode stringNode) { + String fullPath = root.resolve(stringNode.getValue()).normalize().toString(); + if (notFound.contains(fullPath)) { + events.add(ValidationEvent.builder() + .id("FileNotFound") + .severity(Severity.ERROR) + .message("File not found: " + fullPath) + .sourceLocation(stringNode) + .build()); + } } } } - } - // All of this copied from smithy-build SourcesPlugin, except I changed the `accumulator` to - // be a Collection instead of a list. - private static void collectDirectory(Collection accumulator, Path root, Path current) { - try { - if (Files.isDirectory(current)) { - try (Stream paths = Files.list(current)) { - paths.filter(p -> !p.equals(current)) - .filter(p -> Files.isDirectory(p) || Files.isRegularFile(p)) - .forEach(p -> collectDirectory(accumulator, root, p)); - } - } else if (Files.isRegularFile(current)) { - if (current.toString().endsWith(".jar")) { - String jarRoot = root.equals(current) - ? current.toString() - : (current + File.separator); - collectJar(accumulator, jarRoot, current); - } else { - collectFile(accumulator, current); + // All of this copied from smithy-build SourcesPlugin, except I changed the `accumulator` to + // be a Collection instead of a list. + private static void collectDirectory(Collection accumulator, Path root, Path current) { + try { + if (Files.isDirectory(current)) { + try (Stream paths = Files.list(current)) { + paths.filter(p -> !p.equals(current)) + .filter(p -> Files.isDirectory(p) || Files.isRegularFile(p)) + .forEach(p -> collectDirectory(accumulator, root, p)); + } + } else if (Files.isRegularFile(current)) { + if (current.toString().endsWith(".jar")) { + String jarRoot = root.equals(current) + ? current.toString() + : (current + File.separator); + collectJar(accumulator, jarRoot, current); + } else { + collectFile(accumulator, current); + } } + } catch (IOException ignored) { + // For now just ignore this - the assembler would have run into the same issues } - } catch (IOException ignored) { - // For now just ignore this - the assembler would have run into the same issues } - } - private static void collectJar(Collection accumulator, String jarRoot, Path jarPath) { - URL manifestUrl = ModelDiscovery.createSmithyJarManifestUrl(jarPath.toString()); + private static void collectJar(Collection accumulator, String jarRoot, Path jarPath) { + URL manifestUrl = ModelDiscovery.createSmithyJarManifestUrl(jarPath.toString()); - String prefix = computeJarFilePrefix(jarRoot, jarPath); - for (URL model : ModelDiscovery.findModels(manifestUrl)) { - String name = ModelDiscovery.getSmithyModelPathFromJarUrl(model); - Path target = Paths.get(prefix + name); - collectFile(accumulator, target); + String prefix = computeJarFilePrefix(jarRoot, jarPath); + for (URL model : ModelDiscovery.findModels(manifestUrl)) { + String name = ModelDiscovery.getSmithyModelPathFromJarUrl(model); + Path target = Paths.get(prefix + name); + collectFile(accumulator, target); + } } - } - - private static String computeJarFilePrefix(String jarRoot, Path jarPath) { - Path jarFilenamePath = jarPath.getFileName(); - if (jarFilenamePath == null) { - return jarRoot; - } + private static String computeJarFilePrefix(String jarRoot, Path jarPath) { + Path jarFilenamePath = jarPath.getFileName(); - String jarFilename = jarFilenamePath.toString(); - return jarRoot + jarFilename.substring(0, jarFilename.length() - ".jar".length()) + File.separator; - } + if (jarFilenamePath == null) { + return jarRoot; + } - private static void collectFile(Collection accumulator, Path target) { - if (target == null) { - return; + String jarFilename = jarFilenamePath.toString(); + return jarRoot + jarFilename.substring(0, jarFilename.length() - ".jar".length()) + File.separator; } - String filename = target.toString(); - if (filename.endsWith(".smithy") || filename.endsWith(".json")) { - accumulator.add(target); + + private static void collectFile(Collection accumulator, Path target) { + if (target == null) { + return; + } + String filename = target.toString(); + if (filename.endsWith(".smithy") || filename.endsWith(".json")) { + accumulator.add(target); + } } } } 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 6e1823f0..b2a18f9f 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java @@ -57,7 +57,7 @@ public static Project loadDetached(String uri, String text) { return new Project( path, config, - BuildFiles.empty(), + BuildFiles.of(List.of()), result.smithyFiles(), result.assemblerFactory(), Project.Type.DETACHED, @@ -89,7 +89,7 @@ public static Project load(Path root, ManagedFiles managedFiles) throws Exceptio return Project.empty(root); } - ProjectConfigLoader.Result configResult = new ProjectConfigLoader(root, buildFiles).load(); + ProjectConfigLoader.Result configResult = ProjectConfigLoader.load(root, buildFiles); LoadModelResult result = doLoad(managedFiles, configResult.config()); return new Project( diff --git a/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java index f711dd64..6a3c2e50 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java @@ -22,6 +22,14 @@ import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; +/** + * Converts a {@link BuildFile#getParse()} into a Smithy {@link Node}, + * and turning any parse errors into {@link ValidationEvent}s. + * + *

Since the language server's parser is much more lenient than the regular + * {@link Node} parser, the converted {@link Node} will contain only + * the parts of the original text that make up valid {@link Node}s. + */ final class ToSmithyNode { private final String path; private final Document document; diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java index 03984eb3..04f90c5e 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java @@ -229,7 +229,7 @@ public List resolve() { } }; var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); - var result = new ProjectConfigLoader(root, buildFiles, resolverFactory).load(); + var result = ProjectConfigLoader.load(root, buildFiles, resolverFactory); var buildFile = buildFiles.getByType(BuildFileType.SMITHY_BUILD); assertThat(buildFile, notNullValue()); @@ -267,13 +267,11 @@ private static BuildFiles createBuildFiles(Path root, BuildFileType type, String } private static ProjectConfigLoader.Result load(Path root, BuildFiles buildFiles) { - var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); - return loader.load(); + return ProjectConfigLoader.load(root, buildFiles, NoOpResolver::new); } private static ProjectConfigLoader.Result load(Path root) { var buildFiles = BuildFiles.load(root, new ServerState()); - var loader = new ProjectConfigLoader(root, buildFiles, NoOpResolver::new); - return loader.load(); + return ProjectConfigLoader.load(root, buildFiles, NoOpResolver::new); } } diff --git a/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java b/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java new file mode 100644 index 00000000..d987c9a0 --- /dev/null +++ b/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java @@ -0,0 +1,78 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.lsp.project; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static software.amazon.smithy.lsp.UtilMatchers.anOptionalOf; + +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.NodeType; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.validation.ValidatedResult; + +public class ToSmithyNodeTest { + @ParameterizedTest + @MethodSource("differentNodeTypesProvider") + public void convertsDifferentNodeTypes(String text, NodeType expectedNodeType) { + BuildFile buildFile = BuildFile.create("foo", Document.of(text), BuildFileType.SMITHY_BUILD); + ValidatedResult nodeResult = ToSmithyNode.toSmithyNode(buildFile); + + assertThat(nodeResult.getResult().map(Node::getType), anOptionalOf(equalTo(expectedNodeType))); + } + + private static Stream differentNodeTypesProvider() { + return Stream.of( + Arguments.of("null", NodeType.NULL), + Arguments.of("true", NodeType.BOOLEAN), + Arguments.of("false", NodeType.BOOLEAN), + Arguments.of("0", NodeType.NUMBER), + Arguments.of("\"foo\"", NodeType.STRING), + Arguments.of("[]", NodeType.ARRAY), + Arguments.of("{}", NodeType.OBJECT) + ); + } + + @Test + public void skipsMissingElements() { + var text = """ + { + "version": , + "imports": [ + , + "foo" + ], + "projections": { + , + "bar": {} + } + } + """; + BuildFile buildFile = BuildFile.create("foo", Document.of(text), BuildFileType.SMITHY_BUILD); + ValidatedResult nodeResult = ToSmithyNode.toSmithyNode(buildFile); + + ObjectNode node = nodeResult.getResult().get().expectObjectNode(); + assertThat(node.getStringMap().keySet(), containsInAnyOrder("imports", "projections")); + + List imports = node.expectArrayMember("imports") + .getElementsAs(elem -> elem.expectStringNode().getValue()); + assertThat(imports, containsInAnyOrder(equalTo("foo"))); + + Set projections = node.expectObjectMember("projections") + .getStringMap() + .keySet(); + assertThat(projections, containsInAnyOrder("bar")); + } +} From f444ebba134dd406ca8f804e604a407d6b97d15b Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 21 Jan 2025 16:53:51 -0500 Subject: [PATCH 7/9] Fix json node parsing When parsing a node in the idl, commas are treated as whitespace, but they aren't for json. --- .../amazon/smithy/lsp/syntax/Parser.java | 32 +++++++++++++++++-- .../amazon/smithy/lsp/syntax/Syntax.java | 4 +-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/main/java/software/amazon/smithy/lsp/syntax/Parser.java b/src/main/java/software/amazon/smithy/lsp/syntax/Parser.java index 9f2684be..ecc73da4 100644 --- a/src/main/java/software/amazon/smithy/lsp/syntax/Parser.java +++ b/src/main/java/software/amazon/smithy/lsp/syntax/Parser.java @@ -22,10 +22,20 @@ final class Parser extends SimpleParser { final List errors = new ArrayList<>(); final List statements = new ArrayList<>(); private final Document document; + private final boolean isJson; - Parser(Document document) { + private Parser(Document document, boolean isJson) { super(document.borrowText()); this.document = document; + this.isJson = isJson; + } + + static Parser forIdl(Document document) { + return new Parser(document, false); + } + + static Parser forJson(Document document) { + return new Parser(document, true); } Syntax.Node parseNode() { @@ -221,12 +231,25 @@ private Syntax.Node.Obj obj() { return obj; } + if (isJson && is(',')) { + Syntax.Node.Err err = new Syntax.Node.Err("expected key"); + setStart(err); + skip(); + setEnd(err); + ws(); + continue; + } + Syntax.Err kvpErr = kvp(obj.kvps, '}'); if (kvpErr != null) { addError(kvpErr); } ws(); + if (isJson && is(',')) { + skip(); + ws(); + } } Syntax.Node.Err err = new Syntax.Node.Err("missing }"); @@ -336,6 +359,10 @@ private Syntax.Node.Arr arr() { arr.elements.add(elem); } ws(); + if (is(',')) { + skip(); + } + ws(); } Syntax.Node.Err err = nodeErr("missing ]"); @@ -988,7 +1015,8 @@ private boolean isNl() { private boolean isWs() { return switch (peek()) { - case '\n', '\r', ' ', ',', '\t' -> true; + case '\n', '\r', ' ', '\t' -> true; + case ',' -> !isJson; default -> false; }; } diff --git a/src/main/java/software/amazon/smithy/lsp/syntax/Syntax.java b/src/main/java/software/amazon/smithy/lsp/syntax/Syntax.java index e6b27667..18182399 100644 --- a/src/main/java/software/amazon/smithy/lsp/syntax/Syntax.java +++ b/src/main/java/software/amazon/smithy/lsp/syntax/Syntax.java @@ -105,7 +105,7 @@ public record IdlParseResult( * @return The IDL parse result. */ public static IdlParseResult parseIdl(Document document) { - Parser parser = new Parser(document); + Parser parser = Parser.forIdl(document); parser.parseIdl(); List statements = parser.statements; DocumentParser documentParser = DocumentParser.forStatements(document, statements); @@ -130,7 +130,7 @@ public record NodeParseResult(Node value, List errors) {} * @return The Node parse result. */ public static NodeParseResult parseNode(Document document) { - Parser parser = new Parser(document); + Parser parser = Parser.forJson(document); Node node = parser.parseNode(); return new NodeParseResult(node, parser.errors); } From 783d385bf9d3318e97787d74f0572eb72538de04 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Wed, 22 Jan 2025 10:50:23 -0500 Subject: [PATCH 8/9] Report Smithy's json parse errors --- .../smithy/lsp/SmithyLanguageServer.java | 7 ---- .../smithy/lsp/project/ToSmithyNode.java | 32 +++++++++++-------- .../smithy/lsp/project/ToSmithyNodeTest.java | 27 ++++++++++++++++ .../build-exts/build/smithy-dependencies.json | 2 +- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java index 7fd92701..380a9ace 100644 --- a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java +++ b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java @@ -665,11 +665,4 @@ private CompletableFuture sendFileDiagnostics(ProjectAndFile projectAndFil client.publishDiagnostics(publishDiagnosticsParams); }); } - - private CompletableFuture sendFileDiagnostics(String uri, List diagnostics) { - return CompletableFuture.runAsync(() -> { - var params = new PublishDiagnosticsParams(uri, diagnostics); - client.publishDiagnostics(params); - }); - } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java index 6a3c2e50..54600032 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ToSmithyNode.java @@ -5,12 +5,12 @@ package software.amazon.smithy.lsp.project; -import java.util.ArrayList; import java.util.List; import software.amazon.smithy.lsp.document.Document; import software.amazon.smithy.lsp.protocol.LspAdapter; import software.amazon.smithy.lsp.syntax.Syntax; import software.amazon.smithy.model.SourceLocation; +import software.amazon.smithy.model.loader.ModelSyntaxException; import software.amazon.smithy.model.node.ArrayNode; import software.amazon.smithy.model.node.BooleanNode; import software.amazon.smithy.model.node.Node; @@ -18,7 +18,6 @@ import software.amazon.smithy.model.node.NumberNode; import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.node.StringNode; -import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; @@ -33,18 +32,32 @@ final class ToSmithyNode { private final String path; private final Document document; - private final List events; private ToSmithyNode(String path, Document document) { this.path = path; this.document = document; - this.events = new ArrayList<>(); } static ValidatedResult toSmithyNode(BuildFile buildFile) { var toSmithyNode = new ToSmithyNode(buildFile.path(), buildFile.document()); + var smithyNode = toSmithyNode.toSmithyNode(buildFile.getParse().value()); - return new ValidatedResult<>(smithyNode, toSmithyNode.events); + var events = toSmithyNode.getValidationEvents(); + + return new ValidatedResult<>(smithyNode, events); + } + + private List getValidationEvents() { + // The language server's parser isn't going to produce the same errors + // because of its leniency. Reparsing like this does incur a cost, but + // I think it's ok for now considering we get the added benefit of + // having the same errors Smithy itself would produce. + try { + Node.parseJsonWithComments(document.copyText(), path); + return List.of(); + } catch (ModelSyntaxException e) { + return List.of(ValidationEvent.fromSourceException(e)); + } } private Node toSmithyNode(Syntax.Node syntaxNode) { @@ -86,15 +99,6 @@ yield switch (value) { } case Syntax.Node.Str str -> new StringNode(str.stringValue(), sourceLocation); case Syntax.Node.Num num -> new NumberNode(num.value(), sourceLocation); - case Syntax.Node.Err err -> { - events.add(ValidationEvent.builder() - .id("ParseError") - .severity(Severity.ERROR) - .message(err.message()) - .sourceLocation(sourceLocation) - .build()); - yield null; - } default -> null; }; } diff --git a/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java b/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java index d987c9a0..e9bdf20e 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ToSmithyNodeTest.java @@ -6,18 +6,26 @@ package software.amazon.smithy.lsp.project; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithId; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithMessage; +import static software.amazon.smithy.lsp.SmithyMatchers.eventWithSourceLocation; import static software.amazon.smithy.lsp.UtilMatchers.anOptionalOf; import java.util.List; import java.util.Set; import java.util.stream.Stream; +import org.eclipse.lsp4j.Position; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.smithy.lsp.TextWithPositions; import software.amazon.smithy.lsp.document.Document; +import software.amazon.smithy.lsp.protocol.LspAdapter; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.NodeType; import software.amazon.smithy.model.node.ObjectNode; @@ -75,4 +83,23 @@ public void skipsMissingElements() { .keySet(); assertThat(projections, containsInAnyOrder("bar")); } + + @Test + public void emitsValidationEventsForParseErrors() { + var twp = TextWithPositions.from(""" + { + "version": %, + "imports": [] + } + """); + Position eventPosition = twp.positions()[0]; + BuildFile buildFile = BuildFile.create("foo", Document.of(twp.text()), BuildFileType.SMITHY_BUILD); + ValidatedResult nodeResult = ToSmithyNode.toSmithyNode(buildFile); + + assertThat(nodeResult.getValidationEvents(), containsInAnyOrder(allOf( + eventWithId(equalTo("Model")), + eventWithMessage(containsString("Error parsing JSON")), + eventWithSourceLocation(equalTo(LspAdapter.toSourceLocation("foo", eventPosition))) + ))); + } } diff --git a/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json b/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json index 1f9a1005..166f52e0 100644 --- a/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json +++ b/src/test/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json @@ -1,4 +1,4 @@ { "version": "1", "mavenDependencies": ["foo"] -} \ No newline at end of file +} From 58c3dd156e05040494397e3fe2be09ae4bb05566 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Wed, 22 Jan 2025 16:10:30 -0500 Subject: [PATCH 9/9] Address some comments --- .../software/amazon/smithy/lsp/SmithyLanguageServer.java | 6 +++--- .../amazon/smithy/lsp/project/ProjectConfigTest.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java index 380a9ace..78587faa 100644 --- a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java +++ b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java @@ -461,14 +461,14 @@ public void didChange(DidChangeTextDocumentParams params) { // Report any parse/shape/trait loading errors CompletableFuture future = CompletableFuture .runAsync(() -> project.updateModelWithoutValidating(uri)) - .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile)); + .thenRunAsync(() -> sendFileDiagnostics(projectAndFile)); state.lifecycleTasks().putTask(uri, future); } case BuildFile ignored -> { CompletableFuture future = CompletableFuture .runAsync(project::validateConfig) - .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile)); + .thenRunAsync(() -> sendFileDiagnostics(projectAndFile)); state.lifecycleTasks().putTask(uri, future); } @@ -521,7 +521,7 @@ public void didSave(DidSaveTextDocumentParams params) { } else { CompletableFuture future = CompletableFuture .runAsync(() -> project.updateAndValidateModel(uri)) - .thenCompose(unused -> sendFileDiagnostics(projectAndFile)); + .thenRunAsync(() -> sendFileDiagnostics(projectAndFile)); state.lifecycleTasks().putTask(uri, future); } } diff --git a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java index 04f90c5e..2383bd5c 100644 --- a/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java +++ b/src/test/java/software/amazon/smithy/lsp/project/ProjectConfigTest.java @@ -87,7 +87,7 @@ public void validatesSmithyBuildJson() { } """); var eventPosition = text.positions()[0]; - Path root = Path.of("test"); + var root = Path.of("test"); var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); var result = load(root, buildFiles); @@ -110,7 +110,7 @@ public void validatesSmithyProjectJson() { } """); var eventPosition = text.positions()[0]; - Path root = Path.of("test"); + var root = Path.of("test"); var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_PROJECT, text.text()); var result = load(root, buildFiles); @@ -140,7 +140,7 @@ public void validatesMavenConfig() { } """); var eventPosition = text.positions()[0]; - Path root = Path.of("test"); + var root = Path.of("test"); var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, text.text()); var result = load(root, buildFiles);