fuzztest-rust | Add clippy rules and fix stylistic lints across fuzztest-rust sub-crates - Add rust_clippy targets for fuzztest, coverage, engine, and options sub-crates. - Deny clippy::absolute_paths and unused_imports crate-wide. - Clean up existing lints by importing types at the top level and removing redundant lifetimes and returns. PiperOrigin-RevId: 966089879
diff --git a/rust/BUILD b/rust/BUILD index eb43c11..abbffd1 100644 --- a/rust/BUILD +++ b/rust/BUILD
@@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") +load("@rules_rust//rust:defs.bzl", "rust_clippy", "rust_library", "rust_test") licenses(["notice"]) @@ -57,3 +57,10 @@ "@crate_index//:googletest", ], ) + +rust_clippy( + name = "fuzztest_clippy", + deps = [ + ":fuzztest", + ], +)
diff --git a/rust/coverage/BUILD b/rust/coverage/BUILD index 38f65f7..3344a71 100644 --- a/rust/coverage/BUILD +++ b/rust/coverage/BUILD
@@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") +load("@rules_rust//rust:defs.bzl", "rust_clippy", "rust_library", "rust_test") licenses(["notice"]) @@ -48,3 +48,10 @@ "@crate_index//:googletest", ], ) + +rust_clippy( + name = "coverage_clippy", + deps = [ + ":coverage", + ], +)
diff --git a/rust/engine/BUILD b/rust/engine/BUILD index 8603658..42dc853 100644 --- a/rust/engine/BUILD +++ b/rust/engine/BUILD
@@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_rust//rust:defs.bzl", "rust_library") +load("@rules_rust//rust:defs.bzl", "rust_clippy", "rust_library") licenses(["notice"]) @@ -30,3 +30,10 @@ "@com_google_fuzztest//centipede:engine_worker", ], ) + +rust_clippy( + name = "engine_clippy", + deps = [ + ":engine", + ], +)
diff --git a/rust/engine/src/engine_ffi.rs b/rust/engine/src/engine_ffi.rs index 74badf4..276b3e1 100644 --- a/rust/engine/src/engine_ffi.rs +++ b/rust/engine/src/engine_ffi.rs
@@ -185,10 +185,7 @@ if self.data.is_null() { &[] } else { - ptr::slice_from_raw_parts( - self.data as *const u8, - self.size * core::mem::size_of::<u64>(), - ) + ptr::slice_from_raw_parts(self.data as *const u8, self.size * size_of::<u64>()) } } }
diff --git a/rust/engine/src/lib.rs b/rust/engine/src/lib.rs index 58f9be3..f6e0bf8 100644 --- a/rust/engine/src/lib.rs +++ b/rust/engine/src/lib.rs
@@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#![deny(clippy::absolute_paths)] +#![deny(unused_imports)] + pub mod engine_ffi; use std::marker::PhantomData;
diff --git a/rust/options/BUILD b/rust/options/BUILD index 2fc95f3..b56f9a6 100644 --- a/rust/options/BUILD +++ b/rust/options/BUILD
@@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") +load("@rules_rust//rust:defs.bzl", "rust_clippy", "rust_library", "rust_test") licenses(["notice"]) @@ -42,3 +42,10 @@ "@crate_index//:googletest", ], ) + +rust_clippy( + name = "fuzztest_options_clippy", + deps = [ + ":fuzztest_options", + ], +)
diff --git a/rust/options/src/lib.rs b/rust/options/src/lib.rs index 070b00d..2b4c18f 100644 --- a/rust/options/src/lib.rs +++ b/rust/options/src/lib.rs
@@ -12,8 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -// This module provides the core command-line flag and environment variable options -// structure (`FuzzTestOptions`) and domain execution modes (`ExecutionMode`). +//! This module provides the core command-line flag and environment variable options +//! structure (`FuzzTestOptions`) and domain execution modes (`ExecutionMode`). + +#![deny(clippy::absolute_paths)] +#![deny(unused_imports)] use clap::{Parser, ValueEnum}; use humantime::Duration;
diff --git a/rust/src/domains.rs b/rust/src/domains.rs index 0447752..a873c59 100644 --- a/rust/src/domains.rs +++ b/rust/src/domains.rs
@@ -17,6 +17,8 @@ pub mod range; pub mod tuple_of; pub mod utility; +use ::serde::de::DeserializeOwned; +use ::serde::Serialize; use anyhow; use anyhow::Context; @@ -118,7 +120,7 @@ /// the CorpusValue could the owned data structured that the `&str` points to (eg: String). /// The CorpusValue type should implement `serde::Serialize`, `serde::de::DeserializeOwned` and /// `Clone`. - type CorpusValue: ::serde::Serialize + ::serde::de::DeserializeOwned + Clone; + type CorpusValue: Serialize + DeserializeOwned + Clone; /// Initializes a new value drawn from the domain. fn init(&self, rng: &mut dyn rand::Rng) -> anyhow::Result<Self::CorpusValue>;
diff --git a/rust/src/domains/arbitrary.rs b/rust/src/domains/arbitrary.rs index 3ad8bdb..33abb2b 100644 --- a/rust/src/domains/arbitrary.rs +++ b/rust/src/domains/arbitrary.rs
@@ -16,6 +16,8 @@ use super::utility::mutate_integer; use super::utility::shrink_towards; use super::Domain; +use std::char; +use std::marker::PhantomData; use anyhow; use rand::RngExt; @@ -39,9 +41,8 @@ /// let sample = arbitrary_i32.init(&mut rng); /// assert!(sample.is_ok()); /// ``` - pub struct Arbitrary<T> { - _phantom: std::marker::PhantomData<T>, + _phantom: PhantomData<T>, } impl<T> Clone for Arbitrary<T> { @@ -59,14 +60,14 @@ // We cannot just use `#[derive(Default)]` because `T` might not be `Default`. impl<T> Default for Arbitrary<T> { fn default() -> Self { - Self { _phantom: std::marker::PhantomData } + Self { _phantom: PhantomData } } } impl<T> Arbitrary<T> { /// Creates a new `Arbitrary` domain for the given type `T`. pub fn new() -> Self { - Self { _phantom: std::marker::PhantomData } + Self { _phantom: PhantomData } } } @@ -254,7 +255,7 @@ NUM_VALID_CODEPOINTS ); let val = if u >= SURROGATE_START { u + (SURROGATE_END - SURROGATE_START + 1) } else { u }; - std::char::from_u32(val).unwrap() + char::from_u32(val).unwrap() } impl Domain for Arbitrary<char> { @@ -745,19 +746,13 @@ assert_eq!(map_int_to_char(0), '\u{0000}'); let before_surrogate = SURROGATE_START - 1; - assert_eq!( - map_char_to_int(std::char::from_u32(before_surrogate).unwrap()), - before_surrogate - ); - assert_eq!( - map_int_to_char(before_surrogate), - std::char::from_u32(before_surrogate).unwrap() - ); + assert_eq!(map_char_to_int(char::from_u32(before_surrogate).unwrap()), before_surrogate); + assert_eq!(map_int_to_char(before_surrogate), char::from_u32(before_surrogate).unwrap()); let after_surrogate = SURROGATE_END + 1; - let mapped_after_surrogate = map_char_to_int(std::char::from_u32(after_surrogate).unwrap()); + let mapped_after_surrogate = map_char_to_int(char::from_u32(after_surrogate).unwrap()); assert_eq!(mapped_after_surrogate, SURROGATE_START); - assert_eq!(map_int_to_char(SURROGATE_START), std::char::from_u32(after_surrogate).unwrap()); + assert_eq!(map_int_to_char(SURROGATE_START), char::from_u32(after_surrogate).unwrap()); assert_eq!(map_char_to_int('\u{10FFFF}'), NUM_VALID_CODEPOINTS - 1); assert_eq!(map_int_to_char(NUM_VALID_CODEPOINTS - 1), '\u{10FFFF}'); @@ -776,7 +771,7 @@ while value != '\0' && iterations < MAX_ITERATIONS { domain.mutate(&mut value, &mut rng, true).unwrap(); // Ensure that the value is always a valid char after mutation. - assert!(std::char::from_u32(value as u32).is_some()); + assert!(char::from_u32(value as u32).is_some()); iterations += 1; } assert_eq!(
diff --git a/rust/src/domains/utility.rs b/rust/src/domains/utility.rs index 2ce15fa..e14bcef 100644 --- a/rust/src/domains/utility.rs +++ b/rust/src/domains/utility.rs
@@ -18,6 +18,7 @@ use rand::distr::uniform::SampleUniform; use rand::distr::{Distribution, StandardUniform}; use rand::RngExt; +use std::fmt::Display; /// Shrinks a `val` towards a `target` value. /// @@ -32,7 +33,7 @@ /// * `target`: The value to shrink towards. pub fn shrink_towards<T, R: rand::Rng + ?Sized>(rng: &mut R, val: T, target: T) -> T where - T: SampleUniform + PartialOrd + Copy + std::fmt::Display, + T: SampleUniform + PartialOrd + Copy + Display, { match val.partial_cmp(&target) { Some(Ordering::Equal) => val, @@ -81,7 +82,7 @@ max_value: Option<T>, ) -> T where - T: PrimInt + SampleUniform + std::fmt::Display, + T: PrimInt + SampleUniform + Display, { assert!(range > T::zero(), "mutate_integer: range value cannot be <= 0: {range}"); @@ -106,7 +107,7 @@ } 1 => { // 1/3 chance: Flip a random bit - let num_bits = std::mem::size_of::<T>() * 8; + let num_bits = size_of::<T>() * 8; let bit_index = rng.random_range(0..num_bits); let mask = T::one() << bit_index; let result = val ^ mask; @@ -194,7 +195,7 @@ /// /// # Type Parameters /// * `T`: The type of the value to choose. Must implement `SpecialValues` and -/// `StandardUniform` must be able to generate values of type `T`. +/// `StandardUniform` must be able to generate values of type `T`. pub fn choose_value<T, R: rand::Rng + ?Sized>(rng: &mut R) -> T where T: SpecialValues + 'static, @@ -217,7 +218,7 @@ _range: Option<(T, T)>, // TODO: Implement range support for floats. ) -> anyhow::Result<()> where - T: num_traits::Float + SampleUniform + std::fmt::Display + Copy + SpecialValues + 'static, + T: num_traits::Float + SampleUniform + Display + Copy + SpecialValues + 'static, StandardUniform: Distribution<T>, { if only_shrink { @@ -259,6 +260,7 @@ rngs::{SmallRng, SysRng}, SeedableRng, }; + use std::fmt::Debug; fn get_rng() -> SmallRng { SmallRng::try_from_rng(&mut SysRng).unwrap() @@ -266,7 +268,7 @@ fn check_shrink_towards<T>(smaller: T, larger: T) where - T: SampleUniform + PartialOrd + Copy + std::fmt::Display + std::fmt::Debug + PartialEq, + T: SampleUniform + PartialOrd + Copy + Display + Debug + PartialEq, { let mut rng = get_rng(); @@ -317,7 +319,7 @@ fn check_mutate_integer<T>() where - T: PrimInt + SampleUniform + std::fmt::Display + std::fmt::Debug + SpecialValues + 'static, + T: PrimInt + SampleUniform + Display + Debug + SpecialValues + 'static, StandardUniform: Distribution<T>, { let mut rng = get_rng(); @@ -369,13 +371,7 @@ fn check_mutate_float<T>() where - T: num_traits::Float - + SampleUniform - + std::fmt::Display - + std::fmt::Debug - + Copy - + SpecialValues - + 'static, + T: num_traits::Float + SampleUniform + Display + Debug + Copy + SpecialValues + 'static, StandardUniform: Distribution<T>, { let mut rng = get_rng();
diff --git a/rust/src/internal.rs b/rust/src/internal.rs index c83c157..213e66a 100644 --- a/rust/src/internal.rs +++ b/rust/src/internal.rs
@@ -33,7 +33,7 @@ /// (will attempt to downcast to actual user values). /// /// Returns `true` if the property function holds, `false` if it crashes. - fn execute<'a>(&self, args: &'a GenericCorpusValue) -> bool; + fn execute(&self, args: &GenericCorpusValue) -> bool; fn print_finding_report(&self); fn domains(&self) -> &dyn GenericDomain; } @@ -57,6 +57,7 @@ inventory::collect!(FuzzTestRegistration); +#[allow(clippy::type_complexity)] pub static FUZZ_TEST_NAME_TO_FACTORY: LazyLock<HashMap<&str, fn() -> BoxedFuzzTest>> = LazyLock::new(|| { inventory::iter
diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 6a8d7a1..bc663ef 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs
@@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#![deny(clippy::absolute_paths)] +#![deny(unused_imports)] #![feature(cfg_sanitize)] mod crash_handler;
diff --git a/rust/src/options.rs b/rust/src/options.rs index 0d79843..5b659da 100644 --- a/rust/src/options.rs +++ b/rust/src/options.rs
@@ -16,8 +16,10 @@ use ::engine::engine_ffi; use anyhow::Context; use clap::Parser; +use std::env; use std::ffi::CString; use std::ffi::OsString; +use std::iter; use std::path::Path; use std::sync::OnceLock; use tempfile::{NamedTempFile, TempDir}; @@ -34,7 +36,7 @@ // from environment variables (like `FUZZTEST_FUZZ_FOR` etc.). We (currently) do not envisage // support for passing flags on cli as the Rust's libtest harness does not support custom // flags. - OPTIONS.get_or_init(|| FuzzTestOptions::parse_from(std::iter::empty::<OsString>())) + OPTIONS.get_or_init(|| FuzzTestOptions::parse_from(iter::empty::<OsString>())) } trait ExecutionModeExt { @@ -155,7 +157,7 @@ // ============================================================================== // 1. Common Base Arguments (Required across all Centipede executions) // ============================================================================== - let argv0 = std::env::args().next().context("while attempting to get argv[0]")?; + let argv0 = env::args().next().context("while attempting to get argv[0]")?; add_arg(format!("--binary={argv0} {current_test_name} --exact --nocapture"))?; let normalized_test_name = current_test_name.replace("::", ".");
diff --git a/rust/src/worker.rs b/rust/src/worker.rs index 20ac83f..8eae57f 100644 --- a/rust/src/worker.rs +++ b/rust/src/worker.rs
@@ -20,10 +20,16 @@ BytesSink, CoverageDomainRegistry, DiagnosticSink, ExecuteContext, FeedbackSink, InputSink, }; use spin::Mutex; +use std::env; +use std::ffi::c_int; use std::ffi::CString; +use std::fs; use std::path::Path; +use std::process; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::LazyLock; +use std::time::Duration; +use std::time::Instant; /// The DiagnosticSink provided by the engine while creating the adapter. /// @@ -59,7 +65,9 @@ /// This function blocks till it can get the lock on the global DiagnosticSink and is not /// signal-safe. pub(crate) fn emit_error(message: &str) { - DIAGNOSTIC_SINK.lock().as_ref().map(|sink| sink.emit_error(message)); + if let Some(sink) = DIAGNOSTIC_SINK.lock().as_ref() { + sink.emit_error(message) + } } /// Emits a finding into the DiagnosticSink if the DiagnosticSink is set. @@ -99,7 +107,7 @@ return false; }; sink.emit_finding(token, description, signature); - return true; + true } // We double-box the input because `GenericCorpusValue` is a fat pointer (`Box<dyn Any>`), @@ -252,7 +260,7 @@ pub fn get_binary_id(&self, sink: &mut BytesSink) { static ARGV0: LazyLock<CString> = LazyLock::new(|| { CString::new( - Path::new(&std::env::args().nth(0).unwrap()) + Path::new(&env::args().next().unwrap()) .file_name() .and_then(|f| f.to_str()) .unwrap_or(""), @@ -428,7 +436,7 @@ pub unsafe extern "C" fn mutate_callback( ctx: *mut engine_ffi::FuzzTestAdapterCtx, origin: engine_ffi::FuzzTestInputHandle, - shrink: std::ffi::c_int, + shrink: c_int, sink: *const engine_ffi::FuzzTestInputSink, ) { // SAFETY: The engine guarantees `ctx` is a valid pointer to the `RustFuzzTestAdapter` @@ -620,10 +628,10 @@ } pub fn run_smoke_test(fuzztest: &dyn FuzzTest) { - let start_time = std::time::Instant::now(); + let start_time = Instant::now(); // TODO(the-shank): these should be configurable externally. - let smoke_test_duration = std::time::Duration::from_secs(1); + let smoke_test_duration = Duration::from_secs(1); let only_shrink = false; // TODO(the-shank): the rng seed should be configurable @@ -681,7 +689,7 @@ return; } WorkerStatus::Failure => { - std::process::exit(1); + process::exit(1); } } } @@ -701,7 +709,7 @@ } // Now read the file and replay each crash - if let Ok(contents) = std::fs::read_to_string(list_file.path()) { + if let Ok(contents) = fs::read_to_string(list_file.path()) { let options = options::get_fuzztest_options(); for crash_id in contents.lines() { let crash_id = crash_id.trim();