Report Smithy's json parse errors

smithy-lang · Jan 22, 2025 · 783d385 · 783d385
1 parent f444ebb
commit 783d385
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 22 deletions.
diff --git 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<Void> sendFileDiagnostics(ProjectAndFile projectAndFil
             client.publishDiagnostics(publishDiagnosticsParams);
         });
     }
-
-    private CompletableFuture<Void> sendFileDiagnostics(String uri, List<Diagnostic> 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
@@ -5,20 +5,19 @@
 
 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;
 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;
 
@@ -33,18 +32,32 @@
 final class ToSmithyNode {
     private final String path;
     private final Document document;
-    private final List<ValidationEvent> events;
 
     private ToSmithyNode(String path, Document document) {
         this.path = path;
         this.document = document;
-        this.events = new ArrayList<>();
     }
 
     static ValidatedResult<Node> 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<ValidationEvent> 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
@@ -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<Node> 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/...st/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json b/...st/resources/software/amazon/smithy/lsp/project/build-exts/build/smithy-dependencies.json
@@ -1,4 +1,4 @@
 {
   "version": "1",
   "mavenDependencies": ["foo"]
-}
+}