Skip to content

Commit

Permalink
Fix clippy::redundant_closure_for_method_calls (#1380)
Browse files Browse the repository at this point in the history
  • Loading branch information
aaronmondal authored Sep 30, 2024
1 parent 255e0e7 commit 2b24ce2
Show file tree
Hide file tree
Showing 19 changed files with 54 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion nativelink-config/src/serde_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}
Expand Down
4 changes: 3 additions & 1 deletion nativelink-error/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions nativelink-scheduler/src/cache_lookup_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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::<ProtoActionResult>(ac_store, action_digest.into()).await
}
Expand Down
3 changes: 2 additions & 1 deletion nativelink-scheduler/src/simple_scheduler_state_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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!(
Expand Down
5 changes: 3 additions & 2 deletions nativelink-service/src/ac_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

use std::collections::HashMap;
use std::convert::Into;
use std::fmt::Debug;

use bytes::BytesMut;
Expand Down Expand Up @@ -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)]
Expand All @@ -205,6 +206,6 @@ impl ActionCache for AcServer {
self.inner_update_action_result(request),
)
.await
.map_err(|e| e.into())
.map_err(Into::into)
}
}
9 changes: 5 additions & 4 deletions nativelink-service/src/bytestream_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(<stream>)");
Expand Down Expand Up @@ -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::<GrpcStore>(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
Expand All @@ -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)]
Expand All @@ -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)
}
}
11 changes: 6 additions & 5 deletions nativelink-service/src/cas_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

use std::collections::{HashMap, VecDeque};
use std::convert::Into;
use std::pin::Pin;

use bytes::Bytes;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)]
Expand All @@ -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)]
Expand All @@ -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)]
Expand All @@ -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(<stream>)");
}
Expand Down
5 changes: 3 additions & 2 deletions nativelink-service/src/execution_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand All @@ -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(<stream>)");
Expand Down
9 changes: 5 additions & 4 deletions nativelink-service/src/worker_api_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(<stream>)");
}
Expand All @@ -273,7 +274,7 @@ impl WorkerApi for WorkerApiServer {
) -> Result<Response<()>, Status> {
self.inner_keep_alive(grpc_request.into_inner())
.await
.map_err(|e| e.into())
.map_err(Into::into)
}

#[allow(clippy::blocks_in_conditions)]
Expand All @@ -290,7 +291,7 @@ impl WorkerApi for WorkerApiServer {
) -> Result<Response<()>, Status> {
self.inner_going_away(grpc_request.into_inner())
.await
.map_err(|e| e.into())
.map_err(Into::into)
}

#[allow(clippy::blocks_in_conditions)]
Expand All @@ -307,6 +308,6 @@ impl WorkerApi for WorkerApiServer {
) -> Result<Response<()>, Status> {
self.inner_execution_response(grpc_request.into_inner())
.await
.map_err(|e| e.into())
.map_err(Into::into)
}
}
2 changes: 1 addition & 1 deletion nativelink-store/src/grpc_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions nativelink-store/src/memory_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ impl StoreDriver for MemoryStore {
handler: &mut (dyn for<'a> FnMut(&'a StoreKey) -> bool + Send + Sync + '_),
) -> Result<u64, Error> {
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
Expand Down
2 changes: 1 addition & 1 deletion nativelink-store/src/redis_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion nativelink-store/src/size_partitioning_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion nativelink-util/src/action_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions nativelink-util/src/origin_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -186,7 +187,7 @@ pub struct ActiveOriginContext;
impl ActiveOriginContext {
/// Sets the active context for the current thread.
pub fn get() -> Option<Arc<OriginContext>> {
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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 5 additions & 10 deletions nativelink-util/src/resource_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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()
Expand Down
Loading

0 comments on commit 2b24ce2

Please sign in to comment.