[wit-parser] Don't trim leading/trailing whitespace from lines in doc comments (#1954)

* When stripping leading whitespace from lines of doc comments, find the minimum amount of whitespace common to all lines and strip that much, rather than going all-or-nothing.

* Update test expectations

* * Find min leading whitespace using iterators
* Rename intermediate value `d` to `contents`

* Manually update the blessed output for cli/wit-with-all-features

* When running tests, don't remove the trailing newline from the previously-generated output

* Count leading whitespace after stripping the leading comment tokens so that we actually _find_ the leading whitespace.

* Change `assert_output()` so that all test outputs have a single trailing newline. Most outputs already had this, but some had none.

* Re-run tests with `BLESS=1` after updating with latest from main

Author: mjoerussell

crates/wit-component/tests/interfaces/wasi-http.wat

@@ -49,7 +49,7 @@ interface types {
   }
 
   /// These cases are inspired by the IANA HTTP Proxy Error Types:
-  /// https://www.iana.org/assignments/http-proxy-status/http-proxy-status.xhtml#table-http-proxy-error-types
+  ///   https://www.iana.org/assignments/http-proxy-status/http-proxy-status.xhtml#table-http-proxy-error-types
   variant error-code {
     DNS-timeout,
     DNS-error(DNS-error-payload),

crates/wit-component/tests/interfaces/wasi-http/http.wit.print

@@ -1434,13 +1434,34 @@ impl<'a> Resolver<'a> {
 
     fn docs(&mut self, doc: &super::Docs<'_>) -> Docs {
         let mut docs = vec![];
+
         for doc in doc.docs.iter() {
-            if let Some(doc) = doc.strip_prefix("/**") {
-                docs.push(doc.strip_suffix("*/").unwrap().trim());
-            } else {
-                docs.push(doc.trim_start_matches('/').trim());
-            }
+            let contents = match doc.strip_prefix("/**") {
+                Some(doc) => doc.strip_suffix("*/").unwrap(),
+                None => doc.trim_start_matches('/'),
+            };
+
+            docs.push(contents.trim_end());
         }
+
+        // Scan the (non-empty) doc lines to find the minimum amount of leading whitespace.
+        // This amount of whitespace will be removed from the start of all doc lines,
+        // normalizing the output while retaining intentional spacing added by the original authors.
+        let min_leading_ws = docs
+            .iter()
+            .filter(|doc| !doc.is_empty())
+            .map(|doc| doc.bytes().take_while(|c| c.is_ascii_whitespace()).count())
+            .min()
+            .unwrap_or(0);
+
+        if min_leading_ws > 0 {
+            let leading_ws_pattern = " ".repeat(min_leading_ws);
+            docs = docs
+                .iter()
+                .map(|doc| doc.strip_prefix(&leading_ws_pattern).unwrap_or(doc))
+                .collect();
+        }
+
         let contents = if docs.is_empty() {
             None
         } else {

crates/wit-parser/src/ast/resolve.rs

@@ -9,7 +9,7 @@
       },
       "functions": {},
       "docs": {
-        "contents": "hello\nworld\nwhy, yes\nthis is a comment\n* this too */\n* is a comment */\n* this /* is /* a */ nested */ comment */"
+        "contents": " hello\n world\n why, yes\n this is a comment\n* this too */\n* is a comment */\n* this /* is /* a */ nested */ comment */"
       },
       "package": 0
     }

crates/wit-parser/tests/ui/comments.wit.json

@@ -203,7 +203,7 @@ fn assert_output(bless: bool, output: &[u8], path: &Path, tempdir: &TempDir) ->
     // sanitize the output to be consistent across platforms and handle per-test
     // differences such as `%tmpdir`, as well as the version number of the crate being
     // tested in the producers custom section.
-    let output = String::from_utf8_lossy(output)
+    let mut output = String::from_utf8_lossy(output)
         .replace(tempdir, "%tmpdir")
         .replace("\\", "/")
         .lines()
@@ -216,7 +216,14 @@ fn assert_output(bless: bool, output: &[u8], path: &Path, tempdir: &TempDir) ->
             }
         })
         .collect::<Vec<String>>()
-        .join("\n");
+        .join("\n")
+        .trim_end()
+        .to_string();
+
+    // Leave a single trailing newline on all test outputs
+    if !output.is_empty() {
+        output.push_str("\n");
+    }
 
     if bless {
         if output.is_empty() {
@@ -234,13 +241,9 @@ fn assert_output(bless: bool, output: &[u8], path: &Path, tempdir: &TempDir) ->
             Ok(())
         }
     } else {
-        let mut contents = std::fs::read_to_string(path)
+        let contents = std::fs::read_to_string(path)
             .with_context(|| format!("failed to read {path:?}"))?
             .replace("\r\n", "\n");
-        // Drop any trailing newline, the lines iterator on output above will do the same
-        if contents.ends_with('\n') {
-            contents.pop();
-        }
         if output != contents {
             bail!(
                 "failed test: result is not as expected:{}",

tests/cli.rs

@@ -12,4 +12,4 @@
 │ language ┆ bar [1]   │
 ├╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
 │ sdk      ┆ foo [2]   │
-╰──────────┴───────────╯
+╰──────────┴───────────╯

tests/cli/add-metadata-merge-sections.wat.stdout

@@ -6,4 +6,4 @@
 │ name  ┆ foo       │
 ├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
 │ range ┆ 0x0..0x15 │
-╰───────┴───────────╯
+╰───────┴───────────╯

tests/cli/add-metadata-overwrite-name.wat.stdout

@@ -12,4 +12,4 @@
 │ processed-by ┆ baz [1]    │
 ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤
 │ sdk          ┆ my-sdk [2] │
-╰──────────────┴────────────╯
+╰──────────────┴────────────╯

tests/cli/add-metadata.wat.stdout

@@ -13,4 +13,4 @@
  0x26 | 04 06 01 01 | ImportInfo([ImportInfo { module: "a", field: "a", flags: SymbolFlags(0x0) }])
       | 61 01 61 00
  0x2e | 03 04 01 01 | ExportInfo([ExportInfo { name: "a", flags: SymbolFlags(BINDING_WEAK | BINDING_LOCAL | UNDEFINED) }])
-      | 61 13      
+      | 61 13

tests/cli/dump-dylink0.wat.stdout

@@ -235,4 +235,4 @@
  0x25e | 01          | 1 count
  0x25f | 00 09 72 65 | Naming { index: 0, name: "real-wasi" }
        | 61 6c 2d 77
-       | 61 73 69   
+       | 61 73 69