Add bindings for git_reference_name_is_valid, git_remote_name_is_valid & git_tag_name_is_valid

Author: sanjayts

.gitignore

@@ -1,3 +1,3 @@
 target
 Cargo.lock
-src/main.rs
+src/main.rs

libgit2-sys/lib.rs

@@ -2246,7 +2246,14 @@ extern "C" {
         remote: *const git_remote,
     ) -> c_int;
     pub fn git_remote_get_refspec(remote: *const git_remote, n: size_t) -> *const git_refspec;
+
+    // This function is no longer used by kept here for the sake of completeness and to ensure
+    // no one thinks of it as "missing" in terms of FFI coverage. A conscious decision was made to
+    // use `git_remote_name_is_valid` as opposed to this function when trying to figure out whether
+    // a remote name is valid. More details at -- https://github.com/rust-lang/git2-rs/pull/882
     pub fn git_remote_is_valid_name(remote_name: *const c_char) -> c_int;
+
+    pub fn git_remote_name_is_valid(valid: *mut c_int, remote_name: *const c_char) -> c_int;
     pub fn git_remote_list(out: *mut git_strarray, repo: *mut git_repository) -> c_int;
     pub fn git_remote_rename(
         problems: *mut git_strarray,
@@ -2397,7 +2404,14 @@ extern "C" {
     pub fn git_reference_is_note(r: *const git_reference) -> c_int;
     pub fn git_reference_is_remote(r: *const git_reference) -> c_int;
     pub fn git_reference_is_tag(r: *const git_reference) -> c_int;
+
+    // This function is no longer used by kept here for the sake of completeness and to ensure
+    // no one thinks of it as "missing" in terms of FFI coverage. A conscious decision was made to
+    // use `git_reference_name_is_valid` as opposed to this function when trying to figure out whether
+    // a ref name is valid. More details at -- https://github.com/rust-lang/git2-rs/pull/882
     pub fn git_reference_is_valid_name(name: *const c_char) -> c_int;
+
+    pub fn git_reference_name_is_valid(valid: *mut c_int, refname: *const c_char) -> c_int;
     pub fn git_reference_lookup(
         out: *mut *mut git_reference,
         repo: *mut git_repository,
@@ -3209,6 +3223,7 @@ extern "C" {
     pub fn git_tag_target(target_out: *mut *mut git_object, tag: *const git_tag) -> c_int;
     pub fn git_tag_target_id(tag: *const git_tag) -> *const git_oid;
     pub fn git_tag_target_type(tag: *const git_tag) -> git_object_t;
+    pub fn git_tag_name_is_valid(valid: *mut c_int, tag_name: *const c_char) -> c_int;
 
     // checkout
     pub fn git_checkout_head(repo: *mut git_repository, opts: *const git_checkout_options)

src/reference.rs

@@ -62,7 +62,28 @@ impl<'repo> Reference<'repo> {
     pub fn is_valid_name(refname: &str) -> bool {
         crate::init();
         let refname = CString::new(refname).unwrap();
-        unsafe { raw::git_reference_is_valid_name(refname.as_ptr()) == 1 }
+        let mut valid: libc::c_int = 0;
+        let return_code = unsafe {
+            raw::git_reference_name_is_valid(
+                &mut valid,
+                refname.as_ptr()
+            )
+        };
+        return_code >= 0 && valid == 1
+    }
+
+    /// Ensure the reference name is well-formed.
+    pub fn name_is_valid(refname: &str) -> Result<bool, Error> {
+        crate::init();
+        let refname = CString::new(refname)?;
+        let mut valid: libc::c_int = 0;
+        unsafe {
+            try_call!(raw::git_reference_name_is_valid(
+                &mut valid,
+                refname.as_ptr()
+            ));
+        }
+        Ok(valid == 1)
     }
 
     /// Normalize reference name and check validity.
@@ -463,13 +484,23 @@ mod tests {
     use crate::{ObjectType, Reference, ReferenceType};
 
     #[test]
-    fn smoke() {
+    fn is_valid_name() {
         assert!(Reference::is_valid_name("refs/foo"));
         assert!(!Reference::is_valid_name("foo"));
+        assert!(Reference::is_valid_name("FOO_BAR"));
+
+        assert!(!Reference::is_valid_name("foo"));
+        assert!(!Reference::is_valid_name("_FOO_BAR"));
     }
 
     #[test]
-    fn smoke2() {
+    #[should_panic]
+    fn is_valid_name_for_invalid_ref() {
+        Reference::is_valid_name("ab\012");
+    }
+
+    #[test]
+    fn smoke() {
         let (_td, repo) = crate::test::repo_init();
         let mut head = repo.head().unwrap();
         assert!(head.is_branch());

src/remote.rs

@@ -92,7 +92,28 @@ impl<'repo> Remote<'repo> {
     pub fn is_valid_name(remote_name: &str) -> bool {
         crate::init();
         let remote_name = CString::new(remote_name).unwrap();
-        unsafe { raw::git_remote_is_valid_name(remote_name.as_ptr()) == 1 }
+        let mut valid: libc::c_int = 0;
+        let return_code = unsafe {
+            raw::git_remote_name_is_valid(
+                &mut valid,
+                remote_name.as_ptr()
+            )
+        };
+        return_code >= 0 && valid == 1
+    }
+
+    /// Ensure the remote name is well-formed.
+    pub fn name_is_valid(remote_name: &str) -> Result<bool, Error> {
+        crate::init();
+        let remote_name = CString::new(remote_name)?;
+        let mut valid: libc::c_int = 0;
+        unsafe {
+            try_call!(raw::git_remote_name_is_valid(
+                &mut valid,
+                remote_name.as_ptr()
+            ));
+        }
+        Ok(valid == 1)
     }
 
     /// Create a detached remote
@@ -851,11 +872,17 @@ mod tests {
     }
 
     #[test]
-    fn is_valid() {
+    fn is_valid_name() {
         assert!(Remote::is_valid_name("foobar"));
         assert!(!Remote::is_valid_name("\x01"));
     }
 
+    #[test]
+    #[should_panic]
+    fn is_valid_name_for_invalid_remote() {
+        Remote::is_valid_name("ab\012");
+    }
+
     #[test]
     fn transfer_cb() {
         let (td, _repo) = crate::test::repo_init();

src/tag.rs

@@ -1,3 +1,4 @@
+use std::ffi::CString;
 use std::marker;
 use std::mem;
 use std::ptr;
@@ -15,6 +16,19 @@ pub struct Tag<'repo> {
 }
 
 impl<'repo> Tag<'repo> {
+    /// Determine whether a tag name is valid, meaning that (when prefixed with refs/tags/) that
+    /// it is a valid reference name, and that any additional tag name restrictions are imposed
+    /// (eg, it cannot start with a -).
+    pub fn name_is_valid(tag_name: &str) -> Result<bool, Error> {
+        crate::init();
+        let tag_name = CString::new(tag_name)?;
+        let mut valid: libc::c_int = 0;
+        unsafe {
+            try_call!(raw::git_tag_name_is_valid(&mut valid, tag_name.as_ptr()));
+        }
+        Ok(valid == 1)
+    }
+
     /// Get the id (SHA1) of a repository tag
     pub fn id(&self) -> Oid {
         unsafe { Binding::from_raw(raw::git_tag_id(&*self.raw)) }
@@ -141,6 +155,29 @@ impl<'repo> Drop for Tag<'repo> {
 
 #[cfg(test)]
 mod tests {
+    use crate::Tag;
+
+    // Reference -- https://git-scm.com/docs/git-check-ref-format
+    #[test]
+    fn name_is_valid() {
+        assert_eq!(Tag::name_is_valid("blah_blah").unwrap(), true);
+        assert_eq!(Tag::name_is_valid("v1.2.3").unwrap(), true);
+        assert_eq!(Tag::name_is_valid("my/tag").unwrap(), true);
+        assert_eq!(Tag::name_is_valid("@").unwrap(), true);
+
+        assert_eq!(Tag::name_is_valid("-foo").unwrap(), false);
+        assert_eq!(Tag::name_is_valid("foo:bar").unwrap(), false);
+        assert_eq!(Tag::name_is_valid("foo^bar").unwrap(), false);
+        assert_eq!(Tag::name_is_valid("foo.").unwrap(), false);
+        assert_eq!(Tag::name_is_valid("@{").unwrap(), false);
+        assert_eq!(Tag::name_is_valid("as\\cd").unwrap(), false);
+
+        assert_eq!(
+            Tag::name_is_valid("ab\012").err().unwrap().to_string(),
+            "data contained a nul byte that could not be represented as a string"
+        );
+    }
+
     #[test]
     fn smoke() {
         let (_td, repo) = crate::test::repo_init();