Skip to content

Commit

Permalink
Fix clippy::struct_field_names (#1505)
Browse files Browse the repository at this point in the history
  • Loading branch information
SchahinRohani authored Nov 28, 2024
1 parent 524dc11 commit 91f3a2c
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions nativelink-metric/nativelink-metric-macro-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand All @@ -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| {
Expand Down Expand Up @@ -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,
},
};
Expand Down
79 changes: 39 additions & 40 deletions nativelink-scheduler/tests/simple_scheduler_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Result<Option<MockAwaitedActionSubscriber>, Error>>,
tx_get_by_operation_id:
mpsc::UnboundedSender<Result<Option<MockAwaitedActionSubscriber>, Error>>,
tx_get_range_of_actions: mpsc::UnboundedSender<Vec<Result<MockAwaitedActionSubscriber, Error>>>,
tx_update_awaited_action: mpsc::UnboundedSender<Result<(), Error>>,
get_by_operation_id: mpsc::UnboundedSender<Result<Option<MockAwaitedActionSubscriber>, Error>>,
get_range_of_actions: mpsc::UnboundedSender<Vec<Result<MockAwaitedActionSubscriber, Error>>>,
update_awaited_action: mpsc::UnboundedSender<Result<(), Error>>,
}

#[derive(MetricsComponent)]
struct MockAwaitedAction {
rx_get_awaited_action_by_id:
struct RxMockAwaitedAction {
get_awaited_action_by_id:
Mutex<mpsc::UnboundedReceiver<Result<Option<MockAwaitedActionSubscriber>, Error>>>,
rx_get_by_operation_id:
get_by_operation_id:
Mutex<mpsc::UnboundedReceiver<Result<Option<MockAwaitedActionSubscriber>, Error>>>,
rx_get_range_of_actions:
get_range_of_actions:
Mutex<mpsc::UnboundedReceiver<Vec<Result<MockAwaitedActionSubscriber, Error>>>>,
rx_update_awaited_action: Mutex<mpsc::UnboundedReceiver<Result<(), Error>>>,
update_awaited_action: Mutex<mpsc::UnboundedReceiver<Result<(), Error>>>,
}
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<Option<Self::Subscriber>, 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")
Expand All @@ -942,7 +941,7 @@ impl AwaitedActionDb for MockAwaitedAction {
&self,
_operation_id: &OperationId,
) -> Result<Option<Self::Subscriber>, 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")
Expand All @@ -955,15 +954,15 @@ impl AwaitedActionDb for MockAwaitedAction {
_end: Bound<SortedAwaitedAction>,
_desc: bool,
) -> Result<impl Stream<Item = Result<Self::Subscriber, Error>> + 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");
Ok(futures::stream::iter(items))
}

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")
Expand All @@ -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(),
Expand All @@ -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()),
Expand All @@ -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."
Expand All @@ -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(),
Expand All @@ -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()),
Expand All @@ -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."
Expand Down

0 comments on commit 91f3a2c

Please sign in to comment.