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

Derive Eq and Hash wherever possible #2223

Merged
merged 17 commits into from Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Expand Up @@ -370,3 +370,15 @@ set_credentials_cache(
references = ["smithy-rs#2122"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "ysaito1001"

[[smithy-rs]]
message = "`aws_smithy_types::date_time::DateTime`, `aws_smithy_types::Blob` now implement the `Eq` and `Hash` traits"
references = ["smithy-rs#2223"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"}
author = "david-perez"

[[smithy-rs]]
message = "Code-generated types for server SDKs now implement the `Eq` and `Hash` traits when possible"
references = ["smithy-rs#2223"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"}
author = "david-perez"
Expand Up @@ -80,7 +80,7 @@ class RustClientCodegenPlugin : DecoratableBuildPlugin() {
.let { StreamingShapeSymbolProvider(it, model) }
// Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes
.let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf(NonExhaustive)) }
// Streaming shapes need different derives (e.g. they cannot derive Eq)
// Streaming shapes need different derives (e.g. they cannot derive `PartialEq`)
.let { StreamingShapeMetadataProvider(it, model) }
// Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot
// be the name of an operation input
Expand Down
Expand Up @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.core.smithy
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
Expand All @@ -27,7 +30,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private
WrappingSymbolProvider(base) {
override fun toSymbol(shape: Shape): Symbol {
val initial = base.toSymbol(shape)
// We are only targetting member shapes
// We are only targeting member shapes
if (shape !is MemberShape) {
return initial
}
Expand All @@ -49,7 +52,7 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private
}

/**
* SymbolProvider to drop the clone and PartialEq bounds in streaming shapes
* SymbolProvider to drop the `Clone` and `PartialEq` bounds in streaming shapes.
*
* Streaming shapes cannot be cloned and equality cannot be checked without reading the body. Because of this, these shapes
* do not implement `Clone` or `PartialEq`.
Expand All @@ -60,10 +63,6 @@ class StreamingShapeMetadataProvider(
private val base: RustSymbolProvider,
private val model: Model,
) : SymbolMetadataProvider(base) {
override fun memberMeta(memberShape: MemberShape): RustMetadata {
return base.toSymbol(memberShape).expectRustMetadata()
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
val baseMetadata = base.toSymbol(structureShape).expectRustMetadata()
return if (structureShape.hasStreamingMember(model)) {
Expand All @@ -78,7 +77,12 @@ class StreamingShapeMetadataProvider(
} else baseMetadata
}

override fun enumMeta(stringShape: StringShape): RustMetadata {
return base.toSymbol(stringShape).expectRustMetadata()
}
override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata()
override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()

override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata()
override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata()
override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()
override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata()
override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata()
}
Expand Up @@ -8,7 +8,12 @@ package software.amazon.smithy.rust.codegen.core.smithy
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
Expand Down Expand Up @@ -55,9 +60,15 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr
is MemberShape -> memberMeta(shape)
is StructureShape -> structureMeta(shape)
is UnionShape -> unionMeta(shape)
is ListShape -> listMeta(shape)
is MapShape -> mapMeta(shape)
is NumberShape -> numberMeta(shape)
is BlobShape -> blobMeta(shape)
is StringShape -> if (shape.hasTrait<EnumTrait>()) {
enumMeta(shape)
} else null
} else {
stringMeta(shape)
}

else -> null
}
Expand All @@ -68,98 +79,101 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr
abstract fun structureMeta(structureShape: StructureShape): RustMetadata
abstract fun unionMeta(unionShape: UnionShape): RustMetadata
abstract fun enumMeta(stringShape: StringShape): RustMetadata

abstract fun listMeta(listShape: ListShape): RustMetadata
abstract fun mapMeta(mapShape: MapShape): RustMetadata
abstract fun stringMeta(stringShape: StringShape): RustMetadata
abstract fun numberMeta(numberShape: NumberShape): RustMetadata
abstract fun blobMeta(blobShape: BlobShape): RustMetadata
}

fun containerDefaultMetadata(
shape: Shape,
model: Model,
additionalAttributes: List<Attribute> = emptyList(),
): RustMetadata {
val defaultDerives = setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)

val isSensitive = shape.hasTrait<SensitiveTrait>() ||
// Checking the shape's direct members for the sensitive trait should suffice.
// Whether their descendants, i.e. a member's member, is sensitive does not
// affect the inclusion/exclusion of the derived `Debug` trait of _this_ container
// shape; any sensitive descendant should still be printed as redacted.
shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent }

val setOfDerives = if (isSensitive) {
defaultDerives - RuntimeType.Debug
} else {
defaultDerives
}
return RustMetadata(
setOfDerives,
additionalAttributes,
Visibility.PUBLIC,
)
}

/**
* The base metadata supports a list of attributes that are used by generators to decorate code.
* By default, we apply ```#[non_exhaustive]``` only to client structures since model changes should
* be considered as breaking only when generating server code.
* The base metadata supports a set of attributes that are used by generators to decorate code.
*
* By default we apply `#[non_exhaustive]` in [additionalAttributes] only to client structures since breaking model
* changes are fine when generating server code.
*/
class BaseSymbolMetadataProvider(
base: RustSymbolProvider,
private val model: Model,
private val additionalAttributes: List<Attribute>,
) : SymbolMetadataProvider(base) {
private fun containerDefault(shape: Shape): RustMetadata {
val isSensitive = shape.hasTrait<SensitiveTrait>() ||
// Checking the shape's direct members for the sensitive trait should suffice.
// Whether their descendants, i.e. a member's member, is sensitive does not
// affect the inclusion/exclusion of the derived Debug trait of _this_ container
// shape; any sensitive descendant should still be printed as redacted.
shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent }

val derives = if (isSensitive) {
defaultDerives - RuntimeType.Debug
} else {
defaultDerives
}
return RustMetadata(
derives,
additionalAttributes,
Visibility.PUBLIC,
)
}

override fun memberMeta(memberShape: MemberShape): RustMetadata {
val container = model.expectShape(memberShape.container)
return when {
container.isStructureShape -> {
override fun memberMeta(memberShape: MemberShape): RustMetadata =
when (val container = model.expectShape(memberShape.container)) {
is StructureShape -> {
// TODO(https://github.com/awslabs/smithy-rs/issues/943): Once streaming accessors are usable,
// then also make streaming members `#[doc(hidden)]`
if (memberShape.getMemberTrait(model, StreamingTrait::class.java).isPresent) {
RustMetadata(visibility = Visibility.PUBLIC)
} else {
RustMetadata(
// At some point, visibility will be made PRIVATE, so make these `#[doc(hidden)]` for now
// At some point, visibility _may_ be made `PRIVATE`, so make these `#[doc(hidden)]` for now.
visibility = Visibility.PUBLIC,
additionalAttributes = listOf(Attribute.DocHidden),
)
}
}

container.isUnionShape ||
container.isListShape ||
container.isSetShape ||
container.isMapShape
-> RustMetadata(visibility = Visibility.PUBLIC)

is UnionShape, is CollectionShape, is MapShape -> RustMetadata(visibility = Visibility.PUBLIC)
else -> TODO("Unrecognized container type: $container")
}
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
return containerDefault(structureShape)
}

override fun unionMeta(unionShape: UnionShape): RustMetadata {
return containerDefault(unionShape)
}
override fun structureMeta(structureShape: StructureShape) = containerDefaultMetadata(structureShape, model, additionalAttributes)
override fun unionMeta(unionShape: UnionShape) = containerDefaultMetadata(unionShape, model, additionalAttributes)

override fun enumMeta(stringShape: StringShape): RustMetadata {
return containerDefault(stringShape).withDerives(
RuntimeType.Hash,
// enums can be Eq because they can only contain ints and strings
override fun enumMeta(stringShape: StringShape): RustMetadata =
containerDefaultMetadata(stringShape, model, additionalAttributes).withDerives(
// Smithy's `enum` shapes can additionally be `Eq`, `PartialOrd`, `Ord`, and `Hash` because they can
// only contain strings.
RuntimeType.Eq,
// enums can be PartialOrd/Ord because they can only contain ints and strings
RuntimeType.PartialOrd,
RuntimeType.Ord,
RuntimeType.Hash,
)
}

companion object {
private val defaultDerives by lazy {
setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)
}
}
// Only the server subproject uses these, so we provide a sane and conservative default implementation here so that
// the rest of symbol metadata providers can just delegate to it.
private val defaultRustMetadata = RustMetadata(visibility = Visibility.PRIVATE)

override fun listMeta(listShape: ListShape) = defaultRustMetadata
override fun mapMeta(mapShape: MapShape) = defaultRustMetadata
override fun stringMeta(stringShape: StringShape) = defaultRustMetadata
override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata
override fun blobMeta(blobShape: BlobShape) = defaultRustMetadata
}

private const val META_KEY = "meta"
fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder {
return this.putProperty(META_KEY, rustMetadata)
}
fun Symbol.Builder.meta(rustMetadata: RustMetadata?): Symbol.Builder = this.putProperty(META_KEY, rustMetadata)

fun Symbol.expectRustMetadata(): RustMetadata = this.getProperty(META_KEY, RustMetadata::class.java).orElseThrow {
CodegenException(
"Expected $this to have metadata attached but it did not. ",
"Expected `$this` to have metadata attached but it did not.",
)
}
Expand Up @@ -17,7 +17,9 @@ import software.amazon.smithy.rust.codegen.core.smithy.EventStreamSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitor
import software.amazon.smithy.rust.codegen.core.smithy.SymbolVisitorConfig
import software.amazon.smithy.rust.codegen.server.python.smithy.customizations.DECORATORS
import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolMetadataProvider
import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolProvider
import software.amazon.smithy.rust.codegen.server.smithy.DeriveEqAndHashSymbolMetadataProvider
import software.amazon.smithy.rust.codegen.server.smithy.customizations.ServerRequiredCustomizations
import software.amazon.smithy.rust.codegen.server.smithy.customize.CombinedServerCodegenDecorator
import java.util.logging.Level
Expand Down Expand Up @@ -54,7 +56,7 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin {
}

companion object {
/** SymbolProvider
/**
* When generating code, smithy types need to be converted into Rust types—that is the core role of the symbol provider
*
* The Symbol provider is composed of a base [SymbolVisitor] which handles the core functionality, then is layered
Expand All @@ -77,8 +79,12 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin {
.let { EventStreamSymbolProvider(symbolVisitorConfig.runtimeConfig, it, model, CodegenTarget.SERVER) }
// Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes
.let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) }
// Constrained shapes generate newtypes that need the same derives we place on types generated from aggregate shapes.
.let { ConstrainedShapeSymbolMetadataProvider(it, model, constrainedTypes) }
// Streaming shapes need different derives (e.g. they cannot derive Eq)
.let { PythonStreamingShapeMetadataProvider(it, model) }
// Derive `Eq` and `Hash` if possible.
.let { DeriveEqAndHashSymbolMetadataProvider(it, model) }
// Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot
// be the name of an operation input
.let { RustReservedWordSymbolProvider(it, model) }
Expand Down
Expand Up @@ -8,7 +8,10 @@ package software.amazon.smithy.rust.codegen.server.python.smithy
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StringShape
Expand Down Expand Up @@ -55,7 +58,7 @@ class PythonServerSymbolVisitor(
val target = model.expectShape(shape.target)
val container = model.expectShape(shape.container)

// We are only targetting non syntetic inputs and outputs.
// We are only targeting non-synthetic inputs and outputs.
if (!container.hasTrait<SyntheticOutputTrait>() && !container.hasTrait<SyntheticInputTrait>()) {
return initial
}
Expand Down Expand Up @@ -88,10 +91,6 @@ class PythonServerSymbolVisitor(
* Note that since streaming members can only be used on the root shape, this can only impact input and output shapes.
*/
class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider, private val model: Model) : SymbolMetadataProvider(base) {
override fun memberMeta(memberShape: MemberShape): RustMetadata {
return base.toSymbol(memberShape).expectRustMetadata()
}

override fun structureMeta(structureShape: StructureShape): RustMetadata {
val baseMetadata = base.toSymbol(structureShape).expectRustMetadata()
return if (structureShape.hasStreamingMember(model)) {
Expand All @@ -106,7 +105,12 @@ class PythonStreamingShapeMetadataProvider(private val base: RustSymbolProvider,
} else baseMetadata
}

override fun enumMeta(stringShape: StringShape): RustMetadata {
return base.toSymbol(stringShape).expectRustMetadata()
}
override fun memberMeta(memberShape: MemberShape) = base.toSymbol(memberShape).expectRustMetadata()
override fun enumMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()

override fun listMeta(listShape: ListShape) = base.toSymbol(listShape).expectRustMetadata()
override fun mapMeta(mapShape: MapShape) = base.toSymbol(mapShape).expectRustMetadata()
override fun stringMeta(stringShape: StringShape) = base.toSymbol(stringShape).expectRustMetadata()
override fun numberMeta(numberShape: NumberShape) = base.toSymbol(numberShape).expectRustMetadata()
override fun blobMeta(blobShape: BlobShape) = base.toSymbol(blobShape).expectRustMetadata()
david-perez marked this conversation as resolved.
Show resolved Hide resolved
}