refactor: more type safety with newtypes

Author: nik-rev

Cargo.toml

@@ -43,7 +43,7 @@ once_cell = "1.17"
 documented = "0.9"
 log = "0.4"
 env_logger = "0.11"
-nutype = "0.6"
+nutype = { version = "0.6", features = ["serde"] }
 clap = { version = "4.5.39", features = [
   "derive",
   "wrap_help",

src/cli.rs

@@ -7,7 +7,10 @@ use clap::{
     builder::styling::{AnsiColor, Effects},
 };
 
-use crate::{commands, config::Commit, config::Remote};
+use crate::{
+    commands,
+    config::{BranchName, Commit, Remote},
+};
 
 /// A tool which makes it easy to declaratively manage personal forks by automatically merging pull requests
 #[derive(Parser, Debug)]
@@ -51,7 +54,7 @@ pub enum Command {
         remote: Option<Remote>,
         /// Choose a custom branch name for the fetched repo
         #[arg(short, long)]
-        branch: Option<String>,
+        branch: Option<BranchName>,
         /// When fetching this PR, reset to this commit
         #[arg(short = 'C', long)]
         commit: Option<Commit>,
@@ -144,7 +147,7 @@ mod test {
             Remote {
                 owner: "helix-editor".to_string(),
                 repo: "helix".to_string(),
-                branch: "main".to_string(),
+                branch: BranchName::try_new("main").unwrap(),
                 commit: None
             }
         );
@@ -155,7 +158,7 @@ mod test {
             Remote {
                 owner: "helix-editor".to_string(),
                 repo: "helix".to_string(),
-                branch: "master".to_string(),
+                branch: BranchName::try_new("master").unwrap(),
                 commit: Some(Commit::try_new("1a2b3c").unwrap())
             }
         );

src/commands/branch_fetch.rs

@@ -19,7 +19,7 @@ pub async fn branch_fetch(
         remote.owner,
         remote.repo,
         info.branch.upstream_branch_name,
-        info.branch.local_branch_name.bright_cyan(),
+        info.branch.local_branch_name.as_ref().bright_cyan(),
         commit
             .map(|commit_hash| { format!(", at commit {}", commit_hash.as_ref().bright_yellow()) })
             .unwrap_or_default()
@@ -29,7 +29,7 @@ pub async fn branch_fetch(
     let _ = git(["remote", "remove", &info.remote.local_remote_alias]);
 
     if checkout {
-        git(["checkout", &info.branch.local_branch_name]).map_err(|err| {
+        git(["checkout", info.branch.local_branch_name.as_ref()]).map_err(|err| {
             anyhow!(
                 "failed to check out branch {}:\n{err}",
                 info.branch.local_branch_name

src/commands/pr_fetch.rs

@@ -3,7 +3,7 @@
 use anyhow::{Context as _, anyhow};
 use colored::Colorize as _;
 
-use crate::config::{Commit, Remote};
+use crate::config::{BranchName, Commit, Remote};
 use crate::git::{fetch_pull_request, git};
 
 /// Fetch the given `pr` of `remote` at `commit` and store it in local `branch`
@@ -12,7 +12,7 @@ use crate::git::{fetch_pull_request, git};
 pub async fn pr_fetch(
     pr: u32,
     remote: Option<Remote>,
-    branch: Option<String>,
+    branch: Option<BranchName>,
     commit: Option<Commit>,
     checkout: bool,
 ) -> anyhow::Result<()> {
@@ -35,7 +35,7 @@ pub async fn pr_fetch(
                 Ok(Remote {
                     owner: owner.to_string(),
                     repo: repo.to_string(),
-                    branch: "main".to_string(),
+                    branch: BranchName::try_new("main").expect("`main` is a valid branch name"),
                     commit: None,
                 })
             } else {
@@ -48,8 +48,8 @@ pub async fn pr_fetch(
     match fetch_pull_request(
         &format!("{}/{}", remote.owner, remote.repo),
         // TODO: make fetch_pull_request accept a u32 instead
-        &pr.to_string(),
-        branch.as_deref(),
+        pr,
+        branch,
         commit.as_ref(),
     )
     .await
@@ -67,7 +67,7 @@ pub async fn pr_fetch(
                     ),
                     &response.html_url
                 ),
-                info.branch.local_branch_name.bright_cyan(),
+                info.branch.local_branch_name.as_ref().bright_cyan(),
                 commit
                     .clone()
                     .map(|commit_hash| {
@@ -80,7 +80,9 @@ pub async fn pr_fetch(
             let _ = git(["remote", "remove", &info.remote.local_remote_alias]);
 
             if checkout {
-                if let Err(cant_checkout) = git(["checkout", &info.branch.local_branch_name]) {
+                if let Err(cant_checkout) =
+                    git(["checkout", info.branch.local_branch_name.as_ref()])
+                {
                     log::error!(
                         "Could not check out branch {}:\n{cant_checkout}",
                         info.branch.local_branch_name

src/commands/run.rs

@@ -1,6 +1,6 @@
 //! `run` subcommand
 
-use crate::config::{Config, Ref};
+use crate::config::{BranchName, Config, PullRequest};
 use std::ffi::OsString;
 use std::fs::{self, File};
 use std::io::Write as _;
@@ -51,8 +51,8 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
         anyhow!("Could not parse `{root}/{CONFIG_FILE}` configuration file:\n{err}",)
     })?;
 
-    let Ref {
-        item: remote_branch,
+    let crate::config::Branch {
+        name: remote_branch,
         commit,
     } = config.remote_branch;
 
@@ -103,7 +103,8 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
         },
         branch: Branch {
             upstream_branch_name: remote_branch.clone(),
-            local_branch_name: with_uuid(&remote_branch),
+            local_branch_name: BranchName::try_new(with_uuid(remote_branch.as_ref()))
+                .expect("adding UUID to branch name does not invalidate it"),
         },
     };
 
@@ -128,14 +129,14 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
     // TODO: make this concurrent, see https://users.rust-lang.org/t/processing-subprocesses-concurrently/79638/3
     // Git cannot handle multiple threads executing commands in the same repository,
     // so we can't use threads, but we can run processes in the background
-    for Ref {
-        item: pull_request,
+    for PullRequest {
+        number: pull_request,
         commit,
     } in &config.pull_requests
     {
         // TODO: refactor this to not use such deep nesting
         let Ok((response, info)) =
-            git::fetch_pull_request(&config.repo, pull_request, None, commit.as_ref())
+            git::fetch_pull_request(&config.repo, *pull_request, None, commit.as_ref())
                 .await
                 .inspect_err(|err| {
                     log::error!("failed to fetch branch from remote:\n{err}");
@@ -145,7 +146,7 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
         };
 
         if let Err(err) =
-            git::merge_pull_request(&info, pull_request, &response.title, &response.html_url)
+            git::merge_pull_request(&info, *pull_request, &response.title, &response.html_url)
         {
             log::error!("failed to merge {pull_request}: {err}");
             continue;
@@ -157,7 +158,7 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
                 &format!(
                     "{}{}{}{}",
                     "#".bright_blue(),
-                    pull_request.bright_blue(),
+                    pull_request.to_string().bright_blue(),
                     " ".bright_blue(),
                     &response.title.bright_blue().italic()
                 ),
@@ -188,7 +189,7 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
             "Merged branch {}/{}/{} {}",
             owner.bright_blue(),
             repo.bright_blue(),
-            branch.bright_blue(),
+            branch.as_ref().bright_blue(),
             remote
                 .commit
                 .as_ref()
@@ -274,7 +275,7 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
     if yes
         || confirm_prompt!(
             "Overwrite branch {}? This is irreversible.",
-            config.local_branch.cyan()
+            config.local_branch.as_ref().cyan()
         )
     {
         // forcefully renames the branch we are currently on into the branch specified
@@ -285,12 +286,12 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
             "--move",
             "--force",
             &temporary_branch,
-            &config.local_branch,
+            config.local_branch.as_ref(),
         ])?;
         if yes {
             log::info!(
                 "Automatically overwrote branch {} since you supplied the {} flag",
-                config.local_branch.cyan(),
+                config.local_branch.as_ref().cyan(),
                 "--yes".bright_magenta()
             );
         }
@@ -304,7 +305,7 @@ pub async fn run(yes: bool) -> anyhow::Result<()> {
     );
     log::info!(
         "You can still manually overwrite {} with:\n  {overwrite_command}\n",
-        config.local_branch.cyan(),
+        config.local_branch.as_ref().cyan(),
     );
 
     Ok(())