From a0b60ed5e01bfe1c733c9778dd9475e482704c93 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Fri, 28 Jul 2023 13:16:44 -0400 Subject: [PATCH] Add clippy.toml with forbidden methods & fix SystemTime usages (#2882) ## Motivation and Context - #2087 ## Description - remove lingering usages of SystemTime::now() - ensure they don't return by adding a clippy.toml ## Testing - CI - Ran the webassembly example ## Checklist - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _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: John DiSanti --- aws/rust-runtime/aws-config/clippy.toml | 1 + aws/rust-runtime/aws-inlineable/src/presigning.rs | 6 +++++- aws/rust-runtime/aws-runtime/src/request_info.rs | 4 +++- .../aws-runtime/src/service_clock_skew.rs | 11 ++++++++--- aws/rust-runtime/aws-sig-auth/src/event_stream.rs | 3 ++- aws/rust-runtime/aws-sig-auth/src/lib.rs | 2 ++ aws/rust-runtime/clippy.toml | 1 + .../smithy/rustsdk/EndpointsCredentialsTest.kt | 2 +- .../smithy/rustsdk/InvocationIdDecoratorTest.kt | 2 +- .../software/amazon/smithy/rustsdk/TestUtil.kt | 1 + aws/sdk/build.gradle.kts | 2 ++ clippy-root.toml | 9 +++++++++ .../customizations/TimeSourceCustomization.kt | 15 +++++++++++++++ .../core/testutil/CodegenIntegrationTest.kt | 3 ++- rust-runtime/aws-smithy-async/src/time.rs | 2 ++ rust-runtime/clippy.toml | 1 + 16 files changed, 56 insertions(+), 9 deletions(-) create mode 120000 aws/rust-runtime/aws-config/clippy.toml create mode 120000 aws/rust-runtime/clippy.toml create mode 100644 clippy-root.toml create mode 120000 rust-runtime/clippy.toml diff --git a/aws/rust-runtime/aws-config/clippy.toml b/aws/rust-runtime/aws-config/clippy.toml new file mode 120000 index 000000000..85f6167cb --- /dev/null +++ b/aws/rust-runtime/aws-config/clippy.toml @@ -0,0 +1 @@ +../clippy.toml \ No newline at end of file diff --git a/aws/rust-runtime/aws-inlineable/src/presigning.rs b/aws/rust-runtime/aws-inlineable/src/presigning.rs index 1bc9f42e6..c9de75dfb 100644 --- a/aws/rust-runtime/aws-inlineable/src/presigning.rs +++ b/aws/rust-runtime/aws-inlineable/src/presigning.rs @@ -147,7 +147,11 @@ impl PresigningConfigBuilder { return Err(ErrorKind::ExpiresInDurationTooLong.into()); } Ok(PresigningConfig { - start_time: self.start_time.unwrap_or_else(SystemTime::now), + start_time: self.start_time.unwrap_or_else( + // This usage is OK—customers can easily override this. + #[allow(clippy::disallowed_methods)] + SystemTime::now, + ), expires_in, }) } diff --git a/aws/rust-runtime/aws-runtime/src/request_info.rs b/aws/rust-runtime/aws-runtime/src/request_info.rs index 5b7dccee5..04825141b 100644 --- a/aws/rust-runtime/aws-runtime/src/request_info.rs +++ b/aws/rust-runtime/aws-runtime/src/request_info.rs @@ -180,12 +180,14 @@ mod tests { use super::RequestInfoInterceptor; use crate::request_info::RequestPairs; use aws_smithy_http::body::SdkBody; - use aws_smithy_runtime_api::client::interceptors::context::{Input, InterceptorContext}; + use aws_smithy_runtime_api::client::interceptors::context::Input; + use aws_smithy_runtime_api::client::interceptors::context::InterceptorContext; use aws_smithy_runtime_api::client::interceptors::Interceptor; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use aws_smithy_types::config_bag::{ConfigBag, Layer}; use aws_smithy_types::retry::RetryConfig; use aws_smithy_types::timeout::TimeoutConfig; + use http::HeaderValue; use std::time::Duration; diff --git a/aws/rust-runtime/aws-runtime/src/service_clock_skew.rs b/aws/rust-runtime/aws-runtime/src/service_clock_skew.rs index 7ada75c2b..a919f37ca 100644 --- a/aws/rust-runtime/aws-runtime/src/service_clock_skew.rs +++ b/aws/rust-runtime/aws-runtime/src/service_clock_skew.rs @@ -10,7 +10,7 @@ use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreReplace}; use aws_smithy_types::date_time::Format; use aws_smithy_types::DateTime; -use std::time::{Duration, SystemTime}; +use std::time::Duration; /// Amount of clock skew between the client and the service. #[derive(Debug, Clone)] @@ -72,10 +72,15 @@ impl Interceptor for ServiceClockSkewInterceptor { fn modify_before_deserialization( &self, ctx: &mut BeforeDeserializationInterceptorContextMut<'_>, - _runtime_components: &RuntimeComponents, + runtime_components: &RuntimeComponents, cfg: &mut ConfigBag, ) -> Result<(), BoxError> { - let time_received = DateTime::from(SystemTime::now()); + let time_received = DateTime::from( + runtime_components + .time_source() + .ok_or("a time source is required (service clock skew)")? + .now(), + ); let time_sent = match extract_time_sent_from_response(ctx) { Ok(time_sent) => time_sent, Err(e) => { diff --git a/aws/rust-runtime/aws-sig-auth/src/event_stream.rs b/aws/rust-runtime/aws-sig-auth/src/event_stream.rs index 01a1dd55f..bf51ac272 100644 --- a/aws/rust-runtime/aws-sig-auth/src/event_stream.rs +++ b/aws/rust-runtime/aws-sig-auth/src/event_stream.rs @@ -3,8 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -// TODO(enableNewSmithyRuntimeCleanup): Remove this blanket allow once the old implementations are deleted +// this code is dead #![allow(deprecated)] +#![allow(clippy::disallowed_methods)] use crate::middleware::Signature; use aws_credential_types::Credentials; diff --git a/aws/rust-runtime/aws-sig-auth/src/lib.rs b/aws/rust-runtime/aws-sig-auth/src/lib.rs index 643b3ff21..90cc88a7e 100644 --- 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 */ +// TODO(enableNewSmithyRuntimeCleanup): Deprecate this crate and replace it with empty contents. Remove references to it in the code generator. + #![allow(clippy::derive_partial_eq_without_eq)] //! AWS Signature Authentication Package diff --git a/aws/rust-runtime/clippy.toml b/aws/rust-runtime/clippy.toml new file mode 120000 index 000000000..0cbd6319b --- /dev/null +++ b/aws/rust-runtime/clippy.toml @@ -0,0 +1 @@ +../../clippy-root.toml \ No newline at end of file diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointsCredentialsTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointsCredentialsTest.kt index 89e11618a..070803554 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointsCredentialsTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointsCredentialsTest.kt @@ -114,7 +114,7 @@ class EndpointsCredentialsTest { .credentials_provider(#{Credentials}::for_tests()) .build(); let client = $moduleName::Client::from_conf(conf); - let _ = client.custom_auth().send().await; + let _ = dbg!(client.custom_auth().send().await); let req = rcvr.expect_request(); let auth_header = req.headers().get("AUTHORIZATION").unwrap().to_str().unwrap(); assert!(auth_header.contains("/region-custom-auth/name-custom-auth/aws4_request"), "{}", auth_header); diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecoratorTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecoratorTest.kt index 857139c9f..3a792e2a0 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecoratorTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecoratorTest.kt @@ -41,7 +41,7 @@ class InvocationIdDecoratorTest { let client = $moduleName::Client::from_conf(config); - let _ = client.some_operation().send().await; + let _ = dbg!(client.some_operation().send().await); let request = rx.expect_request(); assert_eq!("custom", request.headers().get("amz-sdk-invocation-id").unwrap()); } diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt index 5638aca83..44ed5461a 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt @@ -46,6 +46,7 @@ fun awsSdkIntegrationTest( clientIntegrationTest( model, IntegrationTestParams( + cargoCommand = "cargo test --features test-util", runtimeConfig = AwsTestRuntimeConfig, additionalSettings = ObjectNode.builder().withMember( "customizationConfig", diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 920cb7579..ebf1b3a70 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -338,6 +338,7 @@ tasks.register("generateCargoWorkspace") { doFirst { outputDir.mkdirs() outputDir.resolve("Cargo.toml").writeText(generateCargoWorkspace(awsServices)) + rootProject.rootDir.resolve("clippy-root.toml").copyTo(outputDir.resolve("clippy.toml")) } inputs.property("servicelist", awsServices.moduleNames.toString()) if (awsServices.examples.isNotEmpty()) { @@ -347,6 +348,7 @@ tasks.register("generateCargoWorkspace") { inputs.dir(test.path) } outputs.file(outputDir.resolve("Cargo.toml")) + outputs.file(outputDir.resolve("clippy.toml")) outputs.upToDateWhen { false } } diff --git a/clippy-root.toml b/clippy-root.toml new file mode 100644 index 000000000..87318f316 --- /dev/null +++ b/clippy-root.toml @@ -0,0 +1,9 @@ +# this file is named `clippy-root.toml` so it isn't picked up automagically. Clippy +# will search up the filesystem for a clippy.toml and this causes problems with tools. +disallowed-methods = [ + # fully qualified function/method name: + "std::time::SystemTime::now", + "std::time::SystemTime::elapsed", + "std::time::Instant::now", + "std::time::Instant::elapsed" +] diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/TimeSourceCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/TimeSourceCustomization.kt index 83e111901..7dea61de9 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/TimeSourceCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/TimeSourceCustomization.kt @@ -23,6 +23,9 @@ class TimeSourceCustomization(codegenContext: ClientCodegenContext) : ConfigCust private val codegenScope = arrayOf( *preludeScope, "SharedTimeSource" to RuntimeType.smithyAsync(codegenContext.runtimeConfig).resolve("time::SharedTimeSource"), + "StaticTimeSource" to RuntimeType.smithyAsync(codegenContext.runtimeConfig).resolve("time::StaticTimeSource"), + "UNIX_EPOCH" to RuntimeType.std.resolve("time::UNIX_EPOCH"), + "Duration" to RuntimeType.std.resolve("time::Duration"), ) override fun section(section: ServiceConfig) = @@ -129,6 +132,18 @@ class TimeSourceCustomization(codegenContext: ClientCodegenContext) : ConfigCust } } + is ServiceConfig.DefaultForTests -> { + rustTemplate( + """ + ${section.configBuilderRef} + .set_time_source(#{Some}(#{SharedTimeSource}::new( + #{StaticTimeSource}::new(#{UNIX_EPOCH} + #{Duration}::from_secs(1234567890))) + )); + """, + *codegenScope, + ) + } + else -> emptySection } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/CodegenIntegrationTest.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/CodegenIntegrationTest.kt index d1f208c39..4c1d43520 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/CodegenIntegrationTest.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/CodegenIntegrationTest.kt @@ -24,6 +24,7 @@ data class IntegrationTestParams( val additionalSettings: ObjectNode = ObjectNode.builder().build(), val overrideTestDir: File? = null, val command: ((Path) -> Unit)? = null, + val cargoCommand: String? = null, ) /** @@ -40,6 +41,6 @@ fun codegenIntegrationTest(model: Model, params: IntegrationTestParams, invokePl ) invokePlugin(ctx) ctx.fileManifest.printGeneratedFiles() - params.command?.invoke(testDir) ?: "cargo test".runCommand(testDir, environment = mapOf("RUSTFLAGS" to "-D warnings")) + params.command?.invoke(testDir) ?: (params.cargoCommand ?: "cargo test").runCommand(testDir, environment = mapOf("RUSTFLAGS" to "-D warnings")) return testDir } diff --git a/rust-runtime/aws-smithy-async/src/time.rs b/rust-runtime/aws-smithy-async/src/time.rs index 2abe332c8..b1b3f5ac1 100644 --- a/rust-runtime/aws-smithy-async/src/time.rs +++ b/rust-runtime/aws-smithy-async/src/time.rs @@ -28,6 +28,8 @@ impl SystemTimeSource { impl TimeSource for SystemTimeSource { fn now(&self) -> SystemTime { + // this is the one OK usage + #[allow(clippy::disallowed_methods)] SystemTime::now() } } diff --git a/rust-runtime/clippy.toml b/rust-runtime/clippy.toml new file mode 120000 index 000000000..939baf718 --- /dev/null +++ b/rust-runtime/clippy.toml @@ -0,0 +1 @@ +../clippy-root.toml \ No newline at end of file