From 198d940a7736984f45acc76d83279ac1e0b08dc0 Mon Sep 17 00:00:00 2001 From: Schahin Rouhanizadeh Date: Sun, 29 Sep 2024 15:38:39 +0200 Subject: [PATCH] Fix clippy::missing-panics-doc --- .bazelrc | 2 +- .../nativelink-metric-macro-derive/src/lib.rs | 3 +++ nativelink-scheduler/src/worker.rs | 7 +++++++ nativelink-service/src/worker_api_server.rs | 12 ++++++++++++ nativelink-store/src/s3_store.rs | 10 ++++++++++ nativelink-store/src/shard_store.rs | 10 ++++++++++ nativelink-util/src/buf_channel.rs | 13 +++++++++++++ nativelink-util/src/evicting_map.rs | 5 +++++ nativelink-util/src/fastcdc.rs | 7 +++++++ nativelink-util/src/metrics_utils.rs | 7 +++++++ nativelink-util/src/origin_context.rs | 6 ++++++ nativelink-worker/src/worker_utils.rs | 13 +++++++++++++ 12 files changed, 94 insertions(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index c8e2faa1e..7b1419f78 100644 --- a/.bazelrc +++ b/.bazelrc @@ -46,7 +46,7 @@ build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect # TODO(aaronmondal): Extend these flags until we can run with clippy::pedantic. # TODO(aaronmondal): Fix dependency_on_unit_never_type_fallback. -build --@rules_rust//:clippy_flags=-Dwarnings,-Adependency_on_unit_never_type_fallback,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new +build --@rules_rust//:clippy_flags=-Dwarnings,-Adependency_on_unit_never_type_fallback,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::missing-panics-doc 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 8b8d9c51f..a78058a42 100644 --- a/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs +++ b/nativelink-metric/nativelink-metric-macro-derive/src/lib.rs @@ -190,6 +190,9 @@ impl<'a> ToTokens for MetricStruct<'a> { } } +/// # Panics +/// +/// This function will panic if the `MetricsComponent` macro is applied to anything other than a struct. #[proc_macro_derive(MetricsComponent, attributes(metric))] pub fn metrics_component_derive(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); diff --git a/nativelink-scheduler/src/worker.rs b/nativelink-scheduler/src/worker.rs index 1d9651665..e7305fa4c 100644 --- a/nativelink-scheduler/src/worker.rs +++ b/nativelink-scheduler/src/worker.rs @@ -119,6 +119,13 @@ fn reduce_platform_properties( } impl Worker { + /// Creates a new `Worker` with the specified ID, platform properties, sender, and timestamp. + /// + /// # Panics + /// + /// This function will panic if the system time is earlier than the Unix epoch. + /// The panic occurs due to the call to `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()`, + /// which will fail if the system clock is misconfigured or set to a time before the Unix epoch. pub fn new( id: WorkerId, platform_properties: PlatformProperties, diff --git a/nativelink-service/src/worker_api_server.rs b/nativelink-service/src/worker_api_server.rs index c69fd4749..58f0289ed 100644 --- a/nativelink-service/src/worker_api_server.rs +++ b/nativelink-service/src/worker_api_server.rs @@ -50,6 +50,18 @@ pub struct WorkerApiServer { } impl WorkerApiServer { + /// Creates a new `WorkerApiServer` with the given configuration and schedulers. + /// + /// # Panics + /// + /// This function panics if the system time is earlier than the Unix epoch. + /// The panic occurs in two situations: + /// - When retrieving the system time for scheduler timeouts (`SystemTime::now().duration_since(UNIX_EPOCH)`). + /// - During server creation, if the system time is invalid, it will trigger an internal error. + /// + /// # Errors + /// + /// Returns an error if the server could not be created with the given configuration and schedulers. pub fn new( config: &WorkerApiConfig, schedulers: &HashMap>, diff --git a/nativelink-store/src/s3_store.rs b/nativelink-store/src/s3_store.rs index 699e4684d..effd27aeb 100644 --- a/nativelink-store/src/s3_store.rs +++ b/nativelink-store/src/s3_store.rs @@ -260,6 +260,16 @@ where I: InstantWrapper, NowFn: Fn() -> I + Send + Sync + Unpin + 'static, { + /// Creates a new S3 store with the specified configuration and time function. + /// + /// # Panics + /// + /// This function will panic if the application name provided to the AWS SDK is invalid. + /// The panic occurs with the message `"valid app name"` in the call to `AppName::new()`. + /// + /// # Errors + /// + /// Returns an error if there is an issue with creating the S3 client or other components. pub async fn new( config: &nativelink_config::stores::S3Store, now_fn: NowFn, diff --git a/nativelink-store/src/shard_store.rs b/nativelink-store/src/shard_store.rs index ae2ab82f5..160d3cd33 100644 --- a/nativelink-store/src/shard_store.rs +++ b/nativelink-store/src/shard_store.rs @@ -45,6 +45,16 @@ pub struct ShardStore { } impl ShardStore { + /// Creates a new `ShardStore` with the given configuration and stores. + /// + /// # Panics + /// + /// This function will panic if the `weights` vector is empty when attempting to modify the last element + /// using `unwrap()`. This panic should not occur as long as the `config.stores` is not empty, as validated earlier. + /// + /// # Errors + /// + /// Returns an error if the number of `config.stores` does not match the number of `stores`, or if `config.stores` is empty. pub fn new( config: &nativelink_config::stores::ShardStore, stores: Vec, diff --git a/nativelink-util/src/buf_channel.rs b/nativelink-util/src/buf_channel.rs index 983acab93..9063c6563 100644 --- a/nativelink-util/src/buf_channel.rs +++ b/nativelink-util/src/buf_channel.rs @@ -275,6 +275,14 @@ impl DropCloserReadHalf { /// /// On error the state of the stream is undefined and the caller should not /// attempt to use the stream again. + /// # Panics + /// + /// This function will panic if the `recent_data` is not empty after draining it. + /// Specifically, it panics with the message `"Recent_data should be empty"`. + /// + /// # Errors + /// + /// Returns an error if `bytes_received` exceeds `max_recent_data_size`. pub fn try_reset_stream(&mut self) -> Result<(), Error> { if self.bytes_received > self.max_recent_data_size { return Err(make_err!( @@ -313,6 +321,11 @@ impl DropCloserReadHalf { } /// Peek the next set of bytes in the stream without consuming them. + /// + /// # Panics + /// + /// This function will panic if `queued_data` is empty after attempting to receive data. + /// The panic occurs with the message `"Should have data in the queue"`. pub async fn peek(&mut self) -> &Result { if self.queued_data.is_empty() { let chunk = self.recv().await; diff --git a/nativelink-util/src/evicting_map.rs b/nativelink-util/src/evicting_map.rs index 09e22d897..c6ca92435 100644 --- a/nativelink-util/src/evicting_map.rs +++ b/nativelink-util/src/evicting_map.rs @@ -210,6 +210,11 @@ where /// and return the number of items that were processed. /// The `handler` function should return `true` to continue processing the next item /// or `false` to stop processing. + /// + /// # Panics + /// + /// This function will panic if the `btree` index is not properly initialized and is `None` after attempting to rebuild it. + /// The panic occurs due to calling `unwrap()` on a `None` value. pub async fn range(&self, prefix_range: impl RangeBounds, mut handler: F) -> u64 where F: FnMut(&K, &T) -> bool, diff --git a/nativelink-util/src/fastcdc.rs b/nativelink-util/src/fastcdc.rs index 5d4c89f69..01415a07b 100644 --- a/nativelink-util/src/fastcdc.rs +++ b/nativelink-util/src/fastcdc.rs @@ -51,6 +51,13 @@ pub struct FastCDC { } impl FastCDC { + /// Creates a new instance with the specified minimum, average, and maximum sizes. + /// + /// # Panics + /// + /// This function will panic if: + /// - `min_size` is not less than `avg_size`, in which case it panics with the message `"Expected {min_size} < {avg_size}"`. + /// - `avg_size` is not less than `max_size`, in which case it panics with the message `"Expected {avg_size} < {max_size}"`. pub fn new(min_size: usize, avg_size: usize, max_size: usize) -> Self { assert!(min_size < avg_size, "Expected {min_size} < {avg_size}"); assert!(avg_size < max_size, "Expected {avg_size} < {max_size}"); diff --git a/nativelink-util/src/metrics_utils.rs b/nativelink-util/src/metrics_utils.rs index 3ba95a564..450d0e23d 100644 --- a/nativelink-util/src/metrics_utils.rs +++ b/nativelink-util/src/metrics_utils.rs @@ -324,6 +324,13 @@ pub struct CounterWithTime { impl CounterWithTime { #[inline] + /// Increments the counter and updates the last time to the current system time. + /// + /// # Panics + /// + /// This function will panic if the system time is earlier than the Unix epoch. + /// This happens because `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()` is called, + /// which panics if the system clock is incorrect or misconfigured. pub fn inc(&self) { if !metrics_enabled() { return; diff --git a/nativelink-util/src/origin_context.rs b/nativelink-util/src/origin_context.rs index be466ae94..6421b6405 100644 --- a/nativelink-util/src/origin_context.rs +++ b/nativelink-util/src/origin_context.rs @@ -319,6 +319,12 @@ impl ContextAwareFuture { /// active context. #[must_use = "futures do nothing unless you `.await` or poll them"] #[inline] + /// Creates a new `ContextAwareFuture` from the active origin context. + /// + /// # Panics + /// + /// This function will panic if the `ActiveOriginContext` is not set (i.e., it returns `None`). + /// The panic occurs with the message `"OriginContext must be set"`. pub fn new_from_active(inner: Instrumented) -> ContextAwareFuture { match ActiveOriginContext::get() { Some(ctx) => ContextAwareFuture::new(Some(ctx), inner), diff --git a/nativelink-worker/src/worker_utils.rs b/nativelink-worker/src/worker_utils.rs index 06741037e..857a26797 100644 --- a/nativelink-worker/src/worker_utils.rs +++ b/nativelink-worker/src/worker_utils.rs @@ -26,6 +26,19 @@ use nativelink_proto::com::github::trace_machina::nativelink::remote_execution:: use tokio::process; use tracing::{event, Level}; +/// Creates a `SupportedProperties` object based on the provided worker properties. +/// +/// # Panics +/// +/// This function will panic if the process fails and the exit status code cannot be retrieved +/// (i.e., `process_output.status.code().unwrap()` returns `None`). This might happen if the process was terminated by a signal and no exit code was available. +/// +/// # Errors +/// +/// Returns an error if: +/// - A worker property query command cannot be parsed. +/// - A worker property query command fails to execute. +/// - The process output cannot be converted to UTF-8. pub async fn make_supported_properties( worker_properties: &HashMap, ) -> Result {