From 02d7f3472bed170676e2e60c59998f7121091cf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 14 May 2024 14:01:16 -0700 Subject: [PATCH] Do not delete array tables (#17) --- Cargo.lock | 2 +- Cargo.toml | 2 +- rust/src/build_system.rs | 61 ++++++++++++------------- rust/src/helpers/table.rs | 93 +++++++++++++++++++++++++++------------ rust/src/main.rs | 30 +++++++++++++ rust/src/project.rs | 2 +- rust/src/ruff.rs | 2 +- 7 files changed, 126 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5972274..dc9279f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -520,7 +520,7 @@ dependencies = [ [[package]] name = "pyproject-fmt-rust" -version = "1.0.6" +version = "1.0.7" dependencies = [ "indoc", "lexical-sort", diff --git a/Cargo.toml b/Cargo.toml index c779c61..9e79fe9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyproject-fmt-rust" -version = "1.0.6" +version = "1.0.7" description = "Format pyproject.toml files" repository = "https://github.com/tox-dev/pyproject-fmt" readme = "README.md" diff --git a/rust/src/build_system.rs b/rust/src/build_system.rs index 4407bc5..ca1fb20 100644 --- a/rust/src/build_system.rs +++ b/rust/src/build_system.rs @@ -7,7 +7,7 @@ pub fn fix(tables: &mut Tables, keep_full_version: bool) { if table_element.is_none() { return; } - let table = &mut table_element.unwrap().borrow_mut(); + let table = &mut table_element.unwrap().first().unwrap().borrow_mut(); for_entries(table, &mut |key, entry| match key.as_str() { "requires" => { transform(entry, &|s| format_requirement(s, keep_full_version)); @@ -84,38 +84,33 @@ mod tests { "#}, true )] - // #[case::build_system_order( - // indoc ! {r#" - // [build-system] - // # more - // more = true # more post - // # extra - // extra = 1 # extra post - // # path - // backend-path = ['A'] # path post - // # requires - // requires = ["B"] # requires post - // # backend - // build-backend = "hatchling.build" # backend post - // # post - // "#}, - // indoc ! {r#" - // [build-system] - // # more - // build-backend = "hatchling.build" # backend post - // # post - // requires = ["b"] # requires post - // # backend - // backend-path = ['A'] # path post - // # requires - // more = true # more post - // # extra - // extra = 1 # extra post - // # path - // "#}, - // true - // )] - fn test_normalize_requirement(#[case] start: &str, #[case] expected: &str, #[case] keep_full_version: bool) { + #[case::join( + indoc ! {r#" + [build-system] + requires=["a"] + [build-system] + build-backend = "hatchling.build" + [[build-system.a]] + name = "Hammer" + [[build-system.a]] # empty table within the array + [[build-system.a]] + name = "Nail" + "#}, + indoc ! {r#" + [build-system] + build-backend = "hatchling.build" + requires = [ + "a", + ] + [[build-system.a]] + name = "Hammer" + [[build-system.a]] # empty table within the array + [[build-system.a]] + name = "Nail" + "#}, + false + )] + fn test_format_build_systems(#[case] start: &str, #[case] expected: &str, #[case] keep_full_version: bool) { assert_eq!(evaluate(start, keep_full_version), expected); } } diff --git a/rust/src/helpers/table.rs b/rust/src/helpers/table.rs index cca8f45..a61217d 100644 --- a/rust/src/helpers/table.rs +++ b/rust/src/helpers/table.rs @@ -1,6 +1,7 @@ use std::cell::{RefCell, RefMut}; use std::collections::HashMap; use std::iter::zip; +use std::ops::Index; use taplo::syntax::SyntaxKind::{TABLE_ARRAY_HEADER, TABLE_HEADER}; use taplo::syntax::{SyntaxElement, SyntaxKind, SyntaxNode}; @@ -11,39 +12,57 @@ use crate::helpers::string::load_text; #[derive(Debug)] pub struct Tables { - pub header_to_pos: HashMap, + pub header_to_pos: HashMap>, pub table_set: Vec>>, } impl Tables { - pub(crate) fn get(&mut self, key: &str) -> Option<&RefCell>> { + pub(crate) fn get(&mut self, key: &str) -> Option>>> { if self.header_to_pos.contains_key(key) { - Some(&self.table_set[self.header_to_pos[key]]) + let mut res = Vec::<&RefCell>>::new(); + for pos in &self.header_to_pos[key] { + res.push(&self.table_set[*pos]); + } + Some(res) } else { None } } pub fn from_ast(root_ast: &SyntaxNode) -> Self { - let mut header_to_pos = HashMap::::new(); + let mut header_to_pos = HashMap::>::new(); let mut table_set = Vec::>>::new(); let entry_set = RefCell::new(Vec::::new()); - let mut add_to_table_set = || { + let mut table_kind = TABLE_HEADER; + let mut add_to_table_set = |kind| { let mut entry_set_borrow = entry_set.borrow_mut(); if !entry_set_borrow.is_empty() { - header_to_pos.insert(get_table_name(&entry_set_borrow[0]), table_set.len()); - table_set.push(RefCell::new(entry_set_borrow.clone())); + let table_name = get_table_name(&entry_set_borrow[0]); + let indexes = header_to_pos.entry(table_name).or_default(); + if kind == TABLE_ARRAY_HEADER || (kind == TABLE_HEADER && indexes.is_empty()) { + indexes.push(table_set.len()); + table_set.push(RefCell::new(entry_set_borrow.clone())); + } else if kind == TABLE_HEADER && !indexes.is_empty() { + // join tables + let pos = indexes.first().unwrap(); + let mut res = table_set.index(*pos).borrow_mut(); + for element in entry_set_borrow.clone() { + if element.kind() != TABLE_HEADER { + res.push(element); + } + } + } entry_set_borrow.clear(); } }; for c in root_ast.children_with_tokens() { if [TABLE_ARRAY_HEADER, TABLE_HEADER].contains(&c.kind()) { - add_to_table_set(); + add_to_table_set(table_kind); + table_kind = c.kind(); } entry_set.borrow_mut().push(c); } - add_to_table_set(); - + add_to_table_set(table_kind); Self { header_to_pos, table_set, @@ -61,29 +80,31 @@ impl Tables { } next.push(String::new()); for (name, next_name) in zip(order.iter(), next.iter()) { - let entries = self.get(name).unwrap().borrow(); - if !entries.is_empty() { - entry_count += entries.len(); - let last = entries.last().unwrap(); - if name.is_empty() && last.kind() == SyntaxKind::NEWLINE && entries.len() == 1 { - continue; - } - let mut add = entries.clone(); - if get_key(name) != get_key(next_name) { - if last.kind() == SyntaxKind::NEWLINE { - // replace existing newline to ensure single newline - add.pop(); + for entries in self.get(name).unwrap() { + let got = entries.borrow_mut(); + if !got.is_empty() { + entry_count += got.len(); + let last = got.last().unwrap(); + if name.is_empty() && last.kind() == SyntaxKind::NEWLINE && got.len() == 1 { + continue; } - add.push(make_empty_newline()); + let mut add = got.clone(); + if get_key(name) != get_key(next_name) { + if last.kind() == SyntaxKind::NEWLINE { + // replace existing newline to ensure single newline + add.pop(); + } + add.push(make_empty_newline()); + } + to_insert.extend(add); } - to_insert.extend(add); } } root_ast.splice_children(0..entry_count, to_insert); } } -fn calculate_order(header_to_pos: &HashMap, ordering: &[&str]) -> Vec { +fn calculate_order(header_to_pos: &HashMap>, ordering: &[&str]) -> Vec { let max_ordering = ordering.len() * 2; let key_to_pos = ordering .iter() @@ -91,7 +112,11 @@ fn calculate_order(header_to_pos: &HashMap, ordering: &[&str]) -> .map(|(k, v)| (v, k * 2)) .collect::>(); - let mut header_pos: Vec<(String, usize)> = header_to_pos.clone().into_iter().collect(); + let mut header_pos: Vec<(String, usize)> = header_to_pos + .clone() + .into_iter() + .map(|(k, v)| (k, *v.iter().min().unwrap())) + .collect(); header_pos.sort_by_cached_key(|(k, file_pos)| -> (usize, usize) { let key = get_key(k); @@ -220,12 +245,22 @@ pub fn collapse_sub_tables(tables: &mut Tables, name: &str) { return; } if !tables.header_to_pos.contains_key(name) { - tables.header_to_pos.insert(String::from(name), tables.table_set.len()); + tables + .header_to_pos + .insert(String::from(name), vec![tables.table_set.len()]); tables.table_set.push(RefCell::new(make_table_entry(name))); } - let mut main = tables.table_set[tables.header_to_pos[name]].borrow_mut(); + let main_positions = tables.header_to_pos[name].clone(); + if main_positions.len() != 1 { + return; + } + let mut main = tables.table_set[*main_positions.first().unwrap()].borrow_mut(); for key in sub_table_keys { - let mut sub = tables.table_set[tables.header_to_pos[key]].borrow_mut(); + let sub_positions = tables.header_to_pos[key].clone(); + if sub_positions.len() != 1 { + continue; + } + let mut sub = tables.table_set[*sub_positions.first().unwrap()].borrow_mut(); let sub_name = key.strip_prefix(sub_name_prefix.as_str()).unwrap(); let mut header = false; for child in sub.iter() { diff --git a/rust/src/main.rs b/rust/src/main.rs index e00c89d..c82eaa8 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -209,6 +209,36 @@ mod tests { true, (3, 8) )] + #[case::array_of_tables( + indoc ! {r#" + [tool.commitizen] + name = "cz_customize" + + [tool.commitizen.customize] + message_template = "" + + [[tool.commitizen.customize.questions]] + type = "list" + [[tool.commitizen.customize.questions]] + type = "input" + "#}, + indoc ! {r#" + [tool.commitizen] + name = "cz_customize" + + [tool.commitizen.customize] + message_template = "" + + [[tool.commitizen.customize.questions]] + type = "list" + + [[tool.commitizen.customize.questions]] + type = "input" + "#}, + 2, + true, + (3, 8) + )] fn test_format_toml( #[case] start: &str, #[case] expected: &str, diff --git a/rust/src/project.rs b/rust/src/project.rs index a4e93c4..e7c4c3d 100644 --- a/rust/src/project.rs +++ b/rust/src/project.rs @@ -22,7 +22,7 @@ pub fn fix( if table_element.is_none() { return; } - let table = &mut table_element.unwrap().borrow_mut(); + let table = &mut table_element.unwrap().first().unwrap().borrow_mut(); expand_entry_points_inline_tables(table); for_entries(table, &mut |key, entry| match key.split('.').next().unwrap() { "name" => { diff --git a/rust/src/ruff.rs b/rust/src/ruff.rs index 42f1c01..29f5005 100644 --- a/rust/src/ruff.rs +++ b/rust/src/ruff.rs @@ -9,7 +9,7 @@ pub fn fix(tables: &mut Tables) { if table_element.is_none() { return; } - let table = &mut table_element.unwrap().borrow_mut(); + let table = &mut table_element.unwrap().first().unwrap().borrow_mut(); for_entries(table, &mut |key, entry| match key.as_str() { "target-version" | "cache-dir"