From 91f3a2c65122b0671340bc549d6532f94e6a26b4 Mon Sep 17 00:00:00 2001 From: Schahin Date: Thu, 28 Nov 2024 17:21:16 +0100 Subject: [PATCH] Fix clippy::struct_field_names (#1505) --- .bazelrc | 2 +- .../nativelink-metric-macro-derive/src/lib.rs | 14 ++-- .../tests/simple_scheduler_test.rs | 79 +++++++++---------- 3 files changed, 47 insertions(+), 48 deletions(-) diff --git a/.bazelrc b/.bazelrc index f8b93f764..81198f5de 100644 --- a/.bazelrc +++ b/.bazelrc @@ -43,7 +43,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,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms,-Dclippy::explicit_iter_loop,-Dclippy::items_after_statements,-Dclippy::explicit_into_iter_loop,-Dclippy::stable_sort_primitive,-Dclippy::ptr_as_ptr,-Dclippy::needless_raw_string_hashes,-Dclippy::default_trait_access,-Dclippy::ignored_unit_patterns,-Dclippy::needless_continue,-Dclippy::wildcard_imports,-Dclippy::doc_markdown +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,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms,-Dclippy::explicit_iter_loop,-Dclippy::items_after_statements,-Dclippy::explicit_into_iter_loop,-Dclippy::stable_sort_primitive,-Dclippy::ptr_as_ptr,-Dclippy::needless_raw_string_hashes,-Dclippy::default_trait_access,-Dclippy::ignored_unit_patterns,-Dclippy::needless_continue,-Dclippy::wildcard_imports,-Dclippy::doc_markdown,-Dclippy::struct_field_names build --@rules_rust//:clippy.toml=//:clippy.toml test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml diff --git a/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs b/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs index dad1b511f..96b0f8dc2 100644 --- a/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs +++ b/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs @@ -135,8 +135,8 @@ impl<'a> MetricFieldMetaData<'a> { /// to create the `MetricsComponent` impl. #[derive(Debug)] struct Generics<'a> { - impl_generics: ImplGenerics<'a>, - ty_generics: TypeGenerics<'a>, + implementation: ImplGenerics<'a>, + ty: TypeGenerics<'a>, where_clause: Option<&'a WhereClause>, } @@ -152,8 +152,8 @@ struct MetricStruct<'a> { impl ToTokens for MetricStruct<'_> { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { let name = &self.name; - let impl_generics = &self.generics.impl_generics; - let ty_generics = &self.generics.ty_generics; + let impl_generics = &self.generics.implementation; + let ty_generics = &self.generics.ty; let where_clause = &self.generics.where_clause; let metric_fields = self.metric_fields.iter().map(|field| { @@ -221,13 +221,13 @@ pub fn metrics_component_derive(input: TokenStream) -> TokenStream { } } - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let (implementation, ty, where_clause) = input.generics.split_for_impl(); let metrics_struct = MetricStruct { name: &input.ident, metric_fields, generics: Generics { - impl_generics, - ty_generics, + implementation, + ty, where_clause, }, }; diff --git a/nativelink-scheduler/tests/simple_scheduler_test.rs b/nativelink-scheduler/tests/simple_scheduler_test.rs index 18370ff19..4ce28a243 100644 --- a/nativelink-scheduler/tests/simple_scheduler_test.rs +++ b/nativelink-scheduler/tests/simple_scheduler_test.rs @@ -878,55 +878,54 @@ impl AwaitedActionSubscriber for MockAwaitedActionSubscriber { } } -struct MockSenders { - tx_get_awaited_action_by_id: +struct TxMockSenders { + get_awaited_action_by_id: mpsc::UnboundedSender, Error>>, - tx_get_by_operation_id: - mpsc::UnboundedSender, Error>>, - tx_get_range_of_actions: mpsc::UnboundedSender>>, - tx_update_awaited_action: mpsc::UnboundedSender>, + get_by_operation_id: mpsc::UnboundedSender, Error>>, + get_range_of_actions: mpsc::UnboundedSender>>, + update_awaited_action: mpsc::UnboundedSender>, } #[derive(MetricsComponent)] -struct MockAwaitedAction { - rx_get_awaited_action_by_id: +struct RxMockAwaitedAction { + get_awaited_action_by_id: Mutex, Error>>>, - rx_get_by_operation_id: + get_by_operation_id: Mutex, Error>>>, - rx_get_range_of_actions: + get_range_of_actions: Mutex>>>, - rx_update_awaited_action: Mutex>>, + update_awaited_action: Mutex>>, } -impl MockAwaitedAction { - fn new() -> (MockSenders, Self) { +impl RxMockAwaitedAction { + fn new() -> (TxMockSenders, Self) { let (tx_get_awaited_action_by_id, rx_get_awaited_action_by_id) = mpsc::unbounded_channel(); let (tx_get_by_operation_id, rx_get_by_operation_id) = mpsc::unbounded_channel(); let (tx_get_range_of_actions, rx_get_range_of_actions) = mpsc::unbounded_channel(); let (tx_update_awaited_action, rx_update_awaited_action) = mpsc::unbounded_channel(); ( - MockSenders { - tx_get_awaited_action_by_id, - tx_get_by_operation_id, - tx_get_range_of_actions, - tx_update_awaited_action, + TxMockSenders { + get_awaited_action_by_id: tx_get_awaited_action_by_id, + get_by_operation_id: tx_get_by_operation_id, + get_range_of_actions: tx_get_range_of_actions, + update_awaited_action: tx_update_awaited_action, }, Self { - rx_get_awaited_action_by_id: Mutex::new(rx_get_awaited_action_by_id), - rx_get_by_operation_id: Mutex::new(rx_get_by_operation_id), - rx_get_range_of_actions: Mutex::new(rx_get_range_of_actions), - rx_update_awaited_action: Mutex::new(rx_update_awaited_action), + get_awaited_action_by_id: Mutex::new(rx_get_awaited_action_by_id), + get_by_operation_id: Mutex::new(rx_get_by_operation_id), + get_range_of_actions: Mutex::new(rx_get_range_of_actions), + update_awaited_action: Mutex::new(rx_update_awaited_action), }, ) } } -impl AwaitedActionDb for MockAwaitedAction { +impl AwaitedActionDb for RxMockAwaitedAction { type Subscriber = MockAwaitedActionSubscriber; async fn get_awaited_action_by_id( &self, _client_operation_id: &OperationId, ) -> Result, Error> { - let mut rx_get_awaited_action_by_id = self.rx_get_awaited_action_by_id.lock().await; + let mut rx_get_awaited_action_by_id = self.get_awaited_action_by_id.lock().await; rx_get_awaited_action_by_id .try_recv() .expect("Could not receive msg in mpsc") @@ -942,7 +941,7 @@ impl AwaitedActionDb for MockAwaitedAction { &self, _operation_id: &OperationId, ) -> Result, Error> { - let mut rx_get_by_operation_id = self.rx_get_by_operation_id.lock().await; + let mut rx_get_by_operation_id = self.get_by_operation_id.lock().await; rx_get_by_operation_id .try_recv() .expect("Could not receive msg in mpsc") @@ -955,7 +954,7 @@ impl AwaitedActionDb for MockAwaitedAction { _end: Bound, _desc: bool, ) -> Result> + Send, Error> { - let mut rx_get_range_of_actions = self.rx_get_range_of_actions.lock().await; + let mut rx_get_range_of_actions = self.get_range_of_actions.lock().await; let items = rx_get_range_of_actions .try_recv() .expect("Could not receive msg in mpsc"); @@ -963,7 +962,7 @@ impl AwaitedActionDb for MockAwaitedAction { } async fn update_awaited_action(&self, _new_awaited_action: AwaitedAction) -> Result<(), Error> { - let mut rx_update_awaited_action = self.rx_update_awaited_action.lock().await; + let mut rx_update_awaited_action = self.update_awaited_action.lock().await; rx_update_awaited_action .try_recv() .expect("Could not receive msg in mpsc") @@ -982,7 +981,7 @@ impl AwaitedActionDb for MockAwaitedAction { async fn matching_engine_fails_sends_abort() -> Result<(), Error> { { let task_change_notify = Arc::new(Notify::new()); - let (senders, awaited_action) = MockAwaitedAction::new(); + let (senders, awaited_action) = RxMockAwaitedAction::new(); let (scheduler, _worker_scheduler) = SimpleScheduler::new_with_callback( &SimpleSpec::default(), @@ -992,7 +991,7 @@ async fn matching_engine_fails_sends_abort() -> Result<(), Error> { MockInstantWrapped::default, ); // Initial worker calls do_try_match, so send it no items. - senders.tx_get_range_of_actions.send(vec![]).unwrap(); + senders.get_range_of_actions.send(vec![]).unwrap(); let _worker_rx = setup_new_worker( &scheduler, WorkerId(Uuid::new_v4()), @@ -1002,21 +1001,21 @@ async fn matching_engine_fails_sends_abort() -> Result<(), Error> { .unwrap(); senders - .tx_get_awaited_action_by_id + .get_awaited_action_by_id .send(Ok(Some(MockAwaitedActionSubscriber {}))) .unwrap(); senders - .tx_get_by_operation_id + .get_by_operation_id .send(Ok(Some(MockAwaitedActionSubscriber {}))) .unwrap(); // This one gets called twice because of Abort triggers retry, just return item not exist on retry. - senders.tx_get_by_operation_id.send(Ok(None)).unwrap(); + senders.get_by_operation_id.send(Ok(None)).unwrap(); senders - .tx_get_range_of_actions + .get_range_of_actions .send(vec![Ok(MockAwaitedActionSubscriber {})]) .unwrap(); senders - .tx_update_awaited_action + .update_awaited_action .send(Err(make_err!( Code::Aborted, "This means data version did not match." @@ -1027,7 +1026,7 @@ async fn matching_engine_fails_sends_abort() -> Result<(), Error> { } { let task_change_notify = Arc::new(Notify::new()); - let (senders, awaited_action) = MockAwaitedAction::new(); + let (senders, awaited_action) = RxMockAwaitedAction::new(); let (scheduler, _worker_scheduler) = SimpleScheduler::new_with_callback( &SimpleSpec::default(), @@ -1037,7 +1036,7 @@ async fn matching_engine_fails_sends_abort() -> Result<(), Error> { MockInstantWrapped::default, ); // senders.tx_get_awaited_action_by_id.send(Ok(None)).unwrap(); - senders.tx_get_range_of_actions.send(vec![]).unwrap(); + senders.get_range_of_actions.send(vec![]).unwrap(); let _worker_rx = setup_new_worker( &scheduler, WorkerId(Uuid::new_v4()), @@ -1047,19 +1046,19 @@ async fn matching_engine_fails_sends_abort() -> Result<(), Error> { .unwrap(); senders - .tx_get_awaited_action_by_id + .get_awaited_action_by_id .send(Ok(Some(MockAwaitedActionSubscriber {}))) .unwrap(); senders - .tx_get_by_operation_id + .get_by_operation_id .send(Ok(Some(MockAwaitedActionSubscriber {}))) .unwrap(); senders - .tx_get_range_of_actions + .get_range_of_actions .send(vec![Ok(MockAwaitedActionSubscriber {})]) .unwrap(); senders - .tx_update_awaited_action + .update_awaited_action .send(Err(make_err!( Code::Internal, "This means an internal error happened."