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.

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

Render deprecation hints #599

Merged
merged 13 commits into from
Nov 20, 2022
15 changes: 13 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,13 @@ lazy val `aws-kernel` = projectMatrix
"aws.protocols"
),
Compile / sourceGenerators := Seq(genSmithyScala(Compile).taskValue),
Test / envVars ++= Map("TEST_VAR" -> "hello")
Test / envVars ++= Map("TEST_VAR" -> "hello"),
scalacOptions ++= Seq(
"-Wconf:msg=class AwsQuery in package aws.protocols is deprecated:silent",
"-Wconf:msg=class RestXml in package aws.protocols is deprecated:silent",
"-Wconf:msg=value noErrorWrapping in class RestXml is deprecated:silent",
"-Wconf:msg=class Ec2Query in package aws.protocols is deprecated:silent"
)
)
.jvmPlatform(latest2ScalaVersions, jvmDimSettings)
.jsPlatform(
Expand Down Expand Up @@ -727,6 +733,8 @@ lazy val complianceTests = projectMatrix
/**
* Example application using the custom REST-JSON protocol provided by
* smithy4s.
*
* (almost) all Scala code in this module is generated! The ones that aren't should have a note stating so.
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
*/
lazy val example = projectMatrix
.in(file("modules/example"))
Expand All @@ -744,6 +752,7 @@ lazy val example = projectMatrix
),
smithySpecs := Seq(
(ThisBuild / baseDirectory).value / "sampleSpecs" / "example.smithy",
(ThisBuild / baseDirectory).value / "sampleSpecs" / "deprecations.smithy",
(ThisBuild / baseDirectory).value / "sampleSpecs" / "errors.smithy",
(ThisBuild / baseDirectory).value / "sampleSpecs" / "streaming.smithy",
(ThisBuild / baseDirectory).value / "sampleSpecs" / "operation.smithy",
Expand All @@ -768,7 +777,9 @@ lazy val example = projectMatrix
),
genSmithyOutput := ((ThisBuild / baseDirectory).value / "modules" / "example" / "src"),
genSmithyResourcesOutput := (Compile / resourceDirectory).value,
smithy4sSkip := List("resource")
smithy4sSkip := List("resource"),
// Ignore deprecation warnings here - it's all generated code, anyway.
scalacOptions += "-Wconf:cat=deprecation:silent"
)
.jvmPlatform(List(Scala213), jvmDimSettings)
.settings(Smithy4sBuildPlugin.doNotPublishArtifact)
Expand Down
6 changes: 4 additions & 2 deletions modules/codegen/src/smithy4s/codegen/IR.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ case class Enumeration(
shapeId: ShapeId,
name: String,
values: List[EnumValue],
hints: List[Hint] = Nil
hints: List[Hint]
) extends Decl
case class EnumValue(
value: String,
intValue: Int,
name: String,
hints: List[Hint] = Nil
hints: List[Hint]
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
)

