Fix comment loss and misplacement in match arms, nested records, and raw strings

src/formatting/collections.rs

@@ -266,7 +266,18 @@ impl<'a> Formatter<'a> {
 
         let preserve_multiline_top_level = source_has_newline && self.indent_level == 0;
 
-        if all_simple && items.len() <= 3 && !nested_multiline && !preserve_multiline_top_level {
+        // A record containing standalone comments must stay multiline, otherwise
+        // the inline path emits fields without `write_comments_before` and the
+        // comments are silently dropped. Same `has_comments_in_span` guard that
+        // `format_list` already applies for the same reason.
+        let has_comments_in_record = self.has_comments_in_span(span.start, span.end);
+
+        if all_simple
+            && items.len() <= 3
+            && !nested_multiline
+            && !preserve_multiline_top_level
+            && !has_comments_in_record
+        {
             // Inline format
             let record_start = self.output.len();
             self.write("{");
@@ -461,7 +472,10 @@ impl<'a> Formatter<'a> {
         self.newline();
         self.indent_level += 1;
 
-        for ((_pattern, expr), lhs) in matches.iter().zip(rendered_lhs.iter()) {
+        for ((pattern, expr), lhs) in matches.iter().zip(rendered_lhs.iter()) {
+            // Emit any standalone comments preceding this arm at the arm
+            // boundary, so they are neither dropped nor pulled into a guard.
+            self.write_comments_before(pattern.span.start);
             self.write_indent();
             self.write_bytes(lhs);
 
@@ -582,6 +596,11 @@ impl<'a> Formatter<'a> {
 
     fn render_match_arm_lhs(&self, pattern: &MatchPattern, rhs: &Expression) -> Vec<u8> {
         self.probe_format(|probe| {
+            // Constrain comment emission to this arm's own span. Seeding to the
+            // pattern start stops a parenthesized guard from vacuuming comments
+            // that sit *between* the previous arm and this one — those are
+            // emitted at the arm boundary by `format_match_block` instead.
+            probe.last_pos = pattern.span.start;
             probe.format_match_pattern_for_arm(pattern, rhs);
             if let Some(guard) = &pattern.guard {
                 probe.write(" if ");

src/formatting/comments.rs

@@ -14,29 +14,41 @@ use nu_protocol::Span;
 pub(super) fn extract_comments(source: &[u8]) -> Vec<(Span, Vec<u8>)> {
     let mut comments = Vec::new();
     let mut i = 0;
-    let mut in_string = false;
-    let mut string_char = b'"';
 
     while i < source.len() {
         let c = source[i];
 
-        // Track string state to avoid matching # inside strings
-        if !in_string && (c == b'"' || c == b'\'') {
-            in_string = true;
-            string_char = c;
-            i += 1;
-            continue;
-        }
-
-        if in_string {
-            if c == b'\\' && i + 1 < source.len() {
-                i += 2; // Skip escaped character
+        // Raw string: r#'...'# (or r##'...'##, r###'...'###, ...). Skip the whole
+        // literal so that neither the `#` in its delimiters nor any `#` in its
+        // body is ever mistaken for a comment. Without this, the `#` in `r#'`
+        // starts a bogus comment and the closing `'#` opens a phantom string
+        // that swallows every real comment until the next stray apostrophe.
+        if c == b'r' {
+            if let Some(hashes) = raw_string_open_hashes(source, i) {
+                let body_start = i + 1 + hashes + 1; // 'r' + N*'#' + '\''
+                i = find_raw_string_end(source, body_start, hashes).unwrap_or(source.len());
                 continue;
             }
-            if c == string_char {
-                in_string = false;
-            }
+        }
+
+        // Quoted string. Single-quoted strings are raw (no escapes); only
+        // double-quoted strings process backslash escapes. Consuming each string
+        // inline (rather than a persistent flag) means an unterminated string can
+        // never bleed across the rest of the file.
+        if c == b'"' || c == b'\'' {
+            let quote = c;
             i += 1;
+            while i < source.len() {
+                let b = source[i];
+                if quote == b'"' && b == b'\\' && i + 1 < source.len() {
+                    i += 2;
+                    continue;
+                }
+                i += 1;
+                if b == quote {
+                    break;
+                }
+            }
             continue;
         }
 
@@ -47,6 +59,7 @@ pub(super) fn extract_comments(source: &[u8]) -> Vec<(Span, Vec<u8>)> {
                 i += 1;
             }
             comments.push((Span::new(start, i), source[start..i].to_vec()));
+            continue;
         }
 
         i += 1;
@@ -55,6 +68,38 @@ pub(super) fn extract_comments(source: &[u8]) -> Vec<(Span, Vec<u8>)> {
     comments
 }
 
+/// If `source[i..]` begins a raw-string opener (`r#'`, `r##'`, ...), return the
+/// number of `#` hashes; otherwise `None`. Requires `source[i] == b'r'`.
+fn raw_string_open_hashes(source: &[u8], i: usize) -> Option<usize> {
+    let mut j = i + 1;
+    let mut hashes = 0;
+    while j < source.len() && source[j] == b'#' {
+        hashes += 1;
+        j += 1;
+    }
+    if hashes >= 1 && source.get(j) == Some(&b'\'') {
+        Some(hashes)
+    } else {
+        None
+    }
+}
+
+/// Find the byte index just past a raw-string closer (`'` followed by `hashes`
+/// `#`), scanning from `body_start`. Returns `None` if unterminated.
+fn find_raw_string_end(source: &[u8], body_start: usize, hashes: usize) -> Option<usize> {
+    let mut j = body_start;
+    while j < source.len() {
+        if source[j] == b'\'' {
+            let close = j + 1 + hashes;
+            if source.len() >= close && source[j + 1..close].iter().all(|&b| b == b'#') {
+                return Some(close);
+            }
+        }
+        j += 1;
+    }
+    None
+}
+
 // ─────────────────────────────────────────────────────────────────────────────
 // Formatter comment methods
 // ─────────────────────────────────────────────────────────────────────────────

src/formatting/mod.rs

@@ -239,6 +239,14 @@ impl<'a> Formatter<'a> {
             self.config,
             self.allow_compact_recovered_record_style,
         );
+        // Inherit the parent's comment-emission cursor so a probe never
+        // re-vacuums comments that precede the current position (they are
+        // already emitted / owned by the parent). Without this, rendering a
+        // parenthesized match-arm guard in a probe flushes every earlier
+        // standalone comment inside the guard's `(` (issue: match-guard
+        // comment hoist).
+        probe.last_pos = self.last_pos;
+        probe.written_comments = self.written_comments.clone();
         f(&mut probe);
         probe.output
     }

tests/fixtures/expected/match_guard_paren_does_not_hoist_comments.nu

@@ -0,0 +1,13 @@
+def first [] {
+    # orphan comment that must stay put
+    1
+}
+
+# a leading section comment
+def z [] {
+    match $rest {
+        [] => { '~' }
+        [$arg] if ($arg | path expand | path type) == 'dir' => { $arg }
+        _ => { 0 }
+    }
+}

tests/fixtures/expected/match_inter_arm_comment_not_hoisted_into_guard.nu

@@ -0,0 +1,8 @@
+match $x {
+    a => 1
+    # a comment between arms, before a parenthesized guard
+    b if ($y | foo) == 'z' => 2
+    # before a bare guard
+    c if $y == 'w' => 3
+    _ => 4
+}

tests/fixtures/expected/nested_record_comments_preserved.nu

@@ -0,0 +1,10 @@
+$env.config = {
+    trim: {
+        # wrapping or truncating
+        methodology: wrapping
+        # A strategy used by the 'wrapping' methodology
+        wrapping_try_keep_words: true
+        # A suffix used by the 'truncating' methodology
+        truncating_suffix: "..."
+    }
+}

tests/fixtures/expected/raw_string_hash_delimiter_preserves_comments.nu

@@ -0,0 +1,17 @@
+def mint-token [cache: string] {
+    let remote = r#'
+    set -euo pipefail
+    cache="$1"
+    # -print -quit: find exits after the first hit
+    adm=$(find /nix/store -name atticadm -print -quit)
+  '#
+    # str trim: drop the trailing newline so it can't land inside the
+    # secret and break the Authorization header.
+    $remote | ssh (arca-ssh) bash -s $cache | str trim
+}
+
+# Cache names are validated to [a-z0-9-] — attic's own charset, and it blocks
+# shell-metacharacter injection into the `ssh ... bash -s $cache` above.
+def declared-caches [] {
+    1
+}

tests/fixtures/input/match_guard_paren_does_not_hoist_comments.nu

@@ -0,0 +1,13 @@
+def first [] {
+    # orphan comment that must stay put
+    1
+}
+
+# a leading section comment
+def z [] {
+    match $rest {
+        [] => { '~' }
+        [$arg] if ($arg | path expand | path type) == 'dir' => { $arg }
+        _ => { 0 }
+    }
+}

tests/fixtures/input/match_inter_arm_comment_not_hoisted_into_guard.nu

@@ -0,0 +1,8 @@
+match $x {
+    a => 1
+    # a comment between arms, before a parenthesized guard
+    b if ($y | foo) == 'z' => 2
+    # before a bare guard
+    c if $y == 'w' => 3
+    _ => 4
+}

tests/fixtures/input/nested_record_comments_preserved.nu

@@ -0,0 +1,10 @@
+$env.config = {
+    trim: {
+        # wrapping or truncating
+        methodology: wrapping
+        # A strategy used by the 'wrapping' methodology
+        wrapping_try_keep_words: true
+        # A suffix used by the 'truncating' methodology
+        truncating_suffix: "..."
+    }
+}

tests/fixtures/input/raw_string_hash_delimiter_preserves_comments.nu

@@ -0,0 +1,17 @@
+def mint-token [cache: string] {
+    let remote = r#'
+    set -euo pipefail
+    cache="$1"
+    # -print -quit: find exits after the first hit
+    adm=$(find /nix/store -name atticadm -print -quit)
+  '#
+    # str trim: drop the trailing newline so it can't land inside the
+    # secret and break the Authorization header.
+    $remote | ssh (arca-ssh) bash -s $cache | str trim
+}
+
+# Cache names are validated to [a-z0-9-] — attic's own charset, and it blocks
+# shell-metacharacter injection into the `ssh ... bash -s $cache` above.
+def declared-caches [] {
+    1
+}

tests/ground_truth.rs

@@ -524,6 +524,26 @@ fixture_tests!(
         ground_truth_match_guards_preserved_issue139,
         idempotency_match_guards_preserved_issue139
     ),
+    (
+        "match_guard_paren_does_not_hoist_comments",
+        ground_truth_match_guard_paren_does_not_hoist_comments,
+        idempotency_match_guard_paren_does_not_hoist_comments
+    ),
+    (
+        "match_inter_arm_comment_not_hoisted_into_guard",
+        ground_truth_match_inter_arm_comment_not_hoisted_into_guard,
+        idempotency_match_inter_arm_comment_not_hoisted_into_guard
+    ),
+    (
+        "nested_record_comments_preserved",
+        ground_truth_nested_record_comments_preserved,
+        idempotency_nested_record_comments_preserved
+    ),
+    (
+        "raw_string_hash_delimiter_preserves_comments",
+        ground_truth_raw_string_hash_delimiter_preserves_comments,
+        idempotency_raw_string_hash_delimiter_preserves_comments
+    ),
     (
         "catch_block_indentation_and_closing_brace_preserved_issue140",
         ground_truth_catch_block_indentation_and_closing_brace_preserved_issue140,