Repository: servo/rust-url
Pull request #783 (2022-08-03 22:45:31)

Add Natvis visualizations for the `Url` type
This change adds Natvis visualizations for the `Url` type in the `url` crate to help improve the debugging experience on Windows.

Debugging types such as the Url type under a Windows debugger, WinDbg or the Visual Studio debugger, can be a bit difficult to dissect exactly what data is contained in each type. The Rust compiler does have Natvis support for some types, but this is limited to some of the core libraries and not supported for external crates.

[RFC 3191](https://github.com/rust-lang/rfcs/pull/3191) proposes adding support for embedding debugging visualizations such as Natvis in a Rust crate. This RFC has been approved, merged and implemented.

Natvis is a framework that can be used to specify how types should be viewed under a supported debugger, such as the Windows debugger (WinDbg) and the Visual Studio debugger.

This PR adds:

* Natvis visualizations for the `Url` type in the `url` module.
* Tests for testing visualizers embedded in the url crate.
* Updates to the CI pipeline to ensure tests for visualizers are run so they do not break silently.
* Cleanups to the CI pipeline to manually select features to enable for testing, in addition to the default features. This allows unstable features to be enabled and tested separately from stable features.
* A new debugger_visualizer feature for the url crate to enable the unstable debugger_visualizer feature.

codecov-commenter: # [Codecov](https://codecov.io/gh/servo/rust-url/pull/783?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) Report
> Merging [#783](https://codecov.io/gh/servo/rust-url/pull/783?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) (46ec473) into [master](https://codecov.io/gh/servo/rust-url/commit/a72f83d00294ef7cbc7d1ec1ee870b27dafc0d3e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) (a72f83d) will **not change** coverage.
> The diff coverage is `n/a`.

```diff
@@           Coverage Diff           @@
##           master     #783   +/-   ##
=======================================
  Coverage   85.23%   85.23%           
=======================================
  Files          22       22           
  Lines        3894     3894           
=======================================
  Hits         3319     3319           
  Misses        575      575           
```


| [Impacted Files](https://codecov.io/gh/servo/rust-url/pull/783?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo) | Coverage Δ | |
|---|---|---|
| [url/src/lib.rs](https://codecov.io/gh/servo/rust-url/pull/783/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo#diff-dXJsL3NyYy9saWIucnM=) | `75.76% <ø> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Servo)

ridwanabdillahi: > This is great stuff, thank you. Please fix the warnings if possible.

Great, I'll fix the warnings and update the PR. Thanks for the review!
ridwanabdillahi: > Please fix the `CI / Test (ubuntu-latest, 1.45.0)` and cargo fmt lint.
> Thanks!

Sorry about the lint failures.

Looking at the CI/Test failure, this is actually caused by a new version of the serde_derive crate that was published and was not caused by this change. The version 1.0.143 uses a Cargo toml feature which was not stable in the version of Cargo used by the Rust 1.45.0 toolchain.

To fix this error, this crate would need to explicitly specify that only the versions before 1.0.143 of this crate are allowed to be used. Should I go ahead and make that as a separate change now?
valenting: > One solution would be to explicitly depend on version 1.0.142 or lower of these crates but I'm not sure if that's what we want to do here. Thoughts?

How about we don't run the serde CI task on 1.45.0?
ridwanabdillahi: > How about we don't run the serde CI task on 1.45.0?

From what I can tell there isn't a specific serde CI task. If you mean disabling the entire 1.45.0 CI job then that should probably be a separate change from this PR with a relevant issue opened and linked.

I'm not sure if the url crate has a minimum supported Rust version (MSRV) that's dependent on the 1.45.0 Rust toolchain but the crate maintainers would probably be better served to answer that question.


ridwanabdillahi: @valenting

I opened up an issue for this CI failure https://github.com/servo/rust-url/issues/785 and a corresponding PR https://github.com/servo/rust-url/pull/786 to update the minimum rust-version to 1.51.0 which is when the `resolver` feature was stabilized
bors-servo: :umbrella: The latest upstream changes (presumably #786) made this pull request unmergeable. Please resolve the merge conflicts.

Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 2d20266..a114a27 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -25,8 +25,6 @@ jobs:
             rust: beta
           - os: macos-latest
             rust: nightly
-          - os: windows-latest
-            rust: nightly
 
     runs-on: ${{ matrix.os }}
 
@@ -41,10 +39,27 @@ jobs:
         with:
           command: build
           args: --all-targets
-      - uses: actions-rs/cargo@v1
+        # Run tests
+      - name: Run tests
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+        # Run tests enabling the serde feature
+      - name: Run tests with the serde feature
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --features "url/serde"
+      # The #[debugger_visualizer] attribute is currently gated behind an unstable feature flag.
+      # In order to test the visualizers for the url crate, they have to be tested on a nightly build.
+      - name: Run debugger_visualizer tests
+        if: |
+          matrix.os == 'windows-latest' &&
+          matrix.rust == 'nightly'
+        uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --all-features
+          args: --test debugger_visualizer --features "url/serde,url/debugger_visualizer" -- --test-threads=1
 
   WASM:
     runs-on: ubuntu-latest
diff --git a/debug_metadata/README.md b/debug_metadata/README.md
new file mode 100644
index 0000000..785a454
--- /dev/null
+++ b/debug_metadata/README.md
@@ -0,0 +1,111 @@
+## Debugger Visualizers
+
+Many languages and debuggers enable developers to control how a type is
+displayed in a debugger. These are called "debugger visualizations" or "debugger
+views".
+
+The Windows debuggers (WinDbg\CDB) support defining custom debugger visualizations using
+the `Natvis` framework. To use Natvis, developers write XML documents using the natvis
+schema that describe how debugger types should be displayed with the `.natvis` extension.
+(See: https://docs.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2019)
+The Natvis files provide patterns which match type names a description of how to display
+those types.
+
+The Natvis schema can be found either online (See: https://code.visualstudio.com/docs/cpp/natvis#_schema)
+or locally at `<VS Installation Folder>\Xml\Schemas\1033\natvis.xsd`.
+
+The GNU debugger (GDB) supports defining custom debugger views using Pretty Printers.
+Pretty printers are written as python scripts that describe how a type should be displayed
+when loaded up in GDB/LLDB. (See: https://sourceware.org/gdb/onlinedocs/gdb/Pretty-Printing.html#Pretty-Printing)
+The pretty printers provide patterns, which match type names, and for matching
+types, descibe how to display those types. (For writing a pretty printer, see: https://sourceware.org/gdb/onlinedocs/gdb/Writing-a-Pretty_002dPrinter.html#Writing-a-Pretty_002dPrinter).
+
+### Embedding Visualizers
+
+Through the use of the currently unstable `#[debugger_visualizer]` attribute, the `url`
+crate can embed debugger visualizers into the crate metadata.
+
+Currently the two types of visualizers supported are Natvis and Pretty printers.
+
+For Natvis files, when linking an executable with a crate that includes Natvis files,
+the MSVC linker will embed the contents of all Natvis files into the generated `PDB`.
+
+For pretty printers, the compiler will encode the contents of the pretty printer
+in the `.debug_gdb_scripts` section of the `ELF` generated.
+
+### Testing Visualizers
+
+The `url` crate supports testing debugger visualizers defined for this crate. The entry point for
+these tests are `tests/debugger_visualizer.rs`. These tests are defined using the `debugger_test` and
+`debugger_test_parser` crates. The `debugger_test` crate is a proc macro crate which defines a
+single proc macro attribute, `#[debugger_test]`. For more detailed information about this crate,
+see https://crates.io/crates/debugger_test. The CI pipeline for the `url` crate has been updated
+to run the debugger visualizer tests to ensure debugger visualizers do not become broken/stale.
+
+The `#[debugger_test]` proc macro attribute may only be used on test functions and will run the
+function under the debugger specified by the `debugger` meta item.
+
+This proc macro attribute has 3 required values:
+
+1. The first required meta item, `debugger`, takes a string value which specifies the debugger to launch.
+2. The second required meta item, `commands`, takes a string of new line (`\n`) separated list of debugger
+commands to run.
+3. The third required meta item, `expected_statements`, takes a string of new line (`\n`) separated list of
+statements that must exist in the debugger output. Pattern matching through regular expressions is also
+supported by using the `pattern:` prefix for each expected statement.
+
+#### Example:
+
+```rust
+#[debugger_test(
+    debugger = "cdb",
+    commands = "command1\ncommand2\ncommand3",
+    expected_statements = "statement1\nstatement2\nstatement3")]
+fn test() {
+
+}
+```
+
+Using a multiline string is also supported, with a single debugger command/expected statement per line:
+
+```rust
+#[debugger_test(
+    debugger = "cdb",
+    commands = "
+command1
+command2
+command3",
+    expected_statements = "
+statement1
+pattern:statement[0-9]+
+statement3")]
+fn test() {
+    
+}
+```
+
+In the example above, the second expected statement uses pattern matching through a regular expression
+by using the `pattern:` prefix.
+
+#### Testing Locally
+
+Currently, only Natvis visualizations have been defined for the `url` crate via `debug_metadata/url.natvis`,
+which means the `tests/debugger_visualizer.rs` tests need to be run on Windows using the `*-pc-windows-msvc` targets.
+To run these tests locally, first ensure the debugging tools for Windows are installed or install them following
+the steps listed here, [Debugging Tools for Windows](https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/).
+Once the debugging tools have been installed, the tests can be run in the same manner as they are in the CI
+pipeline.
+
+#### Note
+
+When running the debugger visualizer tests, `tests/debugger_visualizer.rs`, they need to be run consecutively
+and not in parallel. This can be achieved by passing the flag `--test-threads=1` to rustc. This is due to
+how the debugger tests are run. Each test marked with the `#[debugger_test]` attribute launches a debugger
+and attaches it to the current test process. If tests are running in parallel, the test will try to attach
+a debugger to the current process which may already have a debugger attached causing the test to fail.
+
+For example:
+
+```
+cargo test --test debugger_visualizer --features debugger_visualizer -- --test-threads=1
+```
diff --git a/debug_metadata/url.natvis b/debug_metadata/url.natvis
new file mode 100644
index 0000000..3712470
--- /dev/null
+++ b/debug_metadata/url.natvis
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
+  <Type Name="url::Url">
+    <Intrinsic Name="ptr" Expression="serialization.vec.buf.ptr.pointer.pointer" />
+    <DisplayString>{serialization}</DisplayString>
+    <Expand>
+      <Synthetic Name="[scheme]">
+        <DisplayString>{(char*)(ptr()),[scheme_end]s8}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[username]" Condition="username_end > (scheme_end + 3)">
+        <!-- Add 3 to the scheme end to account for the scheme separator which is '://' -->
+        <DisplayString>{(char*)(ptr()+(scheme_end + 3)),[((username_end)-(scheme_end + 3))]s8}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[host]" Condition="host.tag != 0">
+        <DisplayString>{(char*)(ptr()+host_start),[host_end-host_start]s8}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[port]" Condition="port.tag == 1">
+        <DisplayString>{port.variant1.value.__0,d}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[path]">
+        <DisplayString Condition="query_start.tag == 0 &amp;&amp; fragment_start.tag == 0">{(char*)(ptr()+path_start),[(serialization.vec.len-path_start)]s8}</DisplayString>
+        <DisplayString Condition="query_start.tag == 1">{(char*)(ptr()+path_start),[(query_start.variant1.value.__0-path_start)]s8}</DisplayString>
+        <DisplayString Condition="fragment_start.tag == 1">{(char*)(ptr()+path_start),[(fragment_start.variant1.value.__0-path_start)]s8}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[query]" Condition="query_start.tag == 1">
+        <DisplayString Condition="fragment_start.tag == 0">{(char*)(ptr()+query_start.variant1.value.__0+1),[((serialization.vec.len)-(query_start.variant1.value.__0+1))]s8}</DisplayString>
+        <DisplayString Condition="fragment_start.tag == 1">{(char*)(ptr()+query_start.variant1.value.__0+1),[((fragment_start.variant1.value.__0)-(query_start.variant1.value.__0+1))]s8}</DisplayString>
+      </Synthetic>
+      <Synthetic Name="[fragment]" Condition="fragment_start.tag == 1">
+        <DisplayString>{(char*)(ptr()+fragment_start.variant1.value.__0+1),[(serialization.vec.len-fragment_start.variant1.value.__0-1)]s8}</DisplayString>
+      </Synthetic>
+    </Expand>
+  </Type>
+</AutoVisualizer>
diff --git a/url/Cargo.toml b/url/Cargo.toml
index 2231d75..1747e4a 100644
--- a/url/Cargo.toml
+++ b/url/Cargo.toml
@@ -23,6 +23,9 @@ appveyor = { repository = "Manishearth/rust-url" }
 [dev-dependencies]
 serde_json = "1.0"
 bencher = "0.1"
+# To test debugger visualizers defined for the url crate such as url.natvis
+debugger_test = "0.1"
+debugger_test_parser = "0.1"
 
 [dependencies]
 form_urlencoded = { version = "1.0.0", path = "../form_urlencoded" }
@@ -32,8 +35,17 @@ serde = {version = "1.0", optional = true, features = ["derive"]}
 
 [features]
 default = ["idna"]
+# UNSTABLE FEATURES (requires Rust nightly)
+# Enable to use the #[debugger_visualizer] attribute.
+debugger_visualizer = []
 
 [[bench]]
 name = "parse_url"
 path = "benches/parse_url.rs"
 harness = false
+
+[[test]]
+name = "debugger_visualizer"
+path = "tests/debugger_visualizer.rs"
+required-features = ["debugger_visualizer"]
+test = false
diff --git a/url/src/lib.rs b/url/src/lib.rs
index 163cc95..436590c 100644
--- a/url/src/lib.rs
+++ b/url/src/lib.rs
@@ -131,6 +131,11 @@ url = { version = "2", default-features = false }
 */
 
 #![doc(html_root_url = "https://docs.rs/url/2.2.2")]
+#![cfg_attr(
+    feature = "debugger_visualizer",
+    feature(debugger_visualizer),
+    debugger_visualizer(natvis_file = "../../debug_metadata/url.natvis")
+)]
 
 pub use form_urlencoded;
 


Test patch:
diff --git a/url/tests/debugger_visualizer.rs b/url/tests/debugger_visualizer.rs
new file mode 100644
index 0000000..4558e07
--- /dev/null
+++ b/url/tests/debugger_visualizer.rs
@@ -0,0 +1,102 @@
+use debugger_test::debugger_test;
+use url::Url;
+
+#[inline(never)]
+fn __break() {}
+
+#[debugger_test(
+    debugger = "cdb",
+    commands = "
+    .nvlist
+
+    dx base_url
+
+    dx url_with_non_special_scheme
+
+    dx url_with_user_pass_port_query_fragments
+
+    dx url_blob
+
+    dx url_with_base
+
+    dx url_with_base_replaced
+
+    dx url_with_comma",
+    expected_statements = r#"
+    pattern:debugger_visualizer-.*\.exe \(embedded NatVis ".*-[0-9]+\.natvis"\)
+
+    base_url         : "http://example.org/foo/bar" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "http"
+    [host]           : "example.org"
+    [path]           : "/foo/bar"
+
+    url_with_non_special_scheme : "non-special://test/x" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "non-special"
+    [host]           : "test"
+    [path]           : "/x"
+
+    url_with_user_pass_port_query_fragments : "http://user:pass@foo:21/bar;par?b#c" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "http"
+    [username]       : "user"
+    [host]           : "foo"
+    [port]           : 21
+    [path]           : "/bar;par"
+    [query]          : "b"
+    [fragment]       : "c"
+
+    url_blob         : "blob:https://example.com:443/" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "blob"
+    [path]           : "https://example.com:443/"
+
+    url_with_base    : "http://example.org/a%2fc" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "http"
+    [host]           : "example.org"
+    [path]           : "/a%2fc"
+
+    url_with_base_replaced : "http://[::7f00:1]/" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "http"
+    [host]           : "[::7f00:1]"
+    [path]           : "/"
+
+    url_with_comma   : "data:text/html,test#test" [Type: url::Url]
+    [<Raw View>]     [Type: url::Url]
+    [scheme]         : "data"
+    [path]           : "text/html,test"
+    [fragment]       : "test"
+    "#
+)]
+fn test_url_visualizer() {
+    // Copied from https://github.com/web-platform-tests/wpt/blob/master/url/
+    let base_url = Url::parse("http://example.org/foo/bar").unwrap();
+    assert_eq!(base_url.as_str(), "http://example.org/foo/bar");
+
+    let url_with_non_special_scheme = Url::parse("non-special://:@test/x").unwrap();
+    assert_eq!(url_with_non_special_scheme.as_str(), "non-special://test/x");
+
+    let url_with_user_pass_port_query_fragments =
+        Url::parse("http://user:pass@foo:21/bar;par?b#c").unwrap();
+    assert_eq!(
+        url_with_user_pass_port_query_fragments.as_str(),
+        "http://user:pass@foo:21/bar;par?b#c"
+    );
+
+    let url_blob = Url::parse("blob:https://example.com:443/").unwrap();
+    assert_eq!(url_blob.as_str(), "blob:https://example.com:443/");
+
+    let url_with_base = base_url.join("/a%2fc").unwrap();
+    assert_eq!(url_with_base.as_str(), "http://example.org/a%2fc");
+
+    let url_with_base_replaced = base_url.join("http://[::127.0.0.1]").unwrap();
+    assert_eq!(url_with_base_replaced.as_str(), "http://[::7f00:1]/");
+
+    let url_with_comma = base_url.join("data:text/html,test#test").unwrap();
+    assert_eq!(url_with_comma.as_str(), "data:text/html,test#test");
+
+    __break();
+}