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

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

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
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