Complete required fields first (#240)

.github/workflows/ci.yml

@@ -29,12 +29,16 @@ jobs:
     runs-on: ubuntu-20.04
     timeout-minutes: 30
     steps:
+
     - uses: actions/checkout@v3.3.0
+
     - uses: cachix/install-nix-action@v20
+
     - uses: cachix/cachix-action@v12
       with:
         name: kubukoz
         authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
+
     - name: Cache sbt
       uses: actions/cache@v3.2.6
       with:
@@ -47,19 +51,26 @@ jobs:
           ~/AppData/Local/Coursier/Cache/v1
           ~/Library/Caches/Coursier/v1
         key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}
+
+    # This step isn't strictly necessary, but separating it means we can inspect its run time more easily.
+    - name: Setup environment
+      run: nix develop --command echo Environment ready
+
     - name: Server tests
       run: nix develop --command sbt ci
+
     - name: VS Code extension tests
       run: nix develop --command bash -c 'cd vscode-extension && yarn && SERVER_VERSION=$(cat ../.version) xvfb-run --auto-servernum yarn test'
+
     - name: Show extension test logs
       if: job.status == 'failure'
       run: cat vscode-extension/fixture/smithyql-log.txt | tail --lines 1000
-    -
-      name: release
+
+    - name: release
       if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
       run: nix develop --command sbt ci-release
-    -
-      name: release plugin
+
+    - name: release plugin
       if: github.event.inputs.publishExtension
       run: nix develop --command ./release-plugin.sh
       env:

modules/language-support/src/main/scala/playground/language/CompletionVisitor.scala

@@ -114,31 +114,29 @@ object CompletionItem {
   ).copy(detail = describeService(service))
 
   def fromField(
-    field: Field[CompletionResolver, _, _],
-    schema: Schema[_],
+    field: Field[Schema, _, _]
   ): CompletionItem = fromHints(
     kind = CompletionItemKind.Field,
     label = field.label,
     insertText = InsertText.JustString(s"${field.label}: "),
-    schema = schema,
+    schema = field.instance,
   )
 
   def fromAlt(
-    alt: Alt[CompletionResolver, _, _],
-    schema: Schema[_],
+    alt: Alt[Schema, _, _]
   ): CompletionItem = fromHints(
     kind = CompletionItemKind.UnionMember,
     label = alt.label,
     // needs proper completions for the inner schema
     // https://github.com/kubukoz/smithy-playground/pull/120
     insertText =
-      if (describeSchema(schema).apply().startsWith("structure "))
+      if (describeSchema(alt.instance).apply().startsWith("structure "))
         InsertText.SnippetString(s"""${alt.label}: {
                                     |  $$0
                                     |},""".stripMargin)
       else
         InsertText.JustString(s"${alt.label}: "),
-    schema = schema,
+    alt.instance,
   )
 
   def fromHints(
@@ -149,6 +147,13 @@ object CompletionItem {
   ): CompletionItem = {
     val isField = kind == CompletionItemKind.Field
 
+    val sortText =
+      isField match {
+        case true if isRequiredField(schema) => Some(s"1_$label")
+        case true                            => Some(s"2_$label")
+        case false                           => None
+      }
+
     CompletionItem(
       kind = kind,
       label = label,
@@ -164,15 +169,15 @@ object CompletionItem {
         isField,
       ),
       extraTextEdits = Nil,
-      sortText = None,
+      sortText = sortText,
     )
   }
 
   def describeType(
     isField: Boolean,
     schema: Schema[_],
   ): String = {
-    val isOptional = isField && schema.hints.get(smithy.api.Required).isEmpty
+    val isOptional = isField && !isRequiredField(schema)
 
     val optionalPrefix =
       if (isOptional)
@@ -182,6 +187,10 @@ object CompletionItem {
     show"$optionalPrefix: ${describeSchema(schema)()}"
   }
 
+  private def isRequiredField(
+    schema: Schema[_]
+  ): Boolean = schema.hints.has(smithy.api.Required)
+
   private val describePrimitive: Primitive[_] => String = {
     import smithy4s.schema.Primitive._
 
@@ -522,10 +531,10 @@ object CompletionVisitor extends SchemaVisitor[CompletionResolver] {
 
     structLike(
       inBody =
-        compiledFields
+        fields
           // todo: filter out present fields
-          .sortBy { case (field, _) => (field.isRequired, field.label) }
-          .map(CompletionItem.fromField.tupled)
+          .sortBy(field => (field.isRequired, field.label))
+          .map(CompletionItem.fromField)
           .toList,
       inValue =
         (
@@ -551,7 +560,7 @@ object CompletionVisitor extends SchemaVisitor[CompletionResolver] {
     }
 
     structLike(
-      inBody = allWithIds.map(CompletionItem.fromAlt.tupled).toList,
+      inBody = alternatives.map(CompletionItem.fromAlt).toList,
       inValue =
         (
           head,

modules/language-support/src/test/scala/playground/language/CompletionItemTests.scala

@@ -13,6 +13,48 @@ import weaver._
 import Diffs._
 
 object CompletionItemTests extends FunSuite {
+  test("CompletionItem.fromField: required field") {
+    val result = CompletionItem.fromField(
+      Schema.string.required[String]("test", identity(_)).addHints(smithy.api.Required())
+    )
+
+    assertNoDiff(
+      result,
+      CompletionItem(
+        kind = CompletionItemKind.Field,
+        label = "test",
+        insertText = InsertText.JustString("test: "),
+        detail = ": string String",
+        description = Some("smithy.api"),
+        deprecated = false,
+        docs = None,
+        extraTextEdits = Nil,
+        sortText = Some("1_test"),
+      ),
+    )
+  }
+
+  test("CompletionItem.fromField: optional field") {
+    val result = CompletionItem.fromField(
+      Schema.string.optional[Option[String]]("test", identity(_))
+    )
+
+    assertNoDiff(
+      result,
+      CompletionItem(
+        kind = CompletionItemKind.Field,
+        label = "test",
+        insertText = InsertText.JustString("test: "),
+        detail = "?: string String",
+        description = Some("smithy.api"),
+        deprecated = false,
+        docs = Some("**Optional**"),
+        extraTextEdits = Nil,
+        sortText = Some("2_test"),
+      ),
+    )
+  }
+
   test("CompletionItem.forOperation: no use clause") {
     val result = CompletionItem.forOperation(
       insertUseClause = CompletionItem.InsertUseClause.NotRequired,
@@ -93,8 +135,7 @@ object CompletionItemTests extends FunSuite {
 
   test("CompletionItem.fromAlt: struct item") {
     val result = CompletionItem.fromAlt(
-      Hero.GoodCase.alt.mapK(CompletionVisitor),
-      Hero.GoodCase.schema,
+      Hero.GoodCase.alt
     )
 
     assertNoDiff(
@@ -117,8 +158,7 @@ object CompletionItemTests extends FunSuite {
 
   test("CompletionItem.fromAlt: non-struct item") {
     val result = CompletionItem.fromAlt(
-      Hero.GoodCase.alt.mapK(CompletionVisitor),
-      Schema.string,
+      Schema.string.oneOf[String]("good")
     )
 
     assertNoDiff(