refactor

- remove extra accessors

Author: Byron

gix-url/src/parse.rs

@@ -19,7 +19,7 @@ pub enum Error {
     Url {
         url: String,
         kind: UrlKind,
-        source: crate::simple_url::ParseError,
+        source: crate::simple_url::UrlParseError,
     },
 
     #[error("The host portion of the following URL is too long ({} bytes, {len} bytes total): {truncated_url:?}", truncated_url.len())]
@@ -99,36 +99,32 @@ pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error
         });
     }
     let (input, url) = input_to_utf8_and_url(input, UrlKind::Url)?;
-    let scheme = url.scheme().into();
+    let scheme = Scheme::from(url.scheme.as_str());
 
-    if matches!(scheme, Scheme::Git | Scheme::Ssh) && url.path().is_empty() {
+    if matches!(scheme, Scheme::Git | Scheme::Ssh) && url.path.is_empty() {
         return Err(Error::MissingRepositoryPath {
             url: input.into(),
             kind: UrlKind::Url,
         });
     }
 
-    if url.cannot_be_a_base() {
-        return Err(Error::RelativeUrl { url: input.to_owned() });
-    }
-
     // Normalize empty path to "/" for http/https URLs only
-    let path = if url.path().is_empty() && matches!(scheme, Scheme::Http | Scheme::Https) {
+    let path = if url.path.is_empty() && matches!(scheme, Scheme::Http | Scheme::Https) {
         "/".into()
     } else {
-        url.path().into()
+        url.path.into()
     };
 
     Ok(crate::Url {
         serialize_alternative_form: false,
         scheme,
         user: url_user(&url, UrlKind::Url)?,
         password: url
-            .password()
+            .password
             .map(|s| percent_decoded_utf8(s, UrlKind::Url))
             .transpose()?,
-        host: url.host_str().map(Into::into),
-        port: url.port(),
+        host: url.host,
+        port: url.port,
         path,
     })
 }
@@ -172,23 +168,23 @@ pub(crate) fn scp(input: &BStr, colon: usize) -> Result<crate::Url, Error> {
 
     Ok(crate::Url {
         serialize_alternative_form: true,
-        scheme: url.scheme().into(),
+        scheme: Scheme::from(url.scheme.as_str()),
         user: url_user(&url, UrlKind::Scp)?,
         password: url
-            .password()
+            .password
             .map(|s| percent_decoded_utf8(s, UrlKind::Scp))
             .transpose()?,
-        host: url.host_str().map(Into::into),
-        port: url.port(),
+        host: url.host,
+        port: url.port,
         path: path.into(),
     })
 }
 
 fn url_user(url: &crate::simple_url::ParsedUrl<'_>, kind: UrlKind) -> Result<Option<String>, Error> {
-    if url.username().is_empty() && url.password().is_none() {
+    if url.username.is_empty() && url.password.is_none() {
         Ok(None)
     } else {
-        Ok(Some(percent_decoded_utf8(url.username(), kind)?))
+        Ok(Some(percent_decoded_utf8(url.username, kind)?))
     }
 }
 
@@ -285,7 +281,9 @@ fn input_to_utf8_and_url(input: &BStr, kind: UrlKind) -> Result<(&str, crate::si
             // If the parser rejected it as RelativeUrlWithoutBase, map to Error::RelativeUrl
             // to match the expected error type for malformed URLs like "invalid:://"
             match source {
-                crate::simple_url::ParseError::RelativeUrlWithoutBase => Error::RelativeUrl { url: input.to_owned() },
+                crate::simple_url::UrlParseError::RelativeUrlWithoutBase => {
+                    Error::RelativeUrl { url: input.to_owned() }
+                }
                 _ => Error::Url {
                     url: input.to_owned(),
                     kind,

gix-url/src/simple_url.rs

@@ -2,23 +2,26 @@
 /// This is a replacement for the `url` crate dependency.
 #[derive(Debug)]
 pub(crate) struct ParsedUrl<'a> {
-    scheme: String, // Owned to allow normalization to lowercase
-    username: &'a str,
-    password: Option<&'a str>,
-    host: Option<String>, // Owned to allow normalization to lowercase
-    port: Option<u16>,
-    path: &'a str,
+    pub scheme: String, // Owned to allow normalization to lowercase
+    pub username: &'a str,
+    pub password: Option<&'a str>,
+    pub host: Option<String>, // Owned to allow normalization to lowercase
+    pub port: Option<u16>,
+    pub path: &'a str,
 }
 
 /// Minimal parse error type to replace url::ParseError
 #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
-pub enum ParseError {
+#[allow(missing_docs)]
+pub enum UrlParseError {
     #[error("relative URL without a base")]
     RelativeUrlWithoutBase,
-    #[error("invalid port number")]
+    #[error("invalid port number - must be between 1-65535")]
     InvalidPort,
     #[error("invalid domain character")]
     InvalidDomainCharacter,
+    #[error("Scheme requires host")]
+    SchemeRequiresHost,
 }
 
 /// Check if a character is valid in a URL scheme.
@@ -30,30 +33,24 @@ fn is_valid_scheme_char(c: char) -> bool {
 impl<'a> ParsedUrl<'a> {
     /// Parse a URL string into its components.
     /// Expected format: scheme://[user[:password]@]host[:port]/path
-    pub(crate) fn parse(input: &'a str) -> Result<Self, ParseError> {
+    pub(crate) fn parse(input: &'a str) -> Result<Self, UrlParseError> {
         // Find scheme by looking for first ':'
-        let first_colon = input.find(':').ok_or(ParseError::RelativeUrlWithoutBase)?;
-
+        let first_colon = input.find(':').ok_or(UrlParseError::RelativeUrlWithoutBase)?;
         let scheme_str = &input[..first_colon];
         // Normalize scheme to lowercase for case-insensitive matching (matches url crate behavior)
         let scheme = scheme_str.to_ascii_lowercase();
-
-        // Verify it's followed by "//"
-        if !input[first_colon..].starts_with("://") {
-            return Err(ParseError::RelativeUrlWithoutBase);
-        }
-
-        // Start after "://"
-        let after_scheme = &input[first_colon + 3..];
+        let Some(after_scheme) = input[first_colon..].strip_prefix("://") else {
+            return Err(UrlParseError::RelativeUrlWithoutBase);
+        };
 
         // Check for relative URL (scheme without proper authority)
         if scheme_str.is_empty() {
-            return Err(ParseError::RelativeUrlWithoutBase);
+            return Err(UrlParseError::RelativeUrlWithoutBase);
         }
 
         // Validate scheme characters (check original before lowercase conversion)
         if !scheme_str.chars().all(is_valid_scheme_char) {
-            return Err(ParseError::RelativeUrlWithoutBase);
+            return Err(UrlParseError::RelativeUrlWithoutBase);
         }
 
         // Find path start (first '/' after scheme)
@@ -84,7 +81,7 @@ impl<'a> ParsedUrl<'a> {
             let (h, p) = Self::parse_host_port(host_port)?;
             // If we have user info, we must have a host
             if h.is_none() {
-                return Err(ParseError::InvalidDomainCharacter);
+                return Err(UrlParseError::InvalidDomainCharacter);
             }
             (user, pass, h, p)
         } else {
@@ -97,7 +94,7 @@ impl<'a> ParsedUrl<'a> {
         // Scheme is already lowercase at this point
         let requires_host = matches!(scheme.as_str(), "http" | "https" | "git" | "ssh" | "ftp" | "ftps");
         if requires_host && host.is_none() {
-            return Err(ParseError::InvalidDomainCharacter);
+            return Err(UrlParseError::SchemeRequiresHost);
         }
 
         Ok(ParsedUrl {
@@ -110,7 +107,7 @@ impl<'a> ParsedUrl<'a> {
         })
     }
 
-    fn parse_host_port(host_port: &str) -> Result<(Option<String>, Option<u16>), ParseError> {
+    fn parse_host_port(host_port: &str) -> Result<(Option<String>, Option<u16>), UrlParseError> {
         if host_port.is_empty() {
             return Ok((None, None));
         }
@@ -119,23 +116,23 @@ impl<'a> ParsedUrl<'a> {
         if host_port.starts_with('[') {
             if let Some(bracket_end) = host_port.find(']') {
                 // IPv6 addresses are case-insensitive, normalize to lowercase
-                let host = Some(host_port[..bracket_end + 1].to_ascii_lowercase());
+                let host = Some(host_port[..=bracket_end].to_ascii_lowercase());
                 let remaining = &host_port[bracket_end + 1..];
 
                 if remaining.is_empty() {
                     return Ok((host, None));
                 } else if let Some(port_str) = remaining.strip_prefix(':') {
-                    let port = port_str.parse::<u16>().map_err(|_| ParseError::InvalidPort)?;
+                    let port = port_str.parse::<u16>().map_err(|_| UrlParseError::InvalidPort)?;
                     // Validate port is in valid range (1-65535, port 0 is invalid)
                     if port == 0 {
-                        return Err(ParseError::InvalidPort);
+                        return Err(UrlParseError::InvalidPort);
                     }
                     return Ok((host, Some(port)));
                 } else {
-                    return Err(ParseError::InvalidDomainCharacter);
+                    return Err(UrlParseError::InvalidDomainCharacter);
                 }
             } else {
-                return Err(ParseError::InvalidDomainCharacter);
+                return Err(UrlParseError::InvalidDomainCharacter);
             }
         }
 
@@ -151,10 +148,10 @@ impl<'a> ParsedUrl<'a> {
             } else if potential_port.chars().all(|c| c.is_ascii_digit()) {
                 let host_str = &host_port[..colon_pos];
                 let host = Self::normalize_hostname(host_str)?;
-                let port = potential_port.parse::<u16>().map_err(|_| ParseError::InvalidPort)?;
+                let port = potential_port.parse::<u16>().map_err(|_| UrlParseError::InvalidPort)?;
                 // Validate port is in valid range (1-65535, port 0 is invalid)
                 if port == 0 {
-                    return Err(ParseError::InvalidPort);
+                    return Err(UrlParseError::InvalidPort);
                 }
                 return Ok((Some(host), Some(port)));
             }
@@ -175,10 +172,10 @@ impl<'a> ParsedUrl<'a> {
     /// Validate and possibly normalize a hostname
     /// Valid DNS hostnames are normalized to lowercase
     /// Invalid strings (like injection attempts) are preserved as-is but ? is rejected
-    fn normalize_hostname(host: &str) -> Result<String, ParseError> {
+    fn normalize_hostname(host: &str) -> Result<String, UrlParseError> {
         // Reject ? character which git's url parser rejects
         if host.contains('?') {
-            return Err(ParseError::InvalidDomainCharacter);
+            return Err(UrlParseError::InvalidDomainCharacter);
         }
 
         // Only normalize if it looks like a valid DNS hostname
@@ -189,36 +186,6 @@ impl<'a> ParsedUrl<'a> {
             Ok(host.to_owned())
         }
     }
-
-    pub(crate) fn scheme(&self) -> &str {
-        &self.scheme
-    }
-
-    pub(crate) fn username(&self) -> &str {
-        self.username
-    }
-
-    pub(crate) fn password(&self) -> Option<&str> {
-        self.password
-    }
-
-    pub(crate) fn host_str(&self) -> Option<&str> {
-        self.host.as_deref()
-    }
-
-    pub(crate) fn port(&self) -> Option<u16> {
-        self.port
-    }
-
-    pub(crate) fn path(&self) -> &str {
-        self.path
-    }
-
-    /// Check if this URL cannot be a base (is relative).
-    /// For our purposes, since we only parse URLs with "://", they can all be a base.
-    pub(crate) fn cannot_be_a_base(&self) -> bool {
-        false
-    }
 }
 
 #[cfg(test)]
@@ -228,56 +195,56 @@ mod tests {
     #[test]
     fn test_simple_url() {
         let url = ParsedUrl::parse("http://example.com/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.host_str(), Some("example.com"));
-        assert_eq!(url.path(), "/path");
-        assert_eq!(url.username(), "");
-        assert_eq!(url.password(), None);
-        assert_eq!(url.port(), None);
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.host.as_deref(), Some("example.com"));
+        assert_eq!(url.path, "/path");
+        assert_eq!(url.username, "");
+        assert_eq!(url.password, None);
+        assert_eq!(url.port, None);
     }
 
     #[test]
     fn test_url_with_port() {
         let url = ParsedUrl::parse("http://example.com:8080/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.host_str(), Some("example.com"));
-        assert_eq!(url.port(), Some(8080));
-        assert_eq!(url.path(), "/path");
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.host.as_deref(), Some("example.com"));
+        assert_eq!(url.port, Some(8080));
+        assert_eq!(url.path, "/path");
     }
 
     #[test]
     fn test_url_with_user() {
         let url = ParsedUrl::parse("http://user@example.com/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.username(), "user");
-        assert_eq!(url.host_str(), Some("example.com"));
-        assert_eq!(url.path(), "/path");
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.username, "user");
+        assert_eq!(url.host.as_deref(), Some("example.com"));
+        assert_eq!(url.path, "/path");
     }
 
     #[test]
     fn test_url_with_user_and_password() {
         let url = ParsedUrl::parse("http://user:pass@example.com/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.username(), "user");
-        assert_eq!(url.password(), Some("pass"));
-        assert_eq!(url.host_str(), Some("example.com"));
-        assert_eq!(url.path(), "/path");
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.username, "user");
+        assert_eq!(url.password, Some("pass"));
+        assert_eq!(url.host.as_deref(), Some("example.com"));
+        assert_eq!(url.path, "/path");
     }
 
     #[test]
     fn test_url_with_ipv6() {
         let url = ParsedUrl::parse("http://[::1]/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.host_str(), Some("[::1]"));
-        assert_eq!(url.path(), "/path");
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.host.as_deref(), Some("[::1]"));
+        assert_eq!(url.path, "/path");
     }
 
     #[test]
     fn test_url_with_ipv6_and_port() {
         let url = ParsedUrl::parse("http://[::1]:8080/path").unwrap();
-        assert_eq!(url.scheme(), "http");
-        assert_eq!(url.host_str(), Some("[::1]"));
-        assert_eq!(url.port(), Some(8080));
-        assert_eq!(url.path(), "/path");
+        assert_eq!(url.scheme, "http");
+        assert_eq!(url.host.as_deref(), Some("[::1]"));
+        assert_eq!(url.port, Some(8080));
+        assert_eq!(url.path, "/path");
     }
 }