Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Do not evaluate traits of traits members recursively #1305

Merged
merged 4 commits into from
Nov 30, 2023
Merged

fix: Do not evaluate traits of traits members recursively #1305

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df95951
fix: Do not evaluate traits of traits members recursively
ghostbuster91 Nov 21, 2023
3d6a338
fix: Avoid infinite recursion when having self referencing traits
ghostbuster91 Nov 23, 2023
d024e48
Merge branch 'series/0.18' into i1296/recursive-trait
ghostbuster91 Nov 23, 2023
6a68fde
Format code
ghostbuster91 Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,53 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
}
}

def getFieldsPlain = {
ghostbuster91 marked this conversation as resolved.
Show resolved Hide resolved
val noDefault =
if (defaultRenderMode == DefaultRenderMode.NoDefaults)
List(Hint.NoDefault)
else List.empty
val result = shape
.members()
.asScala
.filterNot(isStreaming)
.map { member =>
val default =
if (defaultRenderMode == DefaultRenderMode.Full)
maybeDefault(member)
else List.empty
val defaultable = member.hasTrait(classOf[DefaultTrait]) &&
!member.tpe.exists(_.isExternal)
(
member.getMemberName(),
member.tpe,
member.hasTrait(classOf[RequiredTrait]) || defaultable,
default ++ noDefault // no call to hints(member)
ghostbuster91 marked this conversation as resolved.
Show resolved Hide resolved
)
}
.collect {
case (name, Some(tpe: Type.ExternalType), required, hints) =>
val newHints = hints.filterNot(_ == tpe.refinementHint)
Field(name, tpe, required, newHints)
case (name, Some(tpe), required, hints) =>
Field(name, tpe, required, hints)
}
.toList

val hintsContainsDefault: Field => Boolean = f =>
f.hints.exists {
case _: Hint.Default => true
case _ => false
}

defaultRenderMode match {
case DefaultRenderMode.Full =>
result.sortBy(hintsContainsDefault).sortBy(!_.required)
case DefaultRenderMode.OptionOnly =>
result.sortBy(!_.required)
case DefaultRenderMode.NoDefaults => result
}
}

def alts = {
shape
.members()
Expand Down Expand Up @@ -1183,8 +1230,8 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
case (N.ObjectNode(map), UnRef(S.Structure(struct))) =>
val shapeId = struct.getId()
val ref = Type.Ref(shapeId.getNamespace(), shapeId.getName())
val structFields = struct.fields
val fieldNames = struct.fields.map(_.name)
val structFields = struct.getFieldsPlain
val fieldNames = struct.getFieldsPlain.map(_.name)
val fields: List[TypedNode.FieldTN[NodeAndType]] = structFields.map {
case Field(_, realName, tpe, true, _) =>
val node = map.get(realName).getOrElse {
Expand Down
129 changes: 129 additions & 0 deletions modules/codegen/test/src/smithy4s/codegen/internals/DefaultRenderModeSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,133 @@ final class DefaultRenderModeSpec extends munit.FunSuite {
TestUtils.runTest(smithy, scalaCode)
}

test("allow for self referencing trait - structure") {
ghostbuster91 marked this conversation as resolved.
Show resolved Hide resolved
val smithy = """$version: "2"
|
|namespace input
|
|@trait
|structure Person {
| @Person
| name: String
|}
|""".stripMargin

val scalaCode =
"""package input
|
|import smithy4s.Hints
|import smithy4s.Schema
|import smithy4s.ShapeId
|import smithy4s.ShapeTag
|import smithy4s.schema.Schema.recursive
|import smithy4s.schema.Schema.string
|import smithy4s.schema.Schema.struct
|
|final case class Person(name: Option[String] = None)
|
|object Person extends ShapeTag.Companion[Person] {
| val id: ShapeId = ShapeId("input", "Person")
|
| val hints: Hints = Hints(
| smithy.api.Trait(selector = None, structurallyExclusive = None, conflicts = None, breakingChanges = None),
| )
|
| implicit val schema: Schema[Person] = recursive(struct(
| string.optional[Person]("name", _.name).addHints(input.Person(name = None)),
| ){
| Person.apply
| }.withId(id).addHints(hints))
|}
|""".stripMargin
TestUtils.runTest(smithy, scalaCode)
}

test("allow for self referencing trait - union") {
val smithy = """$version: "2"
|
|namespace input
|
|@trait
|union Person {
| @Person(u: "demo")
| name: String
| u: String
|}
|""".stripMargin
val scalaCode =
"""package input
|
|import smithy4s.Hints
|import smithy4s.Schema
|import smithy4s.ShapeId
|import smithy4s.ShapeTag
|import smithy4s.schema.Schema.bijection
|import smithy4s.schema.Schema.recursive
|import smithy4s.schema.Schema.string
|import smithy4s.schema.Schema.union
|
|sealed trait Person extends scala.Product with scala.Serializable { self =>
| @inline final def widen: Person = this
| def $ordinal: Int
|
| object project {
| def name: Option[String] = Person.NameCase.alt.project.lift(self).map(_.name)
| def u: Option[String] = Person.UCase.alt.project.lift(self).map(_.u)
| }
|
| def accept[A](visitor: Person.Visitor[A]): A = this match {
| case value: Person.NameCase => visitor.name(value.name)
| case value: Person.UCase => visitor.u(value.u)
| }
|}
|object Person extends ShapeTag.Companion[Person] {
|
| def name(name: String): Person = NameCase(name)
| def u(u: String): Person = UCase(u)
|
| val id: ShapeId = ShapeId("input", "Person")
|
| val hints: Hints = Hints(
| smithy.api.Trait(selector = None, structurallyExclusive = None, conflicts = None, breakingChanges = None),
| )
|
| final case class NameCase(name: String) extends Person { final def $ordinal: Int = 0 }
| final case class UCase(u: String) extends Person { final def $ordinal: Int = 1 }
|
| object NameCase {
| val hints: Hints = Hints(
| input.Person.UCase("demo").widen,
| )
| val schema: Schema[Person.NameCase] = bijection(string.addHints(hints), Person.NameCase(_), _.name)
| val alt = schema.oneOf[Person]("name")
| }
| object UCase {
| val hints: Hints = Hints.empty
| val schema: Schema[Person.UCase] = bijection(string.addHints(hints), Person.UCase(_), _.u)
| val alt = schema.oneOf[Person]("u")
| }
|
| trait Visitor[A] {
| def name(value: String): A
| def u(value: String): A
| }
|
| object Visitor {
| trait Default[A] extends Visitor[A] {
| def default: A
| def name(value: String): A = default
| def u(value: String): A = default
| }
| }
|
| implicit val schema: Schema[Person] = recursive(union(
| Person.NameCase.alt,
| Person.UCase.alt,
| ){
| _.$ordinal
| }.withId(id).addHints(hints))
|}""".stripMargin
TestUtils.runTest(smithy, scalaCode)
}
}