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

Rfc30/feature gate for generated crates #2183

Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9b70288
add CfgUnstable for feature-gate
thomas-k-cameron Jan 8, 2023
5c794cf
add features
thomas-k-cameron Jan 8, 2023
6403cec
add feature-gate
thomas-k-cameron Jan 8, 2023
1f33a5c
strings for feature gate
thomas-k-cameron Jan 8, 2023
2fb7178
Revert "strings for feature gate"
thomas-k-cameron Jan 8, 2023
3513d7d
Merge branch 'main' into rfc30/feature-gate-for-generated-crates
thomas-k-cameron Jan 9, 2023
1717d72
Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codeg…
thomas-k-cameron Jan 11, 2023
e16ce62
fix dependency thing on cargo
thomas-k-cameron Jan 11, 2023
14d5e1d
add OutputShape to builder
thomas-k-cameron Jan 11, 2023
cc61ede
EnumGenerator
thomas-k-cameron Jan 11, 2023
22ef698
StructureGenerator
thomas-k-cameron Jan 11, 2023
7aaa744
UnionGenerator
thomas-k-cameron Jan 11, 2023
8b12529
todo
thomas-k-cameron Jan 11, 2023
cd602bf
fixed?
thomas-k-cameron Jan 12, 2023
b90a1b2
SerdeDecorator
thomas-k-cameron Jan 12, 2023
1cefde2
codegen stuff
thomas-k-cameron Jan 12, 2023
af24d35
Merge branch 'main' into rfc30/feature-gate-for-generated-crates
thomas-k-cameron Jan 12, 2023
25ed6ff
update
thomas-k-cameron Jan 12, 2023
6a3757e
fix
thomas-k-cameron Jan 12, 2023
a1dd430
Apply suggestions from code review
thomas-k-cameron Jan 24, 2023
8c0843d
- refactoring
thomas-k-cameron Jan 24, 2023
d64ee3a
adds serde-serialize/deserialize
thomas-k-cameron Jan 24, 2023
296ab7b
Merge branch 'unstable-serde-support' into rfc30/feature-gate-for-gen…
thomas-k-cameron Jan 24, 2023
e0811b2
this one causes null pointer exception
thomas-k-cameron Jan 27, 2023
a5c2eb8
interim solution
thomas-k-cameron Jan 30, 2023
8ceb679
new push
thomas-k-cameron Jan 31, 2023
b3fa786
fix
thomas-k-cameron Feb 3, 2023
9f84174
add Sensitive Warning
thomas-k-cameron Feb 3, 2023
a5970c0
add test for CargoTomlGeneratorTest
thomas-k-cameron Feb 3, 2023
4e8a72c
Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codeg…
thomas-k-cameron Feb 18, 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegen
import software.amazon.smithy.rust.codegen.client.smithy.customize.CombinedClientCodegenDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.NoOpEventStreamSigningDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.RequiredCustomizations
import software.amazon.smithy.rust.codegen.client.smithy.customize.SerdeDecorator
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointsDecorator
import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientDecorator
import software.amazon.smithy.rust.codegen.client.testutil.DecoratableBuildPlugin
Expand Down Expand Up @@ -53,6 +54,7 @@ class RustClientCodegenPlugin : DecoratableBuildPlugin() {
val codegenDecorator =
CombinedClientCodegenDecorator.fromClasspath(
context,
SerdeDecorator(),
ClientCustomizations(),
RequiredCustomizations(),
FluentClientDecorator(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.client.smithy.customize

import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.core.rustlang.Feature
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate

/**
* This class,
* - Adds serde as a dependency
*
*/
class SerdeDecorator : ClientCodegenDecorator {
override val name: String = "SerdeDecorator"
override val order: Byte = -1

override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) {
fun _feature(feature_name: String, crate_name: String): Feature {
return Feature(feature_name, false, listOf(crate_name + "/" + feature_name))
}
rustCrate.mergeFeature(_feature("serde-serialize", "aws-smithy-types"))
rustCrate.mergeFeature(_feature("serde-deserialize", "aws-smithy-types"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import java.nio.file.Path
sealed class DependencyScope {
object Dev : DependencyScope()
object Compile : DependencyScope()
object CfgUnstable : DependencyScope()
Velfi marked this conversation as resolved.
Show resolved Hide resolved
}

sealed class DependencyLocation
Expand Down Expand Up @@ -245,5 +246,8 @@ data class CargoDependency(
fun smithyQuery(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-query")
fun smithyTypes(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-types")
fun smithyXml(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-xml")

// behind feature-gate
val Serde = CargoDependency("serde", CratesIo("1.0"), features = setOf("derive"), scope = DependencyScope.CfgUnstable)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,16 @@ class Attribute(val inner: Writable) {
}
}

// These were supposed to be a part of companion object but we decided to move it out to here to avoid NPE
// You can find the discussion here.
// https://github.com/awslabs/smithy-rs/discussions/2248
public fun SerdeSerialize(): Attribute {
return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))
}
public fun SerdeDeserialize(): Attribute {
return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))
}
Comment on lines +452 to +457
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need for these to be functions. Change them into values and put them in the companion object below.

Suggested change
public fun SerdeSerialize(): Attribute {
return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))
}
public fun SerdeDeserialize(): Attribute {
return Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))
}
val SerdeSerialize = Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-serialize")), derive(RuntimeType.SerdeSerialize)))
val SerdeDeserialize = Attribute(cfgAttr(all(writable("aws_sdk_unstable"), feature("serde-deserialize")), derive(RuntimeType.SerdeDeserialize)))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This causes NPE too.


companion object {
val AllowClippyBoxedLocal = Attribute(allow("clippy::boxed_local"))
val AllowClippyLetAndReturn = Attribute(allow("clippy::let_and_return"))
Expand Down Expand Up @@ -498,6 +508,7 @@ class Attribute(val inner: Writable) {
}

fun all(vararg attrMacros: Writable): Writable = macroWithArgs("all", *attrMacros)
fun cfgAttr(vararg attrMacros: Writable): Writable = macroWithArgs("cfg_attr", *attrMacros)

fun allow(lints: Collection<String>): Writable = macroWithArgs("allow", *lints.toTypedArray())
fun allow(vararg lints: String): Writable = macroWithArgs("allow", *lints)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
val ConstrainedTrait = RuntimeType("crate::constrained::Constrained", InlineDependency.constrained())
val MaybeConstrained = RuntimeType("crate::constrained::MaybeConstrained", InlineDependency.constrained())

// serde types. Gated behind `CfgUnstable`.
val Serde = CargoDependency.Serde.toType()
val SerdeSerialize = Serde.resolve("Serialize")
val SerdeDeserialize = Serde.resolve("Deserialize")

// smithy runtime types
fun smithyAsync(runtimeConfig: RuntimeConfig) = CargoDependency.smithyAsync(runtimeConfig).toType()
fun smithyChecksums(runtimeConfig: RuntimeConfig) = CargoDependency.smithyChecksums(runtimeConfig).toType()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.model.Model
Expand All @@ -27,6 +26,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.documentShape
import software.amazon.smithy.rust.codegen.core.rustlang.render
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
import software.amazon.smithy.rust.codegen.core.rustlang.rustInline
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
Expand All @@ -43,6 +43,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.locatedIn
import software.amazon.smithy.rust.codegen.core.smithy.makeOptional
import software.amazon.smithy.rust.codegen.core.smithy.module
import software.amazon.smithy.rust.codegen.core.smithy.rustType
import software.amazon.smithy.rust.codegen.core.smithy.shape
import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait
import software.amazon.smithy.rust.codegen.core.util.dq
import software.amazon.smithy.rust.codegen.core.util.hasTrait
Expand Down Expand Up @@ -206,13 +207,19 @@ class BuilderGenerator(
}

private fun renderBuilder(writer: RustWriter) {
writer.docs("This is the datatype that Builder of this module build itself into.")
thomas-k-cameron marked this conversation as resolved.
Show resolved Hide resolved
writer.rustInline("pub type OutputShape = #T;", structureSymbol)
writer.docs("A builder for #D.", structureSymbol)
Attribute(derive(builderDerives)).render(writer)
RenderSerdeAttribute.forStructureShape(writer, shape, model)
SensitiveWarning.addDoc(writer, shape)
writer.rustBlock("pub struct $builderName") {
// add serde
for (member in members) {
val memberName = symbolProvider.toMemberName(member)
// All fields in the builder are optional.
val memberSymbol = symbolProvider.toSymbol(member).makeOptional()
SensitiveWarning.addDoc(writer, member)
renderBuilderMember(this, memberName, memberSymbol)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class CargoTomlGenerator(
.associate { it.name to it.toMap() },
"dev-dependencies" to dependencies.filter { it.scope == DependencyScope.Dev }
.associate { it.name to it.toMap() },
"target.'cfg(aws_sdk_unstable)'.dependencies" to dependencies.filter { it.scope == DependencyScope.CfgUnstable }
.associate { it.name to it.toMap() },
"features" to cargoFeatures.toMap(),
).deepMergeWith(manifestCustomizations)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.StringShape
Expand Down Expand Up @@ -139,6 +138,8 @@ open class EnumGenerator(
private fun renderUnnamedEnum() {
writer.documentShape(shape, model)
writer.deprecatedShape(shape)
RenderSerdeAttribute.writeAttributes(writer)
SensitiveWarning.addDoc(writer, shape)
meta.render(writer)
writer.write("struct $enumName(String);")
writer.rustBlock("impl $enumName") {
Expand Down Expand Up @@ -178,7 +179,8 @@ open class EnumGenerator(
renamedWarning.ifBlank { null },
)
writer.deprecatedShape(shape)

RenderSerdeAttribute.writeAttributes(writer)
SensitiveWarning.addDoc(writer, shape)
meta.render(writer)
writer.rustBlock("enum $enumName") {
sortedMembers.forEach { member -> member.render(writer) }
Expand Down Expand Up @@ -225,6 +227,7 @@ open class EnumGenerator(
part of the enums that are public interface.
""".trimIndent(),
)
// adding serde features here adds attribute to the end of the file for some reason
meta.render(this)
rust("struct $UnknownVariantValue(pub(crate) String);")
rustBlock("impl $UnknownVariantValue") {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.util.isEventStream

public object RenderSerdeAttribute {
public fun forStructureShape(writer: RustWriter, shape: StructureShape, model: Model) {
if (shape.members().none { it.isEventStream(model) }) {
writeAttributes(writer)
}
}

public fun writeAttributes(writer: RustWriter) {
Attribute("").SerdeSerialize().render(writer)
Attribute("").SerdeDeserialize().render(writer)
Comment on lines +22 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

After making the change suggested in my earlier comment, you can update this function like so:

Suggested change
Attribute("").SerdeSerialize().render(writer)
Attribute("").SerdeDeserialize().render(writer)
Attribute.SerdeSerialize.render(writer)
Attribute.SerdeDeserialize.render(writer)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this causes NPE.
Here is the discussion.

#2248

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.traits.SensitiveTrait
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.util.hasTrait

object SensitiveWarning {
private const val warningMessage = "/// This data may contain sensitive information; It will not be obscured when serialized.\n"
fun<T : Shape> addDoc(writer: RustWriter, shape: T) {
if (shape.hasTrait<SensitiveTrait>()) {
writer.writeInline(warningMessage)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.model.Model
Expand Down Expand Up @@ -134,6 +133,7 @@ open class StructureGenerator(

open fun renderStructureMember(writer: RustWriter, member: MemberShape, memberName: String, memberSymbol: Symbol) {
writer.renderMemberDoc(member, memberSymbol)
SensitiveWarning.addDoc(writer, shape)
writer.deprecatedShape(member)
memberSymbol.expectRustMetadata().render(writer)
writer.write("$memberName: #T,", memberSymbol)
Expand All @@ -144,10 +144,13 @@ open class StructureGenerator(
val containerMeta = symbol.expectRustMetadata()
writer.documentShape(shape, model)
writer.deprecatedShape(shape)
RenderSerdeAttribute.forStructureShape(writer, shape, model)
SensitiveWarning.addDoc(writer, shape)
containerMeta.render(writer)

writer.rustBlock("struct $name ${lifetimeDeclaration()}") {
writer.forEachMember(members) { member, memberName, memberSymbol ->
SensitiveWarning.addDoc(writer, shape)
renderStructureMember(writer, member, memberName, memberSymbol)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.model.Model
Expand Down Expand Up @@ -61,7 +60,7 @@ class UnionGenerator(
fun render() {
writer.documentShape(shape, model)
writer.deprecatedShape(shape)

RenderSerdeAttribute.writeAttributes(writer)
val containerMeta = unionSymbol.expectRustMetadata()
containerMeta.render(writer)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,32 @@ class CargoTomlGeneratorTest {
}
project.compileAndTest()
}

@Test
fun `check serde features`() {
val project = TestWorkspace.testProject()
/*
["target.'cfg(aws_sdk_unstable)'.dependencies".serde]
version = "1.0"
features = ["derive"]
serde-serialize = ["aws-smithy-types/serde-serialize"]
serde-deserialize = ["aws-smithy-types/serde-deserialize"]
*/
project.lib {
addDependency(CargoMetadata)
unitTest(
"smithy_codegen_serde_features",
"""
let metadata = cargo_metadata::MetadataCommand::new()
.exec()
.expect("could not run `cargo metadata`");

let features = &metadata.root_package().expect("missing root package").features;

assert_eq!(features.get("aws-aws-smithy-types"), Some(vec!["serde-serialize", "serde-deserialize"]));
""",
)
}
project.compileAndTest()
}
}
4 changes: 2 additions & 2 deletions rust-runtime/aws-smithy-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ name = "base64"
harness = false

[features]
"serialize" = []
"deserialize" = []
serde-serialize = []
serde-deserialize = []