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
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] 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 <jdisanti@amazon.com>
This commit is contained in:
Russell Cohen 2023-07-28 13:16:44 -04:00 committed by GitHub
parent cf8df40f18
commit a0b60ed5e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 56 additions and 9 deletions

View File

@ -0,0 +1 @@
../clippy.toml

View File

@ -147,7 +147,11 @@ impl PresigningConfigBuilder {
return Err(ErrorKind::ExpiresInDurationTooLong.into()); return Err(ErrorKind::ExpiresInDurationTooLong.into());
} }
Ok(PresigningConfig { 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, expires_in,
}) })
} }

View File

@ -180,12 +180,14 @@ mod tests {
use super::RequestInfoInterceptor; use super::RequestInfoInterceptor;
use crate::request_info::RequestPairs; use crate::request_info::RequestPairs;
use aws_smithy_http::body::SdkBody; 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::interceptors::Interceptor;
use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder;
use aws_smithy_types::config_bag::{ConfigBag, Layer}; use aws_smithy_types::config_bag::{ConfigBag, Layer};
use aws_smithy_types::retry::RetryConfig; use aws_smithy_types::retry::RetryConfig;
use aws_smithy_types::timeout::TimeoutConfig; use aws_smithy_types::timeout::TimeoutConfig;
use http::HeaderValue; use http::HeaderValue;
use std::time::Duration; use std::time::Duration;

View File

@ -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::config_bag::{ConfigBag, Storable, StoreReplace};
use aws_smithy_types::date_time::Format; use aws_smithy_types::date_time::Format;
use aws_smithy_types::DateTime; use aws_smithy_types::DateTime;
use std::time::{Duration, SystemTime}; use std::time::Duration;
/// Amount of clock skew between the client and the service. /// Amount of clock skew between the client and the service.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
@ -72,10 +72,15 @@ impl Interceptor for ServiceClockSkewInterceptor {
fn modify_before_deserialization( fn modify_before_deserialization(
&self, &self,
ctx: &mut BeforeDeserializationInterceptorContextMut<'_>, ctx: &mut BeforeDeserializationInterceptorContextMut<'_>,
_runtime_components: &RuntimeComponents, runtime_components: &RuntimeComponents,
cfg: &mut ConfigBag, cfg: &mut ConfigBag,
) -> Result<(), BoxError> { ) -> 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) { let time_sent = match extract_time_sent_from_response(ctx) {
Ok(time_sent) => time_sent, Ok(time_sent) => time_sent,
Err(e) => { Err(e) => {

View File

@ -3,8 +3,9 @@
* SPDX-License-Identifier: Apache-2.0 * 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(deprecated)]
#![allow(clippy::disallowed_methods)]
use crate::middleware::Signature; use crate::middleware::Signature;
use aws_credential_types::Credentials; use aws_credential_types::Credentials;

View File

@ -3,6 +3,8 @@
* SPDX-License-Identifier: Apache-2.0 * 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)] #![allow(clippy::derive_partial_eq_without_eq)]
//! AWS Signature Authentication Package //! AWS Signature Authentication Package

View File

@ -0,0 +1 @@
../../clippy-root.toml

View File

@ -114,7 +114,7 @@ class EndpointsCredentialsTest {
.credentials_provider(#{Credentials}::for_tests()) .credentials_provider(#{Credentials}::for_tests())
.build(); .build();
let client = $moduleName::Client::from_conf(conf); 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 req = rcvr.expect_request();
let auth_header = req.headers().get("AUTHORIZATION").unwrap().to_str().unwrap(); 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); assert!(auth_header.contains("/region-custom-auth/name-custom-auth/aws4_request"), "{}", auth_header);

View File

@ -41,7 +41,7 @@ class InvocationIdDecoratorTest {
let client = $moduleName::Client::from_conf(config); 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(); let request = rx.expect_request();
assert_eq!("custom", request.headers().get("amz-sdk-invocation-id").unwrap()); assert_eq!("custom", request.headers().get("amz-sdk-invocation-id").unwrap());
} }

View File

@ -46,6 +46,7 @@ fun awsSdkIntegrationTest(
clientIntegrationTest( clientIntegrationTest(
model, model,
IntegrationTestParams( IntegrationTestParams(
cargoCommand = "cargo test --features test-util",
runtimeConfig = AwsTestRuntimeConfig, runtimeConfig = AwsTestRuntimeConfig,
additionalSettings = ObjectNode.builder().withMember( additionalSettings = ObjectNode.builder().withMember(
"customizationConfig", "customizationConfig",

View File

@ -338,6 +338,7 @@ tasks.register("generateCargoWorkspace") {
doFirst { doFirst {
outputDir.mkdirs() outputDir.mkdirs()
outputDir.resolve("Cargo.toml").writeText(generateCargoWorkspace(awsServices)) outputDir.resolve("Cargo.toml").writeText(generateCargoWorkspace(awsServices))
rootProject.rootDir.resolve("clippy-root.toml").copyTo(outputDir.resolve("clippy.toml"))
} }
inputs.property("servicelist", awsServices.moduleNames.toString()) inputs.property("servicelist", awsServices.moduleNames.toString())
if (awsServices.examples.isNotEmpty()) { if (awsServices.examples.isNotEmpty()) {
@ -347,6 +348,7 @@ tasks.register("generateCargoWorkspace") {
inputs.dir(test.path) inputs.dir(test.path)
} }
outputs.file(outputDir.resolve("Cargo.toml")) outputs.file(outputDir.resolve("Cargo.toml"))
outputs.file(outputDir.resolve("clippy.toml"))
outputs.upToDateWhen { false } outputs.upToDateWhen { false }
} }

9
clippy-root.toml Normal file
View File

@ -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"
]

View File

@ -23,6 +23,9 @@ class TimeSourceCustomization(codegenContext: ClientCodegenContext) : ConfigCust
private val codegenScope = arrayOf( private val codegenScope = arrayOf(
*preludeScope, *preludeScope,
"SharedTimeSource" to RuntimeType.smithyAsync(codegenContext.runtimeConfig).resolve("time::SharedTimeSource"), "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) = 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 else -> emptySection
} }
} }

View File

@ -24,6 +24,7 @@ data class IntegrationTestParams(
val additionalSettings: ObjectNode = ObjectNode.builder().build(), val additionalSettings: ObjectNode = ObjectNode.builder().build(),
val overrideTestDir: File? = null, val overrideTestDir: File? = null,
val command: ((Path) -> Unit)? = null, val command: ((Path) -> Unit)? = null,
val cargoCommand: String? = null,
) )
/** /**
@ -40,6 +41,6 @@ fun codegenIntegrationTest(model: Model, params: IntegrationTestParams, invokePl
) )
invokePlugin(ctx) invokePlugin(ctx)
ctx.fileManifest.printGeneratedFiles() 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 return testDir
} }

View File

@ -28,6 +28,8 @@ impl SystemTimeSource {
impl TimeSource for SystemTimeSource { impl TimeSource for SystemTimeSource {
fn now(&self) -> SystemTime { fn now(&self) -> SystemTime {
// this is the one OK usage
#[allow(clippy::disallowed_methods)]
SystemTime::now() SystemTime::now()
} }
} }

1
rust-runtime/clippy.toml Symbolic link
View File

@ -0,0 +1 @@
../clippy-root.toml