From 351a2067765f3f908d68b3f6220776d486d5d5c4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 15 Dec 2025 19:48:30 -0500 Subject: [PATCH 1/2] Fix clippy and track in CI `flake check` will run clippy. --- Cargo.toml | 9 ++++ flake.nix | 2 +- nix-bindings-expr/Cargo.toml | 3 ++ nix-bindings-expr/src/eval_state.rs | 55 ++++++++++++------------ nix-bindings-expr/src/value/__private.rs | 8 ++++ nix-bindings-fetchers/Cargo.toml | 3 ++ nix-bindings-flake/Cargo.toml | 3 ++ nix-bindings-flake/src/lib.rs | 22 +++++----- nix-bindings-store/Cargo.toml | 3 ++ nix-bindings-store/src/store.rs | 2 +- nix-bindings-util/Cargo.toml | 3 ++ nix-bindings-util/src/context.rs | 17 ++++---- nix-bindings-util/src/nix_version.rs | 2 +- 13 files changed, 82 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 02f532b..d90bd3c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,3 +8,12 @@ members = [ "nix-bindings-util", ] resolver = "2" + +[workspace.lints.rust] +warnings = "deny" +dead-code = "allow" + +[workspace.lints.clippy] +type-complexity = "allow" +# We're still trying to make Nix more thread-safe, want forward-compat +arc-with-non-send-sync = "allow" diff --git a/flake.nix b/flake.nix index e323a3c..7646649 100644 --- a/flake.nix +++ b/flake.nix @@ -148,7 +148,7 @@ key = "nix-bindings-rust-add-checks"; config.checks = lib.concatMapAttrs ( k: v: - lib.optionalAttrs (lib.strings.hasPrefix "nix-bindings-" k && !lib.strings.hasSuffix "-clippy" k) { + lib.optionalAttrs (lib.strings.hasPrefix "nix-bindings-" k) { "dependency-${k}" = v; } ) nix-bindings-rust-perSystemConfig.config.checks; diff --git a/nix-bindings-expr/Cargo.toml b/nix-bindings-expr/Cargo.toml index 3c97117..1c1dfe0 100644 --- a/nix-bindings-expr/Cargo.toml +++ b/nix-bindings-expr/Cargo.toml @@ -16,3 +16,6 @@ lazy_static = "1.4" ctor = "0.2" tempfile = "3.10" cstr = "0.2" + +[lints] +workspace = true diff --git a/nix-bindings-expr/src/eval_state.rs b/nix-bindings-expr/src/eval_state.rs index 5f92b92..7d0808c 100644 --- a/nix-bindings-expr/src/eval_state.rs +++ b/nix-bindings-expr/src/eval_state.rs @@ -1382,7 +1382,7 @@ mod tests { let a = es.eval_from_string("2", "").unwrap(); let v = es.new_value_apply(&f, &a).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == None); + assert!(t.is_none()); let i = es.require_int(&v).unwrap(); assert!(i == 3); }) @@ -1398,9 +1398,9 @@ mod tests { let a = es.eval_from_string("true", "").unwrap(); let v = es.new_value_apply(&f, &a).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == None); + assert!(t.is_none()); let i = es.require_bool(&v).unwrap(); - assert!(i == false); + assert!(!i); }) .unwrap(); } @@ -1423,7 +1423,7 @@ mod tests { let mut es = EvalState::new(store, []).unwrap(); let v = make_thunk(&mut es, "1"); let t = es.value_type_unforced(&v); - assert!(t == None); + assert!(t.is_none()); }) .unwrap(); } @@ -1450,7 +1450,7 @@ mod tests { let mut es = EvalState::new(store, []).unwrap(); let v = make_thunk(&mut es, "{ a = 1; b = 2; }"); let t = es.value_type_unforced(&v); - assert!(t == None); + assert!(t.is_none()); let attrs = es.require_attrs_names_unsorted(&v).unwrap(); assert_eq!(attrs.len(), 2); }) @@ -1507,7 +1507,7 @@ mod tests { let s = format!("{e:#}"); if !s.contains("attribute `c` not found") { eprintln!("unexpected error message: {}", s); - assert!(false); + panic!(); } } } @@ -1542,7 +1542,7 @@ mod tests { Err(e) => { if !e.to_string().contains("oh no the error") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } } } @@ -1594,7 +1594,7 @@ mod tests { Err(e) => { if !e.to_string().contains("oh no the error") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } } } @@ -1730,7 +1730,7 @@ mod tests { let t = es.value_type_unforced(&v); assert!(t == Some(ValueType::String)); let s = es.require_string(&v).unwrap(); - assert!(s == ""); + assert!(s.is_empty()); }) .unwrap(); } @@ -1746,7 +1746,7 @@ mod tests { Err(e) => { if !e.to_string().contains("contains null byte") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -1832,7 +1832,7 @@ mod tests { let list: Vec = es.require_list_strict(&v).unwrap(); assert_eq!(list.len(), 2); assert_eq!(es.require_int(&list[0]).unwrap(), 42); - assert_eq!(es.require_bool(&list[1]).unwrap(), true); + assert!(es.require_bool(&list[1]).unwrap()); }) .unwrap(); } @@ -1971,7 +1971,7 @@ mod tests { let f = es.eval_from_string("x: x + 1", "").unwrap(); let a = es.eval_from_string("2", "").unwrap(); let v = es.new_value_apply(&f, &a).unwrap(); - assert!(es.value_type_unforced(&v) == None); + assert!(es.value_type_unforced(&v).is_none()); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); assert!(t == Some(ValueType::Int)); @@ -1994,7 +1994,7 @@ mod tests { Err(e) => { if !e.to_string().contains("cannot coerce") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2017,7 +2017,7 @@ mod tests { Err(e) => { if !e.to_string().contains("expected an integer but found") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2041,7 +2041,7 @@ mod tests { Err(e) => { if !e.to_string().contains("cannot coerce") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2063,7 +2063,7 @@ mod tests { Err(e) => { if !e.to_string().contains("called without required argument") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2086,7 +2086,7 @@ mod tests { Err(e) => { if !e.to_string().contains("called without required argument") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2111,7 +2111,7 @@ mod tests { Err(e) => { if !e.to_string().contains("called without required argument") { eprintln!("{}", e); - assert!(false); + panic!(); } } } @@ -2179,8 +2179,7 @@ mod tests { for derivation in derivations { assert!(store_contents .iter() - .find(|f| f.as_encoded_bytes().ends_with(derivation)) - .is_some()); + .any(|f| f.as_encoded_bytes().ends_with(derivation))); } assert!(!empty(read_dir(state.path()).unwrap())); @@ -2214,7 +2213,7 @@ mod tests { let a = es.require_int(a)?; let b = es.require_int(b)?; let c = *bias.lock().unwrap(); - Ok(es.new_value_int(a + b + c)?) + es.new_value_int(a + b + c) }), ) .unwrap(); @@ -2268,7 +2267,7 @@ mod tests { Err(e) => { if !e.to_string().contains("error with arg [2]") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } } } @@ -2284,7 +2283,7 @@ mod tests { let v = es .new_value_thunk( "test_thunk", - Box::new(move |es: &mut EvalState| Ok(es.new_value_int(42)?)), + Box::new(move |es: &mut EvalState| es.new_value_int(42)), ) .unwrap(); es.force(&v).unwrap(); @@ -2318,11 +2317,11 @@ mod tests { "error message in test case eval_state_primop_anon_call_no_args_lazy", ) { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } if !e.to_string().contains("test_thunk") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } } } @@ -2345,7 +2344,7 @@ mod tests { Box::new(|es, args| { let a = es.require_int(&args[0])?; let b = es.require_int(&args[1])?; - Ok(es.new_value_int(a + b)?) + es.new_value_int(a + b) }), ) .unwrap(); @@ -2385,11 +2384,11 @@ mod tests { Err(e) => { if !e.to_string().contains("The frob unexpectedly fizzled") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } if !e.to_string().contains("frobnicate") { eprintln!("unexpected error message: {}", e); - assert!(false); + panic!(); } } } diff --git a/nix-bindings-expr/src/value/__private.rs b/nix-bindings-expr/src/value/__private.rs index 571dcd0..558bcbc 100644 --- a/nix-bindings-expr/src/value/__private.rs +++ b/nix-bindings-expr/src/value/__private.rs @@ -3,11 +3,19 @@ use super::Value; use nix_bindings_bindgen_raw as raw; /// See [Value::new]. +/// +/// # Safety +/// +/// See underlying function. pub unsafe fn raw_value_new(ptr: *mut raw::Value) -> Value { Value::new(ptr) } /// See [Value::new_borrowed]. +/// +/// # Safety +/// +/// See underlying function. pub unsafe fn raw_value_new_borrowed(ptr: *mut raw::Value) -> Value { Value::new_borrowed(ptr) } diff --git a/nix-bindings-fetchers/Cargo.toml b/nix-bindings-fetchers/Cargo.toml index fa75657..e8b2787 100644 --- a/nix-bindings-fetchers/Cargo.toml +++ b/nix-bindings-fetchers/Cargo.toml @@ -15,3 +15,6 @@ nix-bindings-bindgen-raw = { path = "../nix-bindings-bindgen-raw" } ctor = "0.2" tempfile = "3.10" cstr = "0.2" + +[lints] +workspace = true diff --git a/nix-bindings-flake/Cargo.toml b/nix-bindings-flake/Cargo.toml index 2b70980..67905cb 100644 --- a/nix-bindings-flake/Cargo.toml +++ b/nix-bindings-flake/Cargo.toml @@ -18,3 +18,6 @@ lazy_static = "1.4" ctor = "0.2" tempfile = "3.10" cstr = "0.2" + +[lints] +workspace = true diff --git a/nix-bindings-flake/src/lib.rs b/nix-bindings-flake/src/lib.rs index 91b4bb2..bd531b9 100644 --- a/nix-bindings-flake/src/lib.rs +++ b/nix-bindings-flake/src/lib.rs @@ -138,7 +138,7 @@ impl FlakeReference { }?; let ptr = NonNull::new(ptr) .context("flake_reference_and_fragment_from_string unexpectedly returned null")?; - Ok((FlakeReference { ptr: ptr }, r?)) + Ok((FlakeReference { ptr }, r?)) } } @@ -293,7 +293,7 @@ mod tests { let b = eval_state.require_bool(&v).unwrap(); - assert_eq!(b, true); + assert!(b); drop(gc_registration); } @@ -353,7 +353,7 @@ mod tests { .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "potato"); @@ -394,7 +394,7 @@ mod tests { // a std::fs::write( - &tmp_dir.path().join("a/flake.nix"), + tmp_dir.path().join("a/flake.nix"), r#" { inputs.b.url = "@flake_dir_b@"; @@ -409,7 +409,7 @@ mod tests { // b std::fs::write( - &tmp_dir.path().join("b/flake.nix"), + tmp_dir.path().join("b/flake.nix"), r#" { outputs = { ... }: { @@ -422,7 +422,7 @@ mod tests { // c std::fs::write( - &tmp_dir.path().join("c/flake.nix"), + tmp_dir.path().join("c/flake.nix"), r#" { outputs = { ... }: { @@ -486,7 +486,7 @@ mod tests { .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "BOB"); @@ -527,7 +527,7 @@ mod tests { let outputs = locked_flake .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "BOB"); @@ -547,7 +547,7 @@ mod tests { let outputs = locked_flake .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "BOB"); @@ -581,7 +581,7 @@ mod tests { let outputs = locked_flake .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "Claire"); @@ -604,7 +604,7 @@ mod tests { let outputs = locked_flake .outputs(&flake_settings, &mut eval_state) .unwrap(); - let hello = eval_state.require_attrs_select(&outputs, &"hello").unwrap(); + let hello = eval_state.require_attrs_select(&outputs, "hello").unwrap(); let hello = eval_state.require_string(&hello).unwrap(); assert_eq!(hello, "BOB"); diff --git a/nix-bindings-store/Cargo.toml b/nix-bindings-store/Cargo.toml index c9bc96b..60cf958 100644 --- a/nix-bindings-store/Cargo.toml +++ b/nix-bindings-store/Cargo.toml @@ -22,3 +22,6 @@ tempfile = "3.10" pkg-config = "0.3" # Needed for version parsing in build.rs nix-bindings-util = { path = "../nix-bindings-util" } + +[lints] +workspace = true diff --git a/nix-bindings-store/src/store.rs b/nix-bindings-store/src/store.rs index aff5a66..fa9adad 100644 --- a/nix-bindings-store/src/store.rs +++ b/nix-bindings-store/src/store.rs @@ -514,7 +514,7 @@ mod tests { #[test] fn parse_store_path_fail() { let mut store = crate::store::Store::open(Some("dummy://"), []).unwrap(); - let store_path_string = format!("bash-interactive-5.2p26"); + let store_path_string = "bash-interactive-5.2p26".to_string(); let r = store.parse_store_path(store_path_string.as_str()); match r { Err(e) => { diff --git a/nix-bindings-util/Cargo.toml b/nix-bindings-util/Cargo.toml index a0fda9d..dfa6863 100644 --- a/nix-bindings-util/Cargo.toml +++ b/nix-bindings-util/Cargo.toml @@ -11,3 +11,6 @@ path = "src/lib.rs" anyhow = "1.0" nix-bindings-bindgen-raw = { path = "../nix-bindings-bindgen-raw" } ctor = "0.2" + +[lints] +workspace = true diff --git a/nix-bindings-util/src/context.rs b/nix-bindings-util/src/context.rs index e28ecb8..4999a7b 100644 --- a/nix-bindings-util/src/context.rs +++ b/nix-bindings-util/src/context.rs @@ -1,6 +1,5 @@ use anyhow::{bail, Result}; use nix_bindings_bindgen_raw as raw; -use std::os::raw::c_char; use std::ptr::null_mut; use std::ptr::NonNull; @@ -11,6 +10,12 @@ pub struct Context { inner: NonNull, } +impl Default for Context { + fn default() -> Self { + Self::new() + } +} + impl Context { pub fn new() -> Self { let ctx = unsafe { raw::c_context_create() }; @@ -48,11 +53,7 @@ impl Context { pub fn clear(&mut self) { unsafe { - raw::set_err_msg( - self.inner.as_ptr(), - raw::err_NIX_OK, - b"\0".as_ptr() as *const c_char, - ); + raw::set_err_msg(self.inner.as_ptr(), raw::err_NIX_OK, c"".as_ptr()); } } @@ -143,8 +144,8 @@ mod tests { unsafe { raw::set_err_msg( ctx_ptr, - raw::err_NIX_ERR_UNKNOWN.try_into().unwrap(), - b"dummy error message\0".as_ptr() as *const std::os::raw::c_char, + raw::err_NIX_ERR_UNKNOWN, + c"dummy error message".as_ptr(), ); } } diff --git a/nix-bindings-util/src/nix_version.rs b/nix-bindings-util/src/nix_version.rs index a577261..b6da8ce 100644 --- a/nix-bindings-util/src/nix_version.rs +++ b/nix-bindings-util/src/nix_version.rs @@ -65,7 +65,7 @@ pub fn parse_version(version_str: &str) -> (u32, u32, i32) { let parts = version_str.split('.').collect::>(); let major = parts[0].parse::().unwrap(); let minor = parts[1].parse::().unwrap(); - let patch = if parts.get(2).map_or(false, |s| s.contains("pre")) { + let patch = if parts.get(2).is_some_and(|s| s.contains("pre")) { -1i32 } else { parts From 278a1379e2c9812446f71eb812f44e5d1e95f9f0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 16 Dec 2025 02:05:44 +0100 Subject: [PATCH 2/2] Document safety requirements for __private functions The previous "See underlying function" text didn't provide a way to find the underlying function, forcing users to search the codebase. Stating the safety contracts explicitly makes the API usable. --- nix-bindings-expr/src/value/__private.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/nix-bindings-expr/src/value/__private.rs b/nix-bindings-expr/src/value/__private.rs index 558bcbc..84151e1 100644 --- a/nix-bindings-expr/src/value/__private.rs +++ b/nix-bindings-expr/src/value/__private.rs @@ -2,20 +2,25 @@ use super::Value; use nix_bindings_bindgen_raw as raw; -/// See [Value::new]. +/// Take ownership of a new [`Value`]. +/// +/// This does not call `nix_gc_incref`, but does call `nix_gc_decref` when dropped. /// /// # Safety /// -/// See underlying function. +/// The caller must ensure that the provided `ptr` has a positive reference count, +/// and that `ptr` is not used after the returned `Value` is dropped. pub unsafe fn raw_value_new(ptr: *mut raw::Value) -> Value { Value::new(ptr) } -/// See [Value::new_borrowed]. +/// Borrow a reference to a [`Value`]. +/// +/// This calls `value_incref`, and the returned Value will call `value_decref` when dropped. /// /// # Safety /// -/// See underlying function. +/// The caller must ensure that the provided `ptr` has a positive reference count. pub unsafe fn raw_value_new_borrowed(ptr: *mut raw::Value) -> Value { Value::new_borrowed(ptr) }