🐛 fix: let the guard compare the marker its own pass chose

A comment the file wrote that spelled the marker was read as one, so an
active entry beside it was left out of nesting, collapse and migration.
The pass now holds the marker it picked while the formatter runs, and the
guard answers with that, so nothing the file says can stand in for it.

Mypy's collapsed overrides are found by their path rather than by the
name they end in, which was sorting an `overrides` array under any tool.

Author: gaborbernat

common/src/disabled.rs

@@ -15,38 +15,53 @@ use std::collections::HashSet;
 /// from this one that the file does not already contain.
 pub const MARKER: &str = "__toml_fmt_disabled__";
 
-/// Whether the entry is one this pass turned back on, which is what a comment carrying [`MARKER`]
-/// says.
+thread_local! {
+    /// The marker the pass running on this thread chose. A guard outside a pass has none, so
+    /// nothing the file wrote is read as one.
+    static IN_USE: std::cell::RefCell<Option<String>> = const { std::cell::RefCell::new(None) };
+}
+
+/// Holds the marker for as long as the pass runs, and takes it back however the pass ends.
+struct InUse;
+
+impl InUse {
+    fn hold(marker: &str) -> Self {
+        IN_USE.with_borrow_mut(|held| *held = Some(marker.to_owned()));
+        Self
+    }
+}
+
+impl Drop for InUse {
+    fn drop(&mut self) {
+        IN_USE.with_borrow_mut(|held| *held = None);
+    }
+}
+
+/// Whether the entry is one this pass turned back on, which is what the marker beside it says.
 ///
 /// A pass that would split, drop or merge such an entry has to leave it alone: what says the entry
 /// is disabled is the comment beside it, and none of those rewrites can say it of the entries they
 /// leave behind.
 #[must_use]
 pub fn is_enabled_here(entry: &toml_doc::Entry<'_>) -> bool {
-    entry
-        .trail
-        .comment
-        .as_ref()
-        .is_some_and(|comment| marks_a_disabled_key(comment))
+    let Some(comment) = entry.trail.comment.as_ref() else {
+        return false;
+    };
+    IN_USE.with_borrow(|held| {
+        held.as_deref()
+            .is_some_and(|marker| marks_a_disabled_key(comment, marker))
+    })
 }
 
