Fix and test shell escaping

It had problems with single quotes on Linux, and lots of problems on
Windows. This is necessary for my new tests to pass on Windows, and I
discovered the Linux bug while writing the test.

test/unit/rustdoc/rustdoc_lib.rs

@@ -12,6 +12,17 @@ make_answer!();
 #[cfg(feature = "with_build_script")]
 pub const CONST: &str = env!("CONST");
 
+/// A value with all characters that might need special handling. Note that this
+/// is duplicated in the Starlark code, which attempts to pass it.
+pub const SPECIAL_VALUE: &str = ")(][ \"\\/&^%><;:\t$#@!*-_=+`~.,?x";
+
+/// ```
+/// assert_eq!(test_crate::SPECIAL_VALUE, test_crate::SPECIAL_VALUE_FROM_RUSTC_ENV);
+/// // Also try grabbing it from the environment when the doctest is built.
+/// assert_eq!(test_crate::SPECIAL_VALUE, env!("SPECIAL_VALUE"));
+/// ```
+pub const SPECIAL_VALUE_FROM_RUSTC_ENV: &str = env!("SPECIAL_VALUE");
+
 /// The answer to the ultimate question
 /// ```
 /// fn answer() -> u32 { 42 }
@@ -35,4 +46,13 @@ mod test {
     fn test_answer() {
         assert_eq!(answer(), 42);
     }
+
+    #[test]
+    fn test_special_value() {
+        assert_eq!(SPECIAL_VALUE, SPECIAL_VALUE_FROM_RUSTC_ENV);
+        // Also try grabbing it from the environment when the test is built.
+        assert_eq!(SPECIAL_VALUE, env!("SPECIAL_VALUE"));
+        // Also try grabbing it from the environment when the test is run.
+        assert_eq!(SPECIAL_VALUE, std::env::var("SPECIAL_VALUE").unwrap());
+    }
 }

test/unit/rustdoc/rustdoc_nodep_lib.rs

@@ -7,6 +7,17 @@ make_answer!();
 #[cfg(feature = "with_build_script")]
 pub const CONST: &str = env!("CONST");
 
+/// A value with all characters that might need special handling. Note that this
+/// is duplicated in the Starlark code, which attempts to pass it.
+pub const SPECIAL_VALUE: &str = ")(][ \"\\/&^%><;:\t$#@!*-_=+`~.,?x";
+
+/// ```
+/// assert_eq!(test_crate::SPECIAL_VALUE, test_crate::SPECIAL_VALUE_FROM_RUSTC_ENV);
+/// // Also try grabbing it from the environment when the doctest is built.
+/// assert_eq!(test_crate::SPECIAL_VALUE, env!("SPECIAL_VALUE"));
+/// ```
+pub const SPECIAL_VALUE_FROM_RUSTC_ENV: &str = env!("SPECIAL_VALUE");
+
 /// The answer to the ultimate question
 /// ```
 /// fn answer() -> u32 { 42 }
@@ -25,4 +36,13 @@ mod test {
     fn test_answer() {
         assert_eq!(answer(), 42);
     }
+
+    #[test]
+    fn test_special_value() {
+        assert_eq!(SPECIAL_VALUE, SPECIAL_VALUE_FROM_RUSTC_ENV);
+        // Also try grabbing it from the environment when the test is built.
+        assert_eq!(SPECIAL_VALUE, env!("SPECIAL_VALUE"));
+        // Also try grabbing it from the environment when the test is run.
+        assert_eq!(SPECIAL_VALUE, std::env::var("SPECIAL_VALUE").unwrap());
+    }
 }

test/unit/rustdoc/rustdoc_unit_test.bzl

@@ -10,6 +10,11 @@ load(
     "assert_argv_contains_prefix_not",
 )
 
+# A value with all characters that might need special handling. Note that this
+# is duplicated in the Rust code, which verifies it comes through unchanged.
+# TODO(bazelbuild/rules_rust#1877): Add a single quote.
+SPECIAL_VALUE = ")(][ \"\\/&^%><;:\t$$#@!*-_=+`~.,?x"
+
 def _common_rustdoc_checks(env, tut):
     actions = tut.actions
     action = actions[0]
