Fix clippy::needless_pass_by_value

.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,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone
+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,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value
 build --@rules_rust//:clippy.toml=//:clippy.toml
 
 test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml

nativelink-error/src/lib.rs

@@ -76,8 +76,8 @@ impl Error {
 
     #[inline]
     #[must_use]
-    pub fn append<S: std::string::ToString>(mut self, msg: S) -> Self {
-        self.messages.push(msg.to_string());
+    pub fn append<S: Into<String>>(mut self, msg: S) -> Self {
+        self.messages.push(msg.into());
         self
     }
 

nativelink-scheduler/src/default_scheduler_factory.rs

@@ -99,7 +99,7 @@ fn simple_scheduler_factory(
             let task_change_notify = Arc::new(Notify::new());
             let awaited_action_db = memory_awaited_action_db_factory(
                 config.retain_completed_for_s,
-                task_change_notify.clone(),
+                &task_change_notify.clone(),
                 SystemTime::now,
             );
             let (action_scheduler, worker_scheduler) =
@@ -141,7 +141,7 @@ fn simple_scheduler_factory(
 
 pub fn memory_awaited_action_db_factory<I, NowFn>(
     mut retain_completed_for_s: u32,
-    task_change_notify: Arc<Notify>,
+    task_change_notify: &Arc<Notify>,
     now_fn: NowFn,
 ) -> MemoryAwaitedActionDb<I, NowFn>
 where

nativelink-scheduler/src/memory_awaited_action_db.rs

@@ -251,7 +251,7 @@ impl SortedAwaitedActions {
     fn insert_sort_map_for_stage(
         &mut self,
         stage: &ActionStage,
-        sorted_awaited_action: SortedAwaitedAction,
+        sorted_awaited_action: &SortedAwaitedAction,
     ) -> Result<(), Error> {
         let newly_inserted = match stage {
             ActionStage::Unknown => self.unknown.insert(sorted_awaited_action.clone()),
@@ -294,7 +294,7 @@ impl SortedAwaitedActions {
             ));
         };
 
-        self.insert_sort_map_for_stage(&new_awaited_action.state().stage, sorted_awaited_action)
+        self.insert_sort_map_for_stage(&new_awaited_action.state().stage, &sorted_awaited_action)
             .err_tip(|| "In AwaitedActionDb::update_awaited_action")?;
         Ok(())
     }
@@ -655,7 +655,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
     /// cleanup of the other maps on drop.
     fn make_client_awaited_action(
         &mut self,
-        operation_id: OperationId,
+        operation_id: &OperationId,
         awaited_action: AwaitedAction,
     ) -> (Arc<ClientAwaitedAction>, watch::Receiver<AwaitedAction>) {
         let (tx, rx) = watch::channel(awaited_action);
@@ -704,7 +704,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
         let sort_key = awaited_action.sort_key();
 
         let (client_awaited_action, rx) =
-            self.make_client_awaited_action(operation_id.clone(), awaited_action);
+            self.make_client_awaited_action(&operation_id.clone(), awaited_action);
 
         event!(
             Level::DEBUG,
@@ -736,7 +736,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
         self.sorted_action_info_hash_keys
             .insert_sort_map_for_stage(
                 &ActionStage::Queued,
-                SortedAwaitedAction {
+                &SortedAwaitedAction {
                     sort_key,
                     operation_id,
                 },

nativelink-scheduler/src/store_awaited_action_db.rs

@@ -133,8 +133,8 @@ impl<S: SchedulerStore> AwaitedActionSubscriber for OperationSubscriber<S> {
     }
 }
 
-fn awaited_action_decode(version: u64, data: Bytes) -> Result<AwaitedAction, Error> {
-    let mut awaited_action: AwaitedAction = serde_json::from_slice(&data)
+fn awaited_action_decode(version: u64, data: &Bytes) -> Result<AwaitedAction, Error> {
+    let mut awaited_action: AwaitedAction = serde_json::from_slice(data)
         .map_err(|e| make_input_err!("In AwaitedAction::decode - {e:?}"))?;
     awaited_action.set_version(version);
     Ok(awaited_action)
@@ -165,7 +165,7 @@ impl SchedulerStoreKeyProvider for OperationIdToAwaitedAction<'_> {
 impl SchedulerStoreDecodeTo for OperationIdToAwaitedAction<'_> {
     type DecodeOutput = AwaitedAction;
     fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
-        awaited_action_decode(version, data)
+        awaited_action_decode(version, &data)
     }
 }
 
@@ -200,7 +200,7 @@ impl SchedulerIndexProvider for SearchUniqueQualifierToAwaitedAction<'_> {
 impl SchedulerStoreDecodeTo for SearchUniqueQualifierToAwaitedAction<'_> {
     type DecodeOutput = AwaitedAction;
     fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
-        awaited_action_decode(version, data)
+        awaited_action_decode(version, &data)
     }
 }
 
@@ -216,7 +216,7 @@ impl SchedulerIndexProvider for SearchSortKeyPrefixToAwaitedAction {
 impl SchedulerStoreDecodeTo for SearchSortKeyPrefixToAwaitedAction {
     type DecodeOutput = AwaitedAction;
     fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
-        awaited_action_decode(version, data)
+        awaited_action_decode(version, &data)
     }
 }
 

nativelink-scheduler/tests/simple_scheduler_test.rs

@@ -163,7 +163,7 @@ async fn basic_add_action_with_one_worker_test() -> Result<(), Error> {
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -225,7 +225,7 @@ async fn client_does_not_receive_update_timeout() -> Result<(), Error> {
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -294,7 +294,7 @@ async fn find_executing_action() -> Result<(), Error> {
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -374,7 +374,7 @@ async fn remove_worker_reschedules_multiple_running_job_test() -> Result<(), Err
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -549,7 +549,7 @@ async fn set_drain_worker_pauses_and_resumes_worker_test() -> Result<(), Error>
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -636,7 +636,7 @@ async fn worker_should_not_queue_if_properties_dont_match_test() -> Result<(), E
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -727,7 +727,7 @@ async fn cacheable_items_join_same_action_queued_test() -> Result<(), Error> {
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -828,7 +828,7 @@ async fn worker_disconnects_does_not_schedule_for_execution_test() -> Result<(),
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1087,7 +1087,7 @@ async fn worker_timesout_reschedules_running_job_test() -> Result<(), Error> {
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1209,7 +1209,7 @@ async fn update_action_sends_completed_result_to_client_test() -> Result<(), Err
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1310,7 +1310,7 @@ async fn update_action_sends_completed_result_after_disconnect() -> Result<(), E
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1428,7 +1428,7 @@ async fn update_action_with_wrong_worker_id_errors_test() -> Result<(), Error> {
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1526,7 +1526,7 @@ async fn does_not_crash_if_operation_joined_then_relaunched() -> Result<(), Erro
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1669,7 +1669,7 @@ async fn run_two_jobs_on_same_worker_with_platform_properties_restrictions() ->
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1831,7 +1831,7 @@ async fn run_jobs_in_the_order_they_were_queued() -> Result<(), Error> {
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -1898,7 +1898,7 @@ async fn worker_retries_on_internal_error_and_fails_test() -> Result<(), Error>
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -2047,7 +2047,7 @@ async fn ensure_scheduler_drops_inner_spawn() -> Result<(), Error> {
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         move || {
@@ -2080,7 +2080,7 @@ async fn ensure_task_or_worker_change_notification_received_test() -> Result<(),
         &nativelink_config::schedulers::SimpleScheduler::default(),
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},
@@ -2155,7 +2155,7 @@ async fn client_reconnect_keeps_action_alive() -> Result<(), Error> {
         },
         memory_awaited_action_db_factory(
             0,
-            task_change_notify.clone(),
+            &task_change_notify.clone(),
             MockInstantWrapped::default,
         ),
         || async move {},

nativelink-service/src/execution_server.rs

@@ -197,7 +197,7 @@ impl ExecutionServer {
     }
 
     fn to_execute_stream(
-        nl_client_operation_id: NativelinkOperationId,
+        nl_client_operation_id: &NativelinkOperationId,
         action_listener: Box<dyn ActionStateResult>,
     ) -> Response<ExecuteStream> {
         let client_operation_id = OperationId::from(nl_client_operation_id.to_string());
@@ -277,7 +277,7 @@ impl ExecutionServer {
             .err_tip(|| "Failed to schedule task")?;
 
         Ok(Self::to_execute_stream(
-            NativelinkOperationId::new(
+            &NativelinkOperationId::new(
                 instance_name,
                 action_listener
                     .as_state()
@@ -315,7 +315,7 @@ impl ExecutionServer {
         else {
             return Err(Status::not_found("Failed to find existing task"));
         };
-        Ok(Self::to_execute_stream(nl_operation_id, rx))
+        Ok(Self::to_execute_stream(&nl_operation_id, rx))
     }
 }
 

nativelink-store/src/compression_store.rs

@@ -220,7 +220,7 @@ pub struct CompressionStore {
 
 impl CompressionStore {
     pub fn new(
-        compression_config: nativelink_config::stores::CompressionStore,
+        compression_config: &nativelink_config::stores::CompressionStore,
         inner_store: Store,
     ) -> Result<Arc<Self>, Error> {
         let lz4_config = match compression_config.compression_algorithm {

nativelink-store/src/default_store_factory.rs

@@ -59,7 +59,7 @@ pub fn store_factory<'a>(
                 store_factory(&config.backend, store_manager, None).await?,
             ),
             StoreConfig::compression(config) => CompressionStore::new(
-                *config.clone(),
+                &config.clone(),
                 store_factory(&config.backend, store_manager, None).await?,
             )?,
             StoreConfig::dedup(config) => DedupStore::new(

nativelink-store/src/s3_store.rs

@@ -324,7 +324,7 @@ where
         }))
     }
 
-    fn make_s3_path(&self, key: StoreKey<'_>) -> String {
+    fn make_s3_path(&self, key: &StoreKey<'_>) -> String {
         format!("{}{}", self.key_prefix, key.as_str(),)
     }
 
@@ -335,7 +335,7 @@ where
                     .s3_client
                     .head_object()
                     .bucket(&self.bucket)
-                    .key(self.make_s3_path(digest.borrow()))
+                    .key(self.make_s3_path(&digest.borrow()))
                     .send()
                     .await;
 
@@ -412,7 +412,7 @@ where
         mut reader: DropCloserReadHalf,
         upload_size: UploadSizeInfo,
     ) -> Result<(), Error> {
-        let s3_path = &self.make_s3_path(digest.borrow());
+        let s3_path = &self.make_s3_path(&digest.borrow());
 
         let max_size = match upload_size {
             UploadSizeInfo::ExactSize(sz) | UploadSizeInfo::MaxSize(sz) => sz,
@@ -687,7 +687,7 @@ where
             return Ok(());
         }
 
-        let s3_path = &self.make_s3_path(key);
+        let s3_path = &self.make_s3_path(&key);
         let end_read_byte = length
             .map_or(Some(None), |length| Some(offset.checked_add(length)))
             .err_tip(|| "Integer overflow protection triggered")?;