Skip to content

Commit

Permalink
[smithy-rs] Respect the sensitive trait on container shapes (#1983)
Browse files Browse the repository at this point in the history 
* Make enum forward-compatible

This commit implements the suggested approach described in
smithy-lang/smithy-rs#627 (comment)

The idea is that once the user writes a match expression against an enum
and assumes that an execution path comes to a particular match arm, we
should guarantee that when the user upgrades a version of SDK, the
execution path should come to the same match arm as before.

* Add unit test to ensure enums are forward-compatible

The test first mimics the user's interaction with the enum generated from
a model where the user writes a match expression on the enum, wishing to
hit the match arm corresponding to Variant3, which is not yet supported
by the model. The test then simulates a scenario where the user now has
access to the updated enum generated from the next version of the model
that does support Variant3.
The user's code should compile in both of the use cases.

* Generate rustdoc for enum's forward-compatibility

This commits generates rustdoc explaining to the users how they might
write a match expression against a generated enum so their code is
forward-compatible. While it is explained in
smithy-lang/smithy-rs#627 (comment),
it can be helpful if the users can read it in rustdoc right below where
the enum's definition is shown.

* Make snippet in rustdoc text

This commit updates a rustdoc code snippet generated for an enum from
`rust,no_run` to `text`. We observed that the snippet fails to compile
in CI because the name of the enum was not fully qualified; code in
rustdoc is treated in a similar way that code in integration tests is
treated as being outside of a crate, but not part of the crate, which
means the name of the crate needs to be spelled out for a `use` statement
if we want to import the enum to the code in rustdoc. However, the name
of the crate is usually one-off, generated on the fly for testing, making
it not obvious to hard-code it or obtain it programmatically from within
Kotlin code.

* Suppress missing doc lint for UnknownVariantValue

This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because
failing to do so will cause tests in CI to fail.

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt

Co-authored-by: Russell Cohen <[email protected]>

* Generate UnknownVariantValue via forInlineFun

This commit attempts to create UnknownVariantValue using
RuntimeType.forInlineFun. Prior to this commit, it was generated per enum
but that caused multiple definitions of UnknownVariantValue in a single
file as there can be multiple enums in the file.
By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue
per module and the enums within the module can refer to the same instance.

This commit, however, fails to pass the unit tests in EnumGeneratorTest.
The issue seems to be that a RustWriter used in the tests does not end up
calling the finalize method, failing to render inline functions.

* Replace "target == CodegenTarget.CLIENT" with a helper

This commit addresses smithy-lang/smithy-rs#1945 (comment)
but uses an existing helper CodegenTarget.renderUnknownVariant.

* Update EnumGeneratorTests to use TestWorkspace

This commit addresses failures for unit tests in EnumGeneratorTests.
Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need
to ensure that the inline functions should be rendered to a Rust
source file during testing. RustWriter, used in EnumGeneratorTests
prior to this commit, was not capable of doing so. We thus update
EnumGeneratorTests to use TestWorkspace by which we ensure that the
inline functions are rendered during testing.

* Make sure to use the passed-in variable for shapeId

This commit fixes a copy-and-paste error introduced in 712c983. The
function should use the passed-in variable rather than the hard-coded
literal "test#SomeEnum".

* Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852

* Update CHANGELOG.next.toml

* Update CHANGELOG.next.toml

This commit addresses smithy-lang/smithy-rs#1945 (comment)

* Avoid potential name collisions by UnknownVariantValue

This commit addresses smithy-lang/smithy-rs#1945 (comment).
It changes the module in which the `UnknownVariantValue` is rendered.
Before the commit, it was emitted to the `model` module but that might cause
name collisions in the future when a Smithy model has a shape named
`UnknownVariantValue`. After this commit, we render it to the `types` module.

* Move re-exports from lib.rs to types.rs

This commit moves re-export statements in client crates from `lib.rs` to
`types.rs`. The motivation is that now that we render the struct
`UnknownVariantValue` in a separate file for the `types` module, we can
no longer have a `pub mod types {...}` block in `lib.rs` as it cannot
coexist with `pub mod types;`.

* Add docs on UnknownVariantValue

This commit adds public docs on `UnknownVariantValue`. Since it is
rendered in `types.rs`, the users may not find the `Unknown` variant
of an enum in the same file and may wonder what this struct is for when
seeing it in isolation.

* Update CHANGELOG.next.toml

This commit addresses smithy-lang/smithy-rs#1945 (comment)

* Update the module documentation for types

This commit updates rustdoc for the types module to reflect that fact
that the module now contains not only re-exports but also an auxiliary
struct `UnknownVariantValue`.

* Add extensions to run code block depending on the target

This commit introduces extensions on CodegenTarget that execute the block
of code depending on the target is for client or for server. These
extensions are intended to replace if-else expressions throughout the
codebase explicitly checking whether the target is for client or for server.
We do not update such if-else expressions all at once in this commit.
Rather, we are planning to make incremental changes to update them
opportunistically.

* Respect the sensitive trait on enums

This commit fixes #1745. It allows enums to respect the sensitive trait.
When annotated as such, an enum will not display its data and instead
will show the redacted text upon debug print.

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt

Co-authored-by: John DiSanti <[email protected]>

* Move extensions into CodegenTarget as methods

This commit addresses smithy-lang/smithy-rs#1945 (comment).
By moving extensions into the CodegenTarget enum, no separate import is
required to use them.

* Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt

Co-authored-by: david-perez <[email protected]>

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt

Co-authored-by: david-perez <[email protected]>

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt

Co-authored-by: david-perez <[email protected]>

* Update CHANGELOG.next.toml

* Configure the (in|ex)clusion of the Debug trait for containers per members' sensitive trait (#2029)

* Removes Debug for shape with sensitive trait

This commit centralizes a place where the codegen excludes the Debug
trait if a shape has the sensitive trait. Previously the exclusion was
handled locally in each shape, e.g. StructureGenerator and EnumGenerator.
However, that approach may overlook a certain shape we also need to treat
as such. We now handle the exclusion of the Debug trait in one place,
BaseSymbolMetadataProvider.

* Stop excluding the Debug trait locally

This commit updates EnumGenerator and StructureGenerator based on the
change made to BaseSymbolMetadataProvider in the previous commit.
Now that the exclusion of the Debug trait was centralized, those
classes in question no longer need to do so individually.

* Implement a custom Debug trait in BuilderGenerator

This commit implements a custom Debug trait in BuilderGenerator now that
the derived Debug trait is excluded from BaseSymbolMetadataProvider for
the structure container. The implementation of the custom Debug trait
pretty much follows that of StructureGenerator.

* Implement a custom Debug trait in UnionGenerator

This commit implements a custom Debug trait in BuilderGenerator now that
the derived Debug trait is excluded from BaseSymbolMetadataProvider for
the union container. The implementation of the custom Debug trait pretty
much follows that of EnumGenerator.

* Implement a custom Debug trait in ServerBuilderGenerator

This commit implements a custom Debug trait in ServerBuilderGenerator now
that the derived Debug trait is excluded from BaseSymbolMetadataProvider
for the structure container. The implementation of the custom Debug trait
pretty much follows that of StructureGenerator.

* Add the Copyright header

* Update CHANGELOG.next.toml

* Update Debug impl for UnionGenerator

This commit updates the implementation of a custom Debug trait impl for
UnionGenerator. Turns out that in a Union, a member target can be marked
as sensitive separately outside the Union. Therefore, the implementation
of a custom Debug trait has two cases depending on where the sensitive
trait appears, either it is applied to the whole Union or to a member
target.

* Peek at member sensitivity for Debug trait (in|ex)clusion

This commit addresses smithy-lang/smithy-rs#2029 (comment).
With this change, structure shapes no longer need to exclude the Debug
trait unconditionally. The upshot is that we may be able to avoid a
custom Debug impl for a structure where the derived Debug will do, i.e.
when there is no sensitive trait either at a container level or at a
member level.

* Remove statement that does not seem to take effect

This commit addresses smithy-lang/smithy-rs#2029 (comment)

* Rename renderDebugImplForUnion -> renderFullyRedactedDebugImpl

This commit addresses smithy-lang/smithy-rs#2029 (comment)

* Rename renderDebugImplForUnionMemberWise -> renderDebugImpl

This commit addresses smithy-lang/smithy-rs#2029 (comment)

Co-authored-by: Yuki Saito <[email protected]>

Co-authored-by: Saito <[email protected]>
Co-authored-by: Russell Cohen <[email protected]>
Co-authored-by: Yuki Saito <[email protected]>
Co-authored-by: John DiSanti <[email protected]>
Co-authored-by: david-perez <[email protected]>
  • Loading branch information
@ysaito1001 @rcoh
@jdisanti @david-perez @aws-sdk-rust-ci
6 people authored and aws-sdk-rust-ci committed Dec 14, 2022
1 parent 116ded5 commit 9852654
Show file tree
Hide file tree
Showing 1,256 changed files with 55,817 additions and 420,966 deletions.
73 changes: 7 additions & 66 deletions sdk/accessanalyzer/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/// <p>Validation exception error.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct ValidationException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -24,15 +24,6 @@ impl ValidationException {
self.field_list.as_deref()
}
}
impl std::fmt::Debug for ValidationException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("ValidationException");
formatter.field("message", &self.message);
formatter.field("reason", &self.reason);
formatter.field("field_list", &self.field_list);
formatter.finish()
}
}
impl ValidationException {
/// Returns the error message.
pub fn message(&self) -> std::option::Option<&str> {
Expand Down Expand Up @@ -124,7 +115,7 @@ impl ValidationException {

/// <p>Throttling limit exceeded error.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct ThrottlingException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -139,14 +130,6 @@ impl ThrottlingException {
self.retry_after_seconds
}
}
impl std::fmt::Debug for ThrottlingException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("ThrottlingException");
formatter.field("message", &self.message);
formatter.field("retry_after_seconds", &self.retry_after_seconds);
formatter.finish()
}
}
impl ThrottlingException {
/// Returns `Some(ErrorKind)` if the error is retryable. Otherwise, returns `None`.
pub fn retryable_error_kind(&self) -> aws_smithy_types::retry::ErrorKind {
Expand Down Expand Up @@ -217,7 +200,7 @@ impl ThrottlingException {

/// <p>Internal server error.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct InternalServerException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -232,14 +215,6 @@ impl InternalServerException {
self.retry_after_seconds
}
}
impl std::fmt::Debug for InternalServerException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("InternalServerException");
formatter.field("message", &self.message);
formatter.field("retry_after_seconds", &self.retry_after_seconds);
formatter.finish()
}
}
impl InternalServerException {
/// Returns `Some(ErrorKind)` if the error is retryable. Otherwise, returns `None`.
pub fn retryable_error_kind(&self) -> aws_smithy_types::retry::ErrorKind {
Expand Down Expand Up @@ -310,19 +285,12 @@ impl InternalServerException {

/// <p>You do not have sufficient access to perform this action.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct AccessDeniedException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
pub message: std::option::Option<std::string::String>,
}
impl std::fmt::Debug for AccessDeniedException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("AccessDeniedException");
formatter.field("message", &self.message);
formatter.finish()
}
}
impl AccessDeniedException {
/// Returns the error message.
pub fn message(&self) -> std::option::Option<&str> {
Expand Down Expand Up @@ -377,7 +345,7 @@ impl AccessDeniedException {

/// <p>Service quote met error.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct ServiceQuotaExceededException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -399,15 +367,6 @@ impl ServiceQuotaExceededException {
self.resource_type.as_deref()
}
}
impl std::fmt::Debug for ServiceQuotaExceededException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("ServiceQuotaExceededException");
formatter.field("message", &self.message);
formatter.field("resource_id", &self.resource_id);
formatter.field("resource_type", &self.resource_type);
formatter.finish()
}
}
impl ServiceQuotaExceededException {
/// Returns the error message.
pub fn message(&self) -> std::option::Option<&str> {
Expand Down Expand Up @@ -489,7 +448,7 @@ impl ServiceQuotaExceededException {

/// <p>A conflict exception error.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct ConflictException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -511,15 +470,6 @@ impl ConflictException {
self.resource_type.as_deref()
}
}
impl std::fmt::Debug for ConflictException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("ConflictException");
formatter.field("message", &self.message);
formatter.field("resource_id", &self.resource_id);
formatter.field("resource_type", &self.resource_type);
formatter.finish()
}
}
impl ConflictException {
/// Returns the error message.
pub fn message(&self) -> std::option::Option<&str> {
Expand Down Expand Up @@ -601,7 +551,7 @@ impl ConflictException {

/// <p>The specified resource could not be found.</p>
#[non_exhaustive]
#[derive(std::clone::Clone, std::cmp::PartialEq)]
#[derive(std::clone::Clone, std::cmp::PartialEq, std::fmt::Debug)]
pub struct ResourceNotFoundException {
#[allow(missing_docs)] // documentation missing in model
#[doc(hidden)]
Expand All @@ -623,15 +573,6 @@ impl ResourceNotFoundException {
self.resource_type.as_deref()
}
}
impl std::fmt::Debug for ResourceNotFoundException {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut formatter = f.debug_struct("ResourceNotFoundException");
formatter.field("message", &self.message);
formatter.field("resource_id", &self.resource_id);
formatter.field("resource_type", &self.resource_type);
formatter.finish()
}
}
impl ResourceNotFoundException {
/// Returns the error message.
pub fn message(&self) -> std::option::Option<&str> {
Expand Down
Loading

0 comments on commit 9852654

Please sign in to comment.