Bump Rust MSRV to 1.63.0 (smithy-lang#2222)

Co-authored-by: Zelda Hessler <zhessler@amazon.com> Co-authored-by: John DiSanti <jdisanti@amazon.com>
thomas-k-cameron · Feb 10, 2023 · de18667 · de18667
1 parent 13b10d5
commit de18667
Show file tree

Hide file tree

Showing 53 changed files with 126 additions and 60 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -29,7 +29,7 @@ on:
         required: false
 
 env:
-  rust_version: 1.62.1
+  rust_version: 1.63.0
   rust_toolchain_components: clippy,rustfmt
   ENCRYPTED_DOCKER_PASSWORD: ${{ secrets.ENCRYPTED_DOCKER_PASSWORD }}
   DOCKER_LOGIN_TOKEN_PASSPHRASE: ${{ secrets.DOCKER_LOGIN_TOKEN_PASSPHRASE }}

diff --git a/.github/workflows/claim-crate-names.yml b/.github/workflows/claim-crate-names.yml
@@ -10,7 +10,7 @@ concurrency:
   cancel-in-progress: true
 
 env:
-  rust_version: 1.62.1
+  rust_version: 1.63.0
 
 name: Claim unpublished crate names on crates.io
 run-name: ${{ github.workflow }}

diff --git a/.github/workflows/pull-request-bot.yml b/.github/workflows/pull-request-bot.yml
@@ -28,7 +28,7 @@ concurrency:
 
 env:
   java_version: 11
-  rust_version: 1.62.1
+  rust_version: 1.63.0
   rust_toolchain_components: clippy,rustfmt
   apt_dependencies: libssl-dev gnuplot jq
 

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -10,16 +10,16 @@ concurrency:
   cancel-in-progress: true
 
 env:
-  rust_version: 1.62.1
+  rust_version: 1.63.0
 
 name: Release smithy-rs
 run-name: ${{ github.workflow }} ${{ inputs.semantic_version }} (${{ inputs.commit_sha }}) - ${{ inputs.dry_run && 'Dry run' || 'Production run' }}
 on:
   workflow_dispatch:
     inputs:
       commit_sha:
-        description: | 
-          The SHA of the git commit that you want to release.  
+        description: |
+          The SHA of the git commit that you want to release.
           You must use the non-abbreviated SHA (e.g. b2318b0 won't work!).
         required: true
         type: string
@@ -75,8 +75,8 @@ jobs:
     # We need `always` here otherwise this job won't run if the previous job has been skipped
     # See https://samanpavel.medium.com/github-actions-conditional-job-execution-e6aa363d2867
     if: |
-      always() && 
-      needs.acquire-base-image.result == 'success' && 
+      always() &&
+      needs.acquire-base-image.result == 'success' &&
       (needs.release-ci.result == 'success' || needs.release-ci.result == 'skipped')
     runs-on: ubuntu-latest
     outputs:
@@ -234,7 +234,7 @@ jobs:
       shell: bash
       run: |
         set -eux
-        
+
         # This will fail if other commits have been pushed to `main` after `commit_sha`
         # In particular, this will ALWAYS fail if you are creating a new release series from
         # a commit that is not the current tip of `main`.

diff --git a/.github/workflows/update-sdk-next.yml b/.github/workflows/update-sdk-next.yml
@@ -32,7 +32,7 @@ jobs:
     - name: Set up Rust
       uses: dtolnay/rust-toolchain@master
       with:
-        toolchain: 1.62.1
+        toolchain: 1.63.0
     - name: Delete old SDK
       run: |
     - name: Generate a fresh SDK

diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -10,3 +10,15 @@
 # references = ["smithy-rs#920"]
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
 # author = "rcoh"
+
+[[aws-sdk-rust]]
+message = "Upgrade Rust MSRV to 1.63.0"
+references = ["smithy-rs#2222"]
+meta = { "breaking" = true, "tada" = true, "bug" = false }
+author = "Nugine"
+
+[[smithy-rs]]
+message = "Upgrade Rust MSRV to 1.63.0"
+references = ["smithy-rs#2222"]
+meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "all" }
+author = "Nugine"
diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs
@@ -917,7 +917,7 @@ pub(crate) mod test {
                 imds_request("http://169.254.169.254/latest/metadata", TOKEN_A),
                 http::Response::builder()
                     .status(200)
-                    .body(SdkBody::from(vec![0xA0 as u8, 0xA1 as u8]))
+                    .body(SdkBody::from(vec![0xA0, 0xA1]))
                     .unwrap(),
             ),
         ]);

diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs
@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_debug_implementations,
     missing_docs,