case class Field(
Expand Down Expand Up @@ -245,6 +245,8 @@ object Hint {
case class Constraint(tr: Type.Ref, native: Native) extends Hint
case class Protocol(traits: List[Type.Ref]) extends Hint
case class Default(typedNode: Fix[TypedNode]) extends Hint
case class Deprecated(message: Option[String], since: Option[String])
extends Hint
// traits that get rendered generically
case class Native(typedNode: Fix[TypedNode]) extends Hint
case object IntEnum extends Hint
Expand Down
127 changes: 86 additions & 41 deletions modules/codegen/src/smithy4s/codegen/Renderer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,35 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
case union @ Union(shapeId, _, alts, recursive, hints) =>
renderUnion(shapeId, union.nameRef, alts, recursive, hints)
case ta @ TypeAlias(shapeId, _, tpe, _, recursive, hints) =>
renderTypeAlias(shapeId, ta.nameRef, tpe, recursive, hints)
renderNewtype(shapeId, ta.nameRef, tpe, recursive, hints)
case enumeration @ Enumeration(shapeId, _, values, hints) =>
renderEnum(shapeId, enumeration.nameRef, values, hints)
case _ => Lines.empty
}

private def deprecationAnnotation(hints: List[Hint]): Line = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the implementation feels a bit dirty, suggestions welcome

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only thing I'd change here is have an extra map, before the that's already there where I call renderStringLiteral before building the line to avoid ${renderStringLiteral(msg)}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I actually like that renderStringLiteral is being interpolated (as long as it doesn't break the line)

hints
.collectFirst { case h: Hint.Deprecated => h }
.foldMap { dep =>
val messagePart = dep.message
.map(msg => line"message = ${renderStringLiteral(msg)}")
val versionPart =
dep.since.map(v => line"since = ${renderStringLiteral(v)}")

val args = List(messagePart, versionPart).flatten.intercalate(comma)

val argListOrEmpty = if (args.nonEmpty) line"($args)" else line""

line"@deprecated$argListOrEmpty"
}
}

def renderPackageContents: Lines = {
val typeAliases = compilationUnit.declarations.collect {
case TypeAlias(_, name, _, _, _, _) =>
line"type $name = ${compilationUnit.namespace}.${name}.Type"
case TypeAlias(_, name, _, _, _, hints) =>
lines(
deprecationAnnotation(hints),
line"type $name = ${compilationUnit.namespace}.${name}.Type"
)
}

val blk =
Expand Down Expand Up @@ -166,6 +185,7 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
val name = s.name
val nameGen = NameRef(s"${name}Gen")
lines(
deprecationAnnotation(s.hints),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it would make sense to deprecate just the service trait and type alias, but we could deprecate the companion object / val alias as well. I don't have a strong opinion on this.

line"type ${NameDef(name)}[F[_]] = $FunctorAlgebra_[$nameGen, F]",
line"val ${NameRef(name)} = $nameGen"
)
Expand All @@ -186,12 +206,16 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
val opTraitNameRef = opTraitName.toNameRef

lines(
deprecationAnnotation(hints),
block(line"trait $genName[F[_, _, _, _, _]]")(
line"self =>",
newline,
ops.map { op =>
line"def ${op.methodName}(${op.renderArgs}) : F[${op.renderAlgParams(genNameRef.name)}]"

lines(
deprecationAnnotation(op.hints),
line"def ${op.methodName}(${op.renderArgs}) : F[${op
.renderAlgParams(genNameRef.name)}]"
)
},
newline,
line"def transform : $Transformation.PartiallyApplied[$genName[F]] = new $Transformation.PartiallyApplied[$genName[F]](this)"
Expand Down Expand Up @@ -504,18 +528,24 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
additionalLines: Lines = Lines.empty
): Lines = {
import product._
if (isMixin)
renderProductMixin(
product,
adtParent,
additionalLines
)
else
renderProductNonMixin(
product,
adtParent,
additionalLines
)
val base =
if (isMixin)
renderProductMixin(
product,
adtParent,
additionalLines
)
else
renderProductNonMixin(
product,
adtParent,
additionalLines
)

lines(
deprecationAnnotation(product.hints),
base
)
}

private def renderGetMessage(field: Field) = field match {
Expand Down Expand Up @@ -572,6 +602,7 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
caseNames.zip(alts.map(_.member == UnionMember.UnitCase))

lines(
deprecationAnnotation(hints),
block(
line"sealed trait ${NameDef(name.name)} extends scala.Product with scala.Serializable"
)(
Expand All @@ -587,23 +618,29 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
val cn = caseName(a)
// format: off
lines(
deprecationAnnotation(altHints),
line"case object $cn extends $name",
line"""private val ${cn}Alt = $Schema_.constant($cn)${renderConstraintValidation(altHints)}.oneOf[$name]("$realName").addHints(hints)""",
line"private val ${cn}AltWithValue = ${cn}Alt($cn)"
)
// format: on
case a @ Alt(altName, _, UnionMember.TypeCase(tpe), _) =>
case a @ Alt(altName, _, UnionMember.TypeCase(tpe), altHints) =>
val cn = caseName(a)
lines(
deprecationAnnotation(altHints),
line"case class $cn(${uncapitalise(altName)}: $tpe) extends $name"
)
case Alt(_, realName, UnionMember.ProductCase(struct), _) =>
case Alt(_, realName, UnionMember.ProductCase(struct), altHints) =>
val additionalLines = lines(
newline,
line"""val alt = schema.oneOf[$name]("$realName")"""
)
// In case of union members that are inline structs (as opposed to structs being referenced and wrapped by a new class),
// we want to put a deprecation note (if it exists on the alt) on the struct - there's nowhere else to put it.
renderProduct(
struct,
// putting alt hints first should result in higher priority of these.
// might need deduplication (although the Hints type will take care of it, just in case)
struct.copy(hints = altHints ++ struct.hints),
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
adtParent = Some(name),
additionalLines
)
Expand Down Expand Up @@ -678,7 +715,8 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
)
}

line"$name: " + tpeAndDefault
deprecationAnnotation(hints).appendIf(_.nonEmpty)(Line.space) +
line"$name: " + tpeAndDefault
}
}
private def renderArgs(fields: List[Field]): Line = fields
Expand All @@ -691,24 +729,29 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
values: List[EnumValue],
hints: List[Hint]
): Lines = lines(
deprecationAnnotation(hints),
block(
line"sealed abstract class ${name.name}(_value: String, _name: String, _intValue: Int) extends $Enumeration_.Value"
line"sealed abstract class ${name.name}(_value: String, _name: String, _intValue: Int, _hints: $Hints_) extends $Enumeration_.Value"
)(
line"override val value: String = _value",
line"override val name: String = _name",
line"override val intValue: Int = _intValue",
line"override val hints: $Hints_ = $Hints_.empty",
line"override val hints: $Hints_ = _hints",
line"@inline final def widen: $name = this"
),
obj(name, ext = line"$Enumeration_[$name]", w = line"${shapeTag(name)}")(
renderId(shapeId),
newline,
renderHintsVal(hints),
newline,
values.map { case e @ EnumValue(value, intValue, _, _) =>
line"""case object ${NameRef(
e.name
)} extends $name("$value", "${e.name}", $intValue)"""
values.map { case e @ EnumValue(value, intValue, _, hints) =>
val valueName = NameRef(e.name)
val valueHints = line"$Hints_(${memberHints(e.hints)})"

lines(
deprecationAnnotation(hints),
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
line"""case object $valueName extends $name("$value", "${e.name}", $intValue, $valueHints)"""
)
},
newline,
line"val values: $list[$name] = $list".args(
Expand All @@ -718,7 +761,7 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
)
)

private def renderTypeAlias(
private def renderNewtype(
shapeId: ShapeId,
name: NameRef,
tpe: Type,
Expand All @@ -732,6 +775,7 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
line".withId(id).addHints(hints)${renderConstraintValidation(hints)}"
val closing = if (recursive) ")" else ""
lines(
deprecationAnnotation(hints),
obj(name, line"$Newtype_[$tpe]")(
renderId(shapeId),
renderHintsVal(hints),
Expand Down Expand Up @@ -949,18 +993,8 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
case Primitive.Int => t => line"${t.toString}"
case Primitive.Short => t => line"${t.toString}"
case Primitive.Bool => t => line"${t.toString}"
case Primitive.Uuid => uuid => line"java.util.UUID.fromString($uuid)"
case Primitive.String => { raw =>
import scala.reflect.runtime.universe._
val str = Literal(Constant(raw))
.toString()
// Replace sequences like "\\uD83D" (how Smithy specs refer to unicode characters)
// with unicode character escapes like "\uD83D" that can be parsed in the regex implementations on all platforms.
// See https://github.com/disneystreaming/smithy4s/pull/499
.replace("\\\\u", "\\u")

line"$str"
}
case Primitive.Uuid => uuid => line"java.util.UUID.fromString($uuid)"
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
case Primitive.String => renderStringLiteral
case Primitive.Document => { (node: Node) =>
node.accept(new NodeVisitor[Line] {
def arrayNode(x: ArrayNode): Line = {
Expand Down Expand Up @@ -988,4 +1022,15 @@ private[codegen] class Renderer(compilationUnit: CompilationUnit) { self =>
case _ => _ => line"null"
}

private def renderStringLiteral(raw: String): Line = {
import scala.reflect.runtime.universe._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the import used for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Literal and Constant

val str = Literal(Constant(raw))
.toString()
// Replace sequences like "\\uD83D" (how Smithy specs refer to unicode characters)
// with unicode character escapes like "\uD83D" that can be parsed in the regex implementations on all platforms.
// See https://github.com/disneystreaming/smithy4s/pull/499
.replace("\\\\u", "\\u")

line"$str"
}
}
30 changes: 25 additions & 5 deletions modules/codegen/src/smithy4s/codegen/SmithyToIR.scala
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
value.getName().asScala,
value.getValue,
index
)
),
hints = Nil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK 1.x enums don't allow hints on values.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct

)
}
.toList
Expand All @@ -287,18 +288,28 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
.asScala
.zipWithIndex
.map { case ((name, value), index) =>
EnumValue(value, index, name)
val member = shape.getMember(name).get()

EnumValue(value, index, name, hints(member))
}
.toList
Enumeration(shape.getId(), shape.name, values).some

Enumeration(
shape.getId(),
shape.name,
values,
hints = hints(shape)
).some
}

override def intEnumShape(shape: IntEnumShape): Option[Decl] = {
val values = shape
.getEnumValues()
.asScala
.map { case (name, value) =>
EnumValue(name, value, name)
val member = shape.getMember(name).get()
Comment on lines 306 to +310
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels a bit odd to do the lookup like this, but it's not that much repetition and it should be safe at all times.


EnumValue(name, value, name, hints(member))
}
.toList
Enumeration(
Expand Down Expand Up @@ -707,6 +718,8 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {
Hint.Protocol(refs.toList)
case _: PackedInputsTrait =>
Hint.PackedInputs
case d: DeprecatedTrait =>
Hint.Deprecated(d.getMessage.asScala, d.getSince.asScala)
case _: ErrorMessageTrait =>
Hint.ErrorMessage
case _: VectorTrait =>
Expand All @@ -722,7 +735,14 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) {

private def traitsToHints(traits: List[Trait]): List[Hint] = {
val nonMetaTraits =
traits.filterNot(_.toShapeId().getNamespace() == "smithy4s.meta")
traits
.filterNot(_.toShapeId().getNamespace() == "smithy4s.meta")
// traits from the synthetic namespace, e.g. smithy.synthetic.enum
// don't have shapes in the model - so we can't generate hints for them.
.filterNot(_.toShapeId().getNamespace() == "smithy.synthetic")
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
// enumValue can be derived from enum schemas anyway, so we're removing it from hints
.filterNot(_.toShapeId() == EnumValueTrait.ID)

val nonConstraintNonMetaTraits = nonMetaTraits.collect {
case t if ConstraintTrait.unapply(t).isEmpty => t
}
Expand Down
4 changes: 4 additions & 0 deletions modules/codegen/src/smithy4s/codegen/ToLine.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ case class Line(segments: Chain[LineSegment]) {
Lines.empty
}
}

def appendIf(condition: this.type => Boolean)(other: Line): Line =
if (condition(this)) this + other else this
}

object Line {
Expand All @@ -150,6 +153,7 @@ object Line {

val empty: Line = Line(Chain.empty)
val comma: Line = Line(", ")
val space: Line = Line(" ")
val dot: Line = Line(".")
implicit val monoid: Monoid[Line] = Monoid.instance(empty, _ + _)

Expand Down
Loading