Skip to content
Draft
13 changes: 10 additions & 3 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -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."},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,6 @@ where
mod tests {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot,
ensure fmt, clippy
cargo clippy --locked --workspace --all-targets --no-deps
also ensure that all tests in modified files pass

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified in commit fcc385e:

  • cargo fmt --check --all passes
  • cargo clippy --locked --workspace --all-targets --no-deps passes (no warnings in our modified files)
  • ✅ All tests in modified files pass:
    • storage::file_storage_provider::tests (4/4 tests passed)
    • utils::generate_synthetic_labels_utils::test (1/1 test passed)

Fixed clippy warning by adding #[expect] attribute to generate_synthetic_labels_utils test module.

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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -2621,7 +2621,7 @@ mod tests {
);
}

let storage = FileStorageProvider;
let storage = VirtualStorageProvider::new_overlay(temp_path);

provider.save_with(&storage, &prefix).await.unwrap();

Expand Down
8 changes: 8 additions & 0 deletions diskann-providers/src/storage/file_storage_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<File>;

Expand All @@ -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};

Expand Down
2 changes: 1 addition & 1 deletion diskann-providers/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down