refactor: use RepoName(String) and RepoOwner(String) newtypes

Author: nik-rev

src/cli.rs

@@ -1,15 +1,13 @@
 //! Parse the command-line arguments
 
-use std::path::PathBuf;
-
 use clap::{
     CommandFactory as _, Parser, Subcommand,
     builder::styling::{AnsiColor, Effects},
 };
 
 use crate::{
     commands,
-    config::{BranchName, Commit, PrNumber, Remote},
+    config::{BranchName, Commit, PatchName, PrNumber, Remote},
 };
 
 /// A tool which makes it easy to declaratively manage personal forks by automatically merging pull requests
@@ -40,7 +38,7 @@ pub enum Command {
         commit: Commit,
         /// Choose a custom file name for the `.patch` file
         #[arg(short, long)]
-        filename: Option<PathBuf>,
+        filename: Option<PatchName>,
     },
     /// Fetch pull request for a GitHub repository as a local branch
     PrFetch {
@@ -138,15 +136,17 @@ pub struct Branch {
 
 #[cfg(test)]
 mod test {
+    use crate::config::{RepoName, RepoOwner};
+
     use super::*;
 
     #[test]
     fn parse_remote() {
         assert_eq!(
             "helix-editor/helix".parse::<Remote>().unwrap(),
             Remote {
-                owner: "helix-editor".to_string(),
-                repo: "helix".to_string(),
+                owner: RepoOwner::try_new("helix-editor").unwrap(),
+                repo: RepoName::try_new("helix").unwrap(),
                 branch: BranchName::try_new("main").unwrap(),
                 commit: None
             }
@@ -156,8 +156,8 @@ mod test {
                 .parse::<Remote>()
                 .unwrap(),
             Remote {
-                owner: "helix-editor".to_string(),
-                repo: "helix".to_string(),
+                owner: RepoOwner::try_new("helix-editor").unwrap(),
+                repo: RepoName::try_new("helix").unwrap(),
                 branch: BranchName::try_new("master").unwrap(),
                 commit: Some(Commit::try_new("1a2b3c").unwrap())
             }

src/commands/gen_patch.rs

@@ -6,12 +6,12 @@ use std::path::PathBuf;
 use anyhow::bail;
 
 use crate::CONFIG_PATH;
-use crate::config::Commit;
+use crate::config::{Commit, PatchName};
 use crate::git::git;
 use crate::utils::normalize_commit_msg;
 
 /// Generate patch `filename` at the given `Commit`
-pub fn gen_patch(commit: Commit, filename: Option<PathBuf>) -> anyhow::Result<()> {
+pub fn gen_patch(commit: Commit, filename: Option<PatchName>) -> anyhow::Result<()> {
     if !CONFIG_PATH.exists() {
         log::info!(
             "Config directory {} does not exist, creating it...",
@@ -23,15 +23,17 @@ pub fn gen_patch(commit: Commit, filename: Option<PathBuf>) -> anyhow::Result<()
     // 1. if the user provides a custom filename for the patch file, use that
     // 2. otherwise use the commit message
     // 3. if all fails use the commit hash
-    let patch_filename = filename.map_or_else(
-        || {
-            git(["log", "--format=%B", "--max-count=1", commit.as_ref()]).map_or_else(
-                |_| commit.clone().into_inner(),
-                |commit_msg| normalize_commit_msg(&commit_msg),
-            )
-        },
-        |filename| filename.to_str().unwrap_or("").to_string(),
-    );
+    let patch_filename = filename.unwrap_or_else(|| {
+        git(["log", "--format=%B", "--max-count=1", commit.as_ref()]).map_or_else(
+            |_| {
+                PatchName::try_new(commit.clone().into_inner().into()).expect("commit is not empty")
+            },
+            |commit_msg| {
+                PatchName::try_new(PathBuf::from(normalize_commit_msg(&commit_msg)))
+                    .expect("normalized commit message is not empty")
+            },
+        )
+    });
 
     let patch_file_path = CONFIG_PATH.join(format!("{patch_filename}.patch"));
 

src/commands/pr_fetch.rs

@@ -3,7 +3,7 @@
 use anyhow::{Context as _, anyhow};
 use colored::Colorize as _;
 
-use crate::config::{BranchName, Commit, PrNumber, Remote};
+use crate::config::{BranchName, Commit, PrNumber, Remote, RepoName, RepoOwner};
 use crate::git::{fetch_pull_request, git};
 
 /// Fetch the given `pr` of `remote` at `commit` and store it in local `branch`
@@ -33,8 +33,8 @@ pub async fn pr_fetch(
                     .and_then(|x| x.split_once('/'))
                     .with_context(err)?;
                 Ok(Remote {
-                    owner: owner.to_string(),
-                    repo: repo.to_string(),
+                    owner: RepoOwner::try_new(owner)?,
+                    repo: RepoName::try_new(repo)?,
                     branch: BranchName::try_new("main").expect("`main` is a valid branch name"),
                     commit: None,
                 })

src/commands/run.rs

@@ -187,8 +187,8 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
 
         log::info!(
             "Merged branch {}/{}/{} {}",
-            owner.bright_blue(),
-            repo.bright_blue(),
+            owner.as_ref().bright_blue(),
+            repo.as_ref().bright_blue(),
             branch.as_ref().bright_blue(),
             remote
                 .commit

src/config.rs

@@ -3,7 +3,7 @@
 use anyhow::{anyhow, bail};
 use itertools::Itertools;
 use nutype::nutype;
-use std::{convert::Infallible, num::NonZeroU32, str::FromStr};
+use std::{convert::Infallible, fmt::Display, num::NonZeroU32, path::PathBuf, str::FromStr};
 use tap::Pipe as _;
 
 use indexmap::IndexSet;
@@ -17,7 +17,7 @@ pub struct Config {
     pub local_branch: BranchName,
     /// List of patches to apply
     #[serde(default)]
-    pub patches: IndexSet<String>,
+    pub patches: IndexSet<PatchName>,
     /// List of pull request to apply
     #[serde(default)]
     pub pull_requests: Vec<PullRequest>,
@@ -34,9 +34,9 @@ pub struct Config {
 #[derive(Debug, Eq, PartialEq, Clone)]
 pub struct Remote {
     /// e.g. `helix-editor`
-    pub owner: String,
+    pub owner: RepoOwner,
     /// e.g. `helix`
-    pub repo: String,
+    pub repo: RepoName,
     /// e.g. `master`
     pub branch: BranchName,
     /// e.g. `1a2b3c`
@@ -66,6 +66,9 @@ impl FromStr for Remote {
             bail!("Invalid branch format: {item}. Expected format: owner/repo/branch");
         };
 
+        let owner = RepoOwner::try_new(owner)?;
+        let repo = RepoName::try_new(repo)?;
+
         let branch = parts
             // insert back the removed '/', this could be part of the branch itself
             // e.g. in `helix-editor/helix/master/main` the branch is considered to be `master/main`
@@ -87,8 +90,8 @@ impl FromStr for Remote {
             .map_err(|err| anyhow!("invalid branch name: {err}"))?;
 
         Ok(Self {
-            owner: owner.to_string(),
-            repo: repo.to_string(),
+            owner,
+            repo,
             branch,
             commit,
         })
@@ -184,11 +187,67 @@ impl FromStr for Ref {
     }
 }
 
+#[cfg(test)]
+impl From<&str> for RepoOwner {
+    fn from(value: &str) -> Self {
+        Self::try_new(value).unwrap()
+    }
+}
+#[cfg(test)]
+impl From<&str> for RepoName {
+    fn from(value: &str) -> Self {
+        Self::try_new(value).unwrap()
+    }
+}
+#[cfg(test)]
+impl From<&str> for BranchName {
+    fn from(value: &str) -> Self {
+        Self::try_new(value).unwrap()
+    }
+}
+#[cfg(test)]
+impl From<&str> for Commit {
+    fn from(value: &str) -> Self {
+        Self::try_new(value).unwrap()
+    }
+}
+
 /// Number of a pull request
 #[nutype(const_fn, derive(Eq, PartialEq, Display, Debug, FromStr, Copy, Clone))]
 pub struct PrNumber(NonZeroU32);
 
+#[cfg(test)]
+impl From<u32> for PrNumber {
+    fn from(value: u32) -> Self {
+        Self::new(NonZeroU32::new(value).unwrap())
+    }
+}
+
+/// Represents owner of a repository
+///
+/// E.g. in `helix-editor/helix/master`, this is `helix-editor`
+#[nutype(
+    validate(not_empty),
+    derive(
+        Debug, Eq, PartialEq, Ord, PartialOrd, Clone, AsRef, Display, Serialize
+    )
+)]
+pub struct RepoOwner(String);
+
+/// Represents name of a repository
+///
+/// E.g. in `helix-editor/helix/master`, this is `helix`
+#[nutype(
+    validate(not_empty),
+    derive(
+        Debug, Eq, PartialEq, Ord, PartialOrd, Clone, AsRef, Display, Serialize
+    )
+)]
+pub struct RepoName(String);
+
 /// Name of a branch in git
+///
+/// E.g. in `helix-editor/helix/master`, this is `master`
 #[nutype(
     validate(not_empty),
     derive(
@@ -207,6 +266,16 @@ impl FromStr for BranchName {
     }
 }
 
+/// File name of a patch
+#[nutype(validate(predicate = |p| !p.as_os_str().is_empty()), derive(Hash, Eq, PartialEq, Debug, AsRef, Deserialize, Clone, FromStr))]
+pub struct PatchName(PathBuf);
+
+impl Display for PatchName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_ref().display())
+    }
+}
+
 /// Represents a git commit hash
 #[nutype(
     validate(not_empty, predicate = is_valid_commit_hash),
@@ -264,45 +333,45 @@ mod tests {
             (
                 "helix-editor/helix/master @ 1a2b3c",
                 Remote {
-                    owner: "helix-editor".to_string(),
-                    repo: "helix".to_string(),
-                    branch: BranchName::try_new("master").unwrap(),
-                    commit: Some(Commit::try_new("1a2b3c".to_string()).unwrap()),
+                    owner: "helix-editor".into(),
+                    repo: "helix".into(),
+                    branch: "master".into(),
+                    commit: Some("1a2b3c".into()),
                 },
             ),
             (
                 "helix-editor/helix @ deadbeef",
                 Remote {
-                    owner: "helix-editor".to_string(),
-                    repo: "helix".to_string(),
-                    branch: BranchName::try_new(Remote::DEFAULT_BRANCH).unwrap(),
-                    commit: Some(Commit::try_new("deadbeef".to_string()).unwrap()),
+                    owner: "helix-editor".into(),
+                    repo: "helix".into(),
+                    branch: Remote::DEFAULT_BRANCH.into(),
+                    commit: Some("deadbeef".into()),
                 },
             ),
             (
                 "helix-editor/helix/feat/feature-x @ abc123",
                 Remote {
-                    owner: "helix-editor".to_string(),
-                    repo: "helix".to_string(),
-                    branch: BranchName::try_new("feat/feature-x").unwrap(),
-                    commit: Some(Commit::try_new("abc123".to_string()).unwrap()),
+                    owner: "helix-editor".into(),
+                    repo: "helix".into(),
+                    branch: "feat/feature-x".into(),
+                    commit: Some("abc123".into()),
                 },
             ),
             (
                 "owner/repo/branch",
                 Remote {
-                    owner: "owner".to_string(),
-                    repo: "repo".to_string(),
-                    branch: BranchName::try_new("branch").unwrap(),
+                    owner: "owner".into(),
+                    repo: "repo".into(),
+                    branch: "branch".into(),
                     commit: None,
                 },
             ),
             (
                 "owner/repo",
                 Remote {
-                    owner: "owner".to_string(),
-                    repo: "repo".to_string(),
-                    branch: BranchName::try_new(Remote::DEFAULT_BRANCH).unwrap(),
+                    owner: "owner".into(),
+                    repo: "repo".into(),
+                    branch: Remote::DEFAULT_BRANCH.into(),
                     commit: None,
                 },
             ),
@@ -338,7 +407,7 @@ patches = ['remove-tab']"#;
             conf,
             Config {
                 local_branch: BranchName::try_new("patchy".to_string()).unwrap(),
-                patches: indexset!["remove-tab".to_string()],
+                patches: indexset![PatchName::try_new("remove-tab".into()).unwrap()],
                 pull_requests: vec![
                     PullRequest {
                         number: pr_number!(10000),