From ed364727257f5b87fd2c1e2400a8ad3d2d2338ed Mon Sep 17 00:00:00 2001 From: Nick Spinale Date: Wed, 25 Oct 2023 07:52:24 +0000 Subject: [PATCH] Clean up Cargo manifest management --- ...rgo_policy.rs => cargo_manifest_policy.rs} | 10 +- .../cargo-manifest-management/src/format.rs | 189 ++++++++++-------- hacking/cargo-manifest-management/src/main.rs | 10 +- hacking/cargo-manifest-management/src/plan.rs | 25 +-- hacking/cargo-manifest-management/tool.nix | 7 +- 5 files changed, 131 insertions(+), 110 deletions(-) rename hacking/cargo-manifest-management/src/{cargo_policy.rs => cargo_manifest_policy.rs} (90%) diff --git a/hacking/cargo-manifest-management/src/cargo_policy.rs b/hacking/cargo-manifest-management/src/cargo_manifest_policy.rs similarity index 90% rename from hacking/cargo-manifest-management/src/cargo_policy.rs rename to hacking/cargo-manifest-management/src/cargo_manifest_policy.rs index a8d6de4a9..42e107453 100644 --- a/hacking/cargo-manifest-management/src/cargo_policy.rs +++ b/hacking/cargo-manifest-management/src/cargo_manifest_policy.rs @@ -8,10 +8,10 @@ use std::cmp::{Ordering, Reverse}; use crate::{PathSegment, Policy}; -pub struct CargoPolicy; +pub struct CargoManifestPolicy; -impl Policy for CargoPolicy { - fn compare_keys(path: &[PathSegment], a: &str, b: &str) -> Ordering { +impl Policy for CargoManifestPolicy { + fn compare_keys(&self, path: &[PathSegment], a: &str, b: &str) -> Ordering { let ranking = if path.len() == 0 { Ranking { front: &[ @@ -72,13 +72,13 @@ impl Policy for CargoPolicy { ranking.compare(a, b) } - fn is_always_table(path: &[PathSegment]) -> bool { + fn is_always_table(&self, path: &[PathSegment]) -> bool { path.len() <= 1 || (path.len() <= 3 && path[0].is_table_key("target")) || (path.len() <= 3 && path[0].is_table_key("profile")) } - fn is_always_array_of_tables(path: &[PathSegment]) -> bool { + fn is_always_array_of_tables(&self, path: &[PathSegment]) -> bool { path.len() == 2 && path[1].is_table_key("bin") } } diff --git a/hacking/cargo-manifest-management/src/format.rs b/hacking/cargo-manifest-management/src/format.rs index 378373f00..3a2f75a5b 100644 --- a/hacking/cargo-manifest-management/src/format.rs +++ b/hacking/cargo-manifest-management/src/format.rs @@ -6,7 +6,6 @@ use std::cmp::Ordering; use std::collections::BTreeMap; -use std::marker::PhantomData; use std::mem; use either::Either; @@ -16,11 +15,11 @@ use toml_edit::{Array, ArrayOfTables, Document, Formatted, InlineTable, Item, Ta const MAX_WIDTH: usize = 100; pub trait Policy { - fn compare_keys(path: &[PathSegment], a: &str, b: &str) -> Ordering; + fn compare_keys(&self, path: &[PathSegment], a: &str, b: &str) -> Ordering; - fn is_always_table(path: &[PathSegment]) -> bool; + fn is_always_table(&self, path: &[PathSegment]) -> bool; - fn is_always_array_of_tables(path: &[PathSegment]) -> bool; + fn is_always_array_of_tables(&self, path: &[PathSegment]) -> bool; } pub enum PathSegment { @@ -41,27 +40,33 @@ impl PathSegment { } } -pub fn format(manifest: &JsonValue) -> Document { - let mut state = ValueTranslation { - current_path: vec![], - _phantom: PhantomData::

, - }; - let item = state.translate(manifest).into_item(); - let mut doc = Document::new(); - let _ = mem::replace(doc.as_item_mut(), item); - doc +pub struct Formatter

{ + policy: P, } -struct ValueTranslation

{ - current_path: Vec, - _phantom: PhantomData

, -} +impl Formatter

{ + pub fn new(policy: P) -> Self { + Self { policy } + } -impl ValueTranslation

{ - fn current_key(&self) -> Option<&str> { - self.current_path.last()?.as_table_key() + pub fn format(&self, manifest: &JsonValue) -> Document { + let mut state = FormatterState { + formatter: self, + current_path: vec![], + }; + let item = state.translate(manifest).into_item(); + let mut doc = Document::new(); + let _ = mem::replace(doc.as_item_mut(), item); + doc } +} + +struct FormatterState<'a, P> { + formatter: &'a Formatter

, + current_path: Vec, +} +impl<'a, P: Policy> FormatterState<'a, P> { fn translate(&mut self, v: &JsonValue) -> Translated { match v { JsonValue::Bool(w) => Value::Boolean(Formatted::new(*w)).into(), @@ -77,22 +82,18 @@ impl ValueTranslation

{ JsonValue::Array(w) => { let mut children = vec![]; for (i, x) in w.iter().enumerate() { - self.current_path.push(PathSegment::ArrayIndex(i)); + self.push(PathSegment::ArrayIndex(i)); children.push(self.translate(x)); - self.current_path.pop(); + self.pop(); } - let homogenous_children = match TranslatedManyArray::homogenize(&children) { - TranslatedManyArray::Values(values) - if P::is_always_array_of_tables(&self.current_path) => - { - TranslatedManyArray::Tables(TranslatedManyArray::tables_from_values( - &values, - )) - } - x => x, - }; + let homogenous_children = + if self.policy().is_always_array_of_tables(self.current_path()) { + TranslatedArray::Tables(TranslatedArray::tables_from_translated(&children)) + } else { + TranslatedArray::homogenize(&children) + }; match homogenous_children { - TranslatedManyArray::Values(values) => { + TranslatedArray::Values(values) => Translated::Value(Value::Array({ let mut array = Array::new(); for v in values.into_iter() { array.push(v); @@ -109,71 +110,85 @@ impl ValueTranslation

{ e.decor_mut().set_suffix("\n"); }); } - Translated::Value(Value::Array(array)) - } - TranslatedManyArray::Tables(tables) => { + array + })), + TranslatedArray::Tables(tables) => Translated::Item(Item::ArrayOfTables({ let mut array = ArrayOfTables::new(); for v in tables.into_iter() { array.push(v); } - Translated::Item(Item::ArrayOfTables(array)) - } + array + })), } } JsonValue::Object(w) => { let mut children = BTreeMap::new(); for (k, x) in w.iter() { - self.current_path.push(PathSegment::TableKey(k.clone())); + self.push(PathSegment::TableKey(k.clone())); children.insert(k.clone(), self.translate(x)); - self.current_path.pop(); + self.pop(); } - let homogenous_children = match TranslatedManyMap::homogenize(&children) { - TranslatedManyMap::Values(values) if P::is_always_table(&self.current_path) => { - TranslatedManyMap::Items(TranslatedManyMap::items_from_values(values)) - } - x => x, + let homogenous_children = if self.policy().is_always_table(self.current_path()) { + TranslatedMap::Items(TranslatedMap::items_from_translated(&children)) + } else { + TranslatedMap::homogenize(&children) }; - let inline_table_or_items = match homogenous_children { - TranslatedManyMap::Values(values) => { + match homogenous_children { + TranslatedMap::Values(values) => { let mut table = InlineTable::new(); for (k, v) in values.clone().into_iter() { table.insert(k, v); } table.sort_values_by(|k1, _v1, k2, _v2| { - P::compare_keys(&self.current_path, k1, k2) + self.policy().compare_keys(self.current_path(), k1, k2) }); if !self .current_key() .map(|k| is_kv_too_long(k, &Value::InlineTable(table.clone()))) .unwrap_or(false) { - Either::Left(table) + Either::Left(Translated::Value(Value::InlineTable(table))) } else { - Either::Right(TranslatedManyMap::items_from_values(values)) - } - } - TranslatedManyMap::Items(items) => Either::Right(items), - }; - match inline_table_or_items { - Either::Left(inline_table) => { - Translated::Value(Value::InlineTable(inline_table)) - } - Either::Right(items) => { - let mut table = Table::new(); - table.set_implicit(true); - for (k, v) in items.into_iter() { - table.insert(&k, v); + Either::Right(TranslatedMap::items_from_values(values)) } - table.sort_values_by(|k1, _v1, k2, _v2| { - P::compare_keys(&self.current_path, k1, k2) - }); - Translated::Item(Item::Table(table)) } + TranslatedMap::Items(items) => Either::Right(items), } + .map_right(|items| { + let mut table = Table::new(); + table.set_implicit(true); + for (k, v) in items.into_iter() { + table.insert(&k, v); + } + table.sort_values_by(|k1, _v1, k2, _v2| { + self.policy().compare_keys(self.current_path(), k1, k2) + }); + Translated::Item(Item::Table(table)) + }) + .into_inner() } _ => panic!(), } } + + fn policy(&self) -> &P { + &self.formatter.policy + } + + fn current_path(&self) -> &[PathSegment] { + &self.current_path + } + + fn current_key(&self) -> Option<&str> { + self.current_path.last()?.as_table_key() + } + fn push(&mut self, seg: PathSegment) { + self.current_path.push(seg); + } + + fn pop(&mut self) { + self.current_path.pop(); + } } #[derive(Clone)] @@ -214,30 +229,32 @@ impl Translated { } } -enum TranslatedManyMap { +enum TranslatedMap { Items(BTreeMap), Values(BTreeMap), } -impl TranslatedManyMap { +impl TranslatedMap { fn homogenize(heterogeneous: &BTreeMap) -> Self { if heterogeneous.values().all(Translated::is_value) { - TranslatedManyMap::Values( + Self::Values( heterogeneous .iter() .map(|(k, v)| (k.clone(), v.as_value().unwrap().clone())) .collect(), ) } else { - TranslatedManyMap::Items( - heterogeneous - .iter() - .map(|(k, v)| (k.clone(), v.clone().into_item())) - .collect(), - ) + Self::Items(Self::items_from_translated(heterogeneous)) } } + fn items_from_translated(translated: &BTreeMap) -> BTreeMap { + translated + .iter() + .map(|(k, v)| (k.clone(), v.clone().into_item())) + .collect() + } + fn items_from_values(values: BTreeMap) -> BTreeMap { values .into_iter() @@ -246,30 +263,32 @@ impl TranslatedManyMap { } } -enum TranslatedManyArray { +enum TranslatedArray { Tables(Vec), Values(Vec), } -impl TranslatedManyArray { +impl TranslatedArray { fn homogenize(heterogeneous: &[Translated]) -> Self { if heterogeneous.iter().all(Translated::is_value) { - TranslatedManyArray::Values( + Self::Values( heterogeneous .iter() .map(|v| v.as_value().unwrap().clone()) .collect(), ) } else { - TranslatedManyArray::Tables( - heterogeneous - .iter() - .map(|v| v.clone().into_item().as_table().unwrap().clone()) - .collect(), - ) + Self::Tables(Self::tables_from_translated(heterogeneous)) } } + fn tables_from_translated(translated: &[Translated]) -> Vec
{ + translated + .iter() + .map(|v| v.clone().into_item().as_table().unwrap().clone()) + .collect() + } + fn tables_from_values(values: &[Value]) -> Vec
{ values .into_iter() diff --git a/hacking/cargo-manifest-management/src/main.rs b/hacking/cargo-manifest-management/src/main.rs index ac550999a..da5a69ac7 100644 --- a/hacking/cargo-manifest-management/src/main.rs +++ b/hacking/cargo-manifest-management/src/main.rs @@ -12,14 +12,14 @@ use std::path::PathBuf; use clap::Parser; -mod cargo_policy; +mod cargo_manifest_policy; mod diff; mod format; mod plan; -use cargo_policy::CargoPolicy; +use cargo_manifest_policy::CargoManifestPolicy; use diff::diff; -use format::{format, PathSegment, Policy}; +use format::{Formatter, PathSegment, Policy}; use plan::Plan; #[derive(Debug, Parser)] @@ -35,13 +35,13 @@ fn main() { let args = Args::parse(); let plan_file = File::open(&args.plan).unwrap(); let plan: Plan = serde_json::from_reader(plan_file).unwrap(); - plan.execute::(args.just_check); + plan.execute(&Formatter::new(CargoManifestPolicy), args.just_check); } // for debugging: fn test_format() { let json_value = serde_json::from_reader(io::stdin()).unwrap(); - let toml_doc = format::(&json_value); + let toml_doc = Formatter::new(CargoManifestPolicy).format(&json_value); print!("{}", toml_doc) } diff --git a/hacking/cargo-manifest-management/src/plan.rs b/hacking/cargo-manifest-management/src/plan.rs index c9b05705a..892680bf5 100644 --- a/hacking/cargo-manifest-management/src/plan.rs +++ b/hacking/cargo-manifest-management/src/plan.rs @@ -13,7 +13,7 @@ use std::str; use serde::Deserialize; use serde_json::Value as JsonValue; -use crate::{diff, format, Policy}; +use crate::{diff, Formatter, Policy}; #[derive(Debug, Deserialize)] #[serde(transparent)] @@ -29,17 +29,10 @@ pub struct Entry { } impl Plan { - pub fn get_entry_by_package_name(&self, name: &str) -> Option<&Entry> { - self.entries - .values() - .filter(|entry| entry.get_package_name() == Some(name)) - .next() - } - - pub fn execute(&self, just_check: bool) { + pub fn execute(&self, formatter: &Formatter

, just_check: bool) { for (path, entry) in self.entries.iter() { assert!(!entry.just_ensure_equivalence); // TODO unimplemented - let rendered = entry.render::

(); + let rendered = entry.render(formatter); let mut write = true; if path.is_file() { let existing = fs::read(path).unwrap(); @@ -61,6 +54,13 @@ impl Plan { } } } + + pub fn get_entry_by_package_name(&self, name: &str) -> Option<&Entry> { + self.entries + .values() + .filter(|entry| entry.get_package_name() == Some(name)) + .next() + } } impl Entry { @@ -79,13 +79,14 @@ impl Entry { ) } - fn render(&self) -> String { + fn render(&self, formatter: &Formatter

) -> String { + let doc = formatter.format(&self.manifest); let mut s = String::new(); if let Some(frontmatter) = self.frontmatter.as_ref() { s.push_str(frontmatter); s.push('\n'); } - s.push_str(&format::

(&self.manifest).to_string()); + s.push_str(&doc.to_string()); s } } diff --git a/hacking/cargo-manifest-management/tool.nix b/hacking/cargo-manifest-management/tool.nix index 402f5a45e..8a8d0e85d 100644 --- a/hacking/cargo-manifest-management/tool.nix +++ b/hacking/cargo-manifest-management/tool.nix @@ -82,6 +82,7 @@ let inherit manifestValue; inherit (elaboratedNix) frontmatter justEnsureEquivalence; }; + in { workspaceRoot, workspaceDirFilter @@ -89,13 +90,13 @@ in }: let - generatedManifestSources = + cargoNixPaths = let dirFilter = relativePathSegments: lib.head relativePathSegments == "crates"; in scanDirForFilesWithName workspaceDirFilter "Cargo.nix" workspaceRoot; - genrateManifest = cargoNixAbsolutePath: + generateManifest = cargoNixAbsolutePath: let absolutePath = builtins.dirOf cargoNixAbsolutePath; manifestExpr = callManifest { @@ -127,7 +128,7 @@ let in generated // manual; - generatedManifestsList = map genrateManifest generatedManifestSources; + generatedManifestsList = map generateManifest cargoNixPaths; generatedManifestsByPackageName = lib.listToAttrs