From 1187eff8a21ff6938d4d00c2a91232fde2d1a796 Mon Sep 17 00:00:00 2001
From: Daniel Silverstone <dsilvers@digital-scurf.org>
Date: Sat, 6 Jun 2020 11:59:40 +0100
Subject: feat: Support reporting error when more than one binding matches a
 step

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
---
 src/ast.rs      | 19 ++++++++++---------
 src/bindings.rs | 23 ++++++++++-------------
 src/error.rs    |  7 +++++++
 src/matches.rs  | 11 +----------
 4 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/src/ast.rs b/src/ast.rs
index bc3547d..4e1d33b 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -803,15 +803,16 @@ fn step(
     }
     let step = step.unwrap();
 
-    let m = bindings.find(&step);
-    if m.is_none() {
-        eprintln!("Could not finding binding for: {}", text);
-        return (
-            error_msg(&format!("Could not find binding for: {}", text)),
-            defkind,
-        );
-    }
-    let m = m.unwrap();
+    let m = match bindings.find(&step) {
+        Ok(m) => m,
+        Err(e) => {
+            eprintln!("Could not select binding: {:?}", e);
+            return (
+                error_msg(&format!("Could not select binding for: {}", text)),
+                defkind,
+            );
+        }
+    };
 
     let mut inlines = Vec::new();
 
diff --git a/src/bindings.rs b/src/bindings.rs
index 4233528..ea35f57 100644
--- a/src/bindings.rs
+++ b/src/bindings.rs
@@ -294,20 +294,17 @@ impl Bindings {
 
     /// Find the binding matching a given scenario step, if there is
     /// exactly one.
-    pub fn find(&self, step: &ScenarioStep) -> Option<MatchedStep> {
+    pub fn find(&self, step: &ScenarioStep) -> Result<MatchedStep> {
         let mut matches = self
             .bindings()
             .iter()
-            .map(|b| b.match_with_step(step))
-            .filter(|o| o.is_some());
-        if let Some(m) = matches.next() {
-            if matches.count() == 0 {
-                m
-            } else {
-                None
-            }
-        } else {
-            None
+            .filter_map(|b| b.match_with_step(step));
+        match matches.next() {
+            None => Err(SubplotError::BindingUnknown(step.to_string())),
+            Some(matched) => match matches.next() {
+                None => Ok(matched),
+                Some(_) => Err(SubplotError::BindingNotUnique(step.to_string())),
+            },
         }
     }
 
@@ -441,7 +438,7 @@ mod test_bindings {
         let binding = Binding::new(StepKind::When, r"I am Tomjon", "set_foo", None).unwrap();
         let mut bindings = Bindings::new();
         bindings.add(&binding);
-        assert!(bindings.find(&step).is_none());
+        assert!(bindings.find(&step).is_err());
     }
 
     #[test]
@@ -451,7 +448,7 @@ mod test_bindings {
             Binding::new(StepKind::Given, r"I am Tomjon of Lancre", "set_foo", None).unwrap();
         let mut bindings = Bindings::new();
         bindings.add(&binding);
-        assert!(bindings.find(&step).is_none());
+        assert!(bindings.find(&step).is_err());
     }
 
     #[test]
diff --git a/src/error.rs b/src/error.rs
index f988c86..daef85c 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -34,6 +34,13 @@ pub enum SubplotError {
     #[error("do not understand binding: {0}")]
     BindingUnknown(String),
 
+    /// Scenario step matches more than one binding
+    ///
+    /// THis may be due to bindings being too general, or having unusual
+    /// overlaps in their matching
+    #[error("more than one binding matches: {0}")]
+    BindingNotUnique(String),
+
     /// A binding in the bindings file doesn't specify a known keyword.
     #[error("binding doesn't specify known keyword: {0}")]
     BindingWithoutKnownKeyword(String),
diff --git a/src/matches.rs b/src/matches.rs
index 27a71e1..2c8f196 100644
--- a/src/matches.rs
+++ b/src/matches.rs
@@ -1,9 +1,7 @@
 use crate::Bindings;
 use crate::Result;
 use crate::Scenario;
-use crate::ScenarioStep;
 use crate::StepKind;
-use crate::SubplotError;
 
 use serde::{Deserialize, Serialize};
 
@@ -20,7 +18,7 @@ impl MatchedScenario {
         let steps: Result<Vec<MatchedStep>> = scen
             .steps()
             .iter()
-            .map(|step| find_binding(step, bindings))
+            .map(|step| bindings.find(step))
             .collect();
         Ok(MatchedScenario {
             title: scen.title().to_string(),
@@ -29,13 +27,6 @@ impl MatchedScenario {
     }
 }
 
-fn find_binding(step: &ScenarioStep, bindings: &Bindings) -> Result<MatchedStep> {
-    match bindings.find(step) {
-        None => Err(SubplotError::BindingUnknown(step.to_string())),
-        Some(ms) => Ok(ms),
-    }
-}
-
 /// A matched binding and scenario step, with captured parts.
 ///
 /// A MatchedStep is a sequence of partial steps, each representing
-- 
cgit v1.2.1