Build file diagnostics (#188)

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.

Additional changes:
- Report Smithy's json parse errors
- Added a 'use-smithy-build' diagnostic to 'legacy' build files
- Fix json node parsing to properly handle commas in the IDL vs actual
json

Author: milesziemer

.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
 

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

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<Path> {
     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<Path> roots = new ArrayList<>();

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<Exception> tryInitProject(Path root) {
         LOGGER.finest("Initializing project at " + root);
         lifecycleTasks.cancelAllTasks();
 
-        Result<Project, List<Exception>> 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<Exception> 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) {

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<Void> future = CompletableFuture
+                        .runAsync(() -> project.updateModelWithoutValidating(uri))
+                        .thenRunAsync(() -> sendFileDiagnostics(projectAndFile));
+
+                state.lifecycleTasks().putTask(uri, future);
+            }
+            case BuildFile ignored -> {
+                CompletableFuture<Void> future = CompletableFuture
+                        .runAsync(project::validateConfig)
+                        .thenRunAsync(() -> 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<Void> future = CompletableFuture
-                    .runAsync(() -> project.updateModelWithoutValidating(uri))
-                    .thenComposeAsync(unused -> sendFileDiagnostics(projectAndFile));
-            state.lifecycleTasks().putTask(uri, future);
+                state.lifecycleTasks().putTask(uri, future);
+            }
         }
     }
 
@@ -512,7 +521,7 @@ public void didSave(DidSaveTextDocumentParams params) {
         } else {
             CompletableFuture<Void> future = CompletableFuture
                     .runAsync(() -> project.updateAndValidateModel(uri))
-                    .thenCompose(unused -> sendFileDiagnostics(projectAndFile));
+                    .thenRunAsync(() -> sendFileDiagnostics(projectAndFile));
             state.lifecycleTasks().putTask(uri, future);
         }
     }

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;
@@ -32,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() {
     }
@@ -51,82 +55,140 @@ public static List<Diagnostic> 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<Diagnostic> diagnostics = project.modelResult().getValidationEvents().stream()
+        List<Diagnostic> 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<ValidationEvent> getValidationEvents();
+
+        EventToDiagnostic getEventToDiagnostic();
+
+        void addExtraDiagnostics(List<Diagnostic> diagnostics);
+    }
+
+    private record DiagnoseSmithy(SmithyFile smithyFile, Project project) implements Diagnose {
+        @Override
+        public List<ValidationEvent> 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<Diagnostic> 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<ValidationEvent> 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<Diagnostic> diagnostics) {
+            switch (buildFile.type()) {
+                case SMITHY_BUILD_EXT_0, SMITHY_BUILD_EXT_1 -> diagnostics.add(useSmithyBuild());
+                default -> {
+                }
+            }
+        }
+
+        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;
+        }
+    }
+
+    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 {