Skip to content

Commit

Permalink
Fix clippy::unnecessary_wraps
Browse files Browse the repository at this point in the history
  • Loading branch information
SchahinRohani committed Oct 17, 2024
1 parent f5de318 commit ef9f13e
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 27 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,-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::unnecessary_wraps
build --@rules_rust//:clippy.toml=//:clippy.toml

test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml
Expand Down
8 changes: 2 additions & 6 deletions nativelink-metric-collector/tests/metric_collector_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use std::io::{BufRead, Cursor};
use std::marker::PhantomData;
use std::str::from_utf8;

use nativelink_error::Error;
use nativelink_metric::{MetricFieldData, MetricKind, MetricsComponent};
use nativelink_metric_collector::{otel_export, MetricsCollectorLayer};
use opentelemetry::metrics::MeterProvider;
Expand Down Expand Up @@ -59,7 +58,7 @@ struct Foo<'a, T: Debug + Send + Sync> {
// Note: Special case to not use nativelink-test macro. We want this test
// to be very lightweight and not depend on other crates.
#[test]
fn test_metric_collector() -> Result<(), Error> {
fn test_metric_collector() {
let multi_struct = MultiStruct {
pub_u64: 1,
str: "str_data".to_string(),
Expand Down Expand Up @@ -108,14 +107,12 @@ fn test_metric_collector() -> Result<(), Error> {
serde_json::to_string(&final_output_metrics).unwrap().len(),
expected_json_data.len()
);

Ok(())
}

// Note: Special case to not use nativelink-test macro. We want this test
// to be very lightweight and not depend on other crates.
#[test]
fn test_prometheus_exporter() -> Result<(), Error> {
fn test_prometheus_exporter() {
let multi_struct = MultiStruct {
pub_u64: 1,
str: "str_data".to_string(),
Expand Down Expand Up @@ -190,5 +187,4 @@ target_info{service_name="unknown_service",telemetry_sdk_language="rust",telemet
expected_output.sort();

assert_eq!(output, expected_output);
Ok(())
}
36 changes: 16 additions & 20 deletions nativelink-scheduler/src/memory_awaited_action_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,25 +502,22 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
&'a self,
state: SortedAwaitedActionState,
range: impl RangeBounds<SortedAwaitedAction> + 'b,
) -> Result<
impl DoubleEndedIterator<
Item = Result<
(
&'a SortedAwaitedAction,
MemoryAwaitedActionSubscriber<I, NowFn>,
),
Error,
>,
> + 'a,
Error,
> {
) -> impl DoubleEndedIterator<
Item = Result<
(
&'a SortedAwaitedAction,
MemoryAwaitedActionSubscriber<I, NowFn>,
),
Error,
>,
> + 'a {
let btree = match state {
SortedAwaitedActionState::CacheCheck => &self.sorted_action_info_hash_keys.cache_check,
SortedAwaitedActionState::Queued => &self.sorted_action_info_hash_keys.queued,
SortedAwaitedActionState::Executing => &self.sorted_action_info_hash_keys.executing,
SortedAwaitedActionState::Completed => &self.sorted_action_info_hash_keys.completed,
};
Ok(btree.range(range).map(|sorted_awaited_action| {
btree.range(range).map(|sorted_awaited_action| {
let operation_id = &sorted_awaited_action.operation_id;
self.get_by_operation_id(operation_id)
.ok_or_else(|| {
Expand All @@ -531,16 +528,16 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
)
})
.map(|subscriber| (sorted_awaited_action, subscriber))
}))
})
}

fn process_state_changes_for_hash_key_map(
action_info_hash_key_to_awaited_action: &mut HashMap<ActionUniqueKey, OperationId>,
new_awaited_action: &AwaitedAction,
) -> Result<(), Error> {
) {
// Only process changes if the stage is not finished.
if !new_awaited_action.state().stage.is_finished() {
return Ok(());
return;
}
match &new_awaited_action.action_info().unique_qualifier {
ActionUniqueQualifier::Cachable(action_key) => {
Expand All @@ -567,13 +564,11 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
);
}
}
Ok(())
}
ActionUniqueQualifier::Uncachable(_action_key) => {
// If we are not cachable, the action should not be in the
// hash_key map, so we don't need to process anything in
// action_info_hash_key_to_awaited_action.
Ok(())
}
}
}
Expand Down Expand Up @@ -635,7 +630,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
Self::process_state_changes_for_hash_key_map(
&mut self.action_info_hash_key_to_awaited_action,
&new_awaited_action,
)?;
);
}
}

Expand Down Expand Up @@ -928,7 +923,8 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync + 'static> Awaite

let iterator = inner
.get_range_of_actions(state, (start.as_ref(), end.as_ref()))
.err_tip(|| "In AwaitedActionDb::get_range_of_actions")?;
.map(|res| res.err_tip(|| "In AwaitedActionDb::get_range_of_actions"));

// TODO(allada) This should probably use the `.left()/right()` pattern,
// but that doesn't exist in the std or any libraries we use.
if desc {
Expand Down
1 change: 1 addition & 0 deletions nativelink-service/tests/worker_api_server_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ struct TestContext {
worker_id: WorkerId,
}

#[allow(clippy::unnecessary_wraps)]
fn static_now_fn() -> Result<Duration, Error> {
Ok(Duration::from_secs(BASE_NOW_S))
}
Expand Down

0 comments on commit ef9f13e

Please sign in to comment.