diff --git a/.bazelrc b/.bazelrc index 31a536423..fed97ae6c 100644 --- a/.bazelrc +++ b/.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::unnecessary_wraps build --@rules_rust//:clippy.toml=//:clippy.toml test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml diff --git a/nativelink-metric-collector/tests/metric_collector_test.rs b/nativelink-metric-collector/tests/metric_collector_test.rs index def159a94..9fdb840dc 100644 --- a/nativelink-metric-collector/tests/metric_collector_test.rs +++ b/nativelink-metric-collector/tests/metric_collector_test.rs @@ -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; @@ -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(), @@ -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(), @@ -190,5 +187,4 @@ target_info{service_name="unknown_service",telemetry_sdk_language="rust",telemet expected_output.sort(); assert_eq!(output, expected_output); - Ok(()) } diff --git a/nativelink-scheduler/src/memory_awaited_action_db.rs b/nativelink-scheduler/src/memory_awaited_action_db.rs index 32f017baf..f950c023e 100644 --- a/nativelink-scheduler/src/memory_awaited_action_db.rs +++ b/nativelink-scheduler/src/memory_awaited_action_db.rs @@ -502,25 +502,22 @@ impl I + Clone + Send + Sync> AwaitedActionDbI &'a self, state: SortedAwaitedActionState, range: impl RangeBounds + 'b, - ) -> Result< - impl DoubleEndedIterator< - Item = Result< - ( - &'a SortedAwaitedAction, - MemoryAwaitedActionSubscriber, - ), - Error, - >, - > + 'a, - Error, - > { + ) -> impl DoubleEndedIterator< + Item = Result< + ( + &'a SortedAwaitedAction, + MemoryAwaitedActionSubscriber, + ), + 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(|| { @@ -531,16 +528,16 @@ impl 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, 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) => { @@ -567,13 +564,11 @@ impl 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(()) } } } @@ -635,7 +630,7 @@ impl 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, - )?; + ); } } @@ -928,7 +923,8 @@ impl 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 { diff --git a/nativelink-service/tests/worker_api_server_test.rs b/nativelink-service/tests/worker_api_server_test.rs index f714788db..1cff7627f 100644 --- a/nativelink-service/tests/worker_api_server_test.rs +++ b/nativelink-service/tests/worker_api_server_test.rs @@ -131,6 +131,7 @@ struct TestContext { worker_id: WorkerId, } +#[allow(clippy::unnecessary_wraps)] fn static_now_fn() -> Result { Ok(Duration::from_secs(BASE_NOW_S)) }