@@ -75,17 +80,20 @@ rustdoc_for_lib_with_proc_macro_test = analysistest.make(_rustdoc_for_lib_with_p
 rustdoc_for_bin_with_transitive_proc_macro_test = analysistest.make(_rustdoc_for_bin_with_transitive_proc_macro_test_impl)
 rustdoc_for_lib_with_cc_lib_test = analysistest.make(_rustdoc_for_lib_with_cc_lib_test_impl)
 
-def _target_maker(rule_fn, name, rustdoc_deps = [], crate_features = [], **kwargs):
+def _target_maker(rule_fn, name, rustdoc_deps = [], crate_features = [], crate_name = "test_crate", **kwargs):
     rule_fn(
         name = name,
         edition = "2018",
         crate_features = crate_features,
+        crate_name = crate_name,
+        rustc_env = {"SPECIAL_VALUE": SPECIAL_VALUE},
         **kwargs
     )
 
     rust_test(
         name = "{}_test".format(name),
         crate = ":{}".format(name),
+        env = {"SPECIAL_VALUE": SPECIAL_VALUE},
         edition = "2018",
     )
 
@@ -130,6 +138,7 @@ def _define_targets():
     _target_maker(
         rust_proc_macro,
         name = "rustdoc_proc_macro",
+        crate_name = "rustdoc_proc_macro",
         srcs = ["rustdoc_proc_macro.rs"],
     )
 

tools/rustdoc/rustdoc_test_writer.rs

@@ -164,6 +164,14 @@ fn expand_params_file(mut options: Options) -> Options {
     options
 }
 
+/// Escapes a string for a single-quoted string in a bash script.
+///
+/// This is pretty simple with single quotes, which escape everything except other single quotes.
+/// We handle those by switching to a double-quoted string for that character.
+fn escape_string_in_single_quoted_bash(s: &str) -> String {
+    s.replace('\'', "'\"'\"'")
+}
+
 /// Write a unix compatible test runner
 fn write_test_runner_unix(
     path: &Path,
@@ -182,7 +190,10 @@ fn write_test_runner_unix(
         "exec env - \\".to_owned(),
     ];
 
-    content.extend(env.iter().map(|(key, val)| format!("{key}='{val}' \\")));
+    content.extend(
+        env.iter()
+            .map(|(key, val)| format!("{key}='{}' \\", escape_string_in_single_quoted_bash(val))),
+    );
 
     let argv_str = argv
         .iter()
@@ -194,7 +205,7 @@ fn write_test_runner_unix(
                 .for_each(|substring| stripped_arg = stripped_arg.replace(substring, ""));
             stripped_arg
         })
-        .map(|arg| format!("'{arg}'"))
+        .map(|arg| format!("'{}'", escape_string_in_single_quoted_bash(&arg)))
         .collect::<Vec<String>>()
         .join(" ");
 
@@ -203,6 +214,37 @@ fn write_test_runner_unix(
     fs::write(path, content.join("\n")).expect("Failed to write test runner");
 }
 
+/// Escapes a string for a batch script to pass as an argument to powershell.
+///
+/// This is amazingly hard. All the obvious approaches that work on Unix-ish shells with quoting
+/// are broken in various ways. Turns out that all Windows wants to do with the strings you pass is
+/// concatenate them with spaces, so the best approach is to escape every (!) special character and
+/// not qoute anything (because the different layers interpret quotes in incompatible ways that
+/// result in no universal cross-version-compatible solution).
+///
+/// I found https://stackoverflow.com/a/31413730 and
+/// https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way
+/// very helpful as references.
+fn escape_string_for_powershell_from_bash_windows(s: &str) -> String {
+    let mut s: String = s.into();
+    // Now escape various special characters so cmd.exe will pass them through.
+    // Note that `'^'` being first is very important or we'll over-escape.
+    for character in &['^', '(', ')', '%', '!', '<', '>', '&'] {
+        s = s.replace(*character, &format!("^{character}"));
+    }
+    s = s.replace('"', "\\`^\\\"");
+    s
+}
+
+/// Quotes a string for powershell.
+///
+/// This is harder than it sounds. The docs claim that `''` works to escape a single quote inside a
+/// single-quoted string, but that doesn't seem to work.
+fn quote_string_for_powershell_windows(s: &str) -> String {
+    // TODO(bazelbuild/rules_rust#1877): Figure out how to escape single quotes.
+    format!("'{s}'")
+}
+
 /// Write a windows compatible test runner
 fn write_test_runner_windows(
     path: &Path,
@@ -212,7 +254,9 @@ fn write_test_runner_windows(
 ) {
     let env_str = env
         .iter()
-        .map(|(key, val)| format!("$env:{key}='{val}'"))
+        // This probably has issues with some weird things in the key, but I'm not smart enough to
+        // make powershell behave for environment variable keys.
+        .map(|(key, val)| format!("$env:{key}={}", quote_string_for_powershell_windows(val)))
         .collect::<Vec<String>>()
         .join(" ; ");
 
@@ -226,7 +270,7 @@ fn write_test_runner_windows(
                 .for_each(|substring| stripped_arg = stripped_arg.replace(substring, ""));
             stripped_arg
         })
-        .map(|arg| format!("'{arg}'"))
+        .map(|arg| quote_string_for_powershell_windows(&arg))
         .collect::<Vec<String>>()
         .join(" ");
 
@@ -239,7 +283,7 @@ fn write_test_runner_windows(
         "powershell.exe -c \"if (!(Test-Path .\\external)) { New-Item -Path .\\external -ItemType SymbolicLink -Value ..\\ }\""
             .to_owned(),
         "".to_owned(),
-        format!("powershell.exe -c \"{env_str} ; & {argv_str}\""),
+        format!("powershell.exe -c \"{}\"", escape_string_for_powershell_from_bash_windows(&format!("{env_str} ; & {argv_str}"))),
         "".to_owned(),
     ];