Rendering fixes (#130)

* Fix rendering of schemas for structures of arity > 22 * Create camel case names from enum values when they don't have names * Add logic to render enum names in worst case scenario Adds logic to create a correct Scala name for enum values, when the values of the enumeration are purely symbolic.
disneystreaming · Mar 7, 2022 · e11e2f6 · e11e2f6
1 parent ca7422f
commit e11e2f6
Show file tree

Hide file tree

Showing 11 changed files with 227 additions and 32 deletions.
diff --git a/build.sbt b/build.sbt
@@ -130,7 +130,7 @@ lazy val core = projectMatrix
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "metadata.smithy",
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "recursive.smithy",
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "bodies.smithy",
-      (ThisBuild / baseDirectory).value / "sampleSpecs" / "empty.smithy",
+      (ThisBuild / baseDirectory).value / "sampleSpecs" / "misc.smithy",
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "product.smithy",
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "weather.smithy",
       (ThisBuild / baseDirectory).value / "sampleSpecs" / "discriminated.smithy",

diff --git a/modules/codegen/src/smithy4s/codegen/CollisionAvoidance.scala b/modules/codegen/src/smithy4s/codegen/CollisionAvoidance.scala
@@ -83,8 +83,9 @@ object CollisionAvoidance {
           hints.map(modHint)
         )
       case Enumeration(name, originalName, values, hints) =>
-        val newValues = values.map { case EnumValue(value, name, hints) =>
-          EnumValue(value, name.map(protect), hints.map(modHint))
+        val newValues = values.map {
+          case EnumValue(value, ordinal, name, hints) =>
+            EnumValue(value, ordinal, name.map(protect), hints.map(modHint))
         }
         Enumeration(
           protect(name.capitalize),
@@ -146,8 +147,8 @@ object CollisionAvoidance {
         Type.Ref(ref.namespace, ref.name.capitalize)
 
       def apply[A](fa: TypedNode[A]): TypedNode[A] = fa match {
-        case EnumerationTN(ref, value, name) =>
-          EnumerationTN(modRef(ref), value, name)
+        case EnumerationTN(ref, value, ordinal, name) =>
+          EnumerationTN(modRef(ref), value, ordinal, name)
         case StructureTN(ref, fields) =>
           StructureTN(modRef(ref), fields)
         case NewTypeTN(ref, target) =>

diff --git a/modules/codegen/src/smithy4s/codegen/IR.scala b/modules/codegen/src/smithy4s/codegen/IR.scala
@@ -83,6 +83,7 @@ case class Enumeration(
 ) extends Decl
 case class EnumValue(
     value: String,
+    ordinal: Int,
     name: Option[String],
     hints: List[Hint] = Nil
 )
@@ -217,7 +218,8 @@ object TypedNode {
 
   implicit val typedNodeFunctor: Functor[TypedNode] = new Functor[TypedNode] {
     def map[A, B](fa: TypedNode[A])(f: A => B): TypedNode[B] = fa match {
-      case EnumerationTN(ref, value, name) => EnumerationTN(ref, value, name)
+      case EnumerationTN(ref, value, ordinal, name) =>
+        EnumerationTN(ref, value, ordinal, name)
       case StructureTN(ref, fields) =>
         StructureTN(ref, fields.map(_.map(_.map(f))))
       case NewTypeTN(ref, target) =>
@@ -235,8 +237,12 @@ object TypedNode {
     }
   }
 
-  case class EnumerationTN(ref: Type.Ref, value: String, name: Option[String])
-      extends TypedNode[Nothing]
+  case class EnumerationTN(
+      ref: Type.Ref,
+      value: String,
+      ordinal: Int,
+      name: Option[String]
+  ) extends TypedNode[Nothing]
   case class StructureTN[A](
       ref: Type.Ref,
       fields: List[(String, FieldTN[A])]

diff --git a/modules/codegen/src/smithy4s/codegen/Renderer.scala b/modules/codegen/src/smithy4s/codegen/Renderer.scala
@@ -370,22 +370,43 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
         renderProtocol(name, hints),
         newline,
         if (fields.nonEmpty) {
-          val definition = if (recursive) "recursive(struct" else "struct"
-          line(s"val schema: $Schema_[$name] = $definition")
-            .args(
-              fields.map { case Field(fieldName, realName, tpe, required, hints) =>
+          val renderedFields =
+            fields.map {
+              case Field(fieldName, realName, tpe, required, hints) =>
                 val req = if (required) "required" else "optional"
                 if (hints.isEmpty) {
                   s"""${tpe.schemaRef}.$req[$name]("$realName", _.$fieldName)"""
                 } else {
                   val mh = memberHints(hints)
                   s"""${tpe.schemaRef}.$req[$name]("$realName", _.$fieldName).withHints($mh)"""
                 }
-              }
-            )
-            .block(s"$name.apply")
-            .appendToLast(".withHints(hints)")
-            .appendToLast(if (recursive) ")" else "")
+            }
+          if (fields.size <= 22) {
+            val definition = if (recursive) "recursive(struct" else "struct"
+            line(s"val schema: $Schema_[$name] = $definition")
+              .args(renderedFields)
+              .block(s"$name.apply")
+              .appendToLast(".withHints(hints)")
+              .appendToLast(if (recursive) ")" else "")
+          } else {
+            val definition =
+              if (recursive) "recursive(bigStruct" else "bigStruct"
+            line(s"val schema: $Schema_[$name] = $definition")
+              .args(renderedFields)
+              .block(
+                line(s"arr => new $name").args(
+                  fields.zipWithIndex.map {
+                    case (Field(_, _, tpe, required, _), idx) =>
+                      val scalaTpe =
+                        if (required) tpe.render
+                        else s"Option[${tpe.render}]"
+                      line(s"arr($idx).asInstanceOf[$scalaTpe]")
+                  }
+                )
+              )
+              .appendToLast(".withHints(hints)")
+              .appendToLast(if (recursive) ")" else "")
+          }
         } else {
           line(
             s"val schema: $Schema_[$name] = constant($name()).withHints(hints)"
@@ -501,9 +522,9 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
       newline,
       renderHintsValWithId(hints),
       newline,
-      values.zipWithIndex.map { case (e @ EnumValue(value, _, _), index) =>
+      values.map { case e @ EnumValue(value, ordinal, _, _) =>
         line(
-          s"""case object ${e.className} extends $name("$value", $index)"""
+          s"""case object ${e.className} extends $name("$value", $ordinal)"""
         )
       },
       newline,
@@ -660,12 +681,35 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
     }
   }
 
-  private def enumValueClassName(name: Option[String], value: String) = {
-    name.getOrElse(value.capitalize)
+  private def toCamelCase(value: String): String = {
+    val (_, output) = value.foldLeft((false, "")) {
+      case ((wasLastSkipped, str), c) =>
+        if (c.isLetterOrDigit) {
+          val newC =
+            if (wasLastSkipped) c.toString.capitalize else c.toString
+          (false, str + newC)
+        } else {
+          (true, str)
+        }
+    }
+    output
+  }
+
+  private def enumValueClassName(
+      name: Option[String],
+      value: String,
+      ordinal: Int
+  ) = {
+    name.getOrElse {
+      val camel = toCamelCase(value).capitalize
+      if (camel.nonEmpty) camel else "Value" + ordinal
+    }
+
   }
 
   private implicit class EnumValueOps(enumValue: EnumValue) {
-    def className = enumValueClassName(enumValue.name, enumValue.value)
+    def className =
+      enumValueClassName(enumValue.name, enumValue.value, enumValue.ordinal)
   }
 
   private def renderHint(hint: Hint): Option[String] = hint match {
@@ -711,8 +755,8 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
   }
 
   private def renderTypedNode(tn: TypedNode[CString]): CString = tn match {
-    case EnumerationTN(ref, value, name) =>
-      (ref.show + "." + enumValueClassName(name, value)).write
+    case EnumerationTN(ref, value, ordinal, name) =>
+      (ref.show + "." + enumValueClassName(name, value, ordinal)).write
     case StructureTN(ref, fields) =>
       val fieldStrings = fields.map {
         case (_, FieldTN.RequiredTN(value))     => value.runDefault

diff --git a/modules/codegen/src/smithy4s/codegen/SmithyToIR.scala b/modules/codegen/src/smithy4s/codegen/SmithyToIR.scala
@@ -101,8 +101,9 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
           val values = e
             .getValues()
             .asScala
-            .map { value =>
-              EnumValue(value.getValue(), value.getName().asScala)
+            .zipWithIndex
+            .map { case (value, index) =>
+              EnumValue(value.getValue(), index, value.getName().asScala)
             }
             .toList
           Enumeration(shape.name, shape.name, values).some
@@ -512,12 +513,14 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
         TypedNode.NewTypeTN(Type.Ref(ns, name), NodeAndType(node, tpe))
       // Enumeration
       case (N.StringNode(str), UnRef(shape @ T.enumeration(e))) =>
-        val enumDef = e.getValues().asScala.find(_.getValue() == str).get
+        val (enumDef, index) =
+          e.getValues().asScala.zipWithIndex.find(_._1.getValue() == str).get
         val shapeId = shape.getId()
         val ref = Type.Ref(shapeId.getNamespace(), shapeId.getName())
         TypedNode.EnumerationTN(
           ref,
           enumDef.getValue(),
+          index,
           enumDef.getName().asScala
         )
       // List

diff --git a/modules/core/test/src/smithy4s/BigStructSmokeSpec.scala b/modules/core/test/src/smithy4s/BigStructSmokeSpec.scala
@@ -0,0 +1,30 @@
+/*
+ *  Copyright 2021 Disney Streaming
+ *
+ *  Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *     https://disneystreaming.github.io/TOST-1.0.txt
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package smithy4s
+
+import weaver._
+
+object BigStructSmokeSpec extends FunSuite {
+
+  test("Big structures do have a schema") {
+    val expected =
+      "struct23(a1: int, a2: int, a3: int, a4: int, a5: int, a6: int, a7: int, a8: int, a9: int, a10: int, a11: int, a12: int, a13: int, a14: int, a15: int, a16: int, a17: int, a18: int, a19: int, a20: int, a21: int, a22: int, a23: int)"
+    val schemaRepr = smithy4s.example.BigStruct.schema.compile(SchematicRepr)
+    expect(schemaRepr == expected)
+  }
+
+}
diff --git a/modules/core/test/src/smithy4s/EnumWithSymbolsSmokeSpec.scala b/modules/core/test/src/smithy4s/EnumWithSymbolsSmokeSpec.scala
@@ -0,0 +1,35 @@
+/*
+ *  Copyright 2021 Disney Streaming
+ *
+ *  Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *     https://disneystreaming.github.io/TOST-1.0.txt
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package smithy4s
+
+import weaver._
+
+object EnumWithSymbolsSmokeSpec extends FunSuite {
+
+  test(
+    "Camel case names are issued from enum values when they don't have names"
+  ) {
+    val values = smithy4s.example.EnumWithSymbols.values
+    val expected = List(
+      smithy4s.example.EnumWithSymbols.FooFooFoo,
+      smithy4s.example.EnumWithSymbols.BarBarBar,
+      smithy4s.example.EnumWithSymbols.Value2
+    )
+    expect(values == expected)
+  }
+
+}
diff --git a/modules/schematic-core/src/generated/struct.scala b/modules/schematic-core/src/generated/struct.scala
@@ -31,6 +31,11 @@ object struct {
       const: Vector[Any] => Z) : schematic.Schema[S, Z] =
     new Schema(fields, const)
 
+    def bigStruct[S[x[_]] <: Schematic[x], Z](
+      fields: StructureField[S, Z, _]*)(
+       const: Vector[Any] => Z) : schematic.Schema[S, Z] =
+    new Schema(fields.toVector, const)
+
     def struct[S[x[_]] <: Schematic[x], Z, A0](a0: StructureField[S, Z, A0])(const : (A0) => Z) : schematic.Schema[S, Z] = new Schema[S, Z](Vector(a0), arr => const(arr(0).asInstanceOf[A0]))
     def struct[S[x[_]] <: Schematic[x], Z, A0, A1](a0: StructureField[S, Z, A0], a1: StructureField[S, Z, A1])(const : (A0, A1) => Z) : schematic.Schema[S, Z] = new Schema[S, Z](Vector(a0, a1), arr => const(arr(0).asInstanceOf[A0], arr(1).asInstanceOf[A1]))
     def struct[S[x[_]] <: Schematic[x], Z, A0, A1, A2](a0: StructureField[S, Z, A0], a1: StructureField[S, Z, A1], a2: StructureField[S, Z, A2])(const : (A0, A1, A2) => Z) : schematic.Schema[S, Z] = new Schema[S, Z](Vector(a0, a1, a2), arr => const(arr(0).asInstanceOf[A0], arr(1).asInstanceOf[A1], arr(2).asInstanceOf[A2]))
@@ -64,6 +69,11 @@ object struct {
       const: Vector[Any] => Z) : schematic.Schema[S, Z] =
     new Schema(fields, const)
 
+    def bigStruct[Z](
+      fields: StructureField[S, Z, _]*)(
+      const: Vector[Any] => Z) : schematic.Schema[S, Z] =
+    new Schema(fields.toVector, const)
+
     def struct[Z, A0](a0: StructureField[S, Z, A0])(const : (A0) => Z) : schematic.Schema[S, Z] =  new Schema[S, Z](Vector(a0), arr => const(arr(0).asInstanceOf[A0]))
     def struct[Z, A0, A1](a0: StructureField[S, Z, A0], a1: StructureField[S, Z, A1])(const : (A0, A1) => Z) : schematic.Schema[S, Z] =  new Schema[S, Z](Vector(a0, a1), arr => const(arr(0).asInstanceOf[A0], arr(1).asInstanceOf[A1]))
     def struct[Z, A0, A1, A2](a0: StructureField[S, Z, A0], a1: StructureField[S, Z, A1], a2: StructureField[S, Z, A2])(const : (A0, A1, A2) => Z) : schematic.Schema[S, Z] =  new Schema[S, Z](Vector(a0, a1, a2), arr => const(arr(0).asInstanceOf[A0], arr(1).asInstanceOf[A1], arr(2).asInstanceOf[A2]))

diff --git a/project/SchematicBoilerplate.scala b/project/SchematicBoilerplate.scala
@@ -202,6 +202,11 @@ object Boilerplate {
       |      const: Vector[Any] => Z) : schematic.Schema[S, Z] =
       |    new Schema(fields, const)
       |
+      |    def bigStruct[S[x[_]] <: Schematic[x], Z](
+      |      fields: StructureField[S, Z, _]*)(
+      |       const: Vector[Any] => Z) : schematic.Schema[S, Z] =
+      |    new Schema(fields.toVector, const)
+      |
       -    $smartCtsrOpen
       |  }
       |
@@ -214,6 +219,11 @@ object Boilerplate {
       |      const: Vector[Any] => Z) : schematic.Schema[S, Z] =
       |    new Schema(fields, const)
       |
+      |    def bigStruct[Z](
+      |      fields: StructureField[S, Z, _]*)(
+      |      const: Vector[Any] => Z) : schematic.Schema[S, Z] =
+      |    new Schema(fields.toVector, const)
+      |
       -    $smartCtsrClosed
       |  }
       |

diff --git a/sampleSpecs/empty.smithy b/sampleSpecs/empty.smithy
diff --git a/sampleSpecs/misc.smithy b/sampleSpecs/misc.smithy
@@ -0,0 +1,61 @@
+namespace smithy4s.example
+
+service EmptyService {
+  version: "1.0"
+}
+
+structure BigStruct{
+  @required
+  a1: Integer,
+  @required
+  a2: Integer,
+  @required
+  a3: Integer,
+  @required
+  a4: Integer,
+  @required
+  a5: Integer,
+  @required
+  a6: Integer,
+  @required
+  a7: Integer,
+  @required
+  a8: Integer,
+  @required
+  a9: Integer,
+  @required
+  a10: Integer,
+  @required
+  a11: Integer,
+  @required
+  a12: Integer,
+  @required
+  a13: Integer,
+  @required
+  a14: Integer,
+  @required
+  a15: Integer,
+  @required
+  a16: Integer,
+  @required
+  a17: Integer,
+  @required
+  a18: Integer,
+  @required
+  a19: Integer,
+  @required
+  a20: Integer,
+  @required
+  a21: Integer,
+  @required
+  a22: Integer,
+  @required
+  a23: Integer
+}
+
+@enum([
+  {value: "foo:foo:foo"},
+  {value: "bar:bar:bar"},
+  {value: "_"},
+])
+string EnumWithSymbols