diff --git a/aws/rust-runtime/aws-credential-types/src/lib.rs b/aws/rust-runtime/aws-credential-types/src/lib.rs
@@ -8,6 +8,7 @@
 //! * An opaque struct representing credentials
 //! * Concrete implementations of credentials caching
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_debug_implementations,
     missing_docs,

diff --git a/aws/rust-runtime/aws-endpoint/src/lib.rs b/aws/rust-runtime/aws-endpoint/src/lib.rs
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#![allow(clippy::derive_partial_eq_without_eq)]
+
 use std::collections::HashMap;
 use std::error::Error;
 use std::fmt;
@@ -270,7 +272,7 @@ mod test {
         let mut req = operation::Request::new(req);
         {
             let mut props = req.properties_mut();
-            props.insert(region.clone());
+            props.insert(region);
             props.insert(SigningService::from_static("qldb"));
             props.insert(endpoint);
         };

diff --git a/aws/rust-runtime/aws-http/src/auth.rs b/aws/rust-runtime/aws-http/src/auth.rs
@@ -188,10 +188,7 @@ mod tests {
             .create_cache(SharedCredentialsProvider::new(provide_credentials_fn(
                 || async { Ok(Credentials::for_tests()) },
             )));
-        set_credentials_cache(
-            &mut req.properties_mut(),
-            SharedCredentialsCache::from(credentials_cache),
-        );
+        set_credentials_cache(&mut req.properties_mut(), credentials_cache);
         let req = CredentialsStage::new()
             .apply(req)
             .await

diff --git a/aws/rust-runtime/aws-http/src/lib.rs b/aws/rust-runtime/aws-http/src/lib.rs
@@ -5,6 +5,7 @@
 
 //! Provides user agent and credentials middleware for the AWS SDK.
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_docs,
     rustdoc::missing_crate_level_docs,

diff --git a/aws/rust-runtime/aws-inlineable/src/http_body_checksum.rs b/aws/rust-runtime/aws-inlineable/src/http_body_checksum.rs
@@ -269,7 +269,7 @@ mod tests {
 
         for i in 0..10000 {
             let line = format!("This is a large file created for testing purposes {}", i);
-            file.as_file_mut().write(line.as_bytes()).unwrap();
+            file.as_file_mut().write_all(line.as_bytes()).unwrap();
             crc32c_checksum.update(line.as_bytes());
         }
 

diff --git a/aws/rust-runtime/aws-inlineable/src/lib.rs b/aws/rust-runtime/aws-inlineable/src/lib.rs
@@ -10,6 +10,7 @@
 //! This is _NOT_ intended to be an actual crate. It is a cargo project to solely to aid
 //! with local development of the SDK.
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_docs,
     rustdoc::missing_crate_level_docs,

diff --git a/aws/rust-runtime/aws-sig-auth/src/lib.rs b/aws/rust-runtime/aws-sig-auth/src/lib.rs
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#![allow(clippy::derive_partial_eq_without_eq)]
+
 //! AWS Signature Authentication Package
 //!
 //! This crate may be used to generate presigned URLs for unmodeled behavior such as `rds-iam-token`

diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/query_writer.rs b/aws/rust-runtime/aws-sigv4/src/http_request/query_writer.rs
@@ -142,7 +142,7 @@ mod test {
                     let mut query_writer = QueryWriter::new(&uri);
                     query_writer.insert("key", value);
 
-                    if let Err(_) = std::panic::catch_unwind(|| query_writer.build_uri()) {
+                    if std::panic::catch_unwind(|| query_writer.build_uri()).is_err() {
                         problematic_chars.push(char::from(byte));
                     };
                 }

diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs
@@ -380,9 +380,11 @@ mod tests {
 
     #[test]
     fn test_sign_vanilla_with_query_params() {
-        let mut settings = SigningSettings::default();
-        settings.signature_location = SignatureLocation::QueryParams;
-        settings.expires_in = Some(Duration::from_secs(35));
+        let settings = SigningSettings {
+            signature_location: SignatureLocation::QueryParams,
+            expires_in: Some(Duration::from_secs(35)),
+            ..Default::default()
+        };
         let params = SigningParams {
             access_key: "AKIDEXAMPLE",
             secret_key: "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY",

diff --git a/aws/rust-runtime/aws-sigv4/src/lib.rs b/aws/rust-runtime/aws-sigv4/src/lib.rs
@@ -6,6 +6,7 @@
 //! Provides functions for calculating Sigv4 signing keys, signatures, and
 //! optional utilities for signing HTTP requests and Event Stream messages.
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_docs,
     rustdoc::missing_crate_level_docs,

diff --git a/aws/rust-runtime/aws-types/src/lib.rs b/aws/rust-runtime/aws-types/src/lib.rs
@@ -5,6 +5,7 @@
 
 //! Cross-service types for the AWS SDK.
 
+#![allow(clippy::derive_partial_eq_without_eq)]
 #![warn(
     missing_docs,
     rustdoc::missing_crate_level_docs,

diff --git a/aws/sdk/integration-tests/s3/tests/endpoints.rs b/aws/sdk/integration-tests/s3/tests/endpoints.rs
@@ -16,7 +16,7 @@ fn test_client(update_builder: fn(Builder) -> Builder) -> (CaptureRequestReceive
     let sdk_config = SdkConfig::builder()
         .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests()))
         .region(Region::new("us-west-4"))
-        .http_connector(conn.clone())
+        .http_connector(conn)
         .build();
     let client = Client::from_conf(update_builder(Builder::from(&sdk_config)).build());
     (captured_request, client)

diff --git a/...oftware/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/ExpressionGenerator.kt b/...oftware/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/ExpressionGenerator.kt
@@ -65,7 +65,14 @@ class ExpressionGenerator(
                 getAttr.path.toList().forEach { part ->
                     when (part) {
                         is GetAttr.Part.Key -> rust(".${part.key().rustName()}()")
-                        is GetAttr.Part.Index -> rust(".get(${part.index()}).cloned()") // we end up with Option<&&T>, we need to get to Option<&T>
+                        is GetAttr.Part.Index -> {
+                            if (part.index() == 0) {
+                                // In this case, `.first()` is more idiomatic and `.get(0)` triggers lint warnings
+                                rust(".first().cloned()")
+                            } else {
+                                rust(".get(${part.index()}).cloned()") // we end up with Option<&&T>, we need to get to Option<&T>
+                            }
+                        }
                     }
                 }
                 if (ownership == Ownership.Owned && getAttr.type() != Type.bool()) {

diff --git a/...are/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGenerator.kt b/...are/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ProtocolTestGenerator.kt
@@ -211,9 +211,9 @@ class ProtocolTestGenerator(
         checkQueryParams(this, httpRequestTestCase.queryParams)
         checkForbidQueryParams(this, httpRequestTestCase.forbidQueryParams)
         checkRequiredQueryParams(this, httpRequestTestCase.requireQueryParams)
-        checkHeaders(this, "&http_request.headers()", httpRequestTestCase.headers)
-        checkForbidHeaders(this, "&http_request.headers()", httpRequestTestCase.forbidHeaders)
-        checkRequiredHeaders(this, "&http_request.headers()", httpRequestTestCase.requireHeaders)
+        checkHeaders(this, "http_request.headers()", httpRequestTestCase.headers)
+        checkForbidHeaders(this, "http_request.headers()", httpRequestTestCase.forbidHeaders)
+        checkRequiredHeaders(this, "http_request.headers()", httpRequestTestCase.requireHeaders)
         if (protocolSupport.requestBodySerialization) {
             // "If no request body is defined, then no assertions are made about the body of the message."
             httpRequestTestCase.body.orNull()?.also { body ->

diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt
@@ -325,6 +325,16 @@ fun RustType.isCopy(): Boolean = when (this) {
     else -> false
 }
 
+/** Returns true if the type implements Eq */
+fun RustType.isEq(): Boolean = when (this) {
+    is RustType.Integer -> true
+    is RustType.Bool -> true
+    is RustType.String -> true
+    is RustType.Unit -> true
+    is RustType.Container -> this.member.isEq()
+    else -> false
+}
+
 enum class Visibility {
     PRIVATE, PUBCRATE, PUBLIC;
 
@@ -546,3 +556,10 @@ class Attribute(val inner: Writable) {
         }
     }
 }
+
+/** Render all attributes in this list, one after another */
+fun Collection<Attribute>.render(writer: RustWriter) {
+    for (attr in this) {
+        attr.render(writer)
+    }
+}
diff --git a/...src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/...src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt
@@ -92,7 +92,7 @@ fun containerDefaultMetadata(
     model: Model,
     additionalAttributes: List<Attribute> = emptyList(),
 ): RustMetadata {
-    val defaultDerives = setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)
+    val derives = mutableSetOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)
 
     val isSensitive = shape.hasTrait<SensitiveTrait>() ||
         // Checking the shape's direct members for the sensitive trait should suffice.
@@ -101,22 +101,17 @@ fun containerDefaultMetadata(
         // shape; any sensitive descendant should still be printed as redacted.
         shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent }
 
-    val setOfDerives = if (isSensitive) {
-        defaultDerives - RuntimeType.Debug
-    } else {
-        defaultDerives
+    if (isSensitive) {
+        derives.remove(RuntimeType.Debug)
     }
-    return RustMetadata(
-        setOfDerives,
-        additionalAttributes,
-        Visibility.PUBLIC,
-    )
+
+    return RustMetadata(derives, additionalAttributes, Visibility.PUBLIC)
 }
 
 /**
  * The base metadata supports a set of attributes that are used by generators to decorate code.
  *
- * By default we apply `#[non_exhaustive]` in [additionalAttributes] only to client structures since breaking model
+ * By default, we apply `#[non_exhaustive]` in [additionalAttributes] only to client structures since breaking model
  * changes are fine when generating server code.
  */
 class BaseSymbolMetadataProvider(

diff --git a/...software/amazon/smithy/rust/codegen/core/smithy/customizations/AllowLintsCustomization.kt b/...software/amazon/smithy/rust/codegen/core/smithy/customizations/AllowLintsCustomization.kt
@@ -34,7 +34,7 @@ private val allowedClippyLints = listOf(
     "should_implement_trait",
 
     // Protocol tests use silly names like `baz`, don't flag that.
-    // TODO(msrv_upgrade): switch
+    // TODO(msrv_upgrade): switch upon MSRV upgrade to Rust 1.65
     "blacklisted_name",
     // "disallowed_names",
 
@@ -48,11 +48,10 @@ private val allowedClippyLints = listOf(
     "needless_return",
 
     // For backwards compatibility, we often don't derive Eq
-    // TODO(msrv_upgrade): enable
-    // "derive_partial_eq_without_eq",
+    "derive_partial_eq_without_eq",
 
     // Keeping errors small in a backwards compatible way is challenging
-    // TODO(msrv_upgrade): enable
+    // TODO(msrv_upgrade): uncomment upon MSRV upgrade to Rust 1.65
     // "result_large_err",
 )
 

diff --git a/...ain/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/...ain/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt
@@ -114,10 +114,10 @@ class BuilderGenerator(
     private val members: List<MemberShape> = shape.allMembers.values.toList()
     private val structureSymbol = symbolProvider.toSymbol(shape)
     private val builderSymbol = shape.builderSymbol(symbolProvider)
-    private val baseDerives = structureSymbol.expectRustMetadata().derives
+    private val metadata = structureSymbol.expectRustMetadata()
 
     // Filter out any derive that isn't Debug, PartialEq, or Clone. Then add a Default derive
-    private val builderDerives = baseDerives.filter { it == RuntimeType.Debug || it == RuntimeType.PartialEq || it == RuntimeType.Clone } + RuntimeType.Default
+    private val builderDerives = metadata.derives.filter { it == RuntimeType.Debug || it == RuntimeType.PartialEq || it == RuntimeType.Clone } + RuntimeType.Default
     private val builderName = "Builder"
 
     fun render(writer: RustWriter) {
@@ -207,6 +207,7 @@ class BuilderGenerator(
 
     private fun renderBuilder(writer: RustWriter) {
         writer.docs("A builder for #D.", structureSymbol)
+        metadata.additionalAttributes.render(writer)
         Attribute(derive(builderDerives)).render(writer)
         writer.rustBlock("pub struct $builderName") {
             for (member in members) {

diff --git a/...n/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt b/...n/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt
@@ -650,12 +650,13 @@ class HttpBindingGenerator(
                 let $safeName = $formatted;
                 if !$safeName.is_empty() {
                     let header_value = $safeName;
-                    let header_value = http::header::HeaderValue::try_from(&*header_value).map_err(|err| {
+                    let header_value: #{HeaderValue} = header_value.parse().map_err(|err| {
                         #{invalid_field_error:W}
                     })?;
                     builder = builder.header("$headerName", header_value);
                 }
                 """,
+                "HeaderValue" to RuntimeType.Http.resolve("HeaderValue"),
                 "invalid_field_error" to renderErrorMessage("header_value"),
             )
         }
@@ -698,13 +699,14 @@ class HttpBindingGenerator(
                     isMultiValuedHeader = false,
                 )
                 };
-                    let header_value = http::header::HeaderValue::try_from(&*header_value).map_err(|err| {
+                    let header_value: #{HeaderValue} = header_value.parse().map_err(|err| {
                         #{invalid_header_value:W}
                     })?;
                     builder = builder.header(header_name, header_value);
                 }
 
                 """,
+                "HeaderValue" to RuntimeType.Http.resolve("HeaderValue"),
                 "invalid_header_name" to OperationBuildError(runtimeConfig).invalidField(memberName) {
                     rust("""format!("`{k}` cannot be used as a header name: {err}")""")
                 },