✨ feat(tox-toml-fmt): add inline table key reordering (#264)

Inline tables in `tox.toml` (like `{ replace = "default", default = ".",
extend = true }`) had no enforced key ordering, leading to inconsistent
formatting across projects. This PR adds schema-driven key reordering
for the four tox inline table types — `replace`, `prefix`, `product`,
and `value` — so keys always appear in a canonical order that puts the
discriminator key first. ✨

The reordering logic lives in the shared `common` crate as a generic
`InlineTableSchema` + `reorder_inline_table_keys` API, making it
reusable by other formatters. Each schema declares a discriminator key
to identify which inline tables it applies to and the desired key order.
The implementation uses the parse-and-extract pattern to rebuild
reordered inline tables, and collects all target nodes before mutation
to avoid iterator invalidation.

Beyond inline tables, this PR also brings the formatter up to date with
recent tox features: 8 new environment keys (`factors`,
`default_base_python`, `virtualenv_spec`, `pylock`, `recreate_commands`,
`fail_fast`, `commands_retry`, `extra_setup_commands`), `env_base.*`
table support, proper `-r`/`-c` file reference handling in
`deps`/`constraints`, alphabetical sorting of remaining envs not in
`env_list`, and product expansion inline tables excluded from `env_list`
sorting. 📝 Documentation and `CONTRIBUTING.md` have been updated to
reflect all changes.

Author: gaborbernat

CONTRIBUTING.md

@@ -330,9 +330,14 @@ fn test_load_text(#[case] input: &str, #[case] kind: SyntaxKind, #[case] expecte
 
 ### Coverage Goals and Measurement
 
-We require **98% line coverage for Rust code** and **100% coverage for Python code**. To generate an HTML coverage
-report for Rust code, run `tox r -e coverage` from the repository root. This generates lcov output and opens an HTML
-report in your browser. For a quick summary, use `cargo llvm-cov --workspace --no-default-features --summary-only`.
+We require **98% line coverage for Rust code** and **100% coverage for Python code**. Additionally, **diff coverage must
+be 100% per test suite** — changes to `common/src/` must be fully covered by the common test suite alone, and changes to
+`tox-toml-fmt/rust/src/` must be fully covered by the tox-toml-fmt test suite alone. Use per-package coverage checks to
+verify: `cargo llvm-cov -p common --summary-only` or `cargo llvm-cov -p tox-toml-fmt --summary-only`.
+
+To generate an HTML coverage report for Rust code, run `tox r -e coverage` from the repository root. This generates lcov
+output and opens an HTML report in your browser. For a quick summary, use
+`cargo llvm-cov --workspace --no-default-features --summary-only`.
 
 #### Testing PyO3 Code from Rust
 
@@ -414,27 +419,30 @@ fn test_format(#[case] input: &str, #[case] expected: &str) {
 }
 ```
 
-Snapshot testing approach (preferred):
+Snapshot testing approach (preferred, using inline snapshots):
 
 ```rust
 #[rstest]
 #[case::simple("input")]
 fn test_format(#[case] input: &str) {
     let result = format_toml(input);
-    insta::assert_snapshot!(result);
+    insta::assert_snapshot!(result, @"");
 }
 ```
 
+The `@""` syntax creates an **inline snapshot** where the expected value is stored directly in the test file. This is
+preferred over file-based snapshots because it keeps the expected output next to the test input, making tests easier to
+read and review.
+
 Snapshot testing workflow:
 
-- Run tests with `cargo insta test` to generate snapshots
-- Review changes with `cargo insta review` (interactive) or view diffs manually
+- Run tests with `cargo insta test --accept` to populate inline snapshots
+- Review changes with `cargo insta review` (interactive) or view diffs in the test file directly
 - Accept all changes with `cargo insta test --accept`
 - Reject changes with `cargo insta reject`
 
 When formatter behavior changes (like switching parsers), you can update all test expectations with a single
-`cargo insta test --accept` instead of manually updating hundreds of inline strings. Snapshots are stored in
-`src/tests/snapshots/` and committed to git.
+`cargo insta test --accept` instead of manually updating hundreds of inline strings.
 
 ## Common Patterns
 
@@ -468,6 +476,25 @@ update_content(value_node, |text| {
 });
 ```
 
+### Reordering Inline Table Keys
+
+When a formatter needs to enforce a consistent key order within inline tables, use the `InlineTableSchema` and
+`reorder_inline_table_keys` from `common::table`. Each schema specifies a discriminator key (used to identify which
+schema applies) and the desired key order. Keys not listed in the schema are appended at the end.
+
+```rust
+use common::table::{reorder_inline_table_keys, InlineTableSchema};
+
+const SCHEMAS: &[InlineTableSchema] = &[
+    InlineTableSchema {
+        discriminator: "replace",
+        key_order: &["replace", "default", "extend"],
+    },
+];
+
+reorder_inline_table_keys(&root_ast, SCHEMAS);
+```
+
 ### Creating New Nodes
 
 When you need to create new syntax nodes, use the functions in `common::create`. These use the parse-and-extract pattern

common/src/string.rs

@@ -298,29 +298,57 @@ where
     }
 }
 
