Ignore non-model JSON imports (#290)

kubukoz · Jun 9, 2023 · a954643 · a954643
1 parent 3317da2
commit a954643
Show file tree

Hide file tree

Showing 10 changed files with 202 additions and 39 deletions.
diff --git a/modules/core/src/main/scala/playground/ModelReader.scala b/modules/core/src/main/scala/playground/ModelReader.scala
diff --git a/modules/core/src/main/scala/playground/PlaygroundConfig.scala b/modules/core/src/main/scala/playground/PlaygroundConfig.scala
@@ -43,7 +43,7 @@ object PlaygroundConfig {
     }
 
     object BuildConfig {
-      implicit val c: JsonValueCodec[BuildConfig] = JsonCodecMaker.make[BuildConfig]
+      implicit val c: JsonValueCodec[BuildConfig] = JsonCodecMaker.make
 
       def fromPlaygroundConfig(
         c: PlaygroundConfig

diff --git a/modules/lsp/src/main/scala/playground/lsp/BuildLoader.scala b/modules/lsp/src/main/scala/playground/lsp/BuildLoader.scala
@@ -2,11 +2,12 @@ package playground.lsp
 
 import cats.effect.kernel.Sync
 import cats.implicits._
+import fs2.io.file.Files
 import fs2.io.file.Path
-import playground.ModelReader
 import playground.PlaygroundConfig
 import playground.language.TextDocumentProvider
 import playground.language.Uri
+import playground.lsp.util.SerializedSmithyModel
 import smithy4s.dynamic.DynamicSchemaIndex
 
 trait BuildLoader[F[_]] {
@@ -37,7 +38,7 @@ object BuildLoader {
     val default: Loaded = Loaded(PlaygroundConfig.empty, Path("/"))
   }
 
-  def instance[F[_]: TextDocumentProvider: Sync]: BuildLoader[F] =
+  def instance[F[_]: TextDocumentProvider: Sync: Files]: BuildLoader[F] =
     new BuildLoader[F] {
 
       def load(
@@ -87,28 +88,59 @@ object BuildLoader {
 
       def buildSchemaIndex(
         loaded: BuildLoader.Loaded
-      ): F[DynamicSchemaIndex] = Sync[F]
+      ): F[DynamicSchemaIndex] = {
+        // This has to be lazy, because for the default, "no imports" config, the file path points to the filesystem root.
+        lazy val workspaceBase = loaded
+          .configFilePath
+          .parent
+          .getOrElse(sys.error("impossible - no parent for " + loaded.configFilePath))
+
+        // "raw" means these can be directories etc., just like in the config file.
+        val rawImportPaths = loaded.config.imports.map(workspaceBase.resolve).toSet
+
+        for {
+          specs <- filterImports(rawImportPaths)
+          model <- loadModel(specs, loaded.config)
+          dsi <- DynamicSchemaIndex.loadModel(model).liftTo[F]
+        } yield dsi
+      }
+
+      private def loadModel(
+        specs: Set[Path],
+        config: PlaygroundConfig,
+      ) = Sync[F]
         .interruptibleMany {
-          ModelLoader
-            .load(
-              specs =
-                loaded
-                  .config
-                  .imports
-                  .map(
-                    loaded
-                      .configFilePath
-                      .parent
-                      .getOrElse(sys.error("impossible - no parent"))
-                      .resolve(_)
-                      .toNioPath
-                      .toFile()
-                  )
-                  .toSet,
-              jars = ModelLoader.resolveModelDependencies(loaded.config),
-            )
+          ModelLoader.load(
+            specs = specs.map(_.toNioPath.toFile),
+            jars = ModelLoader.resolveModelDependencies(config),
+          )
         }
-        .flatMap(ModelReader.buildSchemaIndex[F])
+
+      private def filterImports(
+        specs: Set[Path]
+      ): F[Set[Path]] = fs2
+        .Stream
+        .emits(specs.toSeq)
+        .flatMap(Files[F].walk(_))
+        .evalFilterNot(Files[F].isDirectory)
+        .evalFilter { file =>
+          val isSmithyFile = file.extName === ".smithy"
+
+          if (isSmithyFile)
+            true.pure[F]
+          else
+            isSerializedSmithyModelF(file)
+        }
+        .compile
+        .to(Set)
+
+      private def isSerializedSmithyModelF(
+        file: Path
+      ): F[Boolean] = Files[F]
+        .readAll(file)
+        .compile
+        .to(Array)
+        .map(SerializedSmithyModel.decode(_).isRight)
 
     }
 

diff --git a/modules/lsp/src/main/scala/playground/lsp/util/SerializedSmithyModel.scala b/modules/lsp/src/main/scala/playground/lsp/util/SerializedSmithyModel.scala
@@ -0,0 +1,21 @@
+package playground.lsp.util
+
+import cats.implicits._
+
+case class SerializedSmithyModel(
+  smithy: String
+)
+
+object SerializedSmithyModel {
+  import com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec
+  import com.github.plokhotnyuk.jsoniter_scala.core._
+  import com.github.plokhotnyuk.jsoniter_scala.macros.JsonCodecMaker
+
+  private implicit val c: JsonValueCodec[SerializedSmithyModel] = JsonCodecMaker.make
+
+  val decode: Array[Byte] => Either[Throwable, SerializedSmithyModel] =
+    bytes =>
+      Either
+        .catchNonFatal(readFromArray[SerializedSmithyModel](bytes))
+
+}
diff --git a/modules/lsp/src/test/resources/test-workspaces/json-models/input.smithyql b/modules/lsp/src/test/resources/test-workspaces/json-models/input.smithyql
@@ -0,0 +1 @@
+use service weather#WeatherService
diff --git a/modules/lsp/src/test/resources/test-workspaces/json-models/smithy-build.json b/modules/lsp/src/test/resources/test-workspaces/json-models/smithy-build.json
@@ -0,0 +1,3 @@
+{
+  "imports": ["src"]
+}
diff --git a/modules/lsp/src/test/resources/test-workspaces/json-models/src/valid-smithy-model.json b/modules/lsp/src/test/resources/test-workspaces/json-models/src/valid-smithy-model.json
@@ -0,0 +1,73 @@
+{
+  "smithy": "2.0",
+  "shapes": {
+    "weather#GetWeather": {
+      "type": "operation",
+      "input": {
+        "target": "weather#GetWeatherInput"
+      },
+      "output": {
+        "target": "weather#GetWeatherOutput"
+      },
+      "traits": {
+        "smithy.api#readonly": {}
+      }
+    },
+    "weather#GetWeatherInput": {
+      "type": "structure",
+      "members": {
+        "city": {
+          "target": "smithy.api#String",
+          "traits": {
+            "smithy.api#httpLabel": {},
+            "smithy.api#required": {}
+          }
+        }
+      },
+      "traits": {
+        "smithy.api#input": {}
+      }
+    },
+    "weather#GetWeatherOutput": {
+      "type": "structure",
+      "members": {
+        "weather": {
+          "target": "weather#Weather",
+          "traits": {
+            "smithy.api#required": {}
+          }
+        }
+      },
+      "traits": {
+        "smithy.api#output": {}
+      }
+    },
+    "weather#GoodWeather": {
+      "type": "structure",
+      "members": {
+        "reallyGood": {
+          "target": "smithy.api#Boolean"
+        }
+      }
+    },
+    "weather#Weather": {
+      "type": "union",
+      "members": {
+        "good": {
+          "target": "weather#GoodWeather"
+        },
+        "decent": {
+          "target": "smithy.api#Unit"
+        }
+      }
+    },
+    "weather#WeatherService": {
+      "type": "service",
+      "operations": [
+        {
+          "target": "weather#GetWeather"
+        }
+      ]
+    }
+  }
+}
diff --git a/modules/lsp/src/test/resources/test-workspaces/non-model-jsons/smithy-build.json b/modules/lsp/src/test/resources/test-workspaces/non-model-jsons/smithy-build.json
@@ -0,0 +1,3 @@
+{
+  "imports": ["src"]
+}
diff --git a/modules/lsp/src/test/resources/test-workspaces/non-model-jsons/src/not-a-smithy-model.json b/modules/lsp/src/test/resources/test-workspaces/non-model-jsons/src/not-a-smithy-model.json
@@ -0,0 +1,3 @@
+{
+  "this": "is not a Smithy model file"
+}
diff --git a/modules/lsp/src/test/scala/playground/lsp/LanguageServerReloadIntegrationTests.scala b/modules/lsp/src/test/scala/playground/lsp/LanguageServerReloadIntegrationTests.scala
@@ -6,12 +6,17 @@ import fs2.io.file.Files
 import fs2.io.file.Path
 import org.eclipse.lsp4j.CodeLensParams
 import org.eclipse.lsp4j.DidChangeWatchedFilesParams
+import org.eclipse.lsp4j.DocumentDiagnosticParams
+import org.eclipse.lsp4j.MessageType
 import org.eclipse.lsp4j.TextDocumentIdentifier
 import playground.PlaygroundConfig
 import playground.language.Uri
 import playground.lsp.harness.LanguageServerIntegrationTests
+import playground.lsp.harness.TestClient.MessageLog
 import weaver._
 
+import scala.jdk.CollectionConverters._
+
 object LanguageServerReloadIntegrationTests
   extends SimpleIOSuite
   with LanguageServerIntegrationTests {
@@ -82,15 +87,15 @@ object LanguageServerReloadIntegrationTests
               }
 
             getLenses.flatMap { lensesBefore =>
-              assert.same(lensesBefore, Nil).failFast[IO]
+              assert(lensesBefore.isEmpty).failFast[IO]
             } *>
               addLibrary *>
               f.server.didChangeWatchedFiles(new DidChangeWatchedFilesParams()) *>
               getLenses
           }
       }
       .use { lensesAfter =>
-        assert.same(lensesAfter.length, 1).pure[IO]
+        assert(lensesAfter.length == 1).pure[IO]
       }
   }
 
@@ -112,4 +117,40 @@ object LanguageServerReloadIntegrationTests
       .use_
       .as(success)
   }
+
+  test("workspace can be loaded even if non-model JSON files are included") {
+    makeServer(testWorkspacesBase / "non-model-jsons")
+      .use(_.client.getEvents)
+      .map { events =>
+        val errorLogs = events.collect { case MessageLog(MessageType.Error, msg) => msg }
+        assert(errorLogs.isEmpty)
+      }
+  }
+
+  test("JSON smithy models can be loaded") {
+    makeServer(testWorkspacesBase / "json-models")
+      .use { f =>
+        f.client.getEvents.flatMap { events =>
+          val errorLogs = events.collect { case MessageLog(MessageType.Error, msg) => msg }
+
+          f.server
+            .diagnostic(
+              new DocumentDiagnosticParams(
+                new TextDocumentIdentifier((f.workspaceDir / "input.smithyql").value)
+              )
+            )
+            .map { diags =>
+              val items = diags
+                .getRelatedFullDocumentDiagnosticReport()
+                .getItems()
+                .asScala
+                .toList
+                .map(_.getMessage())
+
+              assert(errorLogs.isEmpty) &&
+              assert(items.isEmpty)
+            }
+        }
+      }
+  }
 }