Globally inline format args (#798)

Much more readable. The new setting is enforced for Bazel tests.

.bazelrc

@@ -43,6 +43,9 @@ build:self_execute --platform_suffix=self-execute
 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=-D,clippy::uninlined_format_args
+
 test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml
 
 # This will make rustfmt and clippy only run on `bazel test`.

nativelink-scheduler/src/worker.rs

@@ -39,7 +39,7 @@ impl std::fmt::Display for WorkerId {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let mut buf = Uuid::encode_buffer();
         let worker_id_str = Uuid::from_u128(self.0).hyphenated().encode_lower(&mut buf);
-        write!(f, "{}", worker_id_str)
+        write!(f, "{worker_id_str}")
     }
 }
 
@@ -188,7 +188,7 @@ impl Worker {
         let id = self.id;
         self.metrics.keep_alive.wrap(move || {
             send_msg_to_worker(tx, update_for_worker::Update::KeepAlive(()))
-                .err_tip(|| format!("Failed to send KeepAlive to worker : {}", id))
+                .err_tip(|| format!("Failed to send KeepAlive to worker : {id}"))
         })
     }
 

nativelink-service/src/ac_server.rs

@@ -82,7 +82,7 @@ impl AcServer {
         let store_info = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?;
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?;
 
         // TODO(blaise.bruer) We should write a test for these errors.
         let digest: DigestInfo = get_action_request
@@ -115,7 +115,7 @@ impl AcServer {
         let store_info = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?;
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?;
 
         if store_info.read_only {
             return Err(make_err!(

nativelink-service/src/bytestream_server.rs

@@ -268,7 +268,7 @@ impl ByteStreamServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         let digest = DigestInfo::try_new(resource_info.hash, resource_info.expected_size)?;
@@ -379,7 +379,7 @@ impl ByteStreamServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         let digest = DigestInfo::try_new(&stream.hash, stream.expected_size)

nativelink-service/src/capabilities_server.rs

@@ -61,7 +61,7 @@ impl CapabilitiesServer {
                 for platform_key in scheduler
                     .get_platform_property_manager(instance_name)
                     .await
-                    .err_tip(|| format!("Failed to get platform properties for {}", instance_name))?
+                    .err_tip(|| format!("Failed to get platform properties for {instance_name}"))?
                     .get_known_properties()
                     .keys()
                 {

nativelink-service/src/cas_server.rs

@@ -73,7 +73,7 @@ impl CasServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         let mut requested_blobs = Vec::with_capacity(inner_request.blob_digests.len());
@@ -105,7 +105,7 @@ impl CasServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         // If we are a GrpcStore we shortcut here, as this is a special store.
@@ -164,7 +164,7 @@ impl CasServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         // If we are a GrpcStore we shortcut here, as this is a special store.
@@ -227,7 +227,7 @@ impl CasServer {
         let store = self
             .stores
             .get(instance_name)
-            .err_tip(|| format!("'instance_name' not configured for '{}'", instance_name))?
+            .err_tip(|| format!("'instance_name' not configured for '{instance_name}'"))?
             .clone();
 
         // If we are a GrpcStore we shortcut here, as this is a special store.

nativelink-service/src/worker_api_server.rs

@@ -216,7 +216,7 @@ impl WorkerApiServer {
                 self.scheduler
                     .update_action(&worker_id, &action_info_hash_key, action_stage)
                     .await
-                    .err_tip(|| format!("Failed to update_action {:?}", action_digest))?;
+                    .err_tip(|| format!("Failed to update_action {action_digest:?}"))?;
             }
             execute_result::Result::InternalError(e) => {
                 self.scheduler

nativelink-service/tests/ac_server_test.rs

@@ -150,8 +150,7 @@ mod get_action_result {
 
         assert!(
             raw_response.is_ok(),
-            "Expected value, got error {:?}",
-            raw_response
+            "Expected value, got error {raw_response:?}"
         );
         assert_eq!(raw_response.unwrap().into_inner(), action_result);
         Ok(())
@@ -236,8 +235,7 @@ mod update_action_result {
 
         assert!(
             raw_response.is_ok(),
-            "Expected success, got error {:?}",
-            raw_response
+            "Expected success, got error {raw_response:?}"
         );
         assert_eq!(raw_response.unwrap().into_inner(), action_result);
 

nativelink-service/tests/cas_server_test.rs

@@ -152,8 +152,7 @@ mod find_missing_blobs {
         let error = raw_response.unwrap_err();
         assert!(
             error.to_string().contains("Invalid sha256 hash: BAD_HASH"),
-            "'Invalid sha256 hash: BAD_HASH' not found in: {:?}",
-            error
+            "'Invalid sha256 hash: BAD_HASH' not found in: {error:?}"
         );
         Ok(())
     }

nativelink-store/src/ac_utils.rs

@@ -74,7 +74,7 @@ pub async fn get_size_and_decode_digest<T: Message + Default>(
         .err_tip_with_code(|e| {
             (
                 Code::NotFound,
-                format!("Stored value appears to be corrupt: {}", e),
+                format!("Stored value appears to be corrupt: {e}"),
             )
         })
         .map(|v| (v, store_data_len))

nativelink-store/src/filesystem_store.rs

@@ -119,7 +119,7 @@ impl Drop for EncodedFilePath {
             );
             let result = fs::remove_file(&file_path)
                 .await
-                .err_tip(|| format!("Failed to remove file {:?}", file_path));
+                .err_tip(|| format!("Failed to remove file {file_path:?}"));
             if let Err(err) = result {
                 error!("\x1b[0;31mFilesystem Store\x1b[0m: {:?}", err);
             }
@@ -213,18 +213,14 @@ impl FileEntry for FileEntryImpl {
         let temp_file_result = fs::create_file(temp_full_path.clone())
             .or_else(|mut err| async {
                 let remove_result = fs::remove_file(&temp_full_path).await.err_tip(|| {
-                    format!(
-                        "Failed to remove file {:?} in filesystem store",
-                        temp_full_path
-                    )
+                    format!("Failed to remove file {temp_full_path:?} in filesystem store")
                 });
                 if let Err(remove_err) = remove_result {
                     err = err.merge(remove_err);
                 }
                 warn!("\x1b[0;31mFilesystem Store\x1b[0m: {:?}", err);
-                Err(err).err_tip(|| {
-                    format!("Failed to create {:?} in filesystem store", temp_full_path)
-                })
+                Err(err)
+                    .err_tip(|| format!("Failed to create {temp_full_path:?} in filesystem store"))
             })
             .await?;
 
@@ -261,10 +257,7 @@ impl FileEntry for FileEntryImpl {
                 let file = fs::open_file(full_content_path.clone(), length)
                     .await
                     .err_tip(|| {
-                        format!(
-                            "Failed to open file in filesystem store {:?}",
-                            full_content_path
-                        )
+                        format!("Failed to open file in filesystem store {full_content_path:?}")
                     })?;
                 Ok((file, full_content_path))
             })
@@ -276,12 +269,7 @@ impl FileEntry for FileEntryImpl {
             .get_mut()
             .seek(SeekFrom::Start(offset))
             .await
-            .err_tip(|| {
-                format!(
-                    "Failed to seek file: {:?}",
-                    full_content_path_for_debug_only
-                )
-            })?;
+            .err_tip(|| format!("Failed to seek file: {full_content_path_for_debug_only:?}"))?;
         Ok(file)
     }
 
@@ -334,10 +322,7 @@ impl LenEntry for FileEntryImpl {
                 let full_content_path = full_content_path.to_os_string();
                 spawn_blocking(move || {
                     set_file_atime(&full_content_path, FileTime::now()).err_tip(|| {
-                        format!(
-                            "Failed to touch file in filesystem store {:?}",
-                            full_content_path
-                        )
+                        format!("Failed to touch file in filesystem store {full_content_path:?}")
                     })
                 })
                 .await
@@ -766,7 +751,7 @@ impl<Fe: FileEntry> Store for FilesystemStore<Fe> {
 
         self.update_file(entry, temp_file, digest, reader)
             .await
-            .err_tip(|| format!("While processing with temp file {:?}", temp_full_path))
+            .err_tip(|| format!("While processing with temp file {temp_full_path:?}"))
     }
 
     fn optimized_for(&self, optimization: StoreOptimizations) -> bool {
@@ -786,16 +771,13 @@ impl<Fe: FileEntry> Store for FilesystemStore<Fe> {
                 .as_reader()
                 .await
                 .err_tip(|| {
-                    format!(
-                        "While getting metadata for {:?} in update_with_whole_file",
-                        path
-                    )
+                    format!("While getting metadata for {path:?} in update_with_whole_file")
                 })?
                 .get_ref()
                 .as_ref()
                 .metadata()
                 .await
-                .err_tip(|| format!("While reading metadata for {:?}", path))?
+                .err_tip(|| format!("While reading metadata for {path:?}"))?
                 .len(),
         };
         let entry = Fe::create(

nativelink-store/tests/ac_utils_test.rs

@@ -33,7 +33,7 @@ async fn make_temp_path(data: &str) -> OsString {
         thread_rng().gen::<u64>(),
     );
     fs::create_dir_all(&dir).await.unwrap();
-    OsString::from(format!("{}/{}", dir, data))
+    OsString::from(format!("{dir}/{data}"))
 }
 
 #[cfg(test)]

nativelink-store/tests/cas_utils_test.rs

@@ -25,7 +25,7 @@ mod cas_utils_tests {
             packed_hash: Sha256::new().finalize().into(),
             size_bytes: 0,
         };
-        assert_eq!(is_zero_digest(&digest), true);
+        assert!(is_zero_digest(&digest));
     }
 
     #[test]
@@ -36,7 +36,7 @@ mod cas_utils_tests {
             packed_hash: hasher.finalize().into(),
             size_bytes: 1,
         };
-        assert_eq!(is_zero_digest(&digest), false);
+        assert!(!is_zero_digest(&digest));
     }
 
     #[test]
@@ -45,7 +45,7 @@ mod cas_utils_tests {
             packed_hash: Blake3::new().finalize().into(),
             size_bytes: 0,
         };
-        assert_eq!(is_zero_digest(&digest), true);
+        assert!(is_zero_digest(&digest));
     }
 
     #[test]
@@ -56,6 +56,6 @@ mod cas_utils_tests {
             packed_hash: hasher.finalize().into(),
             size_bytes: 1,
         };
-        assert_eq!(is_zero_digest(&digest), false);
+        assert!(!is_zero_digest(&digest));
     }
 }

nativelink-store/tests/compression_store_test.rs

@@ -149,10 +149,7 @@ mod compression_store_tests {
                     .get_part_unchunked(digest, offset, Some(read_slice_size), None)
                     .await
                     .err_tip(|| {
-                        format!(
-                            "Failed to get from inner store at {} - {}",
-                            offset, read_slice_size
-                        )
+                        format!("Failed to get from inner store at {offset} - {read_slice_size}")
                     })?;
 
                 let start_pos = cmp::min(RAW_DATA.len(), offset);

nativelink-store/tests/fast_slow_store_test.rs

@@ -63,17 +63,15 @@ async fn check_data<S: Store>(
 ) -> Result<(), Error> {
     assert!(
         check_store.has(digest).await?.is_some(),
-        "Expected data to exist in {} store",
-        debug_name
+        "Expected data to exist in {debug_name} store"
     );
 
     let store_data = check_store
         .get_part_unchunked(digest, 0, None, None)
         .await?;
     assert_eq!(
         store_data, original_data,
-        "Expected data to match in {} store",
-        debug_name
+        "Expected data to match in {debug_name} store"
     );
     Ok(())
 }

nativelink-store/tests/filesystem_store_test.rs

@@ -604,7 +604,7 @@ mod filesystem_store_tests {
             let mut read_dir_stream = ReadDirStream::new(temp_dir_handle);
             if let Some(temp_dir_entry) = read_dir_stream.next().await {
                 let path = temp_dir_entry?.path();
-                panic!("No files should exist in temp directory, found: {:?}", path);
+                panic!("No files should exist in temp directory, found: {path:?}");
             }
         }
 

nativelink-store/tests/verify_store_test.rs

@@ -98,9 +98,7 @@ mod verify_store_tests {
         let err = result.unwrap_err().to_string();
         assert!(
             err.contains(EXPECTED_ERR),
-            "Error should contain '{}', got: {:?}",
-            EXPECTED_ERR,
-            err
+            "Error should contain '{EXPECTED_ERR}', got: {err:?}"
         );
         assert_eq!(
             Pin::new(inner_store.as_ref()).has(digest).await,
@@ -237,15 +235,11 @@ mod verify_store_tests {
         let err = result.unwrap_err().to_string();
         const ACTUAL_HASH: &str =
             "a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3";
-        let expected_err = format!(
-            "Hashes do not match, got: {} but digest hash was {}",
-            HASH, ACTUAL_HASH
-        );
+        let expected_err =
+            format!("Hashes do not match, got: {HASH} but digest hash was {ACTUAL_HASH}");
         assert!(
             err.contains(&expected_err),
-            "Error should contain '{}', got: {:?}",
-            expected_err,
-            err
+            "Error should contain '{expected_err}', got: {err:?}"
         );
         assert_eq!(
             Pin::new(inner_store.as_ref()).has(digest).await,
@@ -315,15 +309,11 @@ mod verify_store_tests {
         let err = result.unwrap_err().to_string();
         const ACTUAL_HASH: &str =
             "b3d4f8803f7e24b8f389b072e75477cdbcfbe074080fb5e500e53e26e054158e";
-        let expected_err = format!(
-            "Hashes do not match, got: {} but digest hash was {}",
-            HASH, ACTUAL_HASH
-        );
+        let expected_err =
+            format!("Hashes do not match, got: {HASH} but digest hash was {ACTUAL_HASH}");
         assert!(
             err.contains(&expected_err),
-            "Error should contain '{}', got: {:?}",
-            expected_err,
-            err
+            "Error should contain '{expected_err}', got: {err:?}"
         );
         assert_eq!(
             Pin::new(inner_store.as_ref()).has(digest).await,

nativelink-util/src/common.rs

@@ -53,7 +53,7 @@ impl DigestInfo {
         T: TryInto<i64> + std::fmt::Display + Copy,
     {
         let packed_hash =
-            <[u8; 32]>::from_hex(hash).err_tip(|| format!("Invalid sha256 hash: {}", hash))?;
+            <[u8; 32]>::from_hex(hash).err_tip(|| format!("Invalid sha256 hash: {hash}"))?;
         let size_bytes = size_bytes
             .try_into()
             .map_err(|_| make_input_err!("Could not convert {} into i64", size_bytes))?;