From 2b24ce28f60ccc6d219f3de8945c4bc1ce0ce1ed Mon Sep 17 00:00:00 2001 From: Aaron Siddhartha Mondal Date: Tue, 1 Oct 2024 00:12:59 +0200 Subject: [PATCH] Fix clippy::redundant_closure_for_method_calls (#1380) --- .bazelrc | 2 +- nativelink-config/src/serde_utils.rs | 3 ++- nativelink-error/src/lib.rs | 4 +++- .../src/cache_lookup_scheduler.rs | 4 ++-- .../src/simple_scheduler_state_manager.rs | 3 ++- nativelink-service/src/ac_server.rs | 5 +++-- nativelink-service/src/bytestream_server.rs | 9 +++++---- nativelink-service/src/cas_server.rs | 11 ++++++----- nativelink-service/src/execution_server.rs | 5 +++-- nativelink-service/src/worker_api_server.rs | 9 +++++---- nativelink-store/src/grpc_store.rs | 2 +- nativelink-store/src/memory_store.rs | 4 ++-- nativelink-store/src/redis_store.rs | 2 +- nativelink-store/src/size_partitioning_store.rs | 2 +- nativelink-util/src/action_messages.rs | 3 ++- nativelink-util/src/origin_context.rs | 5 +++-- nativelink-util/src/resource_info.rs | 15 +++++---------- nativelink-util/src/store_trait.rs | 7 ++++--- nativelink-worker/src/running_actions_manager.rs | 5 +++-- 19 files changed, 54 insertions(+), 46 deletions(-) diff --git a/.bazelrc b/.bazelrc index 6cc425e05..11fca67e7 100644 --- a/.bazelrc +++ b/.bazelrc @@ -45,7 +45,7 @@ build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect # TODO(aaronmondal): Extend these flags until we can run with clippy::pedantic. -build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else +build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls build --@rules_rust//:clippy.toml=//:clippy.toml test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml diff --git a/nativelink-config/src/serde_utils.rs b/nativelink-config/src/serde_utils.rs index dcba57831..99299be90 100644 --- a/nativelink-config/src/serde_utils.rs +++ b/nativelink-config/src/serde_utils.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::borrow::Cow; use std::fmt; use std::marker::PhantomData; use std::str::FromStr; @@ -122,7 +123,7 @@ pub fn convert_vec_string_with_shellexpand<'de, D: Deserializer<'de>>( .map(|s| { shellexpand::env(&s) .map_err(de::Error::custom) - .map(|expanded| expanded.into_owned()) + .map(Cow::into_owned) }) .collect() } diff --git a/nativelink-error/src/lib.rs b/nativelink-error/src/lib.rs index fee8093bd..4e36d82c3 100644 --- a/nativelink-error/src/lib.rs +++ b/nativelink-error/src/lib.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::convert::Into; + use nativelink_metric::{ MetricFieldData, MetricKind, MetricPublishKnownKindData, MetricsComponent, }; @@ -99,7 +101,7 @@ impl Error { } return Some(this.into()); } - other.map(|v| v.into()) + other.map(Into::into) } pub fn to_std_err(self) -> std::io::Error { diff --git a/nativelink-scheduler/src/cache_lookup_scheduler.rs b/nativelink-scheduler/src/cache_lookup_scheduler.rs index 69969ad0b..a2cd22ffd 100644 --- a/nativelink-scheduler/src/cache_lookup_scheduler.rs +++ b/nativelink-scheduler/src/cache_lookup_scheduler.rs @@ -37,7 +37,7 @@ use nativelink_util::store_trait::Store; use parking_lot::{Mutex, MutexGuard}; use scopeguard::guard; use tokio::sync::oneshot; -use tonic::Request; +use tonic::{Request, Response}; use tracing::{event, Level}; /// Actions that are having their cache checked or failed cache lookup and are @@ -84,7 +84,7 @@ async fn get_action_from_store( grpc_store .get_action_result(Request::new(action_result_request)) .await - .map(|response| response.into_inner()) + .map(Response::into_inner) } else { get_and_decode_digest::(ac_store, action_digest.into()).await } diff --git a/nativelink-scheduler/src/simple_scheduler_state_manager.rs b/nativelink-scheduler/src/simple_scheduler_state_manager.rs index 3ccbd8dd7..573a9acf2 100644 --- a/nativelink-scheduler/src/simple_scheduler_state_manager.rs +++ b/nativelink-scheduler/src/simple_scheduler_state_manager.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::ops::Bound; +use std::string::ToString; use std::sync::{Arc, Weak}; use std::time::{Duration, SystemTime}; @@ -505,7 +506,7 @@ where if awaited_action.attempts > self.max_job_retries { ActionStage::Completed(ActionResult { execution_metadata: ExecutionMetadata { - worker: maybe_worker_id.map_or_else(String::default, |v| v.to_string()), + worker: maybe_worker_id.map_or_else(String::default, ToString::to_string), ..ExecutionMetadata::default() }, error: Some(err.clone().merge(make_err!( diff --git a/nativelink-service/src/ac_server.rs b/nativelink-service/src/ac_server.rs index 819251879..cd6be0f8e 100644 --- a/nativelink-service/src/ac_server.rs +++ b/nativelink-service/src/ac_server.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::HashMap; +use std::convert::Into; use std::fmt::Debug; use bytes::BytesMut; @@ -182,7 +183,7 @@ impl ActionCache for AcServer { if resp.is_err() && resp.as_ref().err().unwrap().code != Code::NotFound { event!(Level::ERROR, return = ?resp); } - return resp.map_err(|e| e.into()); + return resp.map_err(Into::into); } #[allow(clippy::blocks_in_conditions)] @@ -205,6 +206,6 @@ impl ActionCache for AcServer { self.inner_update_action_result(request), ) .await - .map_err(|e| e.into()) + .map_err(Into::into) } } diff --git a/nativelink-service/src/bytestream_server.rs b/nativelink-service/src/bytestream_server.rs index 39c817f14..ff00cb8c7 100644 --- a/nativelink-service/src/bytestream_server.rs +++ b/nativelink-service/src/bytestream_server.rs @@ -14,6 +14,7 @@ use std::collections::hash_map::Entry; use std::collections::HashMap; +use std::convert::Into; use std::fmt::{Debug, Formatter}; use std::pin::Pin; use std::sync::atomic::{AtomicU64, Ordering}; @@ -618,7 +619,7 @@ impl ByteStream for ByteStreamServer { ) .await .err_tip(|| "In ByteStreamServer::read") - .map_err(|e| e.into()); + .map_err(Into::into); if resp.is_ok() { event!(Level::DEBUG, return = "Ok()"); @@ -657,7 +658,7 @@ impl ByteStream for ByteStreamServer { // If we are a GrpcStore we shortcut here, as this is a special store. if let Some(grpc_store) = store.downcast_ref::(Some(digest.into())) { - return grpc_store.write(stream).await.map_err(|e| e.into()); + return grpc_store.write(stream).await.map_err(Into::into); } let digest_function = stream @@ -677,7 +678,7 @@ impl ByteStream for ByteStreamServer { ) .await .err_tip(|| "In ByteStreamServer::write") - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -695,6 +696,6 @@ impl ByteStream for ByteStreamServer { self.inner_query_write_status(&grpc_request.into_inner()) .await .err_tip(|| "Failed on query_write_status() command") - .map_err(|e| e.into()) + .map_err(Into::into) } } diff --git a/nativelink-service/src/cas_server.rs b/nativelink-service/src/cas_server.rs index a736820a1..581b704f7 100644 --- a/nativelink-service/src/cas_server.rs +++ b/nativelink-service/src/cas_server.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::{HashMap, VecDeque}; +use std::convert::Into; use std::pin::Pin; use bytes::Bytes; @@ -137,7 +138,7 @@ impl CasServer { .err_tip(|| "Error writing to store"); Ok::<_, Error>(batch_update_blobs_response::Response { digest: Some(digest), - status: Some(result.map_or_else(|e| e.into(), |_| GrpcStatus::default())), + status: Some(result.map_or_else(Into::into, |_| GrpcStatus::default())), }) }) .collect(); @@ -323,7 +324,7 @@ impl ContentAddressableStorage for CasServer { ) .await .err_tip(|| "Failed on find_missing_blobs() command") - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -347,7 +348,7 @@ impl ContentAddressableStorage for CasServer { ) .await .err_tip(|| "Failed on batch_update_blobs() command") - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -371,7 +372,7 @@ impl ContentAddressableStorage for CasServer { ) .await .err_tip(|| "Failed on batch_read_blobs() command") - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -394,7 +395,7 @@ impl ContentAddressableStorage for CasServer { ) .await .err_tip(|| "Failed on get_tree() command") - .map_err(|e| e.into()); + .map_err(Into::into); if resp.is_ok() { event!(Level::DEBUG, return = "Ok()"); } diff --git a/nativelink-service/src/execution_server.rs b/nativelink-service/src/execution_server.rs index c8d376fc4..677d68c4c 100644 --- a/nativelink-service/src/execution_server.rs +++ b/nativelink-service/src/execution_server.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::HashMap; +use std::convert::Into; use std::fmt; use std::pin::Pin; use std::sync::Arc; @@ -343,7 +344,7 @@ impl Execution for ExecutionServer { ) .await .err_tip(|| "Failed on execute() command") - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -361,7 +362,7 @@ impl Execution for ExecutionServer { .inner_wait_execution(grpc_request) .await .err_tip(|| "Failed on wait_execution() command") - .map_err(|e| e.into()); + .map_err(Into::into); if resp.is_ok() { event!(Level::DEBUG, return = "Ok()"); diff --git a/nativelink-service/src/worker_api_server.rs b/nativelink-service/src/worker_api_server.rs index c69fd4749..4ea7403d3 100644 --- a/nativelink-service/src/worker_api_server.rs +++ b/nativelink-service/src/worker_api_server.rs @@ -16,6 +16,7 @@ use std::collections::HashMap; use std::pin::Pin; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use std::convert::Into; use futures::stream::unfold; use futures::Stream; @@ -252,7 +253,7 @@ impl WorkerApi for WorkerApiServer { let resp = self .inner_connect_worker(grpc_request.into_inner()) .await - .map_err(|e| e.into()); + .map_err(Into::into); if resp.is_ok() { event!(Level::DEBUG, return = "Ok()"); } @@ -273,7 +274,7 @@ impl WorkerApi for WorkerApiServer { ) -> Result, Status> { self.inner_keep_alive(grpc_request.into_inner()) .await - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -290,7 +291,7 @@ impl WorkerApi for WorkerApiServer { ) -> Result, Status> { self.inner_going_away(grpc_request.into_inner()) .await - .map_err(|e| e.into()) + .map_err(Into::into) } #[allow(clippy::blocks_in_conditions)] @@ -307,6 +308,6 @@ impl WorkerApi for WorkerApiServer { ) -> Result, Status> { self.inner_execution_response(grpc_request.into_inner()) .await - .map_err(|e| e.into()) + .map_err(Into::into) } } diff --git a/nativelink-store/src/grpc_store.rs b/nativelink-store/src/grpc_store.rs index 11d4017fe..b08b2ef76 100644 --- a/nativelink-store/src/grpc_store.rs +++ b/nativelink-store/src/grpc_store.rs @@ -455,7 +455,7 @@ impl GrpcStore { let action_result = self .get_action_result_from_digest(digest) .await - .map(|response| response.into_inner()) + .map(Response::into_inner) .err_tip(|| "Action result not found")?; // TODO: Would be better to avoid all the encoding and decoding in this // file, however there's no way to currently get raw bytes from a diff --git a/nativelink-store/src/memory_store.rs b/nativelink-store/src/memory_store.rs index a3973d072..99042d37f 100644 --- a/nativelink-store/src/memory_store.rs +++ b/nativelink-store/src/memory_store.rs @@ -106,8 +106,8 @@ impl StoreDriver for MemoryStore { handler: &mut (dyn for<'a> FnMut(&'a StoreKey) -> bool + Send + Sync + '_), ) -> Result { let range = ( - range.0.map(|v| v.into_owned()), - range.1.map(|v| v.into_owned()), + range.0.map(StoreKey::into_owned), + range.1.map(StoreKey::into_owned), ); let iterations = self .evicting_map diff --git a/nativelink-store/src/redis_store.rs b/nativelink-store/src/redis_store.rs index b7ea06c37..338d37a3d 100644 --- a/nativelink-store/src/redis_store.rs +++ b/nativelink-store/src/redis_store.rs @@ -797,7 +797,7 @@ impl RedisSubscriptionManager { let subscribed_keys_mux = subscribed_keys.read(); subscribed_keys_mux .common_prefix_values(&*key) - .for_each(|publisher| publisher.notify()); + .for_each(RedisSubscriptionPublisher::notify); } // Sleep for a small amount of time to ensure we don't reconnect too quickly. sleep(Duration::from_secs(1)).await; diff --git a/nativelink-store/src/size_partitioning_store.rs b/nativelink-store/src/size_partitioning_store.rs index 221b8fdb2..4976c62d5 100644 --- a/nativelink-store/src/size_partitioning_store.rs +++ b/nativelink-store/src/size_partitioning_store.rs @@ -56,7 +56,7 @@ impl StoreDriver for SizePartitioningStore { ) -> Result<(), Error> { let mut non_digest_sample = None; let (lower_digests, upper_digests): (Vec<_>, Vec<_>) = - keys.iter().map(|v| v.borrow()).partition(|k| { + keys.iter().map(StoreKey::borrow).partition(|k| { let StoreKey::Digest(digest) = k else { non_digest_sample = Some(k.borrow().into_owned()); return false; diff --git a/nativelink-util/src/action_messages.rs b/nativelink-util/src/action_messages.rs index 7689b3859..c9bfcfd62 100644 --- a/nativelink-util/src/action_messages.rs +++ b/nativelink-util/src/action_messages.rs @@ -14,6 +14,7 @@ use std::cmp::Ordering; use std::collections::HashMap; +use std::convert::Into; use std::hash::Hash; use std::time::{Duration, SystemTime}; @@ -849,7 +850,7 @@ pub fn to_execute_response(action_result: ActionResult) -> ExecuteResponse { action_result .error .clone() - .map_or_else(Status::default, |v| v.into()), + .map_or_else(Status::default, Into::into), ); let message = action_result.message.clone(); ExecuteResponse { diff --git a/nativelink-util/src/origin_context.rs b/nativelink-util/src/origin_context.rs index 679f9c9ac..23be65408 100644 --- a/nativelink-util/src/origin_context.rs +++ b/nativelink-util/src/origin_context.rs @@ -15,6 +15,7 @@ use core::panic; use std::any::Any; use std::cell::RefCell; +use std::clone::Clone; use std::collections::HashMap; use std::mem::ManuallyDrop; use std::pin::Pin; @@ -186,7 +187,7 @@ pub struct ActiveOriginContext; impl ActiveOriginContext { /// Sets the active context for the current thread. pub fn get() -> Option> { - GLOBAL_ORIGIN_CONTEXT.with_borrow(|maybe_ctx| maybe_ctx.clone()) + GLOBAL_ORIGIN_CONTEXT.with_borrow(Clone::clone) } /// Gets the value current set for a given symbol on the @@ -299,7 +300,7 @@ pin_project! { // Note: If the future panics, the context will not be restored, so // this is a best effort to provide access to our global context // in the desturctors the event of a panic. - let _enter = this.context.take().map(|ctx| ctx.enter()); + let _enter = this.context.take().map(OriginContext::enter); // SAFETY: 1. `Pin::get_unchecked_mut()` is safe, because this isn't // different from wrapping `T` in `Option` and calling // `Pin::set(&mut this.inner, None)`, except avoiding diff --git a/nativelink-util/src/resource_info.rs b/nativelink-util/src/resource_info.rs index 32b54ad9f..4683d7c43 100644 --- a/nativelink-util/src/resource_info.rs +++ b/nativelink-util/src/resource_info.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::borrow::Cow; +use std::convert::AsRef; use nativelink_error::{error_if, make_input_err, Error, ResultExt}; @@ -175,23 +176,17 @@ impl<'a> ResourceInfo<'a> { [ Some(self.instance_name.as_ref()), is_upload.then_some("uploads"), - self.uuid.as_ref().map(|uuid| uuid.as_ref()), + self.uuid.as_ref().map(AsRef::as_ref), Some( self.compressor .as_ref() .map_or("blobs", |_| "compressed-blobs"), ), - self.compressor - .as_ref() - .map(|compressor| compressor.as_ref()), - self.digest_function - .as_ref() - .map(|digest_function| digest_function.as_ref()), + self.compressor.as_ref().map(AsRef::as_ref), + self.digest_function.as_ref().map(AsRef::as_ref), Some(self.hash.as_ref()), Some(self.size.as_ref()), - self.optional_metadata - .as_ref() - .map(|optional_metadata| optional_metadata.as_ref()), + self.optional_metadata.as_ref().map(AsRef::as_ref), ] .into_iter() .flatten() diff --git a/nativelink-util/src/store_trait.rs b/nativelink-util/src/store_trait.rs index 814669d06..7e9ee5620 100644 --- a/nativelink-util/src/store_trait.rs +++ b/nativelink-util/src/store_trait.rs @@ -14,6 +14,7 @@ use std::borrow::{BorrowMut, Cow}; use std::collections::hash_map::DefaultHasher as StdHasher; +use std::convert::Into; use std::hash::{Hash, Hasher}; use std::ops::{Bound, Deref, RangeBounds}; use std::pin::Pin; @@ -320,7 +321,7 @@ impl Store { /// Note: If the store performs complex operations on the data, it should return itself. #[inline] pub fn inner_store<'a, K: Into>>(&self, digest: Option) -> &dyn StoreDriver { - self.inner.inner_store(digest.map(|v| v.into())) + self.inner.inner_store(digest.map(Into::into)) } /// Tries to cast the underlying store to the given type. @@ -430,8 +431,8 @@ pub trait StoreLike: Send + Sync + Sized + Unpin + 'static { self.as_store_driver_pin() .list( ( - range.start_bound().map(|v| v.borrow()), - range.end_bound().map(|v| v.borrow()), + range.start_bound().map(StoreKey::borrow), + range.end_bound().map(StoreKey::borrow), ), &mut handler, ) diff --git a/nativelink-worker/src/running_actions_manager.rs b/nativelink-worker/src/running_actions_manager.rs index ebc0c9114..55a02ff20 100644 --- a/nativelink-worker/src/running_actions_manager.rs +++ b/nativelink-worker/src/running_actions_manager.rs @@ -16,6 +16,7 @@ use std::borrow::Cow; use std::cmp::min; use std::collections::vec_deque::VecDeque; use std::collections::HashMap; +use std::convert::Into; use std::ffi::{OsStr, OsString}; use std::fmt::Debug; #[cfg(target_family = "unix")] @@ -435,7 +436,7 @@ fn upload_directory<'a, P: AsRef + Debug + Send + Sync + Clone + 'a>( .await .err_tip(|| format!("Could not open file {full_path:?}"))?; upload_file(cas_store, &full_path, hasher, metadata) - .map_ok(|v| v.into()) + .map_ok(Into::into) .await }); } else if file_type.is_symlink() { @@ -1871,7 +1872,7 @@ impl RunningActionsManager for RunningActionsManagerImpl { let running_actions = self.running_actions.lock(); running_actions .get(operation_id) - .and_then(|action| action.upgrade()) + .and_then(Weak::upgrade) .ok_or_else(|| make_input_err!("Failed to get running action {operation_id}"))? }; Self::kill_operation(running_action).await;