Add a wast subcommand to the CLI (#2096)

* Add a `wast` subcommand to the CLI

This commit migrates the `tests/roundtrip.rs` file to a `wast`
subcommand of the CLI. This does not actually run tests in terms of
executing WebAssembly but it works as a way to parse the test and
validate, so everything short of runtime execution and related errors.

* Migrate `tests/local/*` to `tests/cli/*`

With the addition of the `wast` subcommand in the previous commit
there's no longer any need to have `tests/local/*`. Additionally it's
now much easier to perform per-test configuration since the flags for
the test are in the test itself.

* Migrate spec tests to `tests/cli/*`

Similarly to the previous commit of moving towards `wasm-tools wast`
this commit migrates the spec test infrastructure to using this as well.
Spec tests are now run by:

* A script, `ci/generate-spec-tests.rs`, generates files in
  `tests/cli/spec/*.wast` to run each spec test.
* Each test lists the features needed to be enabled for the test to run.
* The tests are then naturally run during the `--test cli` test suite run.

New automation is added to ensure that the version in-tree is up-to-date
to ensure we don't creep in too many files by accident.

* Remove the roundtrip test suite

No longer needed!

* Handle some rebase conflicts

* Update submodules in new CI job

* Update testing and contributing docs

* Fix typo in README

* Sync snapshots

Author: alexcrichton

.github/workflows/main.yml

@@ -237,6 +237,7 @@ jobs:
       - run: cargo check --no-default-features --features addr2line
       - run: cargo check --no-default-features --features json-from-wast
       - run: cargo check --no-default-features --features completion
+      - run: cargo check --no-default-features --features wast
       - run: cargo check --no-default-features -p wit-parser
       - run: cargo check --no-default-features -p wit-parser --features wat
       - run: cargo check --no-default-features -p wit-parser --features serde
@@ -283,6 +284,22 @@ jobs:
             exit 1
           fi
 
+  # Double-check that files and such related to the `tests/cli` test suite are
+  # up-to-date.
+  generated_files_up_to_date:
+    name: Check generated files are up-to-date
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
+      - uses: ./.github/actions/install-rust
+      - run: rm -rf tests/snapshots
+      - run: rustc ci/generate-spec-tests.rs && ./generate-spec-tests
+      - run: find tests/cli -name '*.stderr' | xargs rm
+      - run: BLESS=1 cargo test --test cli
+      - run: git diff --exit-code
+
   doc:
     runs-on: ubuntu-latest
     steps:
@@ -329,6 +346,7 @@ jobs:
       - test_extra_features
       - test-prefer-btree-collections
       - clippy
+      - generated_files_up_to_date
     if: always()
 
     steps:

CONTRIBUTING.md

@@ -47,49 +47,48 @@ and then using Cargo to execute all tests:
 $ cargo test --workspace
 ```
 
-Running the spec test suite can be done with:
+The majority of all tests is located in `tests/cli/*` and can be run with:
 
 ```
-$ cargo test --test roundtrip
+$ cargo test --test cli
 ```
 
-and running a single spec test can be done with an argument to this command as a
-string filter on the filename.
+Each individual file is a standalone test and documents at the top how to run
+it. Running individual tests can be done through:
+
+```
+$ cargo test --test cli -- test_name_filter
+```
+
+Or you can use `cargo run` plus the test's arguments to run a single test.
 
 ```
-$ cargo test --test roundtrip binary-leb128.wast
+$ cargo run wast tests/cli/empty.wast
 ```
 
-Many tests are also located in the top-level `tests/*` folder. This is organized
-into a few suites:
+Running just the spec test suite can be done with:
 
-* `tests/cli/*` - these files are run by the `tests/cli.rs` test file and are
-  intended to be tests for the CLI itself. They start with `;; RUN: ...` headers
-  to indicate what commands should run and adjacent files indicate the expected
-  output.
+```
+$ cargo test --test cli -- spec
+```
 
-* `tests/local/*` - these are handwritten `*.wat` and `*.wast` tests. The
-  `*.wat` files must all validate as valid modules and `*.wast` files run their
-  directives in the same manner as the spec test suite. This folder additional
-  subfolders for specific classes of tests, for example `missing-features` has
-  all optional wasm features disabled to test what happens when a feature is
-  implemented but disabled at runtime. The `component-model` folder contains all
-  tests related to enabling the component model feature.
+and running a single spec test can be done with an argument to this command as a
+string filter on the filename.
 
-* `tests/testsuite` - this is a git submodule pointing to the [upstream test
-  suite repository](https://github.com/WebAssembly/testsuite/) and is where spec
-  tests come from.
+```
+$ cargo test --test cli binary-leb128.wast
+$ cargo run wast tests/testsuite/binary-leb128.wast --ignore-error-messages
+```
 
-* `tests/roundtrip.rs` - this is the main driver for the `local` and `testsuite`
-  folders. This will crawl over all files in those folders and execute what
-  tests it can. This means running `*.wast` directives such as `assert_invalid`.
-  Additionally all valid wasm modules are printed with `wasmprinter` and then
-  parsed again with `wat` to ensure that they can be round-tripped through the
-  crates.
+The `tests/cli` tests suite is documented as a self-describing test at
+`tests/cli/readme.wat`. Each test describes what subcommand of `wasm-tools` is
+run and the test is itself the input.
 
-* `tests/snapshots` - this contains golden output files which correspond to the
-  `wasmprinter`-printed version of binaries of all tests. These files are used
-  to view the impact of changes to `wasmprinter`.
+The `tests/testsuite` folder is a git submodule pointing to the [upstream test
+suite repository](https://github.com/WebAssembly/testsuite/) and is where spec
+tests come from. The `tests/snapshots` folder contains golden output files
+which correspond to the `wasmprinter`-printed version of binaries of all tests.
+These files are used to view the impact of changes to `wasmprinter`.
 
 Many tests throughout the repository have automatically generated files
 associated with them which reflect the expected output of an operation. This is

Cargo.toml

@@ -178,6 +178,9 @@ wit-smith = { workspace = true, features = ["clap"], optional = true }
 addr2line = { version = "0.24.0", optional = true }
 gimli = { workspace = true, optional = true }
 
+# Dependencies of `wast`
+pretty_assertions = { workspace = true, optional = true }
+
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
 is_executable = { version = "1.0.1", optional = true }
 
@@ -193,10 +196,6 @@ wast = { workspace = true }
 name = "cli"
 harness = false
 
-[[test]]
-name = "roundtrip"
-harness = false
-
 [features]
 # By default, all subcommands are built
 default = [
@@ -217,6 +216,7 @@ default = [
   'addr2line',
   'completion',
   'json-from-wast',
+  'wast',
 ]
 
 # Each subcommand is gated behind a feature and lists the dependencies it needs
@@ -253,3 +253,13 @@ wit-smith = ['dep:wit-smith', 'arbitrary']
 addr2line = ['dep:addr2line', 'dep:gimli', 'dep:wasmparser']
 completion = ['dep:clap_complete']
 json-from-wast = ['dep:serde_derive', 'dep:serde_json', 'dep:wast', 'dep:serde']
+wast = [
+  'dep:wast',
+  'wasm-encoder/wasmparser',
+  'dep:pretty_assertions',
+  'validate',
+  # These subcommands are executed from `wasm-tools wast` so make sure they're
+  # built-in if this is enabled.
+  'dump',
+  'json-from-wast',
+]

README.md

@@ -155,6 +155,7 @@ that can be use programmatically as well:
 | `wasm-tools addr2line` |  |  | Translate wasm offsets to filename/line numbers with DWARF |
 | `wasm-tools completion` |  |  | Generate shell completion scripts for `wasm-tools` |
 | `wasm-tools json-from-wast` |  |  | Convert a `*.wast` file into JSON commands |
+| `wasm-tools wast` |  |  | Validate the structure of a `*.wast` file |
 
 [wasmparser]: https://crates.io/crates/wasmparser
 [wat]: https://crates.io/crates/wat

ci/generate-spec-tests.rs

@@ -0,0 +1,100 @@
+//! Helper program to generate files in `tests/cli/spec/*` which correspond to
+//! running spec tests in `tests/testsuite/*`.
+
+use std::fs;
+use std::path::Path;
+
+fn main() {
+    let _ = fs::remove_dir_all("./tests/cli/spec");
+    copy_tests("tests/testsuite".as_ref(), "tests/cli/spec".as_ref());
+}
+
+/// Recursively visit `src` and, for all test files, create a file in `dst` to
+/// run the test.
+fn copy_tests(src: &Path, dst: &Path) {
+    fs::create_dir(&dst).unwrap();
+    for entry in src.read_dir().unwrap() {
+        let entry = entry.unwrap();
+
+        let src = entry.path();
+        let dst = dst.join(entry.file_name());
+        if entry.file_type().unwrap().is_dir() {
+            copy_tests(&src, &dst);
+            continue;
+        }
+
+        if src.extension().and_then(|s| s.to_str()) != Some("wast") {
+            continue;
+        }
+
+        copy_test(&src, &dst);
+    }
+}
+
+/// Creates `dst` as a file to run `src` as a test.
+fn copy_test(src: &Path, dst: &Path) {
+    // The legacy exception-handling proposal is not currently supported because
+    // it uses the folded form of s-expressions which are not implemented here.
+    // Regardless just skip these spec tests.
+    if src.iter().any(|p| p == "legacy") {
+        return;
+    }
+
+    let mut contents = format!(";; RUN: wast \\\n");
+    contents.push_str(";;      --assert default \\\n");
+    contents.push_str(";;      --snapshot tests/snapshots \\\n");
+
+    // This test specifically tests various forms of unicode which are
+    // default-disallowed, so explicitly allow it here.
+    if src.ends_with("names.wast") {
+        contents.push_str(";;      --allow-confusing-unicode \\\n");
+    }
+
+    // Historically wasm-tools tried to match the upstream error message. This
+    // generally led to a large sequence of matches here which is not easy to
+    // maintain and is particularly difficult when test suites and proposals
+    // conflict with each other (e.g. one asserts one error message and another
+    // asserts a different error message). Overall we didn't benefit a whole lot
+    // from trying to match errors so just assume the error is roughly the same
+    // and otherwise don't try to match it.
+    contents.push_str(";;      --ignore-error-messages \\\n");
+
+    // Push a `--features=..` flag for the spec tests. Spec tests often need a
+    // precise set of features different from the defaults of `wasm-tools` so
+    // it's always overridden here.
+    let features = match find_proposal(src) {
+        None => "wasm2",
+        Some("annotations") => "wasm2",
+        Some("threads") => "wasm1,threads",
+        Some("function-references") => "wasm2,function-references,tail-call",
+        Some("wasm-3.0") => "wasm3",
+        Some("gc") => "wasm2,function-references,gc,tail-call",
+        Some("multi-memory") => "wasm2,multi-memory",
+        Some("extended-const") => "wasm2,extended-const",
+        Some("exception-handling") => "wasm2,exceptions,tail-call",
+        Some("custom-page-sizes") => "wasm3,custom-page-sizes",
+        Some("wide-arithmetic") => "wasm2,wide-arithmetic",
+        Some("tail-call") => "wasm2,tail-call",
+        Some("relaxed-simd") => "wasm2,relaxed-simd",
+        Some("memory64") => "wasm3",
+        Some(proposal) => panic!("unsupported proposal: {}", proposal),
+    };
+    contents.push_str(&format!(";;      --features={features} \\\n"));
+
+    // And finally push a path to the test itself.
+    contents.push_str(&format!(";;      {}\n", src.display()));
+
+    fs::write(dst, contents).unwrap();
+}
+
+/// Finds the wasm proposal, if present, within `src`.
+fn find_proposal(src: &Path) -> Option<&str> {
+    // Look for `foo` in `.../proposals/foo/...`
+    let mut parts = src.iter();
+    while let Some(next) = parts.next() {
+        if next.to_str() == Some("proposals") {
+            return parts.next()?.to_str();
+        }
+    }
+    None
+}

src/bin/wasm-tools/main.rs

@@ -77,6 +77,7 @@ subcommands! {
     (completion, "completion")
     #[command(alias = "wast2json")]
     (json_from_wast, "json-from-wast")
+    (wast, "wast")
 }
 
 // when all features are disabled then `WasmTools` is an empty enum so suppress

src/bin/wasm-tools/validate.rs

@@ -35,6 +35,16 @@ Examples:
     $ wasm-tools validate --features=mvp mvp.wasm
 ")]
 pub struct Opts {
+    #[clap(flatten)]
+    features: CliFeatures,
+
+    #[clap(flatten)]
+    io: wasm_tools::InputOutput,
+}
+
+// Helper structure extracted used to parse the feature flags for `validate`.
+#[derive(clap::Parser)]
+pub struct CliFeatures {
     /// Comma-separated list of WebAssembly features to enable during
     /// validation.
     ///
@@ -55,9 +65,6 @@ pub struct Opts {
     /// <https://github.com/bytecodealliance/wasm-tools/blob/main/crates/wasmparser/src/features.rs>
     #[clap(long, short = 'f', value_parser = parse_features)]
     features: Vec<Vec<FeatureAction>>,
-
-    #[clap(flatten)]
-    io: wasm_tools::InputOutput,
 }
 
 #[derive(Clone)]
@@ -99,26 +106,6 @@ impl Opts {
         }
     }
 
-    fn features(&self) -> Result<WasmFeatures> {
-        let mut ret = WasmFeatures::default();
-
-        for action in self.features.iter().flat_map(|v| v) {
-            match action {
-                FeatureAction::Enable(features) => {
-                    ret |= *features;
-                }
-                FeatureAction::Disable(features) => {
-                    ret &= !*features;
-                }
-                FeatureAction::Reset(features) => {
-                    ret = *features;
-                }
-            }
-        }
-
-        Ok(ret)
-    }
-
     fn validate(&self, wasm: &[u8]) -> Result<()> {
         // Note that here we're copying the contents of
         // `Validator::validate_all`, but the end is followed up with a parallel
@@ -130,7 +117,7 @@ impl Opts {
         // `Validator` we're using as we navigate nested modules (the module
         // linking proposal) and any functions found are deferred to get
         // validated later.
-        let mut validator = Validator::new_with_features(self.features()?);
+        let mut validator = Validator::new_with_features(self.features.features());
         let mut functions_to_validate = Vec::new();
 
         let start = Instant::now();
@@ -219,6 +206,28 @@ impl Opts {
     }
 }
 
+impl CliFeatures {
+    pub fn features(&self) -> WasmFeatures {
+        let mut ret = WasmFeatures::default();
+
+        for action in self.features.iter().flat_map(|v| v) {
+            match action {
+                FeatureAction::Enable(features) => {
+                    ret |= *features;
+                }
+                FeatureAction::Disable(features) => {
+                    ret &= !*features;
+                }
+                FeatureAction::Reset(features) => {
+                    ret = *features;
+                }
+            }
+        }
+
+        ret
+    }
+}
+
 fn parse_features(arg: &str) -> Result<Vec<FeatureAction>> {
     let mut ret = Vec::new();