Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy::redundant_closure_for_method_calls #1380

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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