Skip to content

Use std::result::Result instead of Result#4010

Merged
drganjoo merged 10 commits intomainfrom
fahadzub/fix-result
Feb 20, 2025
Merged

Use std::result::Result instead of Result#4010
drganjoo merged 10 commits intomainfrom
fahadzub/fix-result

Conversation

@drganjoo
Copy link
Contributor

When the model contains a shape by the name of Result, the generated code fails to compile. For instance the following model fails:

@restJson1
service ConstrainedService {
    operations: [SampleOperation]
}

@http(uri: "/anOperation", method: "POST")
operation SampleOperation {
    output: SampleInputOutput
    input: SampleInputOutput
    errors: [ValidationException]
}

structure SampleInputOutput {
    result: Result
}

structure Result {
    @pattern("^a-z$")
    chat: String
}

This PR ensures:

  1. If rustTemplate is being used, then #{Result} is used
  2. If rust is being used, then that is changed to rustTemplate and then #{Result} is used.
  3. If rustBlock is being used, then the generated code should use std::result::Result

@ysaito1001
Copy link
Contributor

Thanks for going over many places! One question, is there any reason to not use *preludeScope instead of "Result" to std.resolve("result::Result")?

@drganjoo drganjoo marked this pull request as ready for review February 14, 2025 17:21
@drganjoo drganjoo requested review from a team as code owners February 14, 2025 17:21
@drganjoo
Copy link
Contributor Author

drganjoo commented Feb 14, 2025

Thanks for going over many places! One question, is there any reason to not use *preludeScope instead of "Result" to std.resolve("result::Result")?

I maintained the use of preludeScope only in locations where it was already implemented. The array contains over 20 entries, and using *preludeScope would pass all entries onto the stack. While I initially aimed to optimize compilation time, I now realize this optimization would be negligible compared to the total compilation time. I will submit a follow-up commit to consistently use preludeScope instead of "Result" to std.resolve("result::Result")

aws-sdk-rust-ci and others added 5 commits February 14, 2025 17:29
## Motivation and Context
This implements fuzzing for smithy-rs servers

## Description
<!--- Describe your changes in detail -->

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.
- [ ] For changes to the AWS SDK, generated SDK code, or SDK runtime
crates, I have created a changelog entry Markdown file in the
`.changelog` directory, specifying "aws-sdk-rust" in the `applies_to`
key.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: david-perez <d@vidp.dev>
## Motivation and Context
There was downstream breakage (the macro was always broken, but #4001
made it more likely to happen)

## Description
- Use `$crate` to fix the issue when `mock_client` was not otherwise in
scope
- Add a test
## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.
- [ ] For changes to the AWS SDK, generated SDK code, or SDK runtime
crates, I have created a changelog entry Markdown file in the
`.changelog` directory, specifying "aws-sdk-rust" in the `applies_to`
key.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: Landon James <lnj@amazon.com>
When two inline modules with the same name are generated from different
parts of the codebase, their documentation should be merged. However,
other metadata must match exactly, as it is an error for one part of the
codebase to define an inline module with pub visibility while another
defines it with pub(crate) visibility.

This PR enables documentation merging while maintaining strict
validation of other metadata fields.

Currently, the following sample model fails to generate code because
both `SomeList` and `member` are generated in the same inline module but
with different doc comments:

```smithy
@documentation("Outer constraint has some documentation")
@Length(max: 3)
list SomeList {
    @Length(max: 8000)
    member: String
}
```

Co-authored-by: Fahad Zubair <fahadzub@amazon.com>
@drganjoo drganjoo force-pushed the fahadzub/fix-result branch from bdb7681 to f4e533c Compare February 14, 2025 17:41
@drganjoo drganjoo added this pull request to the merge queue Feb 20, 2025
Merged via the queue into main with commit ad119f6 Feb 20, 2025
43 of 45 checks passed
@drganjoo drganjoo deleted the fahadzub/fix-result branch February 20, 2025 09:34
@drganjoo drganjoo mentioned this pull request Feb 20, 2025
aws-sdk-rust-ci pushed a commit that referenced this pull request Feb 20, 2025
This PR adds the CHANGELOG entries that were missed in these two earlier
PRs:

#4010    
#4008
github-merge-queue bot pushed a commit that referenced this pull request Feb 24, 2025
…#4031)

The [earlier PR](#4010),
which ensured that a Smithy shape named `Result` does not conflict with
the Rust type `Result`, had a bug in `TSApplicationGenerator`. The
`rustTemplateBlock` was using `#{Result}` but `preludeScope` was not
passed in as a parameter.

This bug can be addressed in two ways:

1. Pass `*RuntimeType.preludeScope` to the `rustBlockTemplate` call
2. Remove `#{Result}` from `rustBlockTemplate` and continue using
`Result<>`

This PR implements the second approach because:

1. `TSApplicationGenerator` is slated for deprecation
2. The CI step `CI / PR Bot / Generate diff and upload to S3
(pull_request)` currently fails when comparing codegen between HEAD and
this version. Removing `#{Result}` is the most reliable way to avoid
parameter-passing issues with `rustBlockTemplate`

Co-authored-by: Fahad Zubair <fahadzub@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants