From 4c5570aa1d3932dfcad28b0f82efaf0794d4c2b0 Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Sat, 28 May 2022 10:31:42 +0800
Subject: [PATCH] Allow delegates to refuse spec kind changes (#427)

This will be useful for all methods/functions that want to take a
single spec only.

Whenever a range is set, even if no other actual revision is provided,
it indicates the input has different intentions than what we require.
---
 git-revision/src/spec.rs         |  7 +++---
 git-revision/tests/spec/parse.rs | 42 ++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/git-revision/src/spec.rs b/git-revision/src/spec.rs
index 3453464cc6..b529d889c8 100644
--- a/git-revision/src/spec.rs
+++ b/git-revision/src/spec.rs
@@ -42,11 +42,12 @@ pub mod parse {
 
         /// Set the kind of the specification, which happens only once if it happens at all.
         /// In case this method isn't called, assume `Single`.
+        /// Reject a kind by returning `None` to stop the parsing.
         ///
         /// Note that ranges don't necessarily assure that a second specification will be parsed.
         /// If `^rev` is given, this method is called with [`spec::Kind::Range`][crate::spec::Kind::Range]
         /// and no second specification is provided.
-        fn kind(&mut self, kind: crate::spec::Kind);
+        fn kind(&mut self, kind: crate::spec::Kind) -> Option<()>;
     }
 
     pub(crate) mod function {
@@ -65,13 +66,13 @@ pub mod parse {
         pub fn parse(mut input: &BStr, delegate: &mut impl Delegate) -> Result<(), Error> {
             if let Some(b'^') = input.get(0) {
                 input = next(input).1;
-                delegate.kind(spec::Kind::Range);
+                delegate.kind(spec::Kind::Range).ok_or(Error::Delegate)?;
             }
 
             input = revision(input, delegate)?;
             if let Some((rest, kind)) = try_range(input) {
                 // TODO: protect against double-kind calls, invalid for git
-                delegate.kind(kind);
+                delegate.kind(kind).ok_or(Error::Delegate)?;
                 input = rest.as_bstr();
             }
             input = revision(input, delegate)?;
diff --git a/git-revision/tests/spec/parse.rs b/git-revision/tests/spec/parse.rs
index ee5f742130..04dd3f26c8 100644
--- a/git-revision/tests/spec/parse.rs
+++ b/git-revision/tests/spec/parse.rs
@@ -1,12 +1,27 @@
 use git_object::bstr::{BStr, BString};
 use git_revision::spec;
 
+#[derive(Default, Debug)]
+struct Options {
+    reject_kind: bool,
+}
+
 #[derive(Default, Debug)]
 struct Recorder {
     resolve_ref_input: Option<BString>,
     resolve_ref_input2: Option<BString>,
     kind: Option<spec::Kind>,
     calls: usize,
+    opts: Options,
+}
+
+impl Recorder {
+    fn with(options: Options) -> Self {
+        Recorder {
+            opts: options,
+            ..Default::default()
+        }
+    }
 }
 
 impl spec::parse::Delegate for Recorder {
@@ -34,16 +49,24 @@ impl spec::parse::Delegate for Recorder {
         todo!()
     }
 
-    fn kind(&mut self, kind: spec::Kind) {
+    fn kind(&mut self, kind: spec::Kind) -> Option<()> {
+        if self.opts.reject_kind {
+            return None;
+        }
         self.calls += 1;
         self.kind = Some(kind);
+        Some(())
     }
 }
 
 fn parse(spec: &str) -> Recorder {
-    let mut rec = Recorder::default();
-    spec::parse(spec.into(), &mut rec).unwrap();
-    rec
+    try_parse_opts(spec, Options::default()).unwrap()
+}
+
+fn try_parse_opts(spec: &str, options: Options) -> Result<Recorder, spec::parse::Error> {
+    let mut rec = Recorder::with(options);
+    spec::parse(spec.into(), &mut rec)?;
+    Ok(rec)
 }
 
 #[test]
@@ -85,9 +108,18 @@ mod revision {
 }
 
 mod range {
-    use crate::spec::parse::parse;
+    use crate::spec::parse::{parse, try_parse_opts, Options};
     use git_revision::spec;
 
+    #[test]
+    fn delegate_can_refuse_spec_kinds() {
+        let err = try_parse_opts("^HEAD", Options { reject_kind: true }).unwrap_err();
+        assert!(
+            matches!(err, spec::parse::Error::Delegate),
+            "Delegates can refuse spec kind changes to abort parsing early"
+        );
+    }
+
     #[test]
     fn leading_caret_is_range_kind() {
         let rec = parse("^HEAD");