From 6acefda5d3aae4e7b3098f705718a3c3cf245488 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 6 Aug 2024 07:43:28 +0100 Subject: [PATCH] refactor: cleanup pedantic clippy in core crate --- crates/cli/src/repo/handlers/validate_repo.rs | 4 +- .../src/repo/tests/handlers/advance_main.rs | 2 +- .../src/server/actor/handlers/file_updated.rs | 8 +-- crates/cli/src/server/actor/handlers/mod.rs | 4 +- ...server_config.rs => receive_app_config.rs} | 8 +-- ..._config.rs => receive_valid_app_config.rs} | 20 +++---- crates/cli/src/server/actor/messages.rs | 12 ++--- crates/cli/src/server/actor/mod.rs | 22 ++++---- crates/cli/src/server/actor/tests/mod.rs | 2 +- ...server_config.rs => receive_app_config.rs} | 20 ++++--- crates/core/Cargo.toml | 18 +++---- crates/core/src/config/api_token.rs | 6 +-- crates/core/src/config/branch_name.rs | 4 +- crates/core/src/config/common.rs | 30 ++++++----- crates/core/src/config/forge_config.rs | 10 ++-- crates/core/src/config/forge_details.rs | 10 +++- crates/core/src/config/forge_type.rs | 2 +- crates/core/src/config/git_dir.rs | 8 +-- crates/core/src/config/mod.rs | 4 ++ crates/core/src/config/registered_webhook.rs | 3 ++ crates/core/src/config/remote_url.rs | 4 +- crates/core/src/config/repo_branches.rs | 3 ++ crates/core/src/config/repo_config.rs | 10 +++- crates/core/src/config/server.rs | 45 ++++++++++++---- crates/core/src/config/server_repo_config.rs | 11 ++-- crates/core/src/config/tests.rs | 50 ++++++++--------- crates/core/src/config/tests/url.rs | 8 +-- crates/core/src/config/webhook/auth.rs | 12 ++++- .../src/config/webhook/forge_notification.rs | 15 ++++-- crates/core/src/config/webhook/push.rs | 5 ++ crates/core/src/git/commit.rs | 2 + crates/core/src/git/forge/mod.rs | 6 ++- .../core/src/git/forge/{like.rs => trait.rs} | 7 ++- crates/core/src/git/generation.rs | 2 +- crates/core/src/git/git_remote.rs | 6 ++- crates/core/src/git/mod.rs | 7 ++- crates/core/src/git/push.rs | 11 +++- crates/core/src/git/repo_details.rs | 37 ++++++------- crates/core/src/git/repository/factory.rs | 21 ++++++-- crates/core/src/git/repository/mod.rs | 35 ++++++------ crates/core/src/git/repository/open/mod.rs | 39 ++++++++++++-- crates/core/src/git/repository/open/oreal.rs | 29 +++++----- crates/core/src/git/repository/open/otest.rs | 41 ++++++++------ .../src/git/repository/open/tests/fetch.rs | 2 +- crates/core/src/git/repository/test.rs | 3 +- .../core/src/git/repository/tests/factory.rs | 4 -- crates/core/src/git/repository/tests/mod.rs | 1 + crates/core/src/git/tests.rs | 54 ++++--------------- crates/core/src/git/user_notification.rs | 1 + crates/core/src/git/validation/positions.rs | 33 +++++++----- crates/core/src/git/validation/tests.rs | 27 +++++----- crates/core/src/macros/newtype.rs | 1 + 52 files changed, 433 insertions(+), 296 deletions(-) rename crates/cli/src/server/actor/handlers/{receive_server_config.rs => receive_app_config.rs} (72%) rename crates/cli/src/server/actor/handlers/{receive_valid_server_config.rs => receive_valid_app_config.rs} (82%) rename crates/cli/src/server/actor/tests/{receive_server_config.rs => receive_app_config.rs} (76%) rename crates/core/src/git/forge/{like.rs => trait.rs} (87%) diff --git a/crates/cli/src/repo/handlers/validate_repo.rs b/crates/cli/src/repo/handlers/validate_repo.rs index 92a7ba7..a693140 100644 --- a/crates/cli/src/repo/handlers/validate_repo.rs +++ b/crates/cli/src/repo/handlers/validate_repo.rs @@ -9,7 +9,7 @@ use crate::repo::{ notify_user, RepoActor, }; -use git_next_core::git::validation::positions::{validate_positions, Error, Positions}; +use git_next_core::git::validation::positions::{validate, Error, Positions}; impl Handler for RepoActor { type Result = (); @@ -53,7 +53,7 @@ impl Handler for RepoActor { }; logger(self.log.as_ref(), "have repo config"); - match validate_positions(&**open_repository, &self.repo_details, repo_config) { + match validate(&**open_repository, &self.repo_details, &repo_config) { Ok(Positions { main, next, diff --git a/crates/cli/src/repo/tests/handlers/advance_main.rs b/crates/cli/src/repo/tests/handlers/advance_main.rs index 9812c9f..e166ffa 100644 --- a/crates/cli/src/repo/tests/handlers/advance_main.rs +++ b/crates/cli/src/repo/tests/handlers/advance_main.rs @@ -47,7 +47,7 @@ async fn when_repo_config_should_fetch_then_push_then_revalidate() -> TestResult } #[actix::test] -async fn when_server_config_should_fetch_then_push_then_revalidate() -> TestResult { +async fn when_app_config_should_fetch_then_push_then_revalidate() -> TestResult { //given let fs = given::a_filesystem(); let (mut open_repository, mut repo_details) = given::an_open_repository(&fs); diff --git a/crates/cli/src/server/actor/handlers/file_updated.rs b/crates/cli/src/server/actor/handlers/file_updated.rs index 499bac8..95fe7c9 100644 --- a/crates/cli/src/server/actor/handlers/file_updated.rs +++ b/crates/cli/src/server/actor/handlers/file_updated.rs @@ -1,19 +1,19 @@ // use actix::prelude::*; -use git_next_core::server::ServerConfig; +use git_next_core::server::AppConfig; use crate::{ file_watcher::FileUpdated, - server::actor::{messages::ReceiveServerConfig, ServerActor}, + server::actor::{messages::ReceiveAppConfig, ServerActor}, }; impl Handler for ServerActor { type Result = (); fn handle(&mut self, _msg: FileUpdated, ctx: &mut Self::Context) -> Self::Result { - match ServerConfig::load(&self.fs) { - Ok(server_config) => self.do_send(ReceiveServerConfig::new(server_config), ctx), + match AppConfig::load(&self.fs) { + Ok(app_config) => self.do_send(ReceiveAppConfig::new(app_config), ctx), Err(err) => self.abort(ctx, format!("Failed to load config file. Error: {err}")), }; } diff --git a/crates/cli/src/server/actor/handlers/mod.rs b/crates/cli/src/server/actor/handlers/mod.rs index c92aeeb..f488dce 100644 --- a/crates/cli/src/server/actor/handlers/mod.rs +++ b/crates/cli/src/server/actor/handlers/mod.rs @@ -1,4 +1,4 @@ mod file_updated; -mod receive_server_config; -mod receive_valid_server_config; +mod receive_app_config; +mod receive_valid_app_config; mod shutdown; diff --git a/crates/cli/src/server/actor/handlers/receive_server_config.rs b/crates/cli/src/server/actor/handlers/receive_app_config.rs similarity index 72% rename from crates/cli/src/server/actor/handlers/receive_server_config.rs rename to crates/cli/src/server/actor/handlers/receive_app_config.rs index b447ab8..9a17b4f 100644 --- a/crates/cli/src/server/actor/handlers/receive_server_config.rs +++ b/crates/cli/src/server/actor/handlers/receive_app_config.rs @@ -1,15 +1,15 @@ use actix::prelude::*; use crate::server::actor::{ - messages::{ReceiveServerConfig, ReceiveValidServerConfig, ValidServerConfig}, + messages::{ReceiveAppConfig, ReceiveValidAppConfig, ValidAppConfig}, ServerActor, }; -impl Handler for ServerActor { +impl Handler for ServerActor { type Result = (); #[allow(clippy::cognitive_complexity)] - fn handle(&mut self, msg: ReceiveServerConfig, ctx: &mut Self::Context) -> Self::Result { + fn handle(&mut self, msg: ReceiveAppConfig, ctx: &mut Self::Context) -> Self::Result { tracing::info!("recieved server config"); let Ok(socket_addr) = msg.listen_socket_addr() else { return self.abort(ctx, "Unable to parse http.addr"); @@ -24,7 +24,7 @@ impl Handler for ServerActor { } self.do_send( - ReceiveValidServerConfig::new(ValidServerConfig::new( + ReceiveValidAppConfig::new(ValidAppConfig::new( msg.unwrap(), socket_addr, server_storage, diff --git a/crates/cli/src/server/actor/handlers/receive_valid_server_config.rs b/crates/cli/src/server/actor/handlers/receive_valid_app_config.rs similarity index 82% rename from crates/cli/src/server/actor/handlers/receive_valid_server_config.rs rename to crates/cli/src/server/actor/handlers/receive_valid_app_config.rs index 8df1824..2e3b3cd 100644 --- a/crates/cli/src/server/actor/handlers/receive_valid_server_config.rs +++ b/crates/cli/src/server/actor/handlers/receive_valid_app_config.rs @@ -8,7 +8,7 @@ use crate::{ alerts::messages::UpdateShout, repo::{messages::CloneRepo, RepoActor}, server::actor::{ - messages::{ReceiveValidServerConfig, ValidServerConfig}, + messages::{ReceiveValidAppConfig, ValidAppConfig}, ServerActor, }, webhook::{ @@ -18,14 +18,14 @@ use crate::{ }, }; -impl Handler for ServerActor { +impl Handler for ServerActor { type Result = (); - fn handle(&mut self, msg: ReceiveValidServerConfig, _ctx: &mut Self::Context) -> Self::Result { - let ValidServerConfig { - server_config, + fn handle(&mut self, msg: ReceiveValidAppConfig, _ctx: &mut Self::Context) -> Self::Result { + let ValidAppConfig { + app_config, socket_address, - server_storage, + storage: server_storage, } = msg.unwrap(); // shutdown any existing webhook actor if let Some(webhook_actor_addr) = self.webhook_actor_addr.take() { @@ -35,10 +35,10 @@ impl Handler for ServerActor { // Webhook Server info!("Starting Webhook Server..."); let webhook_router = WebhookRouterActor::default().start(); - let listen_url = server_config.listen().url(); + let listen_url = app_config.listen().url(); let notify_user_recipient = self.alerts.clone().recipient(); // Forge Actors - for (forge_alias, forge_config) in server_config.forges() { + for (forge_alias, forge_config) in app_config.forges() { let repo_actors = self .create_forge_repos( forge_config, @@ -68,8 +68,8 @@ impl Handler for ServerActor { let webhook_actor_addr = WebhookActor::new(socket_address, webhook_router.recipient()).start(); self.webhook_actor_addr.replace(webhook_actor_addr); - let shout = server_config.shout().clone(); - self.server_config.replace(server_config); + let shout = app_config.shout().clone(); + self.app_config.replace(app_config); self.alerts.do_send(UpdateShout::new(shout)); } } diff --git a/crates/cli/src/server/actor/messages.rs b/crates/cli/src/server/actor/messages.rs index d4dfe0c..a046e15 100644 --- a/crates/cli/src/server/actor/messages.rs +++ b/crates/cli/src/server/actor/messages.rs @@ -3,13 +3,13 @@ use derive_more::Constructor; use git_next_core::{ message, - server::{ServerConfig, ServerStorage}, + server::{AppConfig, Storage}, }; use std::net::SocketAddr; // receive server config -message!(ReceiveServerConfig: ServerConfig: "Notification of newly loaded server configuration. +message!(ReceiveAppConfig: AppConfig: "Notification of newly loaded server configuration. This message will prompt the `git-next` server to stop and restart all repo-actors. @@ -17,11 +17,11 @@ Contains the new server configuration."); // receive valid server config #[derive(Clone, Debug, PartialEq, Eq, Constructor)] -pub struct ValidServerConfig { - pub server_config: ServerConfig, +pub struct ValidAppConfig { + pub app_config: AppConfig, pub socket_address: SocketAddr, - pub server_storage: ServerStorage, + pub storage: Storage, } -message!(ReceiveValidServerConfig: ValidServerConfig: "Notification of validated server configuration."); +message!(ReceiveValidAppConfig: ValidAppConfig: "Notification of validated server configuration."); message!(Shutdown: "Notification to shutdown the server actor"); diff --git a/crates/cli/src/server/actor/mod.rs b/crates/cli/src/server/actor/mod.rs index 54722b7..fab2b90 100644 --- a/crates/cli/src/server/actor/mod.rs +++ b/crates/cli/src/server/actor/mod.rs @@ -1,6 +1,6 @@ // use actix::prelude::*; -use messages::ReceiveServerConfig; +use messages::ReceiveAppConfig; use tracing::error; #[cfg(test)] @@ -16,7 +16,7 @@ use crate::{ use git_next_core::{ git::{repository::factory::RepositoryFactory, Generation, RepoDetails}, - server::{self, ListenUrl, ServerConfig, ServerStorage}, + server::{self, AppConfig, ListenUrl, Storage}, ForgeAlias, ForgeConfig, GitDir, RepoAlias, ServerRepoConfig, StoragePathType, }; @@ -48,7 +48,7 @@ type Result = core::result::Result; #[derive(derive_with::With)] #[with(message_log)] pub struct ServerActor { - server_config: Option, + app_config: Option, generation: Generation, webhook_actor_addr: Option>, fs: FileSystem, @@ -75,7 +75,7 @@ impl ServerActor { ) -> Self { let generation = Generation::default(); Self { - server_config: None, + app_config: None, generation, webhook_actor_addr: None, fs, @@ -89,10 +89,10 @@ impl ServerActor { } fn create_forge_data_directories( &self, - server_config: &ServerConfig, + app_config: &AppConfig, server_dir: &std::path::Path, ) -> Result<()> { - for (forge_name, _forge_config) in server_config.forges() { + for (forge_name, _forge_config) in app_config.forges() { let forge_dir: PathBuf = (&forge_name).into(); let path = server_dir.join(&forge_dir); if self.fs.path_exists(&path)? { @@ -112,7 +112,7 @@ impl ServerActor { &self, forge_config: &ForgeConfig, forge_name: ForgeAlias, - server_storage: &ServerStorage, + server_storage: &Storage, listen_url: &ListenUrl, notify_user_recipient: &Recipient, ) -> Vec<(ForgeAlias, RepoAlias, RepoActor)> { @@ -143,7 +143,7 @@ impl ServerActor { &self, forge_name: ForgeAlias, forge_config: ForgeConfig, - server_storage: &ServerStorage, + server_storage: &Storage, listen_url: &ListenUrl, ) -> impl Fn( (RepoAlias, &ServerRepoConfig, Recipient), @@ -196,8 +196,8 @@ impl ServerActor { } } - fn server_storage(&self, server_config: &ReceiveServerConfig) -> Option { - let server_storage = server_config.storage().clone(); + fn server_storage(&self, app_config: &ReceiveAppConfig) -> Option { + let server_storage = app_config.storage().clone(); let dir = server_storage.path(); if !dir.exists() { if let Err(err) = self.fs.dir_create(dir) { @@ -209,7 +209,7 @@ impl ServerActor { error!(?dir, "Failed to confirm server storage"); return None; }; - if let Err(err) = self.create_forge_data_directories(server_config, &canon) { + if let Err(err) = self.create_forge_data_directories(app_config, &canon) { error!(?err, "Failure creating forge storage"); return None; } diff --git a/crates/cli/src/server/actor/tests/mod.rs b/crates/cli/src/server/actor/tests/mod.rs index 8c5b19c..a987c11 100644 --- a/crates/cli/src/server/actor/tests/mod.rs +++ b/crates/cli/src/server/actor/tests/mod.rs @@ -1,3 +1,3 @@ -mod receive_server_config; +mod receive_app_config; mod given; diff --git a/crates/cli/src/server/actor/tests/receive_server_config.rs b/crates/cli/src/server/actor/tests/receive_app_config.rs similarity index 76% rename from crates/cli/src/server/actor/tests/receive_server_config.rs rename to crates/cli/src/server/actor/tests/receive_app_config.rs index 8879a17..d5770a6 100644 --- a/crates/cli/src/server/actor/tests/receive_server_config.rs +++ b/crates/cli/src/server/actor/tests/receive_app_config.rs @@ -1,10 +1,10 @@ // use actix::prelude::*; -use crate::server::actor::{tests::given, ReceiveServerConfig, ServerActor}; +use crate::server::actor::{tests::given, ReceiveAppConfig, ServerActor}; use git_next_core::{ git, - server::{Http, Listen, ListenUrl, ServerConfig, ServerStorage, Shout}, + server::{AppConfig, Http, Listen, ListenUrl, Shout, Storage}, }; use std::{ @@ -31,7 +31,7 @@ async fn when_webhook_url_has_trailing_slash_should_not_send() { ListenUrl::new("http://localhost/".to_string()), // with trailing slash ); let shout = Shout::default(); - let server_storage = ServerStorage::new((fs.base()).to_path_buf()); + let server_storage = Storage::new((fs.base()).to_path_buf()); let repos = BTreeMap::default(); // debugging @@ -39,14 +39,12 @@ async fn when_webhook_url_has_trailing_slash_should_not_send() { let server = server.with_message_log(Some(message_log.clone())); //when - server - .start() - .do_send(ReceiveServerConfig::new(ServerConfig::new( - listen, - shout, - server_storage, - repos, - ))); + server.start().do_send(ReceiveAppConfig::new(AppConfig::new( + listen, + shout, + server_storage, + repos, + ))); actix_rt::time::sleep(std::time::Duration::from_millis(1)).await; //then diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index d8112f9..ae3cc40 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -6,6 +6,15 @@ license = { workspace = true } repository = { workspace = true } description = "core for git-next, the trunk-based development manager" +[lints.clippy] +nursery = { level = "warn", priority = -1 } +pedantic = { level = "warn", priority = -1 } +unwrap_used = "warn" +expect_used = "warn" + +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(tarpaulin_include)'] } + [features] default = ["forgejo", "github"] forgejo = [] @@ -54,12 +63,3 @@ assert2 = { workspace = true } rand = { workspace = true } test-log = { workspace = true } pretty_assertions = { workspace = true } - -[lints.clippy] -nursery = { level = "warn", priority = -1 } -# pedantic = "warn" -unwrap_used = "warn" -expect_used = "warn" - -[lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(tarpaulin_include)'] } diff --git a/crates/core/src/config/api_token.rs b/crates/core/src/config/api_token.rs index 42dc049..1c46b1a 100644 --- a/crates/core/src/config/api_token.rs +++ b/crates/core/src/config/api_token.rs @@ -1,6 +1,6 @@ /// The API Token for the [user] -/// ForgeJo: https://{hostname}/user/settings/applications -/// Github: https://github.com/settings/tokens +/// `ForgeJo`: +/// `Github`: #[derive(Clone, Debug, derive_more::Constructor)] pub struct ApiToken(secrecy::Secret); /// The API Token is in effect a password, so it must be explicitly exposed to access its value @@ -11,6 +11,6 @@ impl secrecy::ExposeSecret for ApiToken { } impl Default for ApiToken { fn default() -> Self { - Self("".to_string().into()) + Self(String::new().into()) } } diff --git a/crates/core/src/config/branch_name.rs b/crates/core/src/config/branch_name.rs index 26e329b..694a33c 100644 --- a/crates/core/src/config/branch_name.rs +++ b/crates/core/src/config/branch_name.rs @@ -1,4 +1,6 @@ use derive_more::derive::Display; use serde::Serialize; -crate::newtype!(BranchName: String, Display, Default, Hash, Serialize: "The name of a Git branch"); +use crate::newtype; + +newtype!(BranchName: String, Display, Default, Hash, Serialize: "The name of a Git branch"); diff --git a/crates/core/src/config/common.rs b/crates/core/src/config/common.rs index 049dd70..74c7242 100644 --- a/crates/core/src/config/common.rs +++ b/crates/core/src/config/common.rs @@ -3,6 +3,7 @@ use crate::config::{ RepoConfig, RepoConfigSource, RepoPath, User, }; +#[must_use] pub fn forge_details(n: u32, forge_type: ForgeType) -> ForgeDetails { ForgeDetails::new( forge_name(n), @@ -13,34 +14,35 @@ pub fn forge_details(n: u32, forge_type: ForgeType) -> ForgeDetails { ) } -pub fn api_token(n: u32) -> ApiToken { - ApiToken::new(format!("api-{}", n).into()) +pub(crate) fn api_token(n: u32) -> ApiToken { + ApiToken::new(format!("api-{n}").into()) } -pub fn user(n: u32) -> User { - User::new(format!("user-{}", n)) +pub(crate) fn user(n: u32) -> User { + User::new(format!("user-{n}")) } -pub fn hostname(n: u32) -> Hostname { - Hostname::new(format!("hostname-{}", n)) +pub(crate) fn hostname(n: u32) -> Hostname { + Hostname::new(format!("hostname-{n}")) } -pub fn forge_name(n: u32) -> ForgeAlias { - ForgeAlias::new(format!("forge-name-{}", n)) +pub(crate) fn forge_name(n: u32) -> ForgeAlias { + ForgeAlias::new(format!("forge-name-{n}")) } -pub fn branch_name(n: u32) -> BranchName { - BranchName::new(format!("branch-name-{}", n)) +pub(crate) fn branch_name(n: u32) -> BranchName { + BranchName::new(format!("branch-name-{n}")) } -pub fn repo_path(n: u32) -> RepoPath { - RepoPath::new(format!("repo-path-{}", n)) +pub(crate) fn repo_path(n: u32) -> RepoPath { + RepoPath::new(format!("repo-path-{n}")) } -pub fn repo_alias(n: u32) -> RepoAlias { - RepoAlias::new(format!("repo-alias-{}", n)) +pub(crate) fn repo_alias(n: u32) -> RepoAlias { + RepoAlias::new(format!("repo-alias-{n}")) } +#[must_use] pub fn repo_config(n: u32, source: RepoConfigSource) -> RepoConfig { RepoConfig::new( RepoBranches::new(format!("main-{n}"), format!("next-{n}"), format!("dev-{n}")), diff --git a/crates/core/src/config/forge_config.rs b/crates/core/src/config/forge_config.rs index 75dff39..832b0d1 100644 --- a/crates/core/src/config/forge_config.rs +++ b/crates/core/src/config/forge_config.rs @@ -25,19 +25,19 @@ pub struct ForgeConfig { repos: BTreeMap, } impl ForgeConfig { - pub const fn forge_type(&self) -> ForgeType { + pub(crate) const fn forge_type(&self) -> ForgeType { self.forge_type } - pub fn hostname(&self) -> Hostname { + pub(crate) fn hostname(&self) -> Hostname { Hostname::new(&self.hostname) } - pub fn user(&self) -> User { + pub(crate) fn user(&self) -> User { User::new(self.user.clone()) } - pub fn token(&self) -> ApiToken { + pub(crate) fn token(&self) -> ApiToken { ApiToken::new(self.token.clone().into()) } @@ -47,6 +47,8 @@ impl ForgeConfig { .map(|(name, repo)| (RepoAlias::new(name), repo)) } + #[cfg(test)] + #[must_use] pub fn get_repo(&self, arg: &str) -> Option<&ServerRepoConfig> { self.repos.get(arg) } diff --git a/crates/core/src/config/forge_details.rs b/crates/core/src/config/forge_details.rs index a1c1716..b05ef6d 100644 --- a/crates/core/src/config/forge_details.rs +++ b/crates/core/src/config/forge_details.rs @@ -12,18 +12,26 @@ pub struct ForgeDetails { // Private SSH Key Path } impl ForgeDetails { + #[must_use] pub const fn forge_alias(&self) -> &ForgeAlias { &self.forge_alias } + + #[must_use] pub const fn forge_type(&self) -> ForgeType { self.forge_type } + + #[must_use] pub const fn hostname(&self) -> &Hostname { &self.hostname } - pub const fn user(&self) -> &User { + + pub(crate) const fn user(&self) -> &User { &self.user } + + #[must_use] pub const fn token(&self) -> &ApiToken { &self.token } diff --git a/crates/core/src/config/forge_type.rs b/crates/core/src/config/forge_type.rs index 9154a78..50bb9be 100644 --- a/crates/core/src/config/forge_type.rs +++ b/crates/core/src/config/forge_type.rs @@ -24,6 +24,6 @@ pub enum ForgeType { } impl std::fmt::Display for ForgeType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", self) + write!(f, "{self:?}") } } diff --git a/crates/core/src/config/git_dir.rs b/crates/core/src/config/git_dir.rs index 91aea9e..5607dcd 100644 --- a/crates/core/src/config/git_dir.rs +++ b/crates/core/src/config/git_dir.rs @@ -22,13 +22,15 @@ pub struct GitDir { storage_path_type: StoragePathType, } impl GitDir { - pub const fn pathbuf(&self) -> &PathBuf { + pub(crate) const fn pathbuf(&self) -> &PathBuf { &self.pathbuf } - pub const fn storage_path_type(&self) -> StoragePathType { + + pub(crate) const fn storage_path_type(&self) -> StoragePathType { self.storage_path_type } - pub fn as_fs(&self) -> kxio::fs::FileSystem { + + pub(crate) fn as_fs(&self) -> kxio::fs::FileSystem { pike! { self |> Self::pathbuf diff --git a/crates/core/src/config/mod.rs b/crates/core/src/config/mod.rs index 70411a8..e1d9f7f 100644 --- a/crates/core/src/config/mod.rs +++ b/crates/core/src/config/mod.rs @@ -26,6 +26,7 @@ mod tests; pub use api_token::ApiToken; pub use branch_name::BranchName; pub use forge_alias::ForgeAlias; +#[allow(clippy::module_name_repetitions)] pub use forge_config::ForgeConfig; pub use forge_details::ForgeDetails; pub use forge_type::ForgeType; @@ -36,10 +37,13 @@ pub use registered_webhook::RegisteredWebhook; pub use remote_url::RemoteUrl; pub use repo_alias::RepoAlias; pub use repo_branches::RepoBranches; +#[allow(clippy::module_name_repetitions)] pub use repo_config::RepoConfig; pub use repo_config_source::RepoConfigSource; pub use repo_path::RepoPath; +#[allow(clippy::module_name_repetitions)] pub use server_repo_config::ServerRepoConfig; +#[allow(clippy::module_name_repetitions)] pub use user::User; pub use webhook::auth::WebhookAuth; pub use webhook::forge_notification::ForgeNotification; diff --git a/crates/core/src/config/registered_webhook.rs b/crates/core/src/config/registered_webhook.rs index 31427cb..5f3ece3 100644 --- a/crates/core/src/config/registered_webhook.rs +++ b/crates/core/src/config/registered_webhook.rs @@ -7,9 +7,12 @@ pub struct RegisteredWebhook { auth: WebhookAuth, } impl RegisteredWebhook { + #[must_use] pub const fn id(&self) -> &WebhookId { &self.id } + + #[must_use] pub const fn auth(&self) -> &WebhookAuth { &self.auth } diff --git a/crates/core/src/config/remote_url.rs b/crates/core/src/config/remote_url.rs index 6970969..5d41611 100644 --- a/crates/core/src/config/remote_url.rs +++ b/crates/core/src/config/remote_url.rs @@ -1,3 +1,5 @@ +use std::borrow::ToOwned; + crate::newtype!(RemoteUrl: git_url_parse::GitUrl, derive_more::Display: "The URL of a remote repository"); impl RemoteUrl { pub fn parse(url: impl Into) -> Option { @@ -8,7 +10,7 @@ impl TryFrom for RemoteUrl { type Error = (); fn try_from(url: gix::Url) -> Result { - let pass = url.password().map(|p| p.to_owned()); + let pass = url.password().map(ToOwned::to_owned); let mut parsed = ::git_url_parse::GitUrl::parse(&url.to_string()).map_err(|_| ())?; parsed.token = pass; Ok(Self(parsed)) diff --git a/crates/core/src/config/repo_branches.rs b/crates/core/src/config/repo_branches.rs index 0eed541..b4df8bc 100644 --- a/crates/core/src/config/repo_branches.rs +++ b/crates/core/src/config/repo_branches.rs @@ -21,14 +21,17 @@ pub struct RepoBranches { dev: String, } impl RepoBranches { + #[must_use] pub fn main(&self) -> BranchName { BranchName::new(&self.main) } + #[must_use] pub fn next(&self) -> BranchName { BranchName::new(&self.next) } + #[must_use] pub fn dev(&self) -> BranchName { BranchName::new(&self.dev) } diff --git a/crates/core/src/config/repo_config.rs b/crates/core/src/config/repo_config.rs index e522959..4a859c1 100644 --- a/crates/core/src/config/repo_config.rs +++ b/crates/core/src/config/repo_config.rs @@ -23,14 +23,22 @@ pub struct RepoConfig { source: RepoConfigSource, } impl RepoConfig { + /// Parses the TOML document into a `RepoConfig`. + /// + /// # Errors + /// + /// Will return `Err` if the TOML file is invalid or otherwise doesn't + /// match a `RepoConfig`. pub fn parse(toml: &str) -> Result { - toml::from_str(format!("source = \"Repo\"\n{}", toml).as_str()) + toml::from_str(format!("source = \"Repo\"\n{toml}").as_str()) } + #[must_use] pub const fn branches(&self) -> &RepoBranches { &self.branches } + #[must_use] pub const fn source(&self) -> RepoConfigSource { self.source } diff --git a/crates/core/src/config/server.rs b/crates/core/src/config/server.rs index 794868c..acf4a8c 100644 --- a/crates/core/src/config/server.rs +++ b/crates/core/src/config/server.rs @@ -46,13 +46,13 @@ type Result = core::result::Result; serde::Deserialize, derive_more::Constructor, )] -pub struct ServerConfig { +pub struct AppConfig { listen: Listen, shout: Shout, - storage: ServerStorage, + storage: Storage, pub forge: BTreeMap, } -impl ServerConfig { +impl AppConfig { #[tracing::instrument(skip_all)] pub fn load(fs: &FileSystem) -> Result { let file = fs.base().join("git-next-server.toml"); @@ -67,18 +67,26 @@ impl ServerConfig { .map(|(alias, forge)| (ForgeAlias::new(alias.clone()), forge)) } - pub const fn storage(&self) -> &ServerStorage { + #[must_use] + pub const fn storage(&self) -> &Storage { &self.storage } + #[must_use] pub const fn shout(&self) -> &Shout { &self.shout } + #[must_use] pub const fn listen(&self) -> &Listen { &self.listen } + /// Returns the `SocketAddr` to listen to for incoming webhooks. + /// + /// # Errors + /// + /// Will return an `Err` if the IP address or port from the config file are invalid. pub fn listen_socket_addr(&self) -> Result { self.listen.http.socket_addr() } @@ -102,11 +110,12 @@ pub struct Listen { url: ListenUrl, } impl Listen { - /// Returns the URL a Repo will listen to for updates from the Forge - pub fn repo_url(&self, forge_alias: ForgeAlias, repo_alias: RepoAlias) -> RepoListenUrl { - self.url.repo_url(forge_alias, repo_alias) - } + // /// Returns the URL a Repo will listen to for updates from the Forge + // pub fn repo_url(&self, forge_alias: ForgeAlias, repo_alias: RepoAlias) -> RepoListenUrl { + // self.url.repo_url(forge_alias, repo_alias) + // } + #[must_use] pub const fn url(&self) -> &ListenUrl { &self.url } @@ -118,6 +127,7 @@ newtype!( "The base url for receiving all webhooks from all forges" ); impl ListenUrl { + #[must_use] pub fn repo_url(&self, forge_alias: ForgeAlias, repo_alias: RepoAlias) -> RepoListenUrl { RepoListenUrl::new((self.clone(), forge_alias, repo_alias)) } @@ -182,10 +192,11 @@ impl Http { serde::Deserialize, derive_more::Constructor, )] -pub struct ServerStorage { +pub struct Storage { path: PathBuf, } -impl ServerStorage { +impl Storage { + #[must_use] pub fn path(&self) -> &Path { self.path.as_path() } @@ -212,11 +223,13 @@ pub struct Shout { desktop: Option, } impl Shout { + #[must_use] pub const fn webhook(&self) -> Option<&OutboundWebhook> { self.webhook.as_ref() } - pub fn webhook_url(&self) -> Option { + #[cfg(test)] + pub(crate) fn webhook_url(&self) -> Option { self.webhook.clone().map(|x| x.url) } @@ -224,10 +237,12 @@ impl Shout { self.webhook.clone().map(|x| x.secret).map(Secret::new) } + #[must_use] pub const fn email(&self) -> Option<&EmailConfig> { self.email.as_ref() } + #[must_use] pub const fn desktop(&self) -> Option { self.desktop } @@ -249,9 +264,11 @@ pub struct OutboundWebhook { secret: String, } impl OutboundWebhook { + #[must_use] pub fn url(&self) -> &str { self.url.as_ref() } + #[must_use] pub fn secret(&self) -> Secret { Secret::new(self.secret.clone()) } @@ -275,14 +292,17 @@ pub struct EmailConfig { smtp: Option, } impl EmailConfig { + #[must_use] pub fn from(&self) -> &str { &self.from } + #[must_use] pub fn to(&self) -> &str { &self.to } + #[must_use] pub const fn smtp(&self) -> Option<&SmtpConfig> { self.smtp.as_ref() } @@ -305,14 +325,17 @@ pub struct SmtpConfig { password: String, } impl SmtpConfig { + #[must_use] pub fn username(&self) -> &str { &self.username } + #[must_use] pub fn password(&self) -> &str { &self.password } + #[must_use] pub fn hostname(&self) -> &str { &self.hostname } diff --git a/crates/core/src/config/server_repo_config.rs b/crates/core/src/config/server_repo_config.rs index cd70b9d..44f4456 100644 --- a/crates/core/src/config/server_repo_config.rs +++ b/crates/core/src/config/server_repo_config.rs @@ -6,7 +6,7 @@ use crate::config::{ RepoPath, }; -/// Defines a Repo within a ForgeConfig to be monitored by the server +/// Defines a Repo within a `ForgeConfig` to be monitored by the server /// Maps from `git-next-server.toml` at `forge.{forge}.repos.{name}` #[derive( Clone, @@ -30,14 +30,15 @@ pub struct ServerRepoConfig { dev: Option, } impl ServerRepoConfig { - pub fn repo(&self) -> RepoPath { + pub(crate) fn repo(&self) -> RepoPath { RepoPath::new(self.repo.clone()) } - pub fn branch(&self) -> BranchName { + pub(crate) fn branch(&self) -> BranchName { BranchName::new(&self.branch) } + #[must_use] pub fn gitdir(&self) -> Option { self.gitdir .clone() @@ -45,8 +46,8 @@ impl ServerRepoConfig { .map(|dir| GitDir::new(dir, StoragePathType::External)) } - /// Returns a RepoConfig from the server configuration if ALL THREE branches were provided - pub fn repo_config(&self) -> Option { + /// Returns a `RepoConfig` from the server configuration if ALL THREE branches were provided + pub(crate) fn repo_config(&self) -> Option { match (&self.main, &self.next, &self.dev) { (Some(main), Some(next), Some(dev)) => Some(RepoConfig::new( RepoBranches::new(main.to_string(), next.to_string(), dev.to_string()), diff --git a/crates/core/src/config/tests.rs b/crates/core/src/config/tests.rs index 65325f7..3f34607 100644 --- a/crates/core/src/config/tests.rs +++ b/crates/core/src/config/tests.rs @@ -6,9 +6,9 @@ use secrecy::ExposeSecret; use std::collections::BTreeMap; use std::path::PathBuf; +use crate::server::AppConfig; use crate::server::Http; -use crate::server::ServerConfig; -use crate::server::ServerStorage; +use crate::server::Storage; use crate::webhook::push::Branch; mod url; @@ -450,26 +450,23 @@ mod server { use super::*; #[test] - fn load_should_parse_server_config() -> TestResult { - let server_config = given::a_server_config(); + fn load_should_parse_app_config() -> TestResult { + let app_config = given::an_app_config(); let fs = kxio::fs::temp()?; - let_assert!(Ok(_) = write_server_config(&server_config, &fs), "write"); - let_assert!(Ok(config) = ServerConfig::load(&fs), "load"); - assert_eq!(config, server_config, "compare"); + let_assert!(Ok(()) = write_app_config(&app_config, &fs), "write"); + let_assert!(Ok(config) = AppConfig::load(&fs), "load"); + assert_eq!(config, app_config, "compare"); Ok(()) } - fn write_server_config( - server_config: &ServerConfig, - fs: &kxio::fs::FileSystem, - ) -> TestResult { - let http = &server_config.listen_socket_addr()?; + fn write_app_config(app_config: &AppConfig, fs: &kxio::fs::FileSystem) -> TestResult { + let http = &app_config.listen_socket_addr()?; let http_addr = http.ip(); - let http_port = server_config.listen_socket_addr()?.port(); - let listen_url = server_config.listen().url(); - let storage_path = server_config.storage().path(); - let shout = server_config.shout(); + let http_port = app_config.listen_socket_addr()?.port(); + let listen_url = app_config.listen().url(); + let storage_path = app_config.storage().path(); + let shout = app_config.shout(); let shout_webhook_url = shout.webhook_url().unwrap_or_default(); let shout_webhook_secret = shout .webhook_secret() @@ -482,19 +479,18 @@ mod server { let shout_email_smtp_hostname = shout_email_smtp.hostname(); let shout_email_smtp_username = shout_email_smtp.username(); let shout_email_smtp_password = shout_email_smtp.password(); - let forge_alias = server_config + let forge_alias = app_config .forges() .next() .map(|(fa, _)| fa) .ok_or("forge missing")?; - let forge_default = server_config + let forge_default = app_config .forge .get(forge_alias.as_ref()) .ok_or("forge missing")?; let forge_type = forge_default.forge_type(); let forge_hostname = forge_default.hostname(); let forge_user = forge_default.user(); - use secrecy::ExposeSecret; let forge_token = forge_default.token().expose_secret().to_string(); let mut repos: Vec = vec![]; for (repo_alias, server_repo_config) in forge_default.repos() { @@ -591,7 +587,7 @@ mod webhook { let message = ForgeNotification::new( forge_alias.clone(), given::a_repo_alias(), - Default::default(), + BTreeMap::default(), given::a_webhook_message_body(), ); assert_eq!(message.forge_alias(), &forge_alias); @@ -602,7 +598,7 @@ mod webhook { let message = ForgeNotification::new( given::a_forge_alias(), repo_alias.clone(), - Default::default(), + BTreeMap::default(), given::a_webhook_message_body(), ); assert_eq!(message.repo_alias(), &repo_alias); @@ -613,7 +609,7 @@ mod webhook { let message = ForgeNotification::new( given::a_forge_alias(), given::a_repo_alias(), - Default::default(), + BTreeMap::default(), body.clone(), ); assert_eq!(message.body().as_bytes(), body.as_bytes()); @@ -622,7 +618,7 @@ mod webhook { fn should_return_header() { let key = given::a_name(); let value = given::a_name(); - let mut headers: BTreeMap = Default::default(); + let mut headers = BTreeMap::default(); headers.insert(key.clone(), value.clone()); let message = ForgeNotification::new( given::a_forge_alias(), @@ -730,8 +726,8 @@ mod given { } generate(5) } - pub fn a_server_config() -> ServerConfig { - ServerConfig::new( + pub fn an_app_config() -> AppConfig { + AppConfig::new( a_listen(), a_shout(), a_server_storage(), @@ -770,8 +766,8 @@ mod given { Some(SmtpConfig::new(a_name(), a_name(), a_name())), ) } - pub fn a_server_storage() -> ServerStorage { - ServerStorage::new(a_name().into()) + pub fn a_server_storage() -> Storage { + Storage::new(a_name().into()) } pub fn a_shout() -> Shout { Shout::new( diff --git a/crates/core/src/config/tests/url.rs b/crates/core/src/config/tests/url.rs index 1c5dd99..77b27c4 100644 --- a/crates/core/src/config/tests/url.rs +++ b/crates/core/src/config/tests/url.rs @@ -1,25 +1,21 @@ use super::*; #[test_log::test] -fn url_parse_https_url() -> TestResult { +fn url_parse_https_url() { let url = "https://user:pass@git.host/user/repo.git"; let result = RemoteUrl::parse(url); tracing::debug!(?result); assert!(result.is_some()); - - Ok(()) } #[test_log::test] -fn url_parse_ssh_url() -> TestResult { +fn url_parse_ssh_url() { let url = "git@git.host:user/repo.git"; let result = RemoteUrl::parse(url); tracing::debug!(?result); assert!(result.is_some()); - - Ok(()) } diff --git a/crates/core/src/config/webhook/auth.rs b/crates/core/src/config/webhook/auth.rs index f6306c8..a001257 100644 --- a/crates/core/src/config/webhook/auth.rs +++ b/crates/core/src/config/webhook/auth.rs @@ -5,6 +5,11 @@ newtype!(WebhookAuth: ulid::Ulid, derive_more::Display: r#"The unique token auth Each monitored repository has it's own unique token, and it is different each time `git-next` runs."#); impl WebhookAuth { + /// Parses the authorisation string as a `Ulid` to create a `WebhookAuth`. + /// + /// # Errors + /// + /// Will return an `Err` if the authorisation string is not a valid `Ulid`. pub fn try_new(authorisation: &str) -> Result { use std::str::FromStr as _; let id = ulid::Ulid::from_str(authorisation)?; @@ -12,15 +17,18 @@ impl WebhookAuth { Ok(Self(id)) } + #[must_use] pub fn generate() -> Self { Self(ulid::Ulid::new()) } + #[must_use] pub fn header_value(&self) -> String { - format!("Basic {}", self) + format!("Basic {self}") } - pub const fn to_bytes(&self) -> [u8; 16] { + #[cfg(test)] + pub(crate) const fn to_bytes(&self) -> [u8; 16] { self.0.to_bytes() } } diff --git a/crates/core/src/config/webhook/forge_notification.rs b/crates/core/src/config/webhook/forge_notification.rs index 81bcbd1..35c748f 100644 --- a/crates/core/src/config/webhook/forge_notification.rs +++ b/crates/core/src/config/webhook/forge_notification.rs @@ -3,7 +3,7 @@ use crate::config::{ForgeAlias, RepoAlias}; use derive_more::Constructor; -use std::collections::BTreeMap; +use std::{collections::BTreeMap, string::ToString}; /// A notification receive from a Forge, typically via a Webhook. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, derive_more::Constructor)] @@ -14,28 +14,37 @@ pub struct ForgeNotification { body: Body, } impl ForgeNotification { + #[must_use] pub const fn forge_alias(&self) -> &ForgeAlias { &self.forge_alias } + + #[must_use] pub const fn repo_alias(&self) -> &RepoAlias { &self.repo_alias } + + #[must_use] pub const fn body(&self) -> &Body { &self.body } + + #[must_use] pub fn header(&self, header: &str) -> Option { - self.headers.get(header).map(|value| value.to_string()) + self.headers.get(header).map(ToString::to_string) } } #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Constructor)] pub struct Body(String); impl Body { + #[must_use] pub fn as_str(&self) -> &str { self.0.as_str() } - pub fn as_bytes(&self) -> &[u8] { + #[cfg(test)] + pub(crate) fn as_bytes(&self) -> &[u8] { self.0.as_bytes() } } diff --git a/crates/core/src/config/webhook/push.rs b/crates/core/src/config/webhook/push.rs index 55202e8..76c515f 100644 --- a/crates/core/src/config/webhook/push.rs +++ b/crates/core/src/config/webhook/push.rs @@ -22,13 +22,18 @@ impl Push { tracing::warn!(branch = %self.branch, "Unexpected branch"); None } + + #[must_use] pub fn sha(&self) -> &str { &self.sha } + + #[must_use] pub fn message(&self) -> &str { &self.message } } + #[derive(Debug, PartialEq, Eq)] pub enum Branch { Main, diff --git a/crates/core/src/git/commit.rs b/crates/core/src/git/commit.rs index 8063abd..f1d25ba 100644 --- a/crates/core/src/git/commit.rs +++ b/crates/core/src/git/commit.rs @@ -22,9 +22,11 @@ pub struct Commit { message: Message, } impl Commit { + #[must_use] pub const fn sha(&self) -> &Sha { &self.sha } + #[must_use] pub const fn message(&self) -> &Message { &self.message } diff --git a/crates/core/src/git/forge/mod.rs b/crates/core/src/git/forge/mod.rs index 4fc48f1..034b89c 100644 --- a/crates/core/src/git/forge/mod.rs +++ b/crates/core/src/git/forge/mod.rs @@ -1,3 +1,7 @@ pub mod commit; -pub(super) mod like; +pub(super) mod r#trait; pub mod webhook; + +#[allow(clippy::module_name_repetitions)] +pub use r#trait::ForgeLike; +pub use r#trait::MockForgeLike; diff --git a/crates/core/src/git/forge/like.rs b/crates/core/src/git/forge/trait.rs similarity index 87% rename from crates/core/src/git/forge/like.rs rename to crates/core/src/git/forge/trait.rs index a2f02d5..5c72986 100644 --- a/crates/core/src/git/forge/like.rs +++ b/crates/core/src/git/forge/trait.rs @@ -16,11 +16,16 @@ pub trait ForgeLike: std::fmt::Debug + Send + Sync { /// Checks if the message should be ignored. /// /// Default implementation says that no messages should be ignored. - fn should_ignore_message(&self, _message: &ForgeNotification) -> bool { + #[allow(unused_variables)] + fn should_ignore_message(&self, message: &ForgeNotification) -> bool { false } /// Parses the webhook body into Some(Push) struct if appropriate, or None if not. + /// + /// # Errors + /// + /// Will return an `Err` if the body is not a message in the expected format. fn parse_webhook_body( &self, body: &webhook::forge_notification::Body, diff --git a/crates/core/src/git/generation.rs b/crates/core/src/git/generation.rs index f969d9a..05b9dcf 100644 --- a/crates/core/src/git/generation.rs +++ b/crates/core/src/git/generation.rs @@ -7,6 +7,6 @@ newtype!(Generation: u32, Display, Default, Copy: r#"A counter for the server ge This counter is increased by one each time the server restarts itself when the configuration file is updated."#); impl Generation { pub fn inc(&mut self) { - self.0 += 1 + self.0 += 1; } } diff --git a/crates/core/src/git/git_remote.rs b/crates/core/src/git/git_remote.rs index 6a1c2fc..d4a6a8d 100644 --- a/crates/core/src/git/git_remote.rs +++ b/crates/core/src/git/git_remote.rs @@ -9,11 +9,13 @@ pub struct GitRemote { host: Hostname, repo_path: RepoPath, } + +#[cfg(test)] impl GitRemote { - pub const fn host(&self) -> &Hostname { + pub(crate) const fn host(&self) -> &Hostname { &self.host } - pub const fn repo_path(&self) -> &RepoPath { + pub(crate) const fn repo_path(&self) -> &RepoPath { &self.repo_path } } diff --git a/crates/core/src/git/mod.rs b/crates/core/src/git/mod.rs index 1dfd9c3..d13411a 100644 --- a/crates/core/src/git/mod.rs +++ b/crates/core/src/git/mod.rs @@ -16,10 +16,12 @@ pub mod validation; mod tests; pub use commit::Commit; -pub use forge::like::ForgeLike; -pub use forge::like::MockForgeLike; +pub use forge::ForgeLike; +pub use forge::MockForgeLike; pub use generation::Generation; +#[allow(clippy::module_name_repetitions)] pub use git_ref::GitRef; +#[allow(clippy::module_name_repetitions)] pub use git_remote::GitRemote; pub use repo_details::RepoDetails; pub use repository::Repository; @@ -33,6 +35,7 @@ use crate::ForgeDetails; use crate::GitDir; use crate::RepoConfig; +#[must_use] pub fn repo_details( n: u32, generation: Generation, diff --git a/crates/core/src/git/push.rs b/crates/core/src/git/push.rs index aa2ef28..a791923 100644 --- a/crates/core/src/git/push.rs +++ b/crates/core/src/git/push.rs @@ -10,7 +10,7 @@ impl std::fmt::Display for Force { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::No => write!(f, "fast-forward"), - Self::From(from) => write!(f, "force-if-from:{}", from), + Self::From(from) => write!(f, "force-if-from:{from}"), } } } @@ -45,6 +45,15 @@ pub enum Error { TestResult(#[from] Box), } +/// Resets the position of a branch in the remote repo +/// +/// Performs a 'git fetch' first to ensure we have up-to-date branch positions before +/// performing `git push`. +/// +/// # Errors +/// +/// Will return an `Err` if their is no remote fetch defined in .git/config, or +/// if there are any network connectivity issues with the remote server. pub fn reset( open_repository: &dyn OpenRepositoryLike, repo_details: &git::RepoDetails, diff --git a/crates/core/src/git/repo_details.rs b/crates/core/src/git/repo_details.rs index ca196c3..fc39711 100644 --- a/crates/core/src/git/repo_details.rs +++ b/crates/core/src/git/repo_details.rs @@ -3,7 +3,7 @@ use crate::{ git::{ self, repository::open::{oreal::RealOpenRepository, OpenRepositoryLike}, - Generation, GitRemote, + Generation, }, pike, BranchName, ForgeAlias, ForgeConfig, ForgeDetails, GitDir, Hostname, RemoteUrl, RepoAlias, RepoConfig, RepoPath, ServerRepoConfig, StoragePathType, @@ -11,7 +11,8 @@ use crate::{ use std::sync::{Arc, RwLock}; -use secrecy::Secret; +use secrecy::{ExposeSecret, Secret}; +use tracing::instrument; /// The derived information about a repo, used to interact with it #[derive(Clone, Debug, derive_more::Display, derive_with::With)] @@ -26,6 +27,7 @@ pub struct RepoDetails { pub gitdir: GitDir, } impl RepoDetails { + #[must_use] pub fn new( generation: Generation, repo_alias: &RepoAlias, @@ -50,26 +52,24 @@ impl RepoDetails { ), } } - pub fn origin(&self) -> secrecy::Secret { + pub(crate) fn origin(&self) -> secrecy::Secret { let repo_details = self; let user = &repo_details.forge.user(); let hostname = &repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; let expose_secret = repo_details.forge.token(); - use secrecy::ExposeSecret; + let token = expose_secret.expose_secret(); let origin = format!("https://{user}:{token}@{hostname}/{repo_path}.git"); origin.into() } - pub const fn gitdir(&self) -> &GitDir { + pub(crate) const fn gitdir(&self) -> &GitDir { &self.gitdir } - pub fn git_remote(&self) -> GitRemote { - GitRemote::new(self.forge.hostname().clone(), self.repo_path.clone()) - } - + #[must_use] pub fn with_hostname(mut self, hostname: Hostname) -> Self { let forge = self.forge; self.forge = forge.with_hostname(hostname); @@ -77,21 +77,17 @@ impl RepoDetails { } // url is a secret as it contains auth token - pub fn url(&self) -> Secret { + pub(crate) fn url(&self) -> Secret { let user = self.forge.user(); - use secrecy::ExposeSecret; let token = self.forge.token().expose_secret(); - let auth_delim = match token.is_empty() { - true => "", - false => ":", - }; + let auth_delim = if token.is_empty() { "" } else { ":" }; let hostname = self.forge.hostname(); let repo_path = &self.repo_path; format!("https://{user}{auth_delim}{token}@{hostname}/{repo_path}.git").into() } #[allow(clippy::result_large_err)] - pub fn open(&self) -> Result { + pub(crate) fn open(&self) -> Result { let gix_repo = pike! { self |> Self::gitdir @@ -107,12 +103,13 @@ impl RepoDetails { Ok(repo) } + #[must_use] pub fn remote_url(&self) -> Option { use secrecy::ExposeSecret; RemoteUrl::parse(self.url().expose_secret()) } - #[tracing::instrument] + #[instrument] pub fn assert_remote_url(&self, found: Option) -> git::repository::Result<()> { let Some(found) = found else { tracing::debug!("No remote url found to assert"); @@ -152,10 +149,10 @@ impl RepoDetails { let config_file = fs.file_read_to_string(config_filename)?; let mut config_lines = config_file .lines() - .map(|l| l.to_owned()) + .map(ToOwned::to_owned) .collect::>(); tracing::debug!(?config_lines, "original file"); - let url_line = format!(r#" url = "{}""#, url); + let url_line = format!(r#" url = "{url}""#); if config_lines .iter() .any(|line| *line == r#"[remote "origin"]"#) @@ -163,7 +160,7 @@ impl RepoDetails { tracing::debug!("has an 'origin' remote"); config_lines .iter_mut() - .filter(|line| line.starts_with(r#" url = "#)) + .filter(|line| line.starts_with(r" url = ")) .for_each(|line| line.clone_from(&url_line)); } else { tracing::debug!("has no 'origin' remote"); diff --git a/crates/core/src/git/repository/factory.rs b/crates/core/src/git/repository/factory.rs index 17fc7bd..6ad30c5 100644 --- a/crates/core/src/git/repository/factory.rs +++ b/crates/core/src/git/repository/factory.rs @@ -8,21 +8,37 @@ use crate::git::{ RepoDetails, }; -use derive_more::Deref as _; +use secrecy::ExposeSecret as _; use std::sync::{atomic::AtomicBool, Arc, RwLock}; +#[allow(clippy::module_name_repetitions)] #[mockall::automock] pub trait RepositoryFactory: std::fmt::Debug + Sync + Send { fn duplicate(&self) -> Box; + + /// Opens the repository. + /// + /// # Errors + /// + /// Will return an `Err` if the repository can't be opened. fn open(&self, repo_details: &RepoDetails) -> Result>; + + /// Clones the git repository from the remote server. + /// + /// # Errors + /// + /// Will return an `Err` if there are any network connectivity issues + /// connecting with the server. fn git_clone(&self, repo_details: &RepoDetails) -> Result>; } +#[must_use] pub fn real() -> Box { Box::new(RealRepositoryFactory) } +#[must_use] pub fn mock() -> Box { Box::new(MockRepositoryFactory::new()) } @@ -44,10 +60,9 @@ impl RepositoryFactory for RealRepositoryFactory { fn git_clone(&self, repo_details: &RepoDetails) -> Result> { tracing::info!("creating"); - use secrecy::ExposeSecret; let (gix_repo, _outcome) = gix::prepare_clone_bare( repo_details.origin().expose_secret().as_str(), - repo_details.gitdir.deref(), + &*repo_details.gitdir, )? .fetch_only(gix::progress::Discard, &AtomicBool::new(false))?; tracing::info!("created"); diff --git a/crates/core/src/git/repository/mod.rs b/crates/core/src/git/repository/mod.rs index dfe4124..a9ae6de 100644 --- a/crates/core/src/git/repository/mod.rs +++ b/crates/core/src/git/repository/mod.rs @@ -17,6 +17,7 @@ pub mod factory; pub mod open; mod test; +#[allow(clippy::module_name_repetitions)] pub use factory::RepositoryFactory; #[cfg(test)] @@ -29,7 +30,8 @@ pub enum Repository { Test(TestRepository), } -pub const fn test(fs: kxio::fs::FileSystem) -> TestRepository { +#[cfg(test)] +pub(crate) const fn test(fs: kxio::fs::FileSystem) -> TestRepository { TestRepository::new(fs, vec![], vec![]) } @@ -40,12 +42,12 @@ pub fn open( repository_factory: &dyn factory::RepositoryFactory, repo_details: &RepoDetails, ) -> Result> { - let open_repository = if !repo_details.gitdir.exists() { - info!("Local copy not found - cloning..."); - repository_factory.git_clone(repo_details)? - } else { + let open_repository = if repo_details.gitdir.exists() { info!("Local copy found - opening..."); repository_factory.open(repo_details)? + } else { + info!("Local copy not found - cloning..."); + repository_factory.git_clone(repo_details)? }; info!("Validating..."); validate_default_remotes(&*open_repository, repo_details) @@ -53,20 +55,23 @@ pub fn open( Ok(open_repository) } +#[allow(clippy::module_name_repetitions)] pub trait RepositoryLike { + /// Opens the repository. + /// + /// # Errors + /// + /// Will return an `Err` if the repository can't be opened. fn open(&self, gitdir: &GitDir) -> Result; + + /// Clones the git repository from the remote server. + /// + /// # Errors + /// + /// Will return an `Err` if there are any network connectivity issues + /// connecting with the server. fn git_clone(&self, repo_details: &RepoDetails) -> Result; } -// impl std::ops::Deref for Repository { -// type Target = dyn RepositoryLike; -// -// fn deref(&self) -> &Self::Target { -// match self { -// Self::Real => &real::RealRepository, -// Self::Test(test_repository) => test_repository, -// } -// } -// } #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum Direction { diff --git a/crates/core/src/git/repository/open/mod.rs b/crates/core/src/git/repository/open/mod.rs index 4018eff..1d8d3ed 100644 --- a/crates/core/src/git/repository/open/mod.rs +++ b/crates/core/src/git/repository/open/mod.rs @@ -19,6 +19,7 @@ use std::{ sync::{Arc, RwLock}, }; +#[allow(clippy::module_name_repetitions)] #[derive(Clone, Debug)] pub enum OpenRepository { /// A real git repository. @@ -43,21 +44,44 @@ pub fn real(gix_repo: gix::Repository) -> OpenRepository { } #[cfg(not(tarpaulin_include))] // don't test mocks -pub fn test( +pub(crate) fn test( gitdir: &GitDir, - fs: kxio::fs::FileSystem, + fs: &kxio::fs::FileSystem, on_fetch: Vec, on_push: Vec, ) -> OpenRepository { OpenRepository::Test(TestOpenRepository::new(gitdir, fs, on_fetch, on_push)) } +#[allow(clippy::module_name_repetitions)] #[mockall::automock] pub trait OpenRepositoryLike: std::fmt::Debug + Sync { + /// Creates a clone of the `OpenRepositoryLike`. fn duplicate(&self) -> Box; + + /// Returns a `Vec` of all the branches in the remote repo. + /// + /// # Errors + /// + /// Will return `Err` if there are any network connectivity issues with + /// the remote server. fn remote_branches(&self) -> git::push::Result>; fn find_default_remote(&self, direction: Direction) -> Option; + + /// Performs a `git fetch` + /// + /// # Errors + /// + /// Will return an `Err` if their is no remote fetch defined in .git/config, or + /// if there are any network connectivity issues with the remote server. fn fetch(&self) -> Result<(), git::fetch::Error>; + + /// Performs a `git push` + /// + /// # Errors + /// + /// Will return an `Err` if their is no remote push defined in .git/config, or + /// if there are any network connectivity issues with the remote server. fn push( &self, repo_details: &git::RepoDetails, @@ -67,6 +91,10 @@ pub trait OpenRepositoryLike: std::fmt::Debug + Sync { ) -> git::push::Result<()>; /// List of commits in a branch, optionally up-to any specified commit. + /// + /// # Errors + /// + /// Will return `Err` if there are any network connectivity issues with the remote server. fn commit_log( &self, branch_name: &BranchName, @@ -76,10 +104,15 @@ pub trait OpenRepositoryLike: std::fmt::Debug + Sync { /// Read the contents of a file as a string. /// /// Only handles files in the root of the repo. + /// + /// # Errors + /// + /// Will return `Err` if the file does not exists on the specified branch. fn read_file(&self, branch_name: &BranchName, file_name: &Path) -> git::file::Result; } -pub fn mock() -> Box { +#[cfg(test)] +pub(crate) fn mock() -> Box { Box::new(MockOpenRepositoryLike::new()) } diff --git a/crates/core/src/git/repository/open/oreal.rs b/crates/core/src/git/repository/open/oreal.rs index a7dd192..77b000d 100644 --- a/crates/core/src/git/repository/open/oreal.rs +++ b/crates/core/src/git/repository/open/oreal.rs @@ -9,7 +9,9 @@ use gix::bstr::BStr; use tracing::{info, warn}; use std::{ + borrow::ToOwned, path::Path, + result::Result, sync::{Arc, RwLock}, }; @@ -24,11 +26,12 @@ impl super::OpenRepositoryLike for RealOpenRepository { .and_then(|repo| { Ok(repo.to_thread_local().references()?).and_then(|refs| { Ok(refs.remote_branches().map(|rb| { - rb.filter_map(|rbi| rbi.ok()) + rb.filter_map(Result::ok) .map(|r| r.name().to_owned()) .map(|n| n.to_string()) .filter_map(|p| { - p.strip_prefix("refs/remotes/origin/").map(|v| v.to_owned()) + p.strip_prefix("refs/remotes/origin/") + .map(ToOwned::to_owned) }) .filter(|b| b.as_str() != "HEAD") .map(BranchName::new) @@ -61,6 +64,8 @@ impl super::OpenRepositoryLike for RealOpenRepository { #[tracing::instrument(skip_all)] #[cfg(not(tarpaulin_include))] // would require writing to external service fn fetch(&self) -> Result<(), git::fetch::Error> { + use std::sync::atomic::AtomicBool; + let Ok(repository) = self.0.read() else { #[cfg(not(tarpaulin_include))] // don't test mutex lock failure return Err(git::fetch::Error::Lock); @@ -75,9 +80,12 @@ impl super::OpenRepositoryLike for RealOpenRepository { remote .connect(gix::remote::Direction::Fetch) .map_err(|gix| git::fetch::Error::Connect(gix.to_string()))? - .prepare_fetch(gix::progress::Discard, Default::default()) + .prepare_fetch( + gix::progress::Discard, + gix::remote::ref_map::Options::default(), + ) .map_err(|gix| git::fetch::Error::Prepare(gix.to_string()))? - .receive(gix::progress::Discard, &Default::default()) + .receive(gix::progress::Discard, &AtomicBool::default()) .map_err(|gix| git::fetch::Error::Receive(gix.to_string()))?; info!("Fetch okay"); Ok(()) @@ -93,15 +101,16 @@ impl super::OpenRepositoryLike for RealOpenRepository { to_commit: &git::GitRef, force: &git::push::Force, ) -> Result<(), git::push::Error> { + use secrecy::ExposeSecret as _; + let origin = repo_details.origin(); let force = match force { - git::push::Force::No => "".to_string(), + git::push::Force::No => String::new(), git::push::Force::From(old_ref) => { format!("--force-with-lease={branch_name}:{old_ref}") } }; // INFO: never log the command as it contains the API token within the 'origin' - use secrecy::ExposeSecret; let command: secrecy::Secret = format!( "/usr/bin/git push {} {to_commit}:{branch_name} {force}", origin.expose_secret() @@ -131,10 +140,7 @@ impl super::OpenRepositoryLike for RealOpenRepository { branch_name: &BranchName, find_commits: &[git::Commit], ) -> Result, git::commit::log::Error> { - let limit = match find_commits.is_empty() { - true => 1, - false => 25, - }; + let limit = if find_commits.is_empty() { 1 } else { 25 }; self.0 .read() .map_err(|_| git::commit::log::Error::Lock) @@ -195,8 +201,7 @@ impl super::OpenRepositoryLike for RealOpenRepository { .map_err(|_| git::file::Error::Lock) .and_then(|repo| { let thread_local = repo.to_thread_local(); - let fref = - thread_local.find_reference(format!("origin/{}", branch_name).as_str())?; + let fref = thread_local.find_reference(format!("origin/{branch_name}").as_str())?; let id = fref.try_id().ok_or(git::file::Error::TryId)?; let oid = id.detach(); let obj = thread_local.find_object(oid)?; diff --git a/crates/core/src/git/repository/open/otest.rs b/crates/core/src/git/repository/open/otest.rs index 84b38cc..04d7fa8 100644 --- a/crates/core/src/git/repository/open/otest.rs +++ b/crates/core/src/git/repository/open/otest.rs @@ -7,7 +7,7 @@ use crate::{ BranchName, GitDir, RemoteUrl, RepoBranches, }; -use derive_more::{Constructor, Deref}; +use derive_more::Constructor; use std::{ path::Path, @@ -23,6 +23,12 @@ pub struct OnFetch { action: OnFetchFn, } impl OnFetch { + /// Invokes the action function. + /// + /// # Errors + /// + /// Will return any `Err` if there is no fetch remote defined in .git/config + /// of if there are any network connectivity issues with the remote server. pub fn invoke(&self) -> git::fetch::Result<()> { (self.action)(&self.repo_branches, &self.gitdir, &self.fs) } @@ -45,6 +51,12 @@ pub struct OnPush { action: OnPushFn, } impl OnPush { + /// Invokes the action function. + /// + /// # Errors + /// + /// Will return any `Err` if there is no push remote defined in .git/config + /// of if there are any network connectivity issues with the remote server. pub fn invoke( &self, repo_details: &git::RepoDetails, @@ -86,16 +98,17 @@ impl git::repository::OpenRepositoryLike for TestOpenRepository { let i: usize = *self .fetch_counter .read() - .map_err(|_| git::fetch::Error::Lock)? - .deref(); + .map_err(|_| git::fetch::Error::Lock)?; println!("Fetch: {i}"); self.fetch_counter .write() .map_err(|_| git::fetch::Error::Lock) .map(|mut c| *c += 1)?; - self.on_fetch.get(i).map(|f| f.invoke()).unwrap_or_else(|| { - unimplemented!("Unexpected fetch"); - }) + #[allow(clippy::expect_used)] + self.on_fetch + .get(i) + .map(OnFetch::invoke) + .expect("unexpected fetch") } fn push( @@ -108,19 +121,17 @@ impl git::repository::OpenRepositoryLike for TestOpenRepository { let i: usize = *self .push_counter .read() - .map_err(|_| git::fetch::Error::Lock)? - .deref(); + .map_err(|_| git::fetch::Error::Lock)?; println!("Push: {i}"); self.push_counter .write() .map_err(|_| git::fetch::Error::Lock) .map(|mut c| *c += 1)?; + #[allow(clippy::expect_used)] self.on_push .get(i) .map(|f| f.invoke(repo_details, branch_name, to_commit, force)) - .unwrap_or_else(|| { - unimplemented!("Unexpected push"); - }) + .expect("unexpected push") } fn commit_log( @@ -140,16 +151,16 @@ impl git::repository::OpenRepositoryLike for TestOpenRepository { } } impl TestOpenRepository { - pub fn new( + pub(crate) fn new( gitdir: &GitDir, - fs: kxio::fs::FileSystem, + fs: &kxio::fs::FileSystem, on_fetch: Vec, on_push: Vec, ) -> Self { let pathbuf = fs.base().join(gitdir.to_path_buf()); #[allow(clippy::expect_used)] let gix = gix::init(pathbuf).expect("git init"); - Self::write_origin(gitdir, &fs); + Self::write_origin(gitdir, fs); Self { on_fetch, fetch_counter: Arc::new(RwLock::new(0)), @@ -174,6 +185,6 @@ impl TestOpenRepository { ); #[allow(clippy::expect_used)] fs.file_write(&config_file, &updated_contents) - .expect("write updated .git/config") + .expect("write updated .git/config"); } } diff --git a/crates/core/src/git/repository/open/tests/fetch.rs b/crates/core/src/git/repository/open/tests/fetch.rs index 5e24b00..6eeed67 100644 --- a/crates/core/src/git/repository/open/tests/fetch.rs +++ b/crates/core/src/git/repository/open/tests/fetch.rs @@ -9,5 +9,5 @@ fn should_fetch_from_repo() { let gitdir = GitDir::new(cwd.join("../.."), StoragePathType::External); let repo_details = given::repo_details(&given::a_filesystem()).with_gitdir(gitdir); let_assert!(Ok(repo) = crate::git::repository::factory::real().open(&repo_details)); - let_assert!(Ok(_) = repo.fetch()); + let_assert!(Ok(()) = repo.fetch()); } diff --git a/crates/core/src/git/repository/test.rs b/crates/core/src/git/repository/test.rs index 58e5da4..53e1ae5 100644 --- a/crates/core/src/git/repository/test.rs +++ b/crates/core/src/git/repository/test.rs @@ -16,6 +16,7 @@ use crate::{ GitDir, }; +#[allow(clippy::module_name_repetitions)] #[derive(Clone, Debug, Constructor)] pub struct TestRepository { fs: kxio::fs::FileSystem, @@ -35,7 +36,7 @@ impl RepositoryLike for TestRepository { fn open(&self, gitdir: &GitDir) -> Result { Ok(git::repository::open::test( gitdir, - self.fs.clone(), + &self.fs, self.on_fetch.clone(), self.on_push.clone(), )) diff --git a/crates/core/src/git/repository/tests/factory.rs b/crates/core/src/git/repository/tests/factory.rs index b31d3cf..f3266ec 100644 --- a/crates/core/src/git/repository/tests/factory.rs +++ b/crates/core/src/git/repository/tests/factory.rs @@ -11,7 +11,6 @@ fn open_where_storage_external_auth_matches() -> TestResult { let fs = given::a_filesystem(); let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::External); let repo_details = given::repo_details(&fs).with_gitdir(gitdir); - use secrecy::ExposeSecret; let url = repo_details.url(); let url = url.expose_secret(); given::a_bare_repo_with_url(fs.base(), url, &fs); @@ -37,7 +36,6 @@ fn open_where_storage_external_auth_differs_error() -> TestResult { //given let fs = given::a_filesystem(); let repo_details = given::repo_details(&fs); - use secrecy::ExposeSecret; let original_url = repo_details.url(); let original_url = original_url.expose_secret(); given::a_bare_repo_with_url(fs.base(), original_url, &fs); @@ -75,7 +73,6 @@ fn open_where_storage_internal_auth_matches() -> TestResult { let fs = given::a_filesystem(); let gitdir = GitDir::new(fs.base().to_path_buf(), StoragePathType::Internal); let repo_details = given::repo_details(&fs).with_gitdir(gitdir); - use secrecy::ExposeSecret; let url = repo_details.url(); let url = url.expose_secret(); // create a bare repg with the auth from the forge_config @@ -105,7 +102,6 @@ fn open_where_storage_internal_auth_differs_update_config() -> TestResult { //given let fs = given::a_filesystem(); let repo_details = given::repo_details(&fs); - use secrecy::ExposeSecret; let original_url = repo_details.url(); let original_url = original_url.expose_secret(); given::a_bare_repo_with_url(fs.base(), original_url, &fs); diff --git a/crates/core/src/git/repository/tests/mod.rs b/crates/core/src/git/repository/tests/mod.rs index bfabde9..a9204f9 100644 --- a/crates/core/src/git/repository/tests/mod.rs +++ b/crates/core/src/git/repository/tests/mod.rs @@ -5,6 +5,7 @@ use crate::{ }; use assert2::let_assert; +use secrecy::ExposeSecret as _; mod factory; mod validate; diff --git a/crates/core/src/git/tests.rs b/crates/core/src/git/tests.rs index 2c67806..8bc404d 100644 --- a/crates/core/src/git/tests.rs +++ b/crates/core/src/git/tests.rs @@ -101,7 +101,7 @@ mod push { #[test] fn force_no_should_display() { - assert_eq!(git::push::Force::No.to_string(), "fast-forward") + assert_eq!(git::push::Force::No.to_string(), "fast-forward"); } #[test] @@ -111,7 +111,7 @@ mod push { assert_eq!( git::push::Force::From(GitRef::from(commit)).to_string(), format!("force-if-from:{sha}") - ) + ); } mod reset { @@ -138,7 +138,7 @@ mod push { let commit = given::a_commit(); let gitref = GitRef::from(commit); let_assert!( - Ok(_) = git::push::reset( + Ok(()) = git::push::reset( &*open_repository, &repo_details, branch_name, @@ -182,38 +182,6 @@ mod repo_details { "https://user:token@host/repo.git" ); } - #[test] - fn should_return_git_remote() { - let rd = RepoDetails::new( - Generation::default(), - &RepoAlias::new("foo"), - &ServerRepoConfig::new( - "user/repo".to_string(), - "branch".to_string(), - None, - None, - None, - None, - ), - &ForgeAlias::new("default".to_string()), - &ForgeConfig::new( - ForgeType::MockForge, - "host".to_string(), - "user".to_string(), - "token".to_string(), - BTreeMap::new(), - ), - GitDir::new(PathBuf::default().join("foo"), StoragePathType::Internal), - ); - - assert_eq!( - rd.git_remote(), - GitRemote::new( - Hostname::new("host".to_string()), - RepoPath::new("user/repo".to_string()) - ) - ); - } } pub mod given { use super::*; @@ -267,7 +235,7 @@ pub mod given { format!("hostname-{}", a_name()), format!("user-{}", a_name()), format!("token-{}", a_name()), - Default::default(), // no repos + BTreeMap::default(), // no repos ) } @@ -349,7 +317,7 @@ pub mod given { // add use are origin url let mut config_lines = config_file.lines().collect::>(); config_lines.push(r#"[remote "origin"]"#); - let url_line = format!(r#" url = "{}""#, url); + let url_line = format!(r#" url = "{url}""#); tracing::info!(?url, %url_line, "writing"); config_lines.push(&url_line); // write config file back out @@ -436,7 +404,7 @@ pub mod then { pub fn git_checkout_new_branch(branch_name: &BranchName, gitdir: &GitDir) -> TestResult { exec( - format!("git checkout -b {}", branch_name), + &format!("git checkout -b {branch_name}"), std::process::Command::new("/usr/bin/git") .current_dir(gitdir.to_path_buf()) .args(["checkout", "-b", branch_name.to_string().as_str()]) @@ -447,7 +415,7 @@ pub mod then { pub fn git_switch(branch_name: &BranchName, gitdir: &GitDir) -> TestResult { exec( - format!("git switch {}", branch_name), + &format!("git switch {branch_name}"), std::process::Command::new("/usr/bin/git") .current_dir(gitdir.to_path_buf()) .args(["switch", branch_name.to_string().as_str()]) @@ -455,7 +423,7 @@ pub mod then { ) } - fn exec(label: String, output: Result) -> TestResult { + fn exec(label: &str, output: Result) -> TestResult { println!("== {label}"); match output { Ok(output) => { @@ -479,7 +447,7 @@ pub mod then { fn git_add_file(gitdir: &GitDir, file: &Path) -> TestResult { exec( - format!("git add {file:?}"), + &format!("git add {file:?}"), std::process::Command::new("/usr/bin/git") .current_dir(gitdir.to_path_buf()) .args(["add", file.display().to_string().as_str()]) @@ -489,7 +457,7 @@ pub mod then { fn git_commit(gitdir: &GitDir, file: &Path) -> TestResult { exec( - format!(r#"git commit -m"Added {file:?}""#), + &format!(r#"git commit -m"Added {file:?}""#), std::process::Command::new("/usr/bin/git") .current_dir(gitdir.to_path_buf()) .args([ @@ -502,7 +470,7 @@ pub mod then { pub fn git_log_all(gitdir: &GitDir) -> TestResult { exec( - "git log --all --oneline --decorate --graph".to_string(), + "git log --all --oneline --decorate --graph", std::process::Command::new("/usr/bin/git") .current_dir(gitdir.to_path_buf()) .args(["log", "--all", "--oneline", "--decorate", "--graph"]) diff --git a/crates/core/src/git/user_notification.rs b/crates/core/src/git/user_notification.rs index c25f36b..e698697 100644 --- a/crates/core/src/git/user_notification.rs +++ b/crates/core/src/git/user_notification.rs @@ -29,6 +29,7 @@ pub enum UserNotification { }, } impl UserNotification { + #[must_use] pub fn as_json(&self, timestamp: time::OffsetDateTime) -> serde_json::Value { let timestamp = timestamp.unix_timestamp().to_string(); match self { diff --git a/crates/core/src/git/validation/positions.rs b/crates/core/src/git/validation/positions.rs index 56866ac..8d03ecd 100644 --- a/crates/core/src/git/validation/positions.rs +++ b/crates/core/src/git/validation/positions.rs @@ -15,32 +15,41 @@ pub struct Positions { pub next_is_valid: bool, } +/// Validates the relative positions of the three branches, resetting next back to main if +/// it has gone astry. +/// +/// # Errors +/// +/// Will return an `Err` if any of the branches has no commits, or if user intervention is +/// required, or if there is an error resetting the next branch back to main. #[allow(clippy::result_large_err)] -pub fn validate_positions( +pub fn validate( open_repository: &dyn OpenRepositoryLike, repo_details: &git::RepoDetails, - repo_config: RepoConfig, + repo_config: &RepoConfig, ) -> Result { let main_branch = repo_config.branches().main(); let next_branch = repo_config.branches().next(); let dev_branch = repo_config.branches().dev(); // Collect Commit Histories for `main`, `next` and `dev` branches open_repository.fetch()?; - let commit_histories = get_commit_histories(open_repository, &repo_config)?; + let commit_histories = get_commit_histories(open_repository, repo_config)?; // branch tips - let main = - commit_histories.main.first().cloned().ok_or_else(|| { - Error::NonRetryable(format!("Branch has no commits: {}", main_branch)) - })?; - let next = - commit_histories.next.first().cloned().ok_or_else(|| { - Error::NonRetryable(format!("Branch has no commits: {}", next_branch)) - })?; + let main = commit_histories + .main + .first() + .cloned() + .ok_or_else(|| Error::NonRetryable(format!("Branch has no commits: {main_branch}")))?; + let next = commit_histories + .next + .first() + .cloned() + .ok_or_else(|| Error::NonRetryable(format!("Branch has no commits: {next_branch}")))?; let dev = commit_histories .dev .first() .cloned() - .ok_or_else(|| Error::NonRetryable(format!("Branch has no commits: {}", dev_branch)))?; + .ok_or_else(|| Error::NonRetryable(format!("Branch has no commits: {dev_branch}")))?; // Validations: // Dev must be on main branch, else the USER must rebase it if is_not_based_on(&commit_histories.dev, &main) { diff --git a/crates/core/src/git/validation/tests.rs b/crates/core/src/git/validation/tests.rs index 92d024f..62f2820 100644 --- a/crates/core/src/git/validation/tests.rs +++ b/crates/core/src/git/validation/tests.rs @@ -8,7 +8,7 @@ use crate::{ Direction, RepositoryLike as _, }, tests::{given, then}, - validation::positions::validate_positions, + validation::positions::validate, }, GitDir, StoragePathType, }; @@ -52,7 +52,7 @@ mod repos { mod positions { use super::*; - mod validate_positions { + mod validate { use super::*; @@ -75,7 +75,7 @@ mod positions { ); let repo_config = given::a_repo_config(); - let result = validate_positions(&*repository, &repo_details, repo_config); + let result = validate(&*repository, &repo_details, &repo_config); println!("{result:?}"); let_assert!(Err(err) = result, "validate"); @@ -115,7 +115,7 @@ mod positions { "open repo" ); - let result = validate_positions(&*open_repository, &repo_details, repo_config); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -154,7 +154,7 @@ mod positions { "open repo" ); - let result = validate_positions(&*open_repository, &repo_details, repo_config); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -193,7 +193,7 @@ mod positions { "open repo" ); - let result = validate_positions(&*open_repository, &repo_details, repo_config); + let result = validate(&*open_repository, &repo_details, &repo_config); println!("{result:?}"); assert!(matches!( @@ -240,7 +240,7 @@ mod positions { //when let_assert!( - Err(err) = validate_positions(&*open_repository, &repo_details, repo_config), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -325,7 +325,7 @@ mod positions { //when let_assert!( - Err(err) = validate_positions(&*open_repository, &repo_details, repo_config), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -394,8 +394,7 @@ mod positions { //when let_assert!( - Err(err) = - validate_positions(&*open_repository, &repo_details, repo_config.clone()), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -482,7 +481,7 @@ mod positions { //when let_assert!( - Err(err) = validate_positions(&*open_repository, &repo_details, repo_config), + Err(err) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -568,8 +567,7 @@ mod positions { //when let_assert!( - Ok(positions) = - validate_positions(&*open_repository, &repo_details, repo_config.clone()), + Ok(positions) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); @@ -628,8 +626,7 @@ mod positions { //when let_assert!( - Ok(positions) = - validate_positions(&*open_repository, &repo_details, repo_config.clone()), + Ok(positions) = validate(&*open_repository, &repo_details, &repo_config), "validate" ); diff --git a/crates/core/src/macros/newtype.rs b/crates/core/src/macros/newtype.rs index cacf317..6c7d065 100644 --- a/crates/core/src/macros/newtype.rs +++ b/crates/core/src/macros/newtype.rs @@ -38,6 +38,7 @@ macro_rules! newtype { Self(value.into()) } #[allow(clippy::missing_const_for_fn)] + #[must_use] pub fn unwrap(self) -> $type { self.0 }