From b89c2b892650520a7a4a8f4864f5dcfc3bc157b5 Mon Sep 17 00:00:00 2001 From: Malix - Alix Brunet Date: Wed, 17 Jun 2026 04:11:03 +0200 Subject: [PATCH] refactor: DRY flake metadata extraction and ref-statuses loading --- parse-flake-lock/src/lib.rs | 46 ++++++++++++++++++++++++++++++++----- src/condition.rs | 32 ++++++++------------------ src/flake.rs | 40 +++++++++----------------------- src/main.rs | 11 +++++---- src/ref_statuses.rs | 2 +- src/summary.rs | 26 ++++++--------------- 6 files changed, 74 insertions(+), 83 deletions(-) diff --git a/parse-flake-lock/src/lib.rs b/parse-flake-lock/src/lib.rs index 96963c2..437b44c 100644 --- a/parse-flake-lock/src/lib.rs +++ b/parse-flake-lock/src/lib.rs @@ -114,10 +114,7 @@ impl<'de> Deserialize<'de> for FlakeLock { }; for (root_name, root_input) in root_node.inputs.iter() { - let inputs: VecDeque = match root_input.clone() { - Input::String(s) => [s].into(), - Input::List(keys) => keys.into(), - }; + let inputs: VecDeque = root_input.to_deque(); let real_node = chase_input_node(&nodes, inputs).map_err(|e| { de::Error::custom(format!("failed to chase input {}: {:?}", root_name, e)) @@ -173,7 +170,7 @@ fn chase_input_node( let next_inputs = &node_inputs[&input]; node = match next_inputs { Input::String(s) => &nodes[s], - Input::List(inputs) => chase_input_node(nodes, inputs.to_owned().into())?, + Input::List(_) => chase_input_node(nodes, next_inputs.to_deque())?, }; } @@ -214,7 +211,6 @@ pub enum Node { Fallthrough(serde_json::value::Value), // Covers all other node types } -// A string representation of the node variant (for logging). impl Node { fn variant(&self) -> &'static str { match self { @@ -226,6 +222,34 @@ impl Node { Node::Fallthrough(_) => "Fallthrough", // Covers all other node types } } + + /// Returns the Git reference if this is a repository node. + pub fn git_ref(&self) -> Option<&str> { + match self { + Node::Repo(repo) => repo.original.git_ref.as_deref(), + _ => None, + } + } + + /// Returns the last modified timestamp if available. + pub fn last_modified(&self) -> Option { + match self { + Node::Repo(repo) => Some(repo.locked.last_modified), + Node::Tarball(tarball) => tarball.locked.last_modified, + Node::Path(path) => Some(path.locked.last_modified), + Node::Indirect(indirect) => Some(indirect.locked.last_modified), + _ => None, + } + } + + /// Returns the repository owner if available. + pub fn owner(&self) -> Option<&str> { + match self { + Node::Repo(repo) => Some(&repo.original.owner), + Node::Indirect(indirect) => Some(&indirect.locked.owner), + _ => None, + } + } } /// An enum type representing node input references. @@ -238,6 +262,16 @@ pub enum Input { List(Vec), } +impl Input { + /// Returns the input as a list of keys (VecDeque). + pub fn to_deque(&self) -> VecDeque { + match self { + Input::String(s) => vec![s.clone()].into(), + Input::List(keys) => keys.clone().into(), + } + } +} + /// A flake [Node] representing a raw mapping of strings to [Input]s. #[derive(Clone, Debug, Deserialize)] #[serde(deny_unknown_fields)] diff --git a/src/condition.rs b/src/condition.rs index c79542f..b31fe39 100644 --- a/src/condition.rs +++ b/src/condition.rs @@ -1,5 +1,5 @@ use cel::{Context, Program, Value}; -use parse_flake_lock::{FlakeLock, Node}; +use parse_flake_lock::FlakeLock; use std::collections::{BTreeMap, HashMap}; @@ -34,15 +34,9 @@ pub(super) fn evaluate_condition( let deps = nixpkgs_deps(flake_lock, nixpkgs_keys)?; for (name, node) in deps { - let (git_ref, last_modified, owner) = match node { - Node::Repo(repo) => ( - repo.original.git_ref, - Some(repo.locked.last_modified), - Some(repo.original.owner), - ), - Node::Tarball(tarball) => (None, tarball.locked.last_modified, None), - _ => (None, None, None), - }; + let git_ref = node.git_ref(); + let last_modified = node.last_modified(); + let owner = node.owner(); add_cel_variables(&mut ctx, git_ref, last_modified, owner); @@ -70,22 +64,14 @@ pub(super) fn evaluate_condition( fn add_cel_variables( ctx: &mut Context, - git_ref: Option, + git_ref: Option<&str>, last_modified: Option, - owner: Option, + owner: Option<&str>, ) { - ctx.add_variable_from_value(KEY_GIT_REF, value_or_empty_string(git_ref)); + ctx.add_variable_from_value(KEY_GIT_REF, Value::from(git_ref.unwrap_or(""))); ctx.add_variable_from_value( KEY_NUM_DAYS_OLD, - value_or_zero(last_modified.map(num_days_old)), + Value::from(last_modified.map(num_days_old).unwrap_or(0)), ); - ctx.add_variable_from_value(KEY_OWNER, value_or_empty_string(owner)); -} - -fn value_or_empty_string(value: Option) -> Value { - Value::from(value.unwrap_or(String::from(""))) -} - -fn value_or_zero(value: Option) -> Value { - Value::from(value.unwrap_or(0)) + ctx.add_variable_from_value(KEY_OWNER, Value::from(owner.unwrap_or(""))); } diff --git a/src/flake.rs b/src/flake.rs index 53310c3..c80d3d8 100644 --- a/src/flake.rs +++ b/src/flake.rs @@ -38,12 +38,7 @@ pub(super) fn nixpkgs_deps( for (ref key, node) in flake_lock.root.clone() { match &node { - Node::Repo(_) => { - if keys.contains(key) { - deps.insert(key.to_string(), node); - } - } - Node::Tarball(_) => { + Node::Repo(_) | Node::Tarball(_) => { if keys.contains(key) { deps.insert(key.to_string(), node); } @@ -86,20 +81,14 @@ pub(crate) fn check_flake_lock( let deps = nixpkgs_deps(flake_lock, &config.nixpkgs_keys)?; for (name, node) in deps { - let (git_ref, last_modified, owner) = match node { - Node::Repo(repo) => ( - repo.original.git_ref, - Some(repo.locked.last_modified), - Some(repo.original.owner), - ), - Node::Tarball(tarball) => (None, tarball.locked.last_modified, None), - _ => (None, None, None), - }; + let git_ref = node.git_ref(); + let last_modified = node.last_modified(); + let owner = node.owner(); // Check if not explicitly supported if let Some(git_ref) = git_ref { // Check if not explicitly supported - if config.check_supported && !allowed_refs.contains(&git_ref) { + if config.check_supported && !allowed_refs.iter().any(|r| r == git_ref) { issues.push(Issue { input: name.clone(), kind: IssueKind::Disallowed(Disallowed { @@ -128,7 +117,7 @@ pub(crate) fn check_flake_lock( if config.check_owner && owner.to_lowercase() != "nixos" { issues.push(Issue { input: name.clone(), - kind: IssueKind::NonUpstream(NonUpstream { owner }), + kind: IssueKind::NonUpstream(NonUpstream { owner: owner.to_string() }), }); } } @@ -144,7 +133,6 @@ pub(super) fn num_days_old(timestamp: i64) -> i64 { #[cfg(test)] mod test { - use std::collections::BTreeMap; use std::path::PathBuf; use crate::{ @@ -160,11 +148,9 @@ mod test { let cases: Vec<(&str, bool)> = vec![ (include_str!("../tests/cel-condition.cel"), true), ("supportedRefs.contains(gitRef) && owner != 'NixOS'", false), - ("supportedRefs.contains(gitRef) && owner != 'NixOS'", false), ]; - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = crate::local_ref_statuses(); let supported_refs = supported_refs(ref_statuses.clone()); let path = PathBuf::from("tests/flake.cel.0.lock"); @@ -196,8 +182,7 @@ mod test { #[test] fn clean_flake_locks() { - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = crate::local_ref_statuses(); let allowed_refs = supported_refs(ref_statuses); for n in 0..=7 { let path = PathBuf::from(format!("tests/flake.clean.{n}.lock")); @@ -217,8 +202,7 @@ mod test { #[test] fn dirty_flake_locks() { - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = crate::local_ref_statuses(); let allowed_refs = supported_refs(ref_statuses); let cases: Vec<(&str, Vec)> = vec![ ( @@ -272,8 +256,7 @@ mod test { #[test] fn explicit_nixpkgs_keys() { - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = crate::local_ref_statuses(); let allowed_refs = supported_refs(ref_statuses); let cases: Vec<(&str, Vec, Vec)> = vec![( "flake.explicit-keys.0.lock", @@ -301,8 +284,7 @@ mod test { #[test] fn missing_nixpkgs_keys() { - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = crate::local_ref_statuses(); let allowed_refs = supported_refs(ref_statuses); let cases: Vec<(&str, Vec, String)> = vec![ ( diff --git a/src/main.rs b/src/main.rs index 4cedde7..96c64f9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -96,7 +96,10 @@ struct Cli { condition: Option, } -#[cfg(not(feature = "ref-statuses"))] +pub(crate) fn local_ref_statuses() -> BTreeMap { + serde_json::from_str(include_str!("../ref-statuses.json")).unwrap() +} + pub(crate) fn supported_refs(ref_statuses: BTreeMap) -> Vec { let mut return_value: Vec = ref_statuses .iter() @@ -120,8 +123,7 @@ async fn main() -> Result { .with(EnvFilter::from_default_env()) .init(); - let ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = local_ref_statuses(); let Cli { no_telemetry, @@ -279,8 +281,7 @@ fn main() -> Result { } if check_ref_statuses { - let mut ref_statuses: BTreeMap = - serde_json::from_str(include_str!("../ref-statuses.json")).unwrap(); + let ref_statuses = local_ref_statuses(); match ref_statuses::check_ref_statuses(ref_statuses) { Ok(equals) => { diff --git a/src/ref_statuses.rs b/src/ref_statuses.rs index f463c43..3a57b4e 100644 --- a/src/ref_statuses.rs +++ b/src/ref_statuses.rs @@ -34,7 +34,7 @@ pub(crate) fn check_ref_statuses( } pub(crate) fn fetch_ref_statuses() -> Result, FlakeCheckerError> { - let mut officially_supported: BTreeMap = + let officially_supported: BTreeMap = reqwest::blocking::get(ALLOWED_REFS_URL)? .json::()? .data diff --git a/src/summary.rs b/src/summary.rs index 8ce8fa5..254ac0f 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -10,25 +10,13 @@ use std::path::PathBuf; use handlebars::Handlebars; use serde_json::json; -static CEL_MARKDOWN_TEMPLATE: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/templates/summary.cel.md.hbs" -)); - -static CEL_TEXT_TEMPLATE: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/templates/summary.cel.txt.hbs" -)); - -static STANDARD_MARKDOWN_TEMPLATE: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/templates/summary.standard.md.hbs" -)); - -static STANDARD_TEXT_TEMPLATE: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/templates/summary.standard.txt.hbs" -)); +static CEL_MARKDOWN_TEMPLATE: &str = include_str!("templates/summary.cel.md.hbs"); + +static CEL_TEXT_TEMPLATE: &str = include_str!("templates/summary.cel.txt.hbs"); + +static STANDARD_MARKDOWN_TEMPLATE: &str = include_str!("templates/summary.standard.md.hbs"); + +static STANDARD_TEXT_TEMPLATE: &str = include_str!("templates/summary.standard.txt.hbs"); pub(crate) struct Summary { pub issues: Vec,