-/// Whether the comment is one [`enabled_form`] wrote: a comment holding nothing but the marker, or
-/// the marker written at the end of a comment the file already had. Prose that merely names the
-/// marker is a comment like any other.
-fn marks_a_disabled_key(comment: &str) -> bool {
+/// Whether the comment is the one [`enabled_form`] wrote for this pass: a comment holding nothing
+/// but the marker, or the marker written at the end of a comment the file already had.
+fn marks_a_disabled_key(comment: &str, marker: &str) -> bool {
     let body = comment.trim_start_matches('#').trim();
-    is_a_marker(body)
+    body == marker
         || body
             .rsplit(char::is_whitespace)
             .next()
-            .and_then(|last| last.strip_suffix("-kept"))
-            .is_some_and(is_a_marker)
-}
-
-/// Whether the text is [`MARKER`] or one of the longer ones [`fresh_marker`] builds from it.
-fn is_a_marker(text: &str) -> bool {
-    text.strip_prefix(MARKER)
-        .is_some_and(|rest| rest.chars().all(|held| held == 'x'))
+            .is_some_and(|last| last == kept_marker(marker))
 }
 
 /// A marker the source does not already hold, so nothing the file says can be read as one.
@@ -66,7 +81,12 @@ fn fresh_marker(source: &str) -> String {
 /// wrote a comment on a line of its own, so turning one back on leaves a document that still reads.
 pub fn with_disabled_keys(content: &str, format: impl FnOnce(&str) -> String) -> String {
     let marker = fresh_marker(content);
-    restore_disabled_keys(&format(&enable_disabled_keys(content, &marker)), &marker)
+    let enabled = enable_disabled_keys(content, &marker);
+    let formatted = {
+        let _in_use = InUse::hold(&marker);
+        format(&enabled)
+    };
+    restore_disabled_keys(&formatted, &marker)
 }
 
 /// [`with_disabled_keys`] for a formatter that may reject its input; a rejected pass restores nothing.
@@ -77,7 +97,11 @@ pub fn with_disabled_keys(content: &str, format: impl FnOnce(&str) -> String) ->
 pub fn try_with_disabled_keys<E>(content: &str, format: impl FnOnce(&str) -> Result<String, E>) -> Result<String, E> {
     let marker = fresh_marker(content);
     let enabled = enable_disabled_keys(content, &marker);
-    Ok(restore_disabled_keys(&format(&enabled)?, &marker))
+    let formatted = {
+        let _in_use = InUse::hold(&marker);
+        format(&enabled)
+    };
+    Ok(restore_disabled_keys(&formatted?, &marker))
 }
 
 /// The lines the file wrote a comment on where a key could have stood: what leads an entry or a

common/src/sections.rs

@@ -310,7 +310,7 @@ pub fn for_array_elements(
 
 /// Every value written at the dotted path, wherever the file put the split between the header it
 /// wrote and the keys under it.
-fn values_at<'d, 'a>(document: &'d mut Document<'a>, path: &[String]) -> Vec<&'d mut Value<'a>> {
+pub fn values_at<'d, 'a>(document: &'d mut Document<'a>, path: &[String]) -> Vec<&'d mut Value<'a>> {
     let mut found: Vec<&'d mut Value<'a>> = Vec::new();
     for entry in active(&mut document.root) {
         if entry.key_value.key.segments() == path {

common/tests/disabled.rs

@@ -184,20 +184,31 @@ fn prose_above_a_disabled_key_keeps_its_hashes() {
     );
 }
 
-/// Prose that names the marker is a comment like any other, so what it is written beside is
-/// ordinary configuration a rule reads.
-#[test]
-fn a_comment_naming_the_marker_is_not_a_disabled_key() {
-    let source = format!("[tool.x]\nsub.a = 1  # see {} for why\n", common::disabled::MARKER);
-    let mut document = toml_doc::parse(&source).expect("valid source");
-    common::nesting::expand(&mut document, "tool.x");
-
+/// The marker names the pass that wrote it, so a comment the file wrote is ordinary configuration
+/// however closely it spells one.
+#[test]
+fn a_comment_the_file_wrote_is_not_a_disabled_key() {
+    let wrote = |comment: String| {
+        let source = format!("[tool.x]\nsub.a = 1  # {comment}\nheld = \"{MARKER}\"\n");
+        with_disabled_keys(&source, |content| {
+            let mut document = toml_doc::parse(content).expect("valid source");
+            common::nesting::expand(&mut document, "tool.x");
+            document.to_string()
+        })
+    };
+
+    // the file already holds the marker text, so the pass runs with a longer one
+    assert_eq!(
+        wrote(String::from(MARKER)),
+        format!("[tool.x]\nheld = \"{MARKER}\"\n[tool.x.sub]\na = 1  # {MARKER}\n")
+    );
+    assert_eq!(
+        wrote(format!("{MARKER}-kept")),
+        format!("[tool.x]\nheld = \"{MARKER}\"\n[tool.x.sub]\na = 1  # {MARKER}-kept\n")
+    );
     assert_eq!(
-        document.to_string(),
-        format!(
-            "[tool.x]\n[tool.x.sub]\na = 1  # see {} for why\n",
-            common::disabled::MARKER
-        )
+        wrote(format!("see {MARKER} for why")),
+        format!("[tool.x]\nheld = \"{MARKER}\"\n[tool.x.sub]\na = 1  # see {MARKER} for why\n")
     );
 }
 

common/tests/nesting.rs

@@ -271,17 +271,13 @@ fn an_element_holding_a_comment_keeps_the_array_written_out() {
 /// carry none of that.
 #[test]
 fn a_disabled_key_is_not_written_out_into_a_header() {
-    let source = format!("[tool.x]\nsub.a = 1  # {}\nsub.b = 2\n", common::disabled::MARKER);
-    let mut document = parse(&source);
-    nesting::expand(&mut document, "tool.x");
+    let formatted = common::disabled::with_disabled_keys("[tool.x]\n# sub.a = 1\nsub.b = 2\n", |content| {
+        let mut document = parse(content);
+        nesting::expand(&mut document, "tool.x");
+        document.to_string()
+    });
 
-    assert_eq!(
-        document.to_string(),
-        format!(
-            "[tool.x]\nsub.a = 1  # {}\n[tool.x.sub]\nb = 2\n",
-            common::disabled::MARKER
-        )
-    );
+    assert_eq!(formatted, "[tool.x]\n# sub.a = 1\n[tool.x.sub]\nb = 2\n");
 }
 
 /// A dotted key at the root already writes the parent out, and a header synthesized beside it would
@@ -335,9 +331,12 @@ fn children_of_several_array_elements_stay_written_out() {
 /// parent would leave nothing saying the table is there.
 #[test]
 fn a_table_of_only_disabled_keys_is_not_folded() {
-    let source = format!("[tool.x]\na = 1\n[tool.x.sub]\nb = 2  # {}\n", common::disabled::MARKER);
-    let mut document = parse(&source);
-    nesting::collapse(&mut document, "tool.x");
+    let source = "[tool.x]\na = 1\n[tool.x.sub]\n# b = 2\n";
+    let formatted = common::disabled::with_disabled_keys(source, |content| {
+        let mut document = parse(content);
+        nesting::collapse(&mut document, "tool.x");
+        document.to_string()
+    });
 
-    assert_eq!(document.to_string(), source);
+    assert_eq!(formatted, source);
 }

common/tests/sections.rs

@@ -588,11 +588,13 @@ fn the_tables_of_an_ordered_name_hold_their_place() {
 /// A key the file wrote as a comment says nothing to a rule reading what the file says.
 #[test]
 fn a_disabled_key_is_not_read_as_an_entry() {
-    let source = format!("[tool.x]\na = 1  # {}\nb = 2\n", common::disabled::MARKER);
-    let mut document = parse(&source);
-    let section = sections::first(&mut document, "tool.x").expect("section");
     let mut seen = Vec::new();
-    sections::for_entries(section, |key, _value| seen.push(key.to_owned()));
+    common::disabled::with_disabled_keys("[tool.x]\n# a = 1\nb = 2\n", |content| {
+        let mut document = parse(content);
+        let section = sections::first(&mut document, "tool.x").expect("section");
+        sections::for_entries(section, |key, _value| seen.push(key.to_owned()));
+        content.to_owned()
+    });
 
     assert_eq!(seen, ["b"]);
 }

pyproject-fmt/rust/src/mypy.rs

@@ -241,17 +241,9 @@ pub fn reorder_inline_tables(document: &mut Document<'_>) {
 /// A collapsed `[[tool.mypy.overrides]]` becomes `overrides = [ {...}, {...} ]`, which puts its
 /// arrays inside a value rather than under a table, out of reach of the entry walk above.
 fn sort_arrays_inside_overrides(document: &mut Document<'_>) {
-    let root = common::sections::active(&mut document.root);
-    let held = document
-        .sections
-        .iter_mut()
-        .flat_map(|section| common::sections::active(&mut section.entries));
-    for entry in root.chain(held) {
-        let key = common::sections::dispatch_name(&entry.key_value.key);
-        if key != "overrides" && !key.ends_with(".overrides") {
-            continue;
-        }
-        let Value::Array(array) = &mut entry.key_value.value else {
+    let path = ["tool", "mypy", "overrides"].map(str::to_owned);
+    for value in common::sections::values_at(document, &path) {
+        let Value::Array(array) = value else {
             continue;
         };
         for member in &mut array.members {

pyproject-fmt/rust/src/tests/main_tests.rs

@@ -1813,3 +1813,26 @@ fn test_a_commented_field_keeps_the_order_it_was_written_in() {
     "#);
     assert_eq!(format_toml(&result, &default_settings()).unwrap(), result);
 }
+
+/// An `overrides` key belongs to the tool whose table it sits in, so mypy's rule for its own
+/// overrides leaves another tool's alone.
+#[test]
+fn test_overrides_under_another_tool_keep_their_order() {
+    let start = indoc! {r#"
+        [tool.other]
+        overrides = [{ module = ["z", "a"] }]
+
+        [tool.mypy]
+        overrides = [{ module = ["z", "a"] }]
+    "#};
+    let result = format_toml(start, &default_settings()).unwrap();
+    assert_valid_toml(&result);
+    insta::assert_snapshot!(result, @r#"
+    [tool.mypy]
+    overrides = [ { module = [ "a", "z" ] } ]
+
+    [tool.other]
+    overrides = [ { module = [ "z", "a" ] } ]
+    "#);
+    assert_eq!(format_toml(&result, &default_settings()).unwrap(), result);
+}

pyproject-fmt/rust/src/tests/project_tests.rs

@@ -2226,6 +2226,7 @@ fn test_project_requires_python_reads_the_whole_specifier_set() {
     assert_eq!(minors("<=3.10.1,<=3.10.5"), named(&["9", "10"]));
 }
 
+
 /// Text that is not a specifier set says nothing about what the project supports, so the configured
 /// window stands.
 #[test]

review.md

@@ -214,44 +214,6 @@ rules crate. This preserves one tox implementation without making either end-use
 dependency. Put workflow path filters and shared Rust inputs in one reusable data source, or validate every consumer
 bidirectionally from a repository-level test without mutating Python's import path.
 
-## 76. P2: Mypy's collapsed-override array rules still run across the whole document
-
-Locations: `pyproject-fmt/rust/src/mypy.rs:235-266` and `pyproject-fmt/rust/src/main.rs:235-239`
-
-The path added to `sections::reorder_inline_tables` now keeps every schema under its owning tool, fixing the reproduced
-cross-tool inline-table rewrite. Mypy's additional `sort_arrays_inside_overrides` pass still scans every root and
-section entry, however. It selects any entry named `overrides` or ending in `.overrides`, without checking that its
-semantic path is `tool.mypy.overrides`.
-
-An unrelated tool can therefore have arrays inside an inline-table array sorted according to mypy's schema. For example,
-`[tool.other] overrides = [{ module = ["z", "a"] }]` is rewritten to `module = ["a", "z"]`. This is an end-user semantic
-change if that tool treats the array as ordered.
-
-Find the collapsed override value through a segment-aware `tool.mypy.overrides` path selector instead of scanning by
-leaf name. Add a public case for the same shape under an unrelated tool and keep its member order unchanged.
-
-## 79. P2: Marker-prefix comments can still masquerade as disabled entries
-
-Locations: `common/src/disabled.rs:20-53`, `common/src/sections.rs:102-116`, `common/src/nesting.rs:305-316`,
-`pyproject-fmt/rust/src/project.rs:203-214`, and `tox-toml-fmt/rust/src/global.rs:235-254`
-
-Restricting the guard to a standalone marker removes the broad prose false positive, but `is_enabled_here` still does
-not know which marker `fresh_marker` selected for this formatting run. `is_a_marker` accepts the base marker followed by
-any number of `x` characters. If an active entry already ends in `# __toml_fmt_disabled__`, collision handling chooses
-`__toml_fmt_disabled__x`; the guard nevertheless recognizes the shorter user comment and treats the active entry as
-disabled. The same false positive remains for an exact `__toml_fmt_disabled__-kept` suffix.
-
-The new prose test puts words after the marker, so it cannot exercise either accepted shape. The longer-marker test
-proves that a real disabled entry remains guarded but not that an active colliding comment remains active. Depending on
-the caller, the false positive skips nesting, table collapse, entry-point expansion, or Tox migration. A comment still
-changes which formatter rules run.
-
-Pass the marker selected by `with_disabled_keys` through the formatter and into every active-entry guard, or carry
-explicit disabled state in the parsed model. The guard should compare the exact marker, including the exact `-kept`
-suffix, rather than recognize a marker family. Add public cases where active structural entries end in the exact base
-marker and base `-kept` marker while collision handling selects a longer marker. Keep the new prose and genuine
-longer-marker cases; they protect distinct behavior.
-
 ## 81. P2: Disabled-key preprocessing has cubic and quadratic scans
 
 Locations: `common/src/disabled.rs:31-37` and `common/src/disabled.rs:62-110`