Error on $<num><non_num> capture replacement names (#258)

* Mostly working capture name validation

* Improve inputs for property tests

* Fix advancing when passing over escaped dollar signs

* Switch to inline snapshot captures

* Cleanup invalid capture error formatting code

Cargo.toml

@@ -43,6 +43,11 @@ clap.workspace = true
 assert_cmd = "2.0.12"
 anyhow = "1.0.75"
 clap_mangen = "0.2.14"
+proptest = "1.3.1"
+console = "0.15.7"
+insta = "1.34.0"
+ansi-to-html = "0.1.3"
+regex-automata = "0.4.3"
 
 [profile.release]
 opt-level = 3

proptest-regressions/replacer/tests.txt

@@ -0,0 +1,8 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 3a23ade8355ca034558ea8635e4ea2ee96ecb38b7b1cb9a854509d7633d45795 # shrinks to s = ""
+cc 8c8d1e7497465f26416bddb7607df0de1fce48d098653eeabac0ad2aeba1fa0a # shrinks to s = "$0$0a"

proptest-regressions/replacer/validate.txt

@@ -0,0 +1,10 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc cfacd65058c8dae0ac7b91c56b8096c36ef68cb35d67262debebac005ea9c677 # shrinks to s = ""
+cc 61e5dc6ce0314cde48b5cbc839fbf46a49fcf8d0ba02cfeecdcbff52fca8c786 # shrinks to s = "$a"
+cc 8e5fd9dbb58ae762a751349749320664715056ef63aad58215397e87ee42c722 # shrinks to s = "$$"
+cc 37c2e41ceeddbecbc4e574f82b58a4007923027ad1a6756bf2f547aa3f748d13 # shrinks to s = "$$0"

src/error.rs

@@ -3,6 +3,8 @@ use std::{
     path::PathBuf,
 };
 
+use crate::replacer::InvalidReplaceCapture;
+
 #[derive(thiserror::Error)]
 pub enum Error {
     #[error("invalid regex {0}")]
@@ -15,6 +17,8 @@ pub enum Error {
     InvalidPath(PathBuf),
     #[error("failed processing files:\n{0}")]
     FailedProcessing(FailedJobs),
+    #[error("{0}")]
+    InvalidReplaceCapture(#[from] InvalidReplaceCapture),
 }
 
 pub struct FailedJobs(Vec<(PathBuf, Error)>);

src/main.rs

@@ -5,13 +5,23 @@ mod input;
 pub(crate) mod replacer;
 pub(crate) mod utils;
 
+use std::process;
+
 pub(crate) use self::input::{App, Source};
+use ansi_term::{Color, Style};
 pub(crate) use error::{Error, Result};
 use replacer::Replacer;
 
 use clap::Parser;
 
-fn main() -> Result<()> {
+fn main() {
+    if let Err(e) = try_main() {
+        eprintln!("{}: {}", Style::from(Color::Red).bold().paint("error"), e);
+        process::exit(1);
+    }
+}
+
+fn try_main() -> Result<()> {
     let options = cli::Options::parse();
 
     let source = if options.recursive {

src/replacer.rs → src/replacer/mod.rs

@@ -1,6 +1,14 @@
+use std::{fs, fs::File, io::prelude::*, path::Path};
+
 use crate::{utils, Error, Result};
+
 use regex::bytes::Regex;
-use std::{fs, fs::File, io::prelude::*, path::Path};
+
+#[cfg(test)]
+mod tests;
+mod validate;
+
+pub use validate::{validate_replace, InvalidReplaceCapture};
 
 pub(crate) struct Replacer {
     regex: Regex,
@@ -20,6 +28,8 @@ impl Replacer {
         let (look_for, replace_with) = if is_literal {
             (regex::escape(&look_for), replace_with.into_bytes())
         } else {
+            validate_replace(&replace_with)?;
+
             (
                 look_for,
                 utils::unescape(&replace_with)
@@ -154,66 +164,3 @@ impl Replacer {
         Ok(())
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    fn replace(
-        look_for: impl Into<String>,
-        replace_with: impl Into<String>,
-        literal: bool,
-        flags: Option<&'static str>,
-        src: &'static str,
-        target: &'static str,
-    ) {
-        let replacer = Replacer::new(
-            look_for.into(),
-            replace_with.into(),
-            literal,
-            flags.map(ToOwned::to_owned),
-            None,
-        )
-        .unwrap();
-        assert_eq!(
-            std::str::from_utf8(&replacer.replace(src.as_bytes())),
-            Ok(target)
-        );
-    }
-
-    #[test]
-    fn default_global() {
-        replace("a", "b", false, None, "aaa", "bbb");
-    }
-
-    #[test]
-    fn escaped_char_preservation() {
-        replace("a", "b", false, None, "a\\n", "b\\n");
-    }
-
-    #[test]
-    fn case_sensitive_default() {
-        replace("abc", "x", false, None, "abcABC", "xABC");
-        replace("abc", "x", true, None, "abcABC", "xABC");
-    }
-
-    #[test]
-    fn sanity_check_literal_replacements() {
-        replace("((special[]))", "x", true, None, "((special[]))y", "xy");
-    }
-
-    #[test]
-    fn unescape_regex_replacements() {
-        replace("test", r"\n", false, None, "testtest", "\n\n");
-    }
-
-    #[test]
-    fn no_unescape_literal_replacements() {
-        replace("test", r"\n", true, None, "testtest", r"\n\n");
-    }
-
-    #[test]
-    fn full_word_replace() {
-        replace("abc", "def", false, Some("w"), "abcd abc", "abcd def");
-    }
-}

src/replacer/tests.rs

@@ -0,0 +1,80 @@
+use super::*;
+
+use proptest::prelude::*;
+
+proptest! {
+    #[test]
+    fn validate_doesnt_panic(s in r"(\PC*\$?){0,5}") {
+        let _ = validate::validate_replace(&s);
+    }
+
+    // $ followed by a digit and a non-ident char or an ident char
+    #[test]
+    fn validate_ok(s in r"([^\$]*(\$([0-9][^a-zA-Z_0-9\$]|a-zA-Z_))?){0,5}") {
+        validate::validate_replace(&s).unwrap();
+    }
+
+    // Force at least one $ followed by a digit and an ident char
+    #[test]
+    fn validate_err(s in r"[^\$]*?\$[0-9][a-zA-Z_]\PC*") {
+        validate::validate_replace(&s).unwrap_err();
+    }
+}
+
+fn replace(
+    look_for: impl Into<String>,
+    replace_with: impl Into<String>,
+    literal: bool,
+    flags: Option<&'static str>,
+    src: &'static str,
+    target: &'static str,
+) {
+    let replacer = Replacer::new(
+        look_for.into(),
+        replace_with.into(),
+        literal,
+        flags.map(ToOwned::to_owned),
+        None,
+    )
+    .unwrap();
+    assert_eq!(
+        std::str::from_utf8(&replacer.replace(src.as_bytes())),
+        Ok(target)
+    );
+}
+
+#[test]
+fn default_global() {
+    replace("a", "b", false, None, "aaa", "bbb");
+}
+
+#[test]
+fn escaped_char_preservation() {
+    replace("a", "b", false, None, "a\\n", "b\\n");
+}
+
+#[test]
+fn case_sensitive_default() {
+    replace("abc", "x", false, None, "abcABC", "xABC");
+    replace("abc", "x", true, None, "abcABC", "xABC");
+}
+
+#[test]
+fn sanity_check_literal_replacements() {
+    replace("((special[]))", "x", true, None, "((special[]))y", "xy");
+}
+
+#[test]
+fn unescape_regex_replacements() {
+    replace("test", r"\n", false, None, "testtest", "\n\n");
+}
+
+#[test]
+fn no_unescape_literal_replacements() {
+    replace("test", r"\n", true, None, "testtest", r"\n\n");
+}
+
+#[test]
+fn full_word_replace() {
+    replace("abc", "def", false, Some("w"), "abcd abc", "abcd def");
+}