fix(markdown): remove a fence's indentation before scanning its body

CHANGELOG.md

@@ -55,6 +55,11 @@ All notable changes to OComment will be documented here. The project follows
 
 ### Fixed
 
+- A fenced code block indented under a list item is read the way CommonMark reads it.
+  Up to as many spaces as the opening fence is indented come off each line of the body, and a tab that crosses that column leaves the rest of its width as spaces.
+  The body was scanned with the indentation still on, so a heredoc terminator written at the fence's indentation was not one, and the page was reported as invalid syntax with exit status 2.
+  Spans still name the bytes on the page, so `fix` removes a comment in such a block where it stands.
+
 - A mise file task's header is a load-bearing directive in every language.
   mise and its `usage` library read `#MISE`, `#USAGE`, `#[MISE]` and `#[USAGE]` — and the same after `//` — case-sensitively from the raw line, and `#MISE depends=`, `dir=` and `#USAGE flag` decide what the task runs and which arguments it accepts.
   They were read as prose, so `ocomment fix --tidy` under `wrap = "sentence"` joined a `#MISE` line and the `#USAGE flag` line under it into one `# MISE` line, and the task stopped declaring the flag.

ocaml/lib/ocomment_ref.ml

@@ -938,6 +938,37 @@ let without_c_line_splices source =
   { mapped = Bytes.of_string (Buffer.contents buffer);
     origins = Array.of_list (List.rev !origins); original_length = Bytes.length source }
 
+(** The body of a fenced code block whose opener is indented [indent] columns, with up to that many columns taken off the start of each line (CommonMark 4.5).
+    The body starts at the line break that ends the opener, so its first line is not a line of the block and keeps what it has.
+    A tab reaches the next multiple of four (CommonMark 2.2); when it crosses [indent] it is replaced by the spaces that remain past [indent], each of which maps back to the tab. *)
+let without_fence_indentation source indent =
+  let length = Bytes.length source in
+  let buffer = Buffer.create length and origins = ref [] in
+  let keep character origin =
+    Buffer.add_char buffer character;
+    origins := { start = origin; finish = origin + 1 } :: !origins in
+  let rec strip index column =
+    if column >= indent || index >= length then index
+    else match Bytes.get source index with
+      | ' ' -> strip (index + 1) (column + 1)
+      | '\t' ->
+        let stop = (column / 4 + 1) * 4 in
+        for _ = 1 to stop - indent do keep ' ' index done;
+        strip (index + 1) stop
+      | _ -> index in
+  let rec copy index =
+    if index < length then begin
+      let character = Bytes.get source index in
+      keep character index;
+      let next = index + 1 in
+      let ends_line = character = '\n'
+        || (character = '\r' && (next >= length || Bytes.get source next <> '\n')) in
+      copy (if ends_line then strip next 0 else next)
+    end in
+  copy 0;
+  { mapped = Bytes.of_string (Buffer.contents buffer);
+    origins = Array.of_list (List.rev !origins); original_length = length }
+
 let hex_digit = function
   | '0' .. '9' as value -> Some (Char.code value - Char.code '0')
   | 'a' .. 'f' as value -> Some (Char.code value - Char.code 'a' + 10)
@@ -4953,7 +4984,7 @@ let vue_style_language lang =
    }" opens an expression. *)
 
 (** One Markdown document.
-   An HTML comment is a comment, a fenced code block is scanned as the language its info string names, and a code span or indented block is opaque.
+   An HTML comment is a comment, a fenced code block is scanned as the language its info string names, over its lines with the opener's indentation taken off each, and a code span or indented block is opaque.
    Every construct is recognised at its own start and read forward, so no decision depends on a byte behind a restart. *)
 
 (** One Perl document.
@@ -5311,7 +5342,7 @@ let scan_markdown source language options accumulator =
     if word = "" then None else match language_of_string word with
       | Ok found -> Some found
       | Error _ -> None in
-  let fence opener marker run =
+  let fence opener marker run columns =
     let info_start = opener + run in
     let info_finish = line_end source info_start in
     let embedded = fence_language
@@ -5346,7 +5377,9 @@ let scan_markdown source language options accumulator =
     (match embedded with
      | Some (Html | Markdown | Vue | Svelte | Unknown) | None -> ()
      | Some embedded ->
-       let child_source = Bytes.sub source info_finish (content_finish - info_finish) in
+       let mapping = without_fence_indentation
+         (Bytes.sub source info_finish (content_finish - info_finish)) columns in
+       let child_source = mapping.mapped in
        let child = { comments_rev = []; diagnostics_rev = []; yaml_blocks_rev = [] } in
        (match embedded with
         | Css when options.dialect = Sass -> scan_sass child_source embedded options child
@@ -5370,15 +5403,16 @@ let scan_markdown source language options accumulator =
         | R -> scan_r child_source embedded options child
         | Perl -> scan_perl child_source embedded options child
         | Html | Markdown | Vue | Svelte | Unknown -> ());
+       let place span =
+         let span = mapped_span mapping span in
+         { start = span.start + info_finish; finish = span.finish + info_finish } in
        List.iter (fun (comment : comment) ->
-         accumulator.comments_rev <- { comment with span = {
-           start = comment.span.start + info_finish;
-           finish = comment.span.finish + info_finish } } :: accumulator.comments_rev)
+         accumulator.comments_rev <- { comment with span = place comment.span }
+           :: accumulator.comments_rev)
          (List.rev child.comments_rev);
        List.iter (fun (diagnostic : diagnostic) ->
-         accumulator.diagnostics_rev <- { diagnostic with span = {
-           start = diagnostic.span.start + info_finish;
-           finish = diagnostic.span.finish + info_finish } } :: accumulator.diagnostics_rev)
+         accumulator.diagnostics_rev <- { diagnostic with span = place diagnostic.span }
+           :: accumulator.diagnostics_rev)
          (List.rev child.diagnostics_rev));
     match closer with Some (_, resume) -> resume | None -> length in
   let indented start =
@@ -5407,7 +5441,7 @@ let scan_markdown source language options accumulator =
         let valid_info = marker <> '`'
           || not (String.contains
                     (Bytes.sub_string source (cursor + run) (info_finish - cursor - run)) '`') in
-        if run >= 3 && valid_info then loop (fence cursor marker run)
+        if run >= 3 && valid_info then loop (fence cursor marker run columns)
         else if marker = '`' then loop (inline_code_end cursor)
         else loop (index + 1)
       end else if Bytes.get source index = '`' then loop (inline_code_end index)

ocaml/test/test_core.ml

@@ -321,6 +321,20 @@ let check_markdown_commonmark_boundaries () =
   Alcotest.(check (list string)) "markdown comments"
     ["# r comment"; "<!-- visible -->"] (raw_comments source report)
 
+(* NOTE: A fence indented N spaces is read with up to N spaces off each line of its body, so the EOF under a list item ends the heredoc; a span still covers the page, indentation between its lines included. *)
+let check_markdown_indented_fence () =
+  let shell = Bytes.of_string
+    "1. Step:\n\n   ```sh\n   cat <<'EOF'\n   # heredoc text\n   EOF\n   echo done # remove\n   ```\n" in
+  let report = scan shell Markdown default_scan_options in
+  Alcotest.(check bool) "heredoc under a list item valid" true report.valid;
+  Alcotest.(check (list string)) "only the comment after the heredoc"
+    ["# remove"] (raw_comments shell report);
+  let block = Bytes.of_string
+    "1. Build:\n\n   ```c\n   /* one\n      two */\n   int x;\n   ```\n" in
+  let report = scan block Markdown default_scan_options in
+  Alcotest.(check (list string)) "block comment over the page"
+    ["/* one\n      two */"] (raw_comments block report)
+
 let check_perl_compound_opaque_constructs () =
   let source = Bytes.of_string
     "print $#items, $^X, $!, $1; # variables\nmy $q = \"escaped \\\" # opaque\"; # quote\n$x =~ s/foo#one/bar#two/g; # substitution\n$x =~ tr/a#b/c#d/; # transliteration\nprint <<  \"ONE\", <<~'TWO';\n# first\nONE\n  # second\n  TWO\n=pod\n# pod\n=cutlery\n# still pod\n=cut\nformat STDOUT =\n@<<<<\n# picture\n.\n# after format\n__DATA__\n# data\n" in
@@ -415,6 +429,7 @@ let () = Alcotest.run "ocomment-ref" [
     Alcotest.test_case "kotlin-multi-dollar" `Quick check_kotlin_multi_dollar_and_quote_runs;
     Alcotest.test_case "scala-characters" `Quick check_scala_characters_and_symbols;
     Alcotest.test_case "markdown-commonmark" `Quick check_markdown_commonmark_boundaries;
+    Alcotest.test_case "markdown-indented-fence" `Quick check_markdown_indented_fence;
     Alcotest.test_case "perl-compounds" `Quick check_perl_compound_opaque_constructs;
     Alcotest.test_case "sfc-attributes-and-sass" `Quick check_sfc_exact_attributes_and_sass;
     Alcotest.test_case "span-bounds" `Quick check_all_spans_are_bounded;

rust/ocomment-core/src/scanner.rs

@@ -1012,7 +1012,7 @@ impl<'a> Scanner<'a> {
                 0,
             );
             child.scan_c_family_unmapped();
-            self.merge_mapped(child, &mapped);
+            self.merge_mapped(child, &mapped, 0);
         } else {
             self.scan_c_family_unmapped();
         }
@@ -1683,20 +1683,28 @@ impl<'a> Scanner<'a> {
             }
             index += 1;
         }
-        self.merge_mapped(child, &mapped);
+        self.merge_mapped(child, &mapped, 0);
     }
 
-    fn merge_mapped(&mut self, child: Scanner<'_>, mapped: &MappedBytes) {
+    /// Fold in a child that scanned `mapped`, a remapped copy of this source from `origin` on.
+    /// The child reports spans in the copy's coordinates shifted by its own offset, and each comes back as the original bytes it was read from.
+    fn merge_mapped(&mut self, child: Scanner<'_>, mapped: &MappedBytes, origin: usize) {
+        let shift = child.offset;
+        let base = self.offset + origin;
+        let place = |span: ByteSpan| {
+            let copied = ByteSpan::new(
+                span.start.saturating_sub(shift),
+                span.end.saturating_sub(shift),
+            );
+            let original = mapped.original_span(copied);
+            ByteSpan::new(original.start + base, original.end + base)
+        };
         for mut comment in child.comments {
-            comment.span = mapped.original_span(comment.span);
-            comment.span.start += self.offset;
-            comment.span.end += self.offset;
+            comment.span = place(comment.span);
             self.comments.push(comment);
         }
         for mut diagnostic in child.diagnostics {
-            diagnostic.span = mapped.original_span(diagnostic.span);
-            diagnostic.span.start += self.offset;
-            diagnostic.span.end += self.offset;
+            diagnostic.span = place(diagnostic.span);
             self.diagnostics.push(diagnostic);
         }
     }
@@ -5633,7 +5641,7 @@ impl<'a> Scanner<'a> {
                     let valid_info =
                         marker != b'`' || !bytes[cursor + run..info_end].contains(&b'`');
                     if run >= 3 && valid_info {
-                        index = self.scan_markdown_fence(cursor, marker, run);
+                        index = self.scan_markdown_fence(cursor, marker, run, indent);
                         continue;
                     }
                 }
@@ -5653,9 +5661,18 @@ impl<'a> Scanner<'a> {
         }
     }
 
-    /// One fenced code block, beginning at its opening run of backticks or tildes.
+    /// One fenced code block, beginning at its opening run of backticks or tildes, which stands `indent` columns in.
     /// The body is scanned as the language its info string names, or as nothing when the string names none; the block ends at a line of the same marker with a run at least as long as the opener's, and a block that never closes is a file CommonMark reads to its end, so the rest of the document is opaque.
-    fn scan_markdown_fence(&mut self, opener: usize, marker: u8, run: usize) -> usize {
+    ///
+    /// An indented opener takes up to as much indentation off every line of the body, per CommonMark 4.5, and the body is scanned as those lines rather than as the bytes on the page: a heredoc terminator written at the fence's indentation under a list item is at the start of its line.
+    /// The spans come back to the page through [`MappedBytes`].
+    fn scan_markdown_fence(
+        &mut self,
+        opener: usize,
+        marker: u8,
+        run: usize,
+        indent: usize,
+    ) -> usize {
         let bytes = self.source;
         let info_start = opener + run;
         let info_end = line_end(bytes, info_start);
@@ -5696,15 +5713,29 @@ impl<'a> Scanner<'a> {
                 Language::Html | Language::Vue | Language::Svelte | Language::Markdown
             )
         {
-            let mut child = Scanner::child(
-                &bytes[info_end..content_end],
-                language,
-                self.options.clone(),
-                self.patterns.clone(),
-                self.offset + info_end,
-            );
-            child.scan_language();
-            self.merge_child(child);
+            let content = &bytes[info_end..content_end];
+            if indent == 0 {
+                let mut child = Scanner::child(
+                    content,
+                    language,
+                    self.options.clone(),
+                    self.patterns.clone(),
+                    self.offset + info_end,
+                );
+                child.scan_language();
+                self.merge_child(child);
+            } else {
+                let mapped = MappedBytes::without_fence_indentation(content, indent);
+                let mut child = Scanner::child(
+                    &mapped.bytes,
+                    language,
+                    self.options.clone(),
+                    self.patterns.clone(),
+                    self.offset + info_end,
+                );
+                child.scan_language();
+                self.merge_mapped(child, &mapped, info_end);
+            }
         }
         closer.map_or(bytes.len(), |(_, resume)| resume)
     }
@@ -10680,6 +10711,50 @@ impl MappedBytes {
         }
     }
 
+    /// The body of a fenced code block whose opener stands `indent` columns in, as CommonMark 4.5 hands it on: up to `indent` columns of indentation come off the start of every line.
+    ///
+    /// `content` begins at the line terminator that ends the opener, so its first line is the empty rest of that one and loses nothing.
+    /// A tab counts to the next multiple of four, per CommonMark 2.2, and one that runs past `indent` is taken off and the columns it had left are put back as spaces — `commonmark` 0.31.2 and `markdown-it` 15.0.2 both read `\tEOF` under a three-space opener as ` EOF`.
+    /// Each of those spaces maps back to the tab.
+    fn without_fence_indentation(content: &[u8], indent: usize) -> Self {
+        let mut bytes = Vec::with_capacity(content.len());
+        let mut origins = Vec::with_capacity(content.len());
+        let mut index = 0;
+        let mut line_start = false;
+        while index < content.len() {
+            if line_start {
+                line_start = false;
+                let mut column = 0;
+                while column < indent && index < content.len() {
+                    match content[index] {
+                        b' ' => column += 1,
+                        b'\t' => {
+                            let stop = column + 4 - column % 4;
+                            for _ in indent..stop {
+                                bytes.push(b' ');
+                                origins.push(ByteSpan::new(index, index + 1));
+                            }
+                            column = stop;
+                        }
+                        _ => break,
+                    }
+                    index += 1;
+                }
+                continue;
+            }
+            let byte = content[index];
+            bytes.push(byte);
+            origins.push(ByteSpan::new(index, index + 1));
+            index += 1;
+            line_start = byte == b'\n' || (byte == b'\r' && content.get(index) != Some(&b'\n'));
+        }
+        Self {
+            bytes,
+            origins,
+            original_len: content.len(),
+        }
+    }
+
     fn java_unicode(source: &[u8]) -> (Self, Vec<ByteSpan>) {
         let mut bytes = Vec::with_capacity(source.len());
         let mut origins = Vec::with_capacity(source.len());
@@ -10825,6 +10900,40 @@ mod tests {
         );
     }
 
+    /// The copy an indented fence's body is scanned over, and where each of its bytes came from.
+    /// The body begins at the break that ends the opener's line, every line after a LF, a CRLF or a bare CR loses up to the opener's three columns, and a tab that crosses the third leaves its fourth behind as a space that maps back to the tab.
+    /// A span across lines comes back with the indentation between them.
+    #[test]
+    fn a_fence_body_loses_the_opener_indentation_and_maps_back() {
+        let content = b"\n    a\r\n  b\r\tc\n";
+        let mapped = MappedBytes::without_fence_indentation(content, 3);
+        assert_eq!(mapped.bytes, b"\n a\r\nb\r c\n");
+        assert_eq!(
+            mapped
+                .origins
+                .iter()
+                .map(|origin| origin.start)
+                .collect::<Vec<_>>(),
+            vec![0, 4, 5, 6, 7, 10, 11, 12, 13, 14]
+        );
+        assert_eq!(
+            mapped.original_span(ByteSpan::new(1, 6)),
+            ByteSpan::new(4, 11)
+        );
+        assert_eq!(
+            mapped.original_span(ByteSpan::new(7, 9)),
+            ByteSpan::new(12, 14)
+        );
+        assert_eq!(
+            mapped.original_span(ByteSpan::new(8, 10)),
+            ByteSpan::new(13, 15)
+        );
+        assert_eq!(
+            mapped.original_span(ByteSpan::new(5, 5)),
+            ByteSpan::new(10, 10)
+        );
+    }
+
     /// [`parse_heredoc`] is a lookahead with no line bound — a quoted delimiter word may carry a line terminator as content, so `<<"EO`, a break, `F"` names the delimiter `EO\nF` — and every path that gives up rewinds the scan to the byte after the operator and lexes those bytes again from a state this parse already decided out of them.
     /// The reach it reports is what withdraws the checkpoints in between, so it is asserted here against the parse itself rather than only through a document whose checkpoints might move for some other reason.
     #[test]