Skip to content

Commit

Permalink
[wit-parser] Don't trim leading/trailing whitespace from lines in doc…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
mjoerussell authored Jan 8, 2025
1 parent a43321d commit 34c5c39
Show file tree
Hide file tree
Showing 34 changed files with 65 additions and 42 deletions.
2 changes: 1 addition & 1 deletion crates/wit-component/tests/interfaces/wasi-http.wat

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
31 changes: 26 additions & 5 deletions crates/wit-parser/src/ast/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/wit-parser/tests/ui/comments.wit.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
17 changes: 10 additions & 7 deletions tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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() {
Expand All @@ -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:{}",
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/add-metadata-merge-sections.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
│ language ┆ bar [1] │
├╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
│ sdk ┆ foo [2] │
╰──────────┴───────────╯
╰──────────┴───────────╯
2 changes: 1 addition & 1 deletion tests/cli/add-metadata-overwrite-name.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
│ name ┆ foo │
├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
│ range ┆ 0x0..0x15 │
╰───────┴───────────╯
╰───────┴───────────╯
2 changes: 1 addition & 1 deletion tests/cli/add-metadata.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
│ processed-by ┆ baz [1] │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤
│ sdk ┆ my-sdk [2] │
╰──────────────┴────────────╯
╰──────────────┴────────────╯
2 changes: 1 addition & 1 deletion tests/cli/dump-dylink0.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/cli/dump/bundled.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/cli/dump/component-inline-export-import.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
0x25 | 00 00 04 00 | export ComponentExport { name: ComponentExportName(""), kind: Component, index: 0, ty: None }
| 00
0x2a | 00 01 61 00 | export ComponentExport { name: ComponentExportName("a"), kind: Module, index: 0, ty: None }
| 11 00 00
| 11 00 00
2 changes: 1 addition & 1 deletion tests/cli/dump/component-inline-type.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@
0x37 | 0a 06 | component import section
0x39 | 01 | 1 count
0x3a | 00 01 64 01 | [func 0] ComponentImport { name: ComponentImportName("d"), ty: Func(2) }
| 02
| 02
2 changes: 1 addition & 1 deletion tests/cli/dump/component-instance-type.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@
0x43 | 01 09 03 | type name section
0x46 | 01 | 1 count
0x47 | 00 05 6f 75 | Naming { index: 0, name: "outer" }
| 74 65 72
| 74 65 72
2 changes: 1 addition & 1 deletion tests/cli/dump/instance-type.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
| 00 00 01 00
0x17 | 42 02 01 42 | [type 1] Instance([Type(Instance([])), Export { name: ComponentExportName(""), ty: Instance(0) }])
| 00 04 00 00
| 05 00
| 05 00
2 changes: 1 addition & 1 deletion tests/cli/dump/module-types.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@
0x54 | 01 09 03 | type name section
0x57 | 01 | 1 count
0x58 | 00 05 65 6d | Naming { index: 0, name: "empty" }
| 70 74 79
| 70 74 79
2 changes: 1 addition & 1 deletion tests/cli/dump/nested-component.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,4 @@
0xbc | 0b 07 | component export section
0xbe | 01 | 1 count
0xbf | 00 01 61 04 | export ComponentExport { name: ComponentExportName("a"), kind: Component, index: 3, ty: None }
| 03 00
| 03 00
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.simple-rename.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.simple-toplevel.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.simple.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.toplevel-deps.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.tricky-import.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.trim-imports.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/importize.wit.with-deps.stdout
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// RUN[simple]: component wit --importize-world simple %
/// RUN[simple-rename]: component wit --importize-world simple-rename --importize-out-world-name test-rename %
/// RUN[simple-component]: component embed --dummy --world simple % | /
/// component wit --importize
/// component wit --importize
/// RUN[with-deps]: component wit --importize-world with-deps %
/// RUN[simple-toplevel]: component wit --importize-world simple-toplevel %
/// RUN[toplevel-deps]: component wit --importize-world toplevel-deps %
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/metadata-add-component.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
│ name ┆ <unknown> │
├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
│ range ┆ 0xa..0x31 │
╰───────┴───────────╯
╰───────┴───────────╯
2 changes: 1 addition & 1 deletion tests/cli/metadata-component.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@
│ name ┆ another submodule │
├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤
│ range ┆ 0x27..0x4a │
╰───────┴───────────────────╯
╰───────┴───────────────────╯
2 changes: 1 addition & 1 deletion tests/cli/metadata.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
│ name ┆ <unknown> │
├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
│ range ┆ 0x0..0x8 │
╰───────┴───────────╯
╰───────┴───────────╯
2 changes: 1 addition & 1 deletion tests/cli/print-code-section-overflow.wat.stdout
Original file line number Diff line number Diff line change
@@ -1 +1 @@
(module
(module
2 changes: 1 addition & 1 deletion tests/cli/print-custom-indent-width.wat.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
(func (;0;) (type 0) (param i32) (result i32)
local.get 0
)
)
)
2 changes: 1 addition & 1 deletion tests/cli/print-dont-reserve-the-world.wat.stdout
Original file line number Diff line number Diff line change
@@ -1 +1 @@
(module
(module
2 changes: 1 addition & 1 deletion tests/cli/print-locals-overflow.wat.stdout
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
(module
(type (;0;) (func))
(func (;0;) (type 0)
(func (;0;) (type 0)
2 changes: 1 addition & 1 deletion tests/cli/print-memarg-too-big.wat.stdout
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
(module
(data (;0;) (i32.load16_s
(data (;0;) (i32.load16_s
Binary file modified tests/cli/unbundle-print-module.wat.gen.stdout
Binary file not shown.
Binary file modified tests/cli/wat2wasm-alias.wat.stdout
Binary file not shown.
1 change: 0 additions & 1 deletion tests/cli/wit-with-all-features.wit.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,3 @@ interface bar {
y: t,
}
}

0 comments on commit 34c5c39

Please sign in to comment.