diff --git a/clippy.toml b/clippy.toml index 0f58a29c9..0f77d950b 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,9 +1,16 @@ disallowed-methods = [ { path = "rand::thread_rng", reason = "Do not use rand::* directly. Instead, use functions from random.rs" }, - # We still have `std::fs` references. - # { path = "std::fs", reason = "Do not use std::fs directly. Instead, use the FileStorageProvider." }, + { path = "std::fs::File::create", reason = "Do not use std::fs::File::create directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::File::create_new", reason = "Do not use std::fs::File::create_new directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::OpenOptions::open", reason = "Do not use std::fs::OpenOptions::open for writing. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::write", reason = "Do not use std::fs::write directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::remove_file", reason = "Do not use std::fs::remove_file directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::remove_dir", reason = "Do not use std::fs::remove_dir directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::remove_dir_all", reason = "Do not use std::fs::remove_dir_all directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::create_dir", reason = "Do not use std::fs::create_dir directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, + { path = "std::fs::create_dir_all", reason = "Do not use std::fs::create_dir_all directly. Use FileStorageProvider in production code or VirtualStorageProvider in tests." }, { path = "vfs::PhysicalFS::new", reason = "Do not use vfs::PhysicalFS in tests. Instead, use the VirtualStorageProvider::new_overlay()." }, - { path = "diskann_providers::storage::virtual_storage_provider::VirtualStorageProvider::new_physical", reason = "Do not use VirtualStorageProvider::new_physical() in tests. Instead, use VirtualStorageProvider::new_overlay()." }, + { path = "diskann_providers::storage::virtual_storage_provider::VirtualStorageProvider::new_physical", reason = "In most tests, use VirtualStorageProvider::new_memory() or new_overlay(). Only use new_physical() for integration tests that require actual filesystem operations (e.g., testing BfTree with its disk backend).", allow-invalid = true }, # Disallowed methods for the rayon crate to enforce execution within a specified thread pool instead of the global thread pool. { path = "rayon::iter::ParallelIterator::for_each", reason = "Use `for_each_in_pool` from rayon_utils.rs instead to enforce execution within a specified thread pool."}, { path = "rayon::iter::ParallelIterator::for_each_with", reason = "Use `for_each_with_in_pool` instead to enforce execution within a specified thread pool."}, diff --git a/diskann-providers/src/model/graph/provider/async_/bf_tree/provider.rs b/diskann-providers/src/model/graph/provider/async_/bf_tree/provider.rs index 8650b969f..79388a246 100644 --- a/diskann-providers/src/model/graph/provider/async_/bf_tree/provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/bf_tree/provider.rs @@ -2176,7 +2176,6 @@ where mod tests { use super::*; use crate::model::graph::provider::async_::common::TableBasedDeletes; - use crate::storage::file_storage_provider::FileStorageProvider; #[tokio::test] async fn test_data_provider_and_delete_interface() { @@ -2365,6 +2364,7 @@ mod tests { // SaveWith/LoadWith Tests for BfTree-based // /////////////////////////////////////////////// + use crate::storage::VirtualStorageProvider; use tempfile::tempdir; /// Test saving and loading of BfTreeProvider without quantization, including deleted vertices @@ -2453,7 +2453,7 @@ mod tests { assert_eq!(vector_config.get_leaf_page_size(), 8192); assert_eq!(vector_config.get_cb_max_record_size(), 1024); - let storage = FileStorageProvider; + let storage = VirtualStorageProvider::new_overlay(temp_path); provider.save_with(&storage, &prefix).await.unwrap(); @@ -2621,7 +2621,7 @@ mod tests { ); } - let storage = FileStorageProvider; + let storage = VirtualStorageProvider::new_overlay(temp_path); provider.save_with(&storage, &prefix).await.unwrap(); diff --git a/diskann-providers/src/storage/file_storage_provider.rs b/diskann-providers/src/storage/file_storage_provider.rs index 9d4d351a4..b0163613b 100644 --- a/diskann-providers/src/storage/file_storage_provider.rs +++ b/diskann-providers/src/storage/file_storage_provider.rs @@ -31,6 +31,10 @@ impl StorageReadProvider for FileStorageProvider { } } +#[expect( + clippy::disallowed_methods, + reason = "FileStorageProvider is the abstraction layer that wraps std::fs for production use" +)] impl StorageWriteProvider for FileStorageProvider { type Writer = BufWriter; @@ -54,6 +58,10 @@ impl StorageWriteProvider for FileStorageProvider { } #[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "These tests verify FileStorageProvider's functionality with real filesystem operations" +)] mod tests { use std::io::{Read, Seek, SeekFrom, Write}; diff --git a/diskann-providers/src/storage/mod.rs b/diskann-providers/src/storage/mod.rs index 1233b11f6..41532851a 100644 --- a/diskann-providers/src/storage/mod.rs +++ b/diskann-providers/src/storage/mod.rs @@ -20,7 +20,7 @@ pub(crate) mod bin; pub(crate) mod file_storage_provider; // Use VirtualStorageProvider in tests to avoid filesystem side-effects -#[cfg(not(test))] +#[cfg(not(any(test, feature = "testing", feature = "virtual_storage")))] pub use file_storage_provider::FileStorageProvider; mod pq_storage; diff --git a/diskann-providers/src/utils/generate_synthetic_labels_utils.rs b/diskann-providers/src/utils/generate_synthetic_labels_utils.rs index 38720914c..5f26e0b97 100644 --- a/diskann-providers/src/utils/generate_synthetic_labels_utils.rs +++ b/diskann-providers/src/utils/generate_synthetic_labels_utils.rs @@ -80,6 +80,10 @@ impl ZipfDistribution { } } +#[expect( + clippy::disallowed_methods, + reason = "Utility function for generating synthetic label files, needs direct file creation" +)] pub fn generate_labels( output_file: &str, distribution_type: &str, @@ -128,6 +132,10 @@ pub fn generate_labels( } #[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "Test for generate_labels function which creates files, needs cleanup with fs::remove_file" +)] mod test { use std::fs;