+fn is_valid_bare_key(s: &str) -> bool {
+    !s.is_empty() && s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+}
+
+fn normalize_key_segment(kind: SyntaxKind, text: &str) -> String {
+    match kind {
+        BARE_KEY => text.to_string(),
+        LITERAL_STRING => {
+            let inner = &text[1..text.len() - 1];
+            if is_valid_bare_key(inner) {
+                return inner.to_string();
+            }
+            let escaped = inner.replace('\\', "\\\\").replace('"', "\\\"");
+            format!("\"{escaped}\"")
+        }
+        BASIC_STRING => {
+            let inner = &text[1..text.len() - 1];
+            if is_valid_bare_key(inner) {
+                return inner.to_string();
+            }
+            text.to_string()
+        }
+        _ => text.to_string(),
+    }
+}
+
 pub fn normalize_key_quotes(root: &SyntaxNode) {
     use crate::create::make_key;
 
-    for descendant in root.descendants() {
-        if descendant.kind() != KEYS {
-            continue;
-        }
-        let has_literal = descendant.children_with_tokens().any(|c| c.kind() == LITERAL_STRING);
-        if !has_literal {
+    let keys_nodes: Vec<SyntaxNode> = root.descendants().filter(|n| n.kind() == KEYS).collect();
+    for descendant in keys_nodes {
+        let needs_normalization = descendant.children_with_tokens().any(|c| {
+            let kind = c.kind();
+            if kind == LITERAL_STRING {
+                return true;
+            }
+            if kind == BASIC_STRING {
+                let text = c.to_string();
+                let inner = &text[1..text.len() - 1];
+                return is_valid_bare_key(inner);
+            }
+            false
+        });
+        if !needs_normalization {
             continue;
         }
         let mut key_parts = Vec::new();
         for child in descendant.children_with_tokens() {
-            match child.kind() {
-                BARE_KEY => key_parts.push(child.to_string()),
-                LITERAL_STRING => {
-                    let text = child.to_string();
-                    let inner = &text[1..text.len() - 1];
-                    let escaped = inner.replace('\\', "\\\\").replace('"', "\\\"");
-                    key_parts.push(format!("\"{escaped}\""));
-                }
-                BASIC_STRING => key_parts.push(child.to_string()),
-                _ => {}
+            let kind = child.kind();
+            if matches!(kind, BARE_KEY | LITERAL_STRING | BASIC_STRING) {
+                key_parts.push(normalize_key_segment(kind, &child.to_string()));
             }
         }
         let new_key = make_key(&key_parts.join("."));

common/src/table.rs

@@ -5,8 +5,8 @@ use std::ops::Index;
 
 use tombi_syntax::SyntaxKind::{
     ARRAY_OF_TABLE, BARE_KEY, BASIC_STRING, BRACKET_END, BRACKET_START, COMMENT, DANGLING_COMMENT_GROUP,
-    DOUBLE_BRACKET_START, EQUAL, KEY_VALUE, KEY_VALUE_GROUP, KEY_VALUE_WITH_COMMA_GROUP, KEYS, LINE_BREAK,
-    LITERAL_STRING, TABLE, WHITESPACE,
+    DOUBLE_BRACKET_START, EQUAL, INLINE_TABLE, KEY_VALUE, KEY_VALUE_GROUP, KEY_VALUE_WITH_COMMA_GROUP, KEYS,
+    LINE_BREAK, LITERAL_STRING, TABLE, WHITESPACE,
 };
 use tombi_syntax::{SyntaxElement, SyntaxKind, SyntaxNode};
 
@@ -1063,3 +1063,78 @@ fn add_intermediate_parents(table_name: &str, prefix_dots: usize, result: &mut V
         current = parent;
     }
 }
+
+pub struct InlineTableSchema {
+    pub discriminator: &'static str,
+    pub key_order: &'static [&'static str],
+}
+
+fn inline_table_key_name(kv_node: &SyntaxNode) -> String {
+    let raw = kv_node
+        .children_with_tokens()
+        .find(|c| c.kind() == KEYS)
+        .and_then(|c| c.as_node().map(|n| n.text().to_string().trim().to_string()))
+        .unwrap_or_default();
+    raw.strip_prefix('"')
+        .and_then(|s| s.strip_suffix('"'))
+        .or_else(|| raw.strip_prefix('\'').and_then(|s| s.strip_suffix('\'')))
+        .unwrap_or(&raw)
+        .to_string()
+}
+
+fn detect_schema<'a>(keys: &[String], schemas: &'a [InlineTableSchema]) -> Option<&'a [&'static str]> {
+    schemas
+        .iter()
+        .find(|s| keys.iter().any(|k| k == s.discriminator))
+        .map(|s| s.key_order)
+}
+
+fn reorder_single_inline_table(node: &SyntaxNode, schemas: &[InlineTableSchema]) {
+    let kv_pairs: Vec<(String, String)> = node
+        .children()
+        .filter(|n| n.kind() == KEY_VALUE_WITH_COMMA_GROUP)
+        .flat_map(|group| {
+            group
+                .children()
+                .filter(|n| n.kind() == KEY_VALUE)
+                .map(|kv| (inline_table_key_name(&kv), kv.text().to_string().trim().to_string()))
+                .collect::<Vec<_>>()
+        })
+        .collect();
+
+    if kv_pairs.len() < 2 {
+        return;
+    }
+
+    let keys: Vec<String> = kv_pairs.iter().map(|(k, _)| k.clone()).collect();
+    let Some(schema) = detect_schema(&keys, schemas) else {
+        return;
+    };
+
+    let key_position = |k: &str| -> usize { schema.iter().position(|s| *s == k).unwrap_or(usize::MAX) };
+    let mut sorted = kv_pairs.clone();
+    sorted.sort_by_key(|(k, _)| key_position(k));
+
+    if sorted.iter().map(|(k, _)| k).eq(kv_pairs.iter().map(|(k, _)| k)) {
+        return;
+    }
+
+    let entries: Vec<&str> = sorted.iter().map(|(_, v)| v.as_str()).collect();
+    let rebuilt = format!("_x = {{ {} }}\n", entries.join(", "));
+    let parsed = parse(&rebuilt);
+    let new_children: Option<Vec<SyntaxElement>> = parsed
+        .descendants()
+        .find(|n| n.kind() == INLINE_TABLE)
+        .map(|n| n.children_with_tokens().collect());
+
+    if let Some(children) = new_children {
+        node.splice_children(0..node.children_with_tokens().count(), children);
+    }
+}
+
+pub fn reorder_inline_table_keys(root_ast: &SyntaxNode, schemas: &[InlineTableSchema]) {
+    let inline_tables: Vec<SyntaxNode> = root_ast.descendants().filter(|n| n.kind() == INLINE_TABLE).collect();
+    for node in inline_tables {
+        reorder_single_inline_table(&node, schemas);
+    }
+}

common/src/tests/string_tests.rs

@@ -602,8 +602,8 @@ fn test_strip_quotes_triple_quotes() {
 fn test_normalize_key_quotes_simple_literal() {
     let root = parse("'key' = 1\n");
     normalize_key_quotes(&root);
-    insta::assert_snapshot!(root.to_string(), @r#""key" = 1
-"#);
+    insta::assert_snapshot!(root.to_string(), @"key = 1
+");
 }
 
 #[test]
@@ -615,11 +615,11 @@ fn test_normalize_key_quotes_bare_key_unchanged() {
 }
 
 #[test]
-fn test_normalize_key_quotes_basic_string_unchanged() {
+fn test_normalize_key_quotes_basic_string_stripped() {
     let root = parse("\"key\" = 1\n");
     normalize_key_quotes(&root);
-    insta::assert_snapshot!(root.to_string(), @r#""key" = 1
-"#);
+    insta::assert_snapshot!(root.to_string(), @"key = 1
+");
 }
 
 #[test]
@@ -674,15 +674,31 @@ fn test_normalize_key_quotes_literal_with_backslash_and_quote() {
 fn test_normalize_key_quotes_multiple_literal_segments() {
     let root = parse("'first'.'second' = 1\n");
     normalize_key_quotes(&root);
-    insta::assert_snapshot!(root.to_string(), @r#""first"."second" = 1
-"#);
+    insta::assert_snapshot!(root.to_string(), @"first.second = 1
+");
 }
 
 #[test]
 fn test_normalize_key_quotes_preserves_basic_in_dotted() {
     let root = parse("bare.\"basic\".'literal' = 1\n");
     normalize_key_quotes(&root);
-    insta::assert_snapshot!(root.to_string(), @r#"bare."basic"."literal" = 1
+    insta::assert_snapshot!(root.to_string(), @"bare.basic.literal = 1
+");
+}
+
+#[test]
+fn test_normalize_key_quotes_preserves_required_quotes() {
+    let root = parse("\"key with spaces\" = 1\n");
+    normalize_key_quotes(&root);
+    insta::assert_snapshot!(root.to_string(), @r#""key with spaces" = 1
+"#);
+}
+
+#[test]
+fn test_normalize_key_quotes_inline_table_keys() {
+    let root = parse("val = { \"else\" = \"no\", \"then\" = \"yes\" }\n");
+    normalize_key_quotes(&root);
+    insta::assert_snapshot!(root.to_string(), @r#"val = { else = "no", then = "yes" }
 "#);
 }