Merge branch 'kinnison/fix-50' into 'master'

Error when implicit simple pattern contains regex metacharacters

Closes #50

See merge request larswirzenius/subplot!44

3 files changed, 95 insertions, 11 deletions

diff --git a/src/bindings.rs b/src/bindings.rs
index 67c45d6..9c0df9e 100644
--- a/src/bindings.rs
+++ b/src/bindings.rs
@@ -362,7 +362,8 @@ fn from_hashmap(parsed: &ParsedBinding) -> Result<Binding> {
     let pattern = if parsed.regex.unwrap_or(false) {
         pattern.to_string()
     } else {
-        regex_from_simple_pattern(pattern)?
+        // if we get here parsed.regex is either None or Some(false)
+        regex_from_simple_pattern(pattern, parsed.regex.is_some())?
     };
 
     Ok(Binding::new(
@@ -496,8 +497,8 @@ mod test_bindings {
     }
 }
 
-fn regex_from_simple_pattern(pattern: &str) -> Result<String> {
-    let pat = Regex::new(r"\{\S+\}").unwrap();
+fn regex_from_simple_pattern(pattern: &str, explicit_plain: bool) -> Result<String> {
+    let pat = Regex::new(r"\{[^\s\{\}]+\}").unwrap();
     let mut r = String::new();
     let mut end = 0;
     for m in pat.find_iter(pattern) {
@@ -505,6 +506,11 @@ fn regex_from_simple_pattern(pattern: &str) -> Result<String> {
         if before.find('{').is_some() || before.find('}').is_some() {
             return Err(SubplotError::StrayBraceInSimplePattern(pattern.to_string()));
         }
+        if !explicit_plain && before.chars().any(|c| r"$^*.()+\?|[]".contains(c)) {
+            return Err(SubplotError::SimplePatternHasMetaCharacters(
+                pattern.to_owned(),
+            ));
+        }
         r.push_str(&escape(before));
         let name = &pattern[m.start() + 1..m.end() - 1];
         r.push_str(&format!(r"(?P<{}>\S+)", name));
@@ -514,6 +520,11 @@ fn regex_from_simple_pattern(pattern: &str) -> Result<String> {
     if after.find('{').is_some() || after.find('}').is_some() {
         return Err(SubplotError::StrayBraceInSimplePattern(pattern.to_string()));
     }
+    if !explicit_plain && after.chars().any(|c| r"$^*.()+\?|[]".contains(c)) {
+        return Err(SubplotError::SimplePatternHasMetaCharacters(
+            pattern.to_owned(),
+        ));
+    }
     r.push_str(&escape(after));
     Ok(r)
 }
@@ -525,31 +536,31 @@ mod test_regex_from_simple_pattern {
 
     #[test]
     fn returns_empty_string_as_is() {
-        let ret = regex_from_simple_pattern("").unwrap();
+        let ret = regex_from_simple_pattern("", false).unwrap();
         assert_eq!(ret, "");
     }
 
     #[test]
     fn returns_boring_pattern_as_is() {
-        let ret = regex_from_simple_pattern("boring").unwrap();
+        let ret = regex_from_simple_pattern("boring", false).unwrap();
         assert_eq!(ret, "boring");
     }
 
     #[test]
     fn returns_pattern_with_regexp_chars_escaped() {
-        let ret = regex_from_simple_pattern(r".[]*\\").unwrap();
+        let ret = regex_from_simple_pattern(r".[]*\\", true).unwrap();
         assert_eq!(ret, r"\.\[\]\*\\\\");
     }
 
     #[test]
     fn returns_simple_pattern_expressed_as_regexp() {
-        let ret = regex_from_simple_pattern("I am {name}").unwrap();
+        let ret = regex_from_simple_pattern("I am {name}", false).unwrap();
         assert_eq!(ret, r"I am (?P<name>\S+)");
     }
 
     #[test]
     fn returns_error_for_stray_opening_brace() {
-        match regex_from_simple_pattern("{") {
+        match regex_from_simple_pattern("{", false) {
             Err(SubplotError::StrayBraceInSimplePattern(_)) => (),
             Err(e) => panic!("unexpected error: {}", e),
             _ => unreachable!(),
@@ -558,7 +569,7 @@ mod test_regex_from_simple_pattern {
 
     #[test]
     fn returns_error_for_stray_closing_brace() {
-        match regex_from_simple_pattern("}") {
+        match regex_from_simple_pattern("}", false) {
             Err(SubplotError::StrayBraceInSimplePattern(_)) => (),
             Err(e) => panic!("unexpected error: {}", e),
             _ => unreachable!(),
@@ -567,7 +578,7 @@ mod test_regex_from_simple_pattern {
 
     #[test]
     fn returns_error_for_stray_opening_brace_before_capture() {
-        match regex_from_simple_pattern("{") {
+        match regex_from_simple_pattern("{{foo}", false) {
             Err(SubplotError::StrayBraceInSimplePattern(_)) => (),
             Err(e) => panic!("unexpected error: {}", e),
             _ => unreachable!(),
@@ -576,7 +587,7 @@ mod test_regex_from_simple_pattern {
 
     #[test]
     fn returns_error_for_stray_closing_brace_before_capture() {
-        match regex_from_simple_pattern("}") {
+        match regex_from_simple_pattern("}{foo}", false) {
             Err(SubplotError::StrayBraceInSimplePattern(_)) => (),
             Err(e) => panic!("unexpected error: {}", e),
             _ => unreachable!(),
diff --git a/src/error.rs b/src/error.rs
index 53128b2..60c850e 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -10,6 +10,14 @@ pub enum SubplotError {
     #[error("simple pattern contains a stray {{ or }}")]
     StrayBraceInSimplePattern(String),
 
+    /// The simple pattern has regex metacharacters.
+    ///
+    /// Simple patterns are not permitted to have regex metacharacters in them
+    /// unless the pattern is explicitly marked `regex: false` to indicate that
+    /// the binding author understands what they're up to.
+    #[error("simple pattern contains regex metacharacters: {0}")]
+    SimplePatternHasMetaCharacters(String),
+
     /// Scenario step does not match a known binding
     ///
     /// This may be due to the binding missing entirely, or that the
diff --git a/subplot.md b/subplot.md
index 74e23cf..f16c6fe 100644
--- a/subplot.md
+++ b/subplot.md
@@ -919,6 +919,71 @@ def func(ctx, name=None):
     print('function got argument name as', name)
 ~~~
 
+### Simple patterns with regex metacharacters: forbidden case
+
+Help use to avoid accidental regular expression versus simple pattern
+confusion. The rule is that a simple pattern mustn't contain regular
+expression meta characters unless the rule is explicitly marked as not
+being a regular expression pattern.
+
+~~~scenario
+given file confusedpattern.md
+and file confusedpattern.yaml
+and file capture.py
+when I try to run sp-codegen --run confusedpattern.md -o test.py
+then exit code is non-zero
+and stderr matches /simple pattern contains regex/
+~~~
+
+~~~~{#confusedpattern.md .file .markdown .numberLines}
+---
+title: Simple pattern capture
+bindings: confusedpattern.yaml
+functions: capture.py
+...
+
+# Simple pattern
+
+~~~scenario
+given I* am Tomjon
+~~~
+~~~~
+
+~~~{#confusedpattern.yaml .file .yaml .numberLines}
+- given: I* am {name}
+  function: func
+~~~
+
+### Simple patterns with regex metacharacters: allowed case
+
+~~~scenario
+given file confusedbutok.md
+and file confusedbutok.yaml
+and file capture.py
+when I run sp-codegen --run confusedbutok.md -o test.py
+then program finished successfully
+~~~
+
+~~~~{#confusedbutok.md .file .markdown .numberLines}
+---
+title: Simple pattern capture
+bindings: confusedbutok.yaml
+functions: capture.py
+...
+
+# Simple pattern
+
+~~~scenario
+given I* am Tomjon
+~~~
+~~~~
+
+~~~{#confusedbutok.yaml .file .yaml .numberLines}
+- given: I* am {name}
+  function: func
+  regex: false
+~~~
+
 ### Capture using regular expressions
 
 ~~~scenario