refactor

- fmt
- clippy

Author: Byron

gitoxide-core/src/repository/credential.rs

@@ -15,12 +15,14 @@ pub fn function(repo: gix::Repository, action: gix::credentials::program::main::
         std::io::stdin(),
         std::io::stdout(),
         |action, context| -> Result<_, Error> {
-            let url = context.url.clone().or_else(|| context.to_url()).ok_or_else(|| {
-                Error::Protocol(gix::credentials::protocol::Error::UrlMissing)
-            })?;
-            let (mut cascade, _action, prompt_options) = repo.config_snapshot().credential_helpers(gix::url::parse(
-                url.as_ref(),
-            )?)?;
+            let url = context
+                .url
+                .clone()
+                .or_else(|| context.to_url())
+                .ok_or(Error::Protocol(gix::credentials::protocol::Error::UrlMissing))?;
+            let (mut cascade, _action, prompt_options) = repo
+                .config_snapshot()
+                .credential_helpers(gix::url::parse(url.as_ref())?)?;
             cascade
                 .invoke(
                     match action {

gix-credentials/src/program/main.rs

@@ -92,14 +92,16 @@ pub(crate) mod function {
         if ctx.url.is_none() && (ctx.protocol.is_none() || ctx.host.is_none()) {
             return Err(Error::UrlMissing);
         }
-        let res = credentials(action, ctx).map_err(|err| Error::Helper { source: Box::new(err) })?;
+        let res = credentials(action, ctx.clone()).map_err(|err| Error::Helper { source: Box::new(err) })?;
         match (action, res) {
             (Action::Get, None) => {
-                let ctx_for_error = Context::from_bytes(&buf)?;
-                let url = ctx_for_error.url.clone()
+                let ctx_for_error = ctx;
+                let url = ctx_for_error
+                    .url
+                    .clone()
                     .or_else(|| ctx_for_error.to_url())
-                    .expect("URL is available either directly or via protocol+host");
-                return Err(Error::CredentialsMissing { url })
+                    .expect("URL is available either directly or via protocol+host which we checked for");
+                return Err(Error::CredentialsMissing { url });
             }
             (Action::Get, Some(ctx)) => ctx.write_to(stdout)?,
             (Action::Erase | Action::Store, None) => {}

gix-credentials/src/protocol/context/mod.rs

@@ -93,14 +93,9 @@ mod mutate {
         #[allow(clippy::result_large_err)]
         pub fn destructure_url_in_place(&mut self, use_http_path: bool) -> Result<&mut Self, protocol::Error> {
             if self.url.is_none() {
-                // If URL is not present but protocol and host are, construct the URL
-                if let Some(constructed_url) = self.to_url() {
-                    self.url = Some(constructed_url);
-                } else {
-                    return Err(protocol::Error::UrlMissing);
-                }
+                self.url = Some(self.to_url().ok_or(protocol::Error::UrlMissing)?);
             }
-            
+
             let url = gix_url::parse(self.url.as_ref().expect("URL is present after check above").as_ref())?;
             self.protocol = Some(url.scheme.as_str().into());
             self.username = url.user().map(ToOwned::to_owned);

gix-credentials/tests/program/main.rs

@@ -0,0 +1,94 @@
+use gix_credentials::program::main;
+use std::io::Cursor;
+
+#[derive(Debug, thiserror::Error)]
+#[error("Test error")]
+struct TestError;
+
+#[test]
+fn protocol_and_host_without_url_is_valid() {
+    let input = b"protocol=https\nhost=github.com\n";
+    let mut output = Vec::new();
+
+    let mut called = false;
+    let result = main(
+        ["get".into()],
+        Cursor::new(input),
+        &mut output,
+        |_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
+            assert_eq!(context.protocol.as_deref(), Some("https"));
+            assert_eq!(context.host.as_deref(), Some("github.com"));
+            assert_eq!(context.url, None, "the URL isn't automatically populated");
+            called = true;
+
+            Ok(None)
+        },
+    );
+
+    // This should fail because our mock helper returned None (no credentials found)
+    // but it should NOT fail because of missing URL
+    match result {
+        Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
+            assert!(
+                called,
+                "The helper gets called, but as nothing is provided in the function it ulimately fails"
+            );
+        }
+        other => panic!("Expected CredentialsMissing error, got: {other:?}"),
+    }
+}
+
+#[test]
+fn missing_protocol_with_only_host_or_protocol_fails() {
+    for input in ["host=github.com\n", "protocol=https\n"] {
+        let mut output = Vec::new();
+
+        let mut called = false;
+        let result = main(
+            ["get".into()],
+            Cursor::new(input),
+            &mut output,
+            |_action, _context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
+                called = true;
+                Ok(None)
+            },
+        );
+
+        match result {
+            Err(gix_credentials::program::main::Error::UrlMissing) => {
+                assert!(!called, "the context is lacking, hence nothing gets called");
+            }
+            other => panic!("Expected UrlMissing error, got: {other:?}"),
+        }
+    }
+}
+
+#[test]
+fn url_alone_is_valid() {
+    let input = b"url=https://github.com\n";
+    let mut output = Vec::new();
+
+    let mut called = false;
+    let result = main(
+        ["get".into()],
+        Cursor::new(input),
+        &mut output,
+        |_action, context| -> Result<Option<gix_credentials::protocol::Context>, TestError> {
+            called = true;
+            assert_eq!(context.url.unwrap(), "https://github.com");
+            assert_eq!(context.host, None, "not auto-populated");
+            assert_eq!(context.protocol, None, "not auto-populated");
+
+            Ok(None)
+        },
+    );
+
+    // This should fail because our mock helper returned None (no credentials found)
+    // but it should NOT fail because of missing URL
+    match result {
+        Err(gix_credentials::program::main::Error::CredentialsMissing { .. }) => {
+            assert!(called);
+        }
+        other => panic!("Expected CredentialsMissing error, got: {other:?}"),
+    }
+}

gix-credentials/tests/program/main_validation.rs

@@ -1,2 +1,2 @@
 mod from_custom_definition;
-mod main_validation;
+mod main;

gix-credentials/tests/program/mod.rs

@@ -73,38 +73,28 @@ mod destructure_url_in_place {
         );
     }
 
-    #[test]
-    fn protocol_and_host_without_url_constructs_url_first() {
-        let mut ctx = Context {
-            protocol: Some("https".into()),
-            host: Some("github.com".into()),
-            ..Default::default()
-        };
-        ctx.destructure_url_in_place(false).expect("should work with protocol and host");
-        
-        // URL should be constructed from protocol and host
-        assert_eq!(ctx.url.as_deref().map(|b| &**b), Some("https://github.com".as_bytes()));
-        // Original fields should be preserved 
-        assert_eq!(ctx.protocol.as_deref(), Some("https"));
-        assert_eq!(ctx.host.as_deref(), Some("github.com"));
-    }
-
     #[test]
     fn protocol_and_host_with_path_without_url_constructs_full_url() {
         let mut ctx = Context {
             protocol: Some("https".into()),
             host: Some("github.com".into()),
             path: Some("org/repo".into()),
+            username: Some("user".into()),
+            password: Some("pass-to-be-ignored".into()),
             ..Default::default()
         };
-        ctx.destructure_url_in_place(false).expect("should work with protocol, host and path");
-        
-        // URL should be constructed from all provided fields
-        assert_eq!(ctx.url.as_deref().map(|b| &**b), Some("https://github.com/org/repo".as_bytes()));
+        ctx.destructure_url_in_place(false)
+            .expect("should work with protocol, host and path");
+
+        assert_eq!(
+            ctx.url.unwrap(),
+            "https://[email protected]/org/repo",
+            "URL should be constructed from all provided fields, except password"
+        );
         // Original fields should be preserved
         assert_eq!(ctx.protocol.as_deref(), Some("https"));
         assert_eq!(ctx.host.as_deref(), Some("github.com"));
-        assert_eq!(ctx.path.as_deref().map(|b| &**b), Some("org/repo".as_bytes()));
+        assert_eq!(ctx.path.unwrap(), "org/repo");
     }
 
     #[test]
@@ -113,7 +103,10 @@ mod destructure_url_in_place {
             host: Some("github.com".into()),
             ..Default::default()
         };
-        assert!(ctx_no_protocol.destructure_url_in_place(false).is_err());
+        assert_eq!(
+            ctx_no_protocol.destructure_url_in_place(false).unwrap_err().to_string(),
+            "Either 'url' field or both 'protocol' and 'host' fields must be provided"
+        );
 
         let mut ctx_no_host = Context {
             protocol: Some("https".into()),