Avoid collisions in typed node namespaces, add missing uppercase in a…

…lt nodes (#1602)
disneystreaming · Oct 3, 2024 · 92d56f2 · 92d56f2
1 parent e5aa5b6
commit 92d56f2
Show file tree

Hide file tree

Showing 22 changed files with 177 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ Thank you!
 
 # 0.18.25
 
+* Fixes issues in which applications of some Smithy traits would be incorrectly rendered in Scala code (see [#1602](https://github.com/disneystreaming/smithy4s/pull/1602)).
 * Fixes an issue in which refinements wouldn't work on custom simple shapes (newtypes) (see [#1595](https://github.com/disneystreaming/smithy4s/pull/1595))
 * Fixes a regression from 0.18.4 which incorrectly rendered default values for certain types (see [#1593](https://github.com/disneystreaming/smithy4s/pull/1593))
 * Fixes an issue in which union members targetting Unit would fail to compile when used as traits (see [#1600](https://github.com/disneystreaming/smithy4s/pull/1600)).

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/DeprecatedUnion.scala b/modules/bootstrapped/src/generated/smithy4s/example/DeprecatedUnion.scala
@@ -51,15 +51,15 @@ object DeprecatedUnion extends ShapeTag.Companion[DeprecatedUnion] {
     def $ordinal: Int = 2
   }
 
-  object DeprecatedUnionProductCase extends ShapeTag.Companion[DeprecatedUnionProductCase] {
+  object DeprecatedUnionProductCase {
     val id: ShapeId = ShapeId("smithy4s.example", "DeprecatedUnionProductCase")
 
     val hints: Hints = Hints(
       smithy.api.Deprecated(message = None, since = None),
     ).lazily
 
 
-    implicit val schema: Schema[DeprecatedUnionProductCase] = constant(DeprecatedUnionProductCase()).withId(id).addHints(hints)
+    val schema: Schema[DeprecatedUnionProductCase] = constant(DeprecatedUnionProductCase()).withId(id).addHints(hints)
 
     val alt = schema.oneOf[DeprecatedUnion]("p")
   }
@@ -68,15 +68,15 @@ object DeprecatedUnion extends ShapeTag.Companion[DeprecatedUnion] {
     def $ordinal: Int = 3
   }
 
-  object UnionProductCaseDeprecatedAtCallSite extends ShapeTag.Companion[UnionProductCaseDeprecatedAtCallSite] {
+  object UnionProductCaseDeprecatedAtCallSite {
     val id: ShapeId = ShapeId("smithy4s.example", "UnionProductCaseDeprecatedAtCallSite")
 
     val hints: Hints = Hints(
       smithy.api.Deprecated(message = None, since = None),
     ).lazily
 
 
-    implicit val schema: Schema[UnionProductCaseDeprecatedAtCallSite] = constant(UnionProductCaseDeprecatedAtCallSite()).withId(id).addHints(hints)
+    val schema: Schema[UnionProductCaseDeprecatedAtCallSite] = constant(UnionProductCaseDeprecatedAtCallSite()).withId(id).addHints(hints)
 
     val alt = schema.oneOf[DeprecatedUnion]("p2")
   }

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala b/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala
diff --git a/modules/bootstrapped/src/generated/smithy4s/example/OrderType.scala b/modules/bootstrapped/src/generated/smithy4s/example/OrderType.scala
@@ -48,7 +48,7 @@ object OrderType extends ShapeTag.Companion[OrderType] {
     def $ordinal: Int = 1
   }
 
-  object InStoreOrder extends ShapeTag.Companion[InStoreOrder] {
+  object InStoreOrder {
     val id: ShapeId = ShapeId("smithy4s.example", "InStoreOrder")
 
     val hints: Hints = Hints(

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/PersonUnion.scala b/modules/bootstrapped/src/generated/smithy4s/example/PersonUnion.scala
@@ -32,7 +32,7 @@ object PersonUnion extends ShapeTag.Companion[PersonUnion] {
     def $ordinal: Int = 0
   }
 
-  object OtherPerson extends ShapeTag.Companion[OtherPerson] {
+  object OtherPerson {
     val id: ShapeId = ShapeId("smithy4s.example", "OtherPerson")
 
     val hints: Hints = Hints.empty

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/Podcast.scala b/modules/bootstrapped/src/generated/smithy4s/example/Podcast.scala
@@ -43,7 +43,7 @@ object Podcast extends ShapeTag.Companion[Podcast] {
     def $ordinal: Int = 0
   }
 
-  object Video extends ShapeTag.Companion[Video] {
+  object Video {
     val id: ShapeId = ShapeId("smithy4s.example", "Video")
 
     val hints: Hints = Hints.empty
@@ -69,7 +69,7 @@ object Podcast extends ShapeTag.Companion[Podcast] {
     def $ordinal: Int = 1
   }
 
-  object Audio extends ShapeTag.Companion[Audio] {
+  object Audio {
     val id: ShapeId = ShapeId("smithy4s.example", "Audio")
 
     val hints: Hints = Hints.empty

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/TestAdt.scala b/modules/bootstrapped/src/generated/smithy4s/example/TestAdt.scala
@@ -40,7 +40,7 @@ object TestAdt extends ShapeTag.Companion[TestAdt] {
     def $ordinal: Int = 0
   }
 
-  object AdtOne extends ShapeTag.Companion[AdtOne] {
+  object AdtOne {
     val id: ShapeId = ShapeId("smithy4s.example", "AdtOne")
 
     val hints: Hints = Hints.empty
@@ -61,7 +61,7 @@ object TestAdt extends ShapeTag.Companion[TestAdt] {
     def $ordinal: Int = 1
   }
 
-  object AdtTwo extends ShapeTag.Companion[AdtTwo] {
+  object AdtTwo {
     val id: ShapeId = ShapeId("smithy4s.example", "AdtTwo")
 
     val hints: Hints = Hints.empty

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/TestMixinAdt.scala b/modules/bootstrapped/src/generated/smithy4s/example/TestMixinAdt.scala
@@ -33,7 +33,7 @@ object TestMixinAdt extends ShapeTag.Companion[TestMixinAdt] {
     def $ordinal: Int = 0
   }
 
-  object TestAdtMemberWithMixin extends ShapeTag.Companion[TestAdtMemberWithMixin] {
+  object TestAdtMemberWithMixin {
     val id: ShapeId = ShapeId("smithy4s.example", "TestAdtMemberWithMixin")
 
     val hints: Hints = Hints.empty

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/TestString.scala b/modules/bootstrapped/src/generated/smithy4s/example/TestString.scala
@@ -10,7 +10,7 @@ import smithy4s.schema.Schema.string
 object TestString extends Newtype[String] {
   val id: ShapeId = ShapeId("smithy4s.example", "TestString")
   val hints: Hints = Hints(
-    smithy4s.example.TestTrait(orderType = Some(smithy4s.example.OrderType.InStoreOrder(id = smithy4s.example.OrderNumber(100), locationId = Some("someLocation")))),
+    smithy4s.example.TestTrait(orderType = Some(smithy4s.example.OrderType.InStoreOrder(id = smithy4s.example.OrderNumber(100), locationId = Some("someLocation")).widen)),
   ).lazily
   val underlyingSchema: Schema[String] = string.withId(id).addHints(hints)
   implicit val schema: Schema[TestString] = bijection(underlyingSchema, asBijection)

diff --git a/modules/bootstrapped/src/generated/smithy4s/example/_package/MyPackageStringTrait.scala b/modules/bootstrapped/src/generated/smithy4s/example/_package/MyPackageStringTrait.scala
@@ -0,0 +1,18 @@
+package smithy4s.example._package
+
+import smithy4s.Hints
+import smithy4s.Newtype
+import smithy4s.Schema
+import smithy4s.ShapeId
+import smithy4s.schema.Schema.bijection
+import smithy4s.schema.Schema.recursive
+import smithy4s.schema.Schema.string
+
+object MyPackageStringTrait extends Newtype[String] {
+  val id: ShapeId = ShapeId("smithy4s.example.package", "MyPackageStringTrait")
+  val hints: Hints = Hints(
+    smithy.api.Trait(selector = None, structurallyExclusive = None, conflicts = None, breakingChanges = None),
+  ).lazily
+  val underlyingSchema: Schema[String] = string.withId(id).addHints(hints)
+  implicit val schema: Schema[MyPackageStringTrait] = recursive(bijection(underlyingSchema, asBijection))
+}
diff --git a/modules/bootstrapped/src/generated/smithy4s/example/_package/package.scala b/modules/bootstrapped/src/generated/smithy4s/example/_package/package.scala
@@ -3,5 +3,6 @@ package smithy4s.example
 package object _package {
 
   type MyPackageString = smithy4s.example._package.MyPackageString.Type
+  type MyPackageStringTrait = smithy4s.example._package.MyPackageStringTrait.Type
 
 }
diff --git a/modules/bootstrapped/src/generated/smithy4s/example/collision/Class.scala b/modules/bootstrapped/src/generated/smithy4s/example/collision/Class.scala
@@ -0,0 +1,65 @@
+package smithy4s.example.collision
+
+import smithy4s.Hints
+import smithy4s.Schema
+import smithy4s.ShapeId
+import smithy4s.ShapeTag
+import smithy4s.schema.Schema.constant
+import smithy4s.schema.Schema.recursive
+import smithy4s.schema.Schema.union
+
+sealed trait Class extends scala.Product with scala.Serializable { self =>
+  @inline final def widen: Class = this
+  def $ordinal: Int
+
+  object project {
+    def _package: Option[Class.AdtStruct] = Class.AdtStruct.alt.project.lift(self)
+  }
+
+  def accept[A](visitor: Class.Visitor[A]): A = this match {
+    case value: Class.AdtStruct => visitor._package(value)
+  }
+}
+object Class extends ShapeTag.Companion[Class] {
+
+  def adtStruct():AdtStruct = AdtStruct()
+
+  val id: ShapeId = ShapeId("smithy4s.example.collision", "class")
+
+  val hints: Hints = Hints(
+    smithy.api.Trait(selector = None, structurallyExclusive = None, conflicts = None, breakingChanges = None),
+  ).lazily
+
+  final case class AdtStruct() extends Class {
+    def $ordinal: Int = 0
+  }
+
+  object AdtStruct {
+    val id: ShapeId = ShapeId("smithy4s.example.collision", "AdtStruct")
+
+    val hints: Hints = Hints.empty
+
+
+    val schema: Schema[AdtStruct] = constant(AdtStruct()).withId(id).addHints(hints)
+
+    val alt = schema.oneOf[Class]("package")
+  }
+
+
+  trait Visitor[A] {
+    def _package(value: Class.AdtStruct): A
+  }
+
+  object Visitor {
+    trait Default[A] extends Visitor[A] {
+      def default: A
+      def _package(value: Class.AdtStruct): A = default
+    }
+  }
+
+  implicit val schema: Schema[Class] = recursive(union(
+    Class.AdtStruct.alt,
+  ){
+    _.$ordinal
+  }.withId(id).addHints(hints))
+}
diff --git a/modules/bootstrapped/src/generated/smithy4s/example/collision/ReservedNameUnionTrait.scala b/modules/bootstrapped/src/generated/smithy4s/example/collision/ReservedNameUnionTrait.scala
@@ -0,0 +1,17 @@
+package smithy4s.example.collision
+
+import smithy4s.Hints
+import smithy4s.Newtype
+import smithy4s.Schema
+import smithy4s.ShapeId
+import smithy4s.schema.Schema.bijection
+import smithy4s.schema.Schema.string
+
+object ReservedNameUnionTrait extends Newtype[java.lang.String] {
+  val id: ShapeId = ShapeId("smithy4s.example.collision", "ReservedNameUnionTrait")
+  val hints: Hints = Hints(
+    smithy4s.example.collision.Class.AdtStruct().widen,
+  ).lazily
+  val underlyingSchema: Schema[java.lang.String] = string.withId(id).addHints(hints)
+  implicit val schema: Schema[ReservedNameUnionTrait] = bijection(underlyingSchema, asBijection)
+}
diff --git a/...es/bootstrapped/src/generated/smithy4s/example/collision/TestReservedNamespaceTrait.scala b/...es/bootstrapped/src/generated/smithy4s/example/collision/TestReservedNamespaceTrait.scala
@@ -0,0 +1,17 @@
+package smithy4s.example.collision
+
+import smithy4s.Hints
+import smithy4s.Newtype
+import smithy4s.Schema
+import smithy4s.ShapeId
+import smithy4s.schema.Schema.bijection
+import smithy4s.schema.Schema.string
+
+object TestReservedNamespaceTrait extends Newtype[java.lang.String] {
+  val id: ShapeId = ShapeId("smithy4s.example.collision", "TestReservedNamespaceTrait")
+  val hints: Hints = Hints(
+    smithy4s.example._package.MyPackageStringTrait("test"),
+  ).lazily
+  val underlyingSchema: Schema[java.lang.String] = string.withId(id).addHints(hints)
+  implicit val schema: Schema[TestReservedNamespaceTrait] = bijection(underlyingSchema, asBijection)
+}
diff --git a/modules/bootstrapped/src/generated/smithy4s/example/collision/package.scala b/modules/bootstrapped/src/generated/smithy4s/example/collision/package.scala
@@ -9,6 +9,8 @@ package object collision {
   type MySet = smithy4s.example.collision.MySet.Type
   type ReservedKeywordTraitExampleCollection = smithy4s.example.collision.ReservedKeywordTraitExampleCollection.Type
   type ReservedKeywordTraitExamplePrimitive = smithy4s.example.collision.ReservedKeywordTraitExamplePrimitive.Type
+  type ReservedNameUnionTrait = smithy4s.example.collision.ReservedNameUnionTrait.Type
   type String = smithy4s.example.collision.String.Type
+  type TestReservedNamespaceTrait = smithy4s.example.collision.TestReservedNamespaceTrait.Type
 
 }
diff --git a/modules/codegen/src/smithy4s/codegen/internals/CollisionAvoidance.scala b/modules/codegen/src/smithy4s/codegen/internals/CollisionAvoidance.scala
@@ -243,6 +243,11 @@ private[internals] object CollisionAvoidance {
         case ValidatedNewTypeTN(ref, target) =>
           ValidatedNewTypeTN(modRef(ref), target)
         case AltTN(ref, altName, alt) =>
+          // note: technically we should probably escape altName here
+          // but it'd only really break if it matched a capitalized keyword,
+          // and Scala has none of those, so it's impossible to write a failing test.
+          // Alt names in this context are always capitalized before being printed
+          // (Renderer.scala:1614 at the time of writing).
           AltTN(modRef(ref), altName, alt)
         case MapTN(values) =>
           MapTN(values)

diff --git a/modules/codegen/src/smithy4s/codegen/internals/IR.scala b/modules/codegen/src/smithy4s/codegen/internals/IR.scala
@@ -305,7 +305,10 @@ private[internals] object Type {
       valueHints: List[Hint]
   ) extends Type
   case class Ref(namespace: String, name: String) extends Type {
-    def show = namespace + "." + name
+    def show: String = NameRef
+      .splitPath(namespace)
+      .map(CollisionAvoidance.protectKeyword)
+      .mkString(".") + "." + name
   }
   case class Alias(
       namespace: String,

diff --git a/modules/codegen/src/smithy4s/codegen/internals/LineSegment.scala b/modules/codegen/src/smithy4s/codegen/internals/LineSegment.scala
@@ -95,13 +95,17 @@ private[codegen] object LineSegment {
     )
     implicit val nameRefShow: Show[NameRef] = Show.show[NameRef](_.asImport)
     def apply(pkg: String, name: String): NameRef =
-      NameRef(pkg.split("\\.").toList, name, List.empty)
+      NameRef(splitPath(pkg), name, List.empty)
     def apply(fqn: String): NameRef = {
-      val parts = fqn.split("\\.").toList.toNel.get
+      val parts =
+        splitPath(fqn).toNel.getOrElse(sys.error(s"Invalid FQN: $fqn"))
       NameRef(parts.toList.dropRight(1), parts.last, List.empty)
     }
     def apply(fqn: String, typeParams: List[NameRef]): NameRef =
       apply(fqn).copy(typeParams = typeParams)
+
+    private[internals] def splitPath(pkg: String): List[String] =
+      pkg.split("\\.").toList
   }
 
   implicit val lineSegmentShow: Show[LineSegment] = Show.show {

diff --git a/modules/codegen/src/smithy4s/codegen/internals/Renderer.scala b/modules/codegen/src/smithy4s/codegen/internals/Renderer.scala
@@ -711,7 +711,7 @@ private[internals] class Renderer(compilationUnit: CompilationUnit) { self =>
           }
       },
       newline,
-      obj(product.nameRef, shapeTag(product.nameRef))(
+      obj(product.nameRef, if (adtParent.isEmpty) shapeTag(product.nameRef) else Line.empty)(
         renderId(shapeId),
         newline,
         renderHintsVal(hints),
@@ -776,7 +776,7 @@ private[internals] class Renderer(compilationUnit: CompilationUnit) { self =>
               .appendToLast(if (recursive) ")" else "")
           }
         } else {
-          line"implicit val schema: $Schema_[${product.nameRef}] = $constant_(${product.nameRef}()).withId(id).addHints(hints)"
+          line"${schemaImplicit}val schema: $Schema_[${product.nameRef}] = $constant_(${product.nameRef}()).withId(id).addHints(hints)"
         },
         renderTypeclasses(product.hints, product.nameRef),
         additionalLines
@@ -1583,7 +1583,7 @@ private[internals] class Renderer(compilationUnit: CompilationUnit) { self =>
 
   private def renderTypedNode(tn: TypedNode[CString]): CString = tn match {
     case EnumerationTN(ref, _, _, name) =>
-      line"${ref.show + "." + name + ".widen"}".write
+      line"${ref.show}.$name.widen".write
     case StructureTN(ref, fields) =>
       val fieldStrings = fields.map {
         case (name, FieldTN.RequiredTN(value)) =>
@@ -1617,7 +1617,9 @@ private[internals] class Renderer(compilationUnit: CompilationUnit) { self =>
       line"${ref.show}.${altName.capitalize}Case.widen".write
 
     case AltTN(_, _, AltValueTN.ProductAltTN(alt)) =>
-      alt.runDefault.write
+      // The `widen` is necessary in Scala 2.
+      // Without it, there is no ShapeTag to use for the conversion to Hints.Binding.
+      line"${alt.runDefault}.widen".write
 
     case CollectionTN(collectionType, values) =>
       val col = collectionType.tpe

diff --git a/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala b/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala
@@ -1237,7 +1237,7 @@ private[codegen] class SmithyToIR(
             case Some(parent) =>
               val cId = shape.getId
               val newNs =
-                cId.getNamespace + "." + parent.getName
+                cId.getNamespace + "." + parent.getName.capitalize
               val error = new Exception(
                 s"Shapes annotated with the adtMemberTrait must be structures. $cId is not a structure."
               )

diff --git a/sampleSpecs/reservedNamespace.smithy b/sampleSpecs/reservedNamespace.smithy
@@ -3,3 +3,6 @@ $version: "2.0"
 namespace smithy4s.example.package
 
 string MyPackageString
+
+@trait
+string MyPackageStringTrait