Deny warnings in generated crates #3194
Labels
Comments
|
For the record, as of writing (cfd8942), there's 210 tests in the server that fail to compile if we |
This was referenced May 3, 2024
ysaito1001
added a commit
that referenced
this issue
May 6, 2024
## Motivation and Context Mark unused code as such for [the latest compiler](https://blog.rust-lang.org/2024/05/02/Rust-1.78.0.html). ## Description We use the latest compiler as part of our release of AWS SDKs, which ensures that 3rd party dependencies should build even if they release a new version requiring a newer MSRV than we specify within this repository. At the time of writing, the latest compiler is 1.78.0, and it is now able to detect more unused code patterns that our current MSRV 1.75.0. The compiler 1.78.0 unfortunately caused a build failure in [check-sdk-codegen-unit-tests](https://github.com/smithy-lang/smithy-rs/blob/e8449bd152ffd6471516a125f565c63dd690c034/tools/ci-scripts/check-sdk-codegen-unit-tests) as follows ``` src/smithy-rs/aws/rust-runtime/aws-runtime/src/env_config/section.rs:21:8 | 16 | pub(crate) trait Section { | ------- methods in this trait ... 21 | fn properties(&self) -> &HashMap<String, Property>; | ^^^^^^^^^^ ... 27 | fn is_empty(&self) -> bool; | ^^^^^^^^ | = note: `-D dead-code` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(dead_code)] ``` We cannot eliminate this failure even if we specify `RUSTFLAGS="-D warnings"` because the generated client tests will render the following in its dot cargo directory: ``` cat .cargo/config.toml [build] rustflags = ["--deny","warnings"] ``` Plus, trying to eliminate `--deny warnings` contradicts what #3194 is trying to achieve. To fix it, we will follow what the compiler suggests to fix. ## Testing Verified it fixed our internal release pipeline. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
aws-sdk-rust-ci
pushed a commit
to awslabs/aws-sdk-rust
that referenced
this issue
May 7, 2024
… (#3624) ## Motivation and Context Mark unused code as such for [the latest compiler](https://blog.rust-lang.org/2024/05/02/Rust-1.78.0.html). ## Description We use the latest compiler as part of our release of AWS SDKs, which ensures that 3rd party dependencies should build even if they release a new version requiring a newer MSRV than we specify within this repository. At the time of writing, the latest compiler is 1.78.0, and it is now able to detect more unused code patterns that our current MSRV 1.75.0. The compiler 1.78.0 unfortunately caused a build failure in [check-sdk-codegen-unit-tests](https://github.com/smithy-lang/smithy-rs/blob/e8449bd152ffd6471516a125f565c63dd690c034/tools/ci-scripts/check-sdk-codegen-unit-tests) as follows ``` src/smithy-rs/aws/rust-runtime/aws-runtime/src/env_config/section.rs:21:8 | 16 | pub(crate) trait Section { | ------- methods in this trait ... 21 | fn properties(&self) -> &HashMap<String, Property>; | ^^^^^^^^^^ ... 27 | fn is_empty(&self) -> bool; | ^^^^^^^^ | = note: `-D dead-code` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(dead_code)] ``` We cannot eliminate this failure even if we specify `RUSTFLAGS="-D warnings"` because the generated client tests will render the following in its dot cargo directory: ``` cat .cargo/config.toml [build] rustflags = ["--deny","warnings"] ``` Plus, trying to eliminate `--deny warnings` contradicts what smithy-lang/smithy-rs#3194 is trying to achieve. To fix it, we will follow what the compiler suggests to fix. ## Testing Verified it fixed our internal release pipeline. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
|
We also generate crates with doc warnings (mostly broken links), but CI doesn't fail, despite us baking in |
david-perez
added a commit
that referenced
this issue
Jun 6, 2024
…ests This commit gets rid of some Clippy and Rust doc warnings we produce in generated code, which are surfaced in our codegen integration tests. The aim is that eventually we will be able to deny Clippy and Rust doc warnings in #3678 (despite us baking in `RUSTDOCFLAGS="-D warnings"` and `RUSTFLAGS="-D warnings"` in our CI Docker image, we curently clear these in codegen integration tests). Note that denying compiler warnings is separately tracked in #3194. The commit contains fixes for the following: - Unconditionally referring to the `crate::types` module in Rust docs when the Smithy model is such that it is not generated. - Broken intra-crate doc links for client documentation. - Incorrectly escaping Rust reserved keywords when referring to operation names in Rust docs. - An unnecessary semi-colon when rendering additional client plugins. - Not escaping brackets in text when rendering Rust docs containing HTML, that are interpreted as broken links. - A broken link to unit variants of unions. - Using `TryFrom` instead of `From` when infallibly converting from `&str` to `String` when deserializing an `@httpPayload` in the server. - Using a redundant closure with `unwrap_or_else` when falling back to a `@default` boolean or number value for a `document` shape, instead of `unwrap_or`. - Not opting into `clippy::enum_variant_names` when rendering the `Operation` enum in the server (since we render the operation names that were modeled as-is).
github-merge-queue bot
pushed a commit
that referenced
this issue
Jun 11, 2024
…ests (#3684) This commit gets rid of some Clippy and Rust doc warnings we produce in generated code, which are surfaced in our codegen integration tests. The aim is that eventually we will be able to deny Clippy and Rust doc warnings in #3678 (despite us baking in `RUSTDOCFLAGS="-D warnings"` and `RUSTFLAGS="-D warnings"` in our CI Docker image, we curently clear these in codegen integration tests). Note that denying compiler warnings is separately tracked in #3194. The commit contains fixes for the following: - Unconditionally referring to the `crate::types` module in Rust docs when the Smithy model is such that it is not generated. - Broken intra-crate doc links for client documentation. - Incorrectly escaping Rust reserved keywords when referring to operation names in Rust docs. - An unnecessary semi-colon when rendering additional client plugins. - Not escaping brackets in text when rendering Rust docs containing HTML, that are interpreted as broken links. - A broken link to unit variants of unions. - Using `TryFrom` instead of `From` when infallibly converting from `&str` to `String` when deserializing an `@httpPayload` in the server. - Using a redundant closure with `unwrap_or_else` when falling back to a `@default` boolean or number value for a `document` shape, instead of `unwrap_or`. - Not opting into `clippy::enum_variant_names` when rendering the `Operation` enum in the server (since we render the operation names that were modeled as-is). ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We already
--deny warningsin:codegen-{core,client,server}-testGradle subprojects),{client,server}IntegrationTests; andRustWriter.compileAndTest, via theRUSTFLAGSwe bake in the CI Docker image (note though that we should delete usages ofRustWriter.compileAndTestRemoveRustWriter.compileAndTestin favor of{client,server}IntegrationTestorTestWriterDelegator.compileAndTest#3193).However, we do not deny warnings in
TestWriterDelegator.compileAndTest; we generate crates with tests that have warnings.It seems worthwhile to modify the tests so that they don't generate warnings to be consistent across the board. If a test can't be modified so as not to emit warnings, that specific test should opt-in into allowing warnings.
The text was updated successfully, but these errors were encountered: