Rollup merge of #78400 - GuillaumeGomez:fix-unindent, r=jyn514

Fix unindent in doc comments

Fixes #70732

r? ``@jyn514``

src/librustdoc/passes/unindent_comments.rs

@@ -1,7 +1,6 @@
 use std::cmp;
-use std::string::String;
 
-use crate::clean::{self, DocFragment, Item};
+use crate::clean::{self, DocFragment, DocFragmentKind, Item};
 use crate::core::DocContext;
 use crate::fold::{self, DocFolder};
 use crate::passes::Pass;
@@ -35,54 +34,73 @@ impl clean::Attributes {
 }
 
 fn unindent_fragments(docs: &mut Vec<DocFragment>) {
-    for fragment in docs {
-        fragment.doc = unindent(&fragment.doc);
-    }
-}
-
-fn unindent(s: &str) -> String {
-    let lines = s.lines().collect::<Vec<&str>>();
-    let mut saw_first_line = false;
-    let mut saw_second_line = false;
-    let min_indent = lines.iter().fold(usize::MAX, |min_indent, line| {
-        // After we see the first non-whitespace line, look at
-        // the line we have. If it is not whitespace, and therefore
-        // part of the first paragraph, then ignore the indentation
-        // level of the first line
-        let ignore_previous_indents =
-            saw_first_line && !saw_second_line && !line.chars().all(|c| c.is_whitespace());
-
-        let min_indent = if ignore_previous_indents { usize::MAX } else { min_indent };
-
-        if saw_first_line {
-            saw_second_line = true;
-        }
+    // `add` is used in case the most common sugared doc syntax is used ("/// "). The other
+    // fragments kind's lines are never starting with a whitespace unless they are using some
+    // markdown formatting requiring it. Therefore, if the doc block have a mix between the two,
+    // we need to take into account the fact that the minimum indent minus one (to take this
+    // whitespace into account).
+    //
+    // For example:
+    //
+    // /// hello!
+    // #[doc = "another"]
+    //
+    // In this case, you want "hello! another" and not "hello!  another".
+    let add = if docs.windows(2).any(|arr| arr[0].kind != arr[1].kind)
+        && docs.iter().any(|d| d.kind == DocFragmentKind::SugaredDoc)
+    {
+        // In case we have a mix of sugared doc comments and "raw" ones, we want the sugared one to
+        // "decide" how much the minimum indent will be.
+        1
+    } else {
+        0
+    };
 
+    // `min_indent` is used to know how much whitespaces from the start of each lines must be
+    // removed. Example:
+    //
+    // ///     hello!
+    // #[doc = "another"]
+    //
+    // In here, the `min_indent` is 1 (because non-sugared fragment are always counted with minimum
+    // 1 whitespace), meaning that "hello!" will be considered a codeblock because it starts with 4
+    // (5 - 1) whitespaces.
+    let min_indent = match docs
+        .iter()
+        .map(|fragment| {
+            fragment.doc.lines().fold(usize::MAX, |min_indent, line| {
                 if line.chars().all(|c| c.is_whitespace()) {
                     min_indent
                 } else {
-            saw_first_line = true;
-            let mut whitespace = 0;
-            line.chars().all(|char| {
-                // Compare against either space or tab, ignoring whether they
-                // are mixed or not
-                if char == ' ' || char == '\t' {
-                    whitespace += 1;
-                    true
-                } else {
-                    false
-                }
-            });
+                    // Compare against either space or tab, ignoring whether they are
+                    // mixed or not.
+                    let whitespace = line.chars().take_while(|c| *c == ' ' || *c == '\t').count();
                     cmp::min(min_indent, whitespace)
+                        + if fragment.kind == DocFragmentKind::SugaredDoc { 0 } else { add }
                 }
-    });
+            })
+        })
+        .min()
+    {
+        Some(x) => x,
+        None => return,
+    };
 
-    if !lines.is_empty() {
-        let mut unindented = vec![lines[0].trim_start().to_string()];
-        unindented.extend_from_slice(
-            &lines[1..]
-                .iter()
-                .map(|&line| {
+    for fragment in docs {
+        if fragment.doc.lines().count() == 0 {
+            continue;
+        }
+
+        let min_indent = if fragment.kind != DocFragmentKind::SugaredDoc && min_indent > 0 {
+            min_indent - add
+        } else {
+            min_indent
+        };
+
+        fragment.doc = fragment
+            .doc
+            .lines()
+            .map(|line| {
                 if line.chars().all(|c| c.is_whitespace()) {
                     line.to_string()
                 } else {
@@ -90,10 +108,7 @@ fn unindent(s: &str) -> String {
                     line[min_indent..].to_string()
                 }
             })
-                .collect::<Vec<_>>(),
-        );
-        unindented.join("\n")
-    } else {
-        s.to_string()
+            .collect::<Vec<_>>()
+            .join("\n");
     }
 }

src/librustdoc/passes/unindent_comments/tests.rs

@@ -1,72 +1,63 @@
 use super::*;
+use rustc_span::source_map::DUMMY_SP;
+
+fn create_doc_fragment(s: &str) -> Vec<DocFragment> {
+    vec![DocFragment {
+        line: 0,
+        span: DUMMY_SP,
+        parent_module: None,
+        doc: s.to_string(),
+        kind: DocFragmentKind::SugaredDoc,
+    }]
+}
+
+#[track_caller]
+fn run_test(input: &str, expected: &str) {
+    let mut s = create_doc_fragment(input);
+    unindent_fragments(&mut s);
+    assert_eq!(s[0].doc, expected);
+}
 
 #[test]
 fn should_unindent() {
-    let s = "    line1\n    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\nline2");
+    run_test("    line1\n    line2", "line1\nline2");
 }
 
 #[test]
 fn should_unindent_multiple_paragraphs() {
-    let s = "    line1\n\n    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\n\nline2");
+    run_test("    line1\n\n    line2", "line1\n\nline2");
 }
 
 #[test]
 fn should_leave_multiple_indent_levels() {
     // Line 2 is indented another level beyond the
     // base indentation and should be preserved
-    let s = "    line1\n\n        line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\n\n    line2");
+    run_test("    line1\n\n        line2", "line1\n\n    line2");
 }
 
 #[test]
 fn should_ignore_first_line_indent() {
-    // The first line of the first paragraph may not be indented as
-    // far due to the way the doc string was written:
-    //
-    // #[doc = "Start way over here
-    //          and continue here"]
-    let s = "line1\n    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\nline2");
+    run_test("line1\n    line2", "line1\n    line2");
 }
 
 #[test]
 fn should_not_ignore_first_line_indent_in_a_single_line_para() {
-    let s = "line1\n\n    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\n\n    line2");
+    run_test("line1\n\n    line2", "line1\n\n    line2");
 }
 
 #[test]
 fn should_unindent_tabs() {
-    let s = "\tline1\n\tline2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\nline2");
+    run_test("\tline1\n\tline2", "line1\nline2");
 }
 
 #[test]
 fn should_trim_mixed_indentation() {
-    let s = "\t    line1\n\t    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\nline2");
-
-    let s = "    \tline1\n    \tline2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1\nline2");
+    run_test("\t    line1\n\t    line2", "line1\nline2");
+    run_test("    \tline1\n    \tline2", "line1\nline2");
 }
 
 #[test]
 fn should_not_trim() {
-    let s = "\t    line1  \n\t    line2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1  \nline2");
-
-    let s = "    \tline1  \n    \tline2".to_string();
-    let r = unindent(&s);
-    assert_eq!(r, "line1  \nline2");
+    run_test("\t    line1  \n\t    line2", "line1  \nline2");
+    run_test("    \tline1  \n    \tline2", "line1  \nline2");
 }

src/test/rustdoc/unindent.md

@@ -0,0 +1 @@
+Just some text.

src/test/rustdoc/unindent.rs

@@ -0,0 +1,64 @@
+#![feature(external_doc)]
+
+#![crate_name = "foo"]
+
+// @has foo/struct.Example.html
+// @matches - '//pre[@class="rust rust-example-rendered"]' \
+//     '(?m)let example = Example::new\(\)\n    \.first\(\)\n    \.second\(\)\n    \.build\(\);\Z'
+/// ```rust
+/// let example = Example::new()
+///     .first()
+#[cfg_attr(not(feature = "one"), doc = "    .second()")]
+///     .build();
+/// ```
+pub struct Example;
+
+// @has foo/struct.F.html
+// @matches - '//pre[@class="rust rust-example-rendered"]' \
+//     '(?m)let example = Example::new\(\)\n    \.first\(\)\n    \.another\(\)\n    \.build\(\);\Z'
+///```rust
+///let example = Example::new()
+///    .first()
+#[cfg_attr(not(feature = "one"), doc = "    .another()")]
+///    .build();
+/// ```
+pub struct F;
+
+// @has foo/struct.G.html
+// @matches - '//pre[@class="rust rust-example-rendered"]' \
+//     '(?m)let example = Example::new\(\)\n\.first\(\)\n    \.another\(\)\n\.build\(\);\Z'
+///```rust
+///let example = Example::new()
+///.first()
+#[cfg_attr(not(feature = "one"), doc = "    .another()")]
+///.build();
+///```
+pub struct G;
+
+// @has foo/struct.H.html
+// @has - '//div[@class="docblock"]/p' 'no whitespace lol'
+///no whitespace
+#[doc = " lol"]
+pub struct H;
+
+// @has foo/struct.I.html
+// @matches - '//pre[@class="rust rust-example-rendered"]' '(?m)4 whitespaces!\Z'
+///     4 whitespaces!
+#[doc = "something"]
+pub struct I;
+
+// @has foo/struct.J.html
+// @matches - '//div[@class="docblock"]/p' '(?m)a\nno whitespace\nJust some text.\Z'
+///a
+///no whitespace
+#[doc(include = "unindent.md")]
+pub struct J;
+
+// @has foo/struct.K.html
+// @matches - '//pre[@class="rust rust-example-rendered"]' '(?m)4 whitespaces!\Z'
+///a
+///
+///    4 whitespaces!
+///
+#[doc(include = "unindent.md")]
+pub struct K;