regex-cli: validate patterns before combining alternations

regex-cli/args/patterns.rs

@@ -52,23 +52,16 @@ impl Config {
     /// one pattern is returned.
     ///
     /// Note that it is legal for this to return zero patterns!
-    pub fn get(&self) -> anyhow::Result<Vec<String>> {
+    pub fn get(
+        &self,
+        syntax: &crate::args::syntax::Config,
+    ) -> anyhow::Result<Vec<String>> {
         let mut pats = self.patterns.clone();
         if self.fixed_strings {
             pats = pats.iter().map(|p| regex_syntax::escape(p)).collect();
         }
         if self.combine {
-            // FIXME: This is... not technically correct, since someone could
-            // provide a pattern `ab(cd` and then `ef)gh`. Neither are valid
-            // patterns, but by joining them with a |, we get `ab(cd|ef)gh`
-            // which is valid. The solution to this is I think to try and
-            // parse the regex to make sure it's valid, but we should be
-            // careful to only use the AST parser. The problem here is that
-            // we don't technically have the configuration of the parser at
-            // this point. We could *ask* for it. We could also just assume a
-            // default configuration since the AST parser doesn't have many
-            // configuration knobs. But probably we should just ask for the
-            // parser configuration here.
+            syntax.asts(&pats)?;
             pats = vec![pats.join("|")];
         }
         Ok(pats)
@@ -186,3 +179,52 @@ impl Default for Mode {
         Mode::OnlyFlags
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::Config;
+
+    #[test]
+    fn combine_patterns_rejects_individually_invalid_patterns() {
+        let config = Config {
+            patterns: vec!["ab(cd".to_string(), "ef)gh".to_string()],
+            combine: true,
+            ..Config::default()
+        };
+
+        assert!(config.get(&crate::args::syntax::Config::default()).is_err());
+    }
+
+    #[test]
+    fn combine_patterns_accepts_individually_valid_patterns() {
+        let config = Config {
+            patterns: vec!["ab(cd)".to_string(), "ef(gh)".to_string()],
+            combine: true,
+            ..Config::default()
+        };
+
+        let got = config.get(&crate::args::syntax::Config::default()).unwrap();
+        assert_eq!(got, vec!["ab(cd)|ef(gh)".to_string()]);
+    }
+
+    #[test]
+    fn combine_patterns_respects_syntax_configuration() {
+        let mut syntax = crate::args::syntax::Config::default();
+        let config = Config {
+            patterns: vec![r"\141".to_string(), r"\142".to_string()],
+            combine: true,
+            ..Config::default()
+        };
+
+        assert!(config.get(&syntax).is_err());
+        let mut parser = lexopt::Parser::from_args(["--octal"]);
+        crate::args::configure(
+            &mut parser,
+            "",
+            &mut [&mut syntax as &mut dyn crate::args::Configurable],
+        )
+        .unwrap();
+        let got = config.get(&syntax).unwrap();
+        assert_eq!(got, vec![r"\141|\142".to_string()]);
+    }
+}

regex-cli/cmd/debug/dfa.rs

@@ -64,7 +64,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -126,7 +126,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -223,7 +223,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -284,7 +284,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/debug/literal.rs

@@ -43,7 +43,7 @@ OPTIONS:
         &mut [&mut common, &mut patterns, &mut syntax, &mut literal],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     anyhow::ensure!(
         pats.len() == 1,
         "only one pattern is allowed, but {} were given",

regex-cli/cmd/debug/mod.rs

@@ -62,7 +62,7 @@ OPTIONS:
     let mut syntax = args::syntax::Config::default();
     args::configure(p, USAGE, &mut [&mut common, &mut patterns, &mut syntax])?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     anyhow::ensure!(
         pats.len() == 1,
         "only one pattern is allowed, but {} were given",
@@ -104,7 +104,7 @@ OPTIONS:
     let mut syntax = args::syntax::Config::default();
     args::configure(p, USAGE, &mut [&mut common, &mut patterns, &mut syntax])?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     anyhow::ensure!(
         pats.len() == 1,
         "only one pattern is allowed, but {} were given",
@@ -159,7 +159,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -210,7 +210,7 @@ OPTIONS:
         &mut [&mut common, &mut patterns, &mut syntax, &mut thompson],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/capture/dfa.rs

@@ -43,7 +43,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/capture/mod.rs

@@ -84,7 +84,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| api.from_patterns(&syn, &pats))?;
@@ -185,7 +185,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
 
     let re = if meta.build_from_patterns() {
@@ -263,7 +263,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| lite.from_patterns(&syn, &pats))?;

regex-cli/cmd/find/capture/nfa.rs

@@ -43,7 +43,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -121,7 +121,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/half/dfa.rs

@@ -47,7 +47,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -151,7 +151,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -253,7 +253,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/half/mod.rs

@@ -83,7 +83,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| api.from_patterns(&syn, &pats))?;
@@ -141,7 +141,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
 
     let re = if meta.build_from_patterns() {

regex-cli/cmd/find/match/dfa.rs

@@ -43,7 +43,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -124,7 +124,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -202,7 +202,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -281,7 +281,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/match/mod.rs

@@ -82,7 +82,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| api.from_patterns(&syn, &pats))?;
@@ -140,7 +140,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
 
     let re = if meta.build_from_patterns() {
@@ -212,7 +212,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| lite.from_patterns(&syn, &pats))?;

regex-cli/cmd/find/match/nfa.rs

@@ -43,7 +43,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -118,7 +118,7 @@ OPTIONS:
         ],
     )?;
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/which/dfa.rs

@@ -47,7 +47,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -117,7 +117,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);
@@ -185,7 +185,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);

regex-cli/cmd/find/which/mod.rs

@@ -100,7 +100,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let syn = syntax.syntax()?;
     let mut table = Table::empty();
     let (re, time) = util::timeitr(|| api.from_patterns_set(&syn, &pats))?;
@@ -169,7 +169,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
 
     let re = if meta.build_from_patterns() {

regex-cli/cmd/find/which/nfa.rs

@@ -47,7 +47,7 @@ OPTIONS:
         "'which' command does not support reporting counts",
     );
 
-    let pats = patterns.get()?;
+    let pats = patterns.get(&syntax)?;
     let mut table = Table::empty();
     let (asts, time) = util::timeitr(|| syntax.asts(&pats))?;
     table.add("parse time", time);