From dbb8e442c81955e2d2f9ecf6c60b7e0a2588d9f4 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Wed, 10 Jan 2024 10:42:48 +0100 Subject: [PATCH] Fixes from clippy --- flake.nix | 4 ++- src/domain/acme/manager.rs | 28 ++++++++++--------- src/domain/acme/store/direct_fs_store.rs | 7 ++--- src/domain/acme/store/json_fs_store.rs | 1 - src/domain/config/proxied_domain.rs | 16 +++-------- src/domain/gemini.rs | 2 +- src/domain/manager.rs | 34 +++++++++++++----------- src/domain/settings.rs | 6 ++--- src/domain/store.rs | 4 +-- src/domain/tls/certificate.rs | 5 ++-- src/main.rs | 4 +-- src/origin.rs | 6 +---- src/origin/git.rs | 6 ++--- src/origin/mux.rs | 4 +-- src/service.rs | 7 ++--- src/service/http.rs | 15 ++++++----- src/service/http/proxy.rs | 2 +- src/service/http/tasks.rs | 4 +-- src/token.rs | 6 +++++ src/util.rs | 2 +- 20 files changed, 79 insertions(+), 84 deletions(-) diff --git a/flake.nix b/flake.nix index a5143bd..f813ca1 100644 --- a/flake.nix +++ b/flake.nix @@ -196,9 +196,11 @@ (buildSystem: targetSystem: let pkgs = mkPkgs buildSystem null; toolchain = mkToolchain buildSystem targetSystem; + # make clippy available directly + clippy = pkgs.writeShellScriptBin "clippy" ''exec cargo clippy "$@"''; in pkgs.mkShell ({ - inputsFrom = []; # extra packages for dev + packages = [ clippy ]; # extra packages for dev } // (buildEnv buildSystem targetSystem)) ); }; diff --git a/src/domain/acme/manager.rs b/src/domain/acme/manager.rs index 2fcf576..8d462b1 100644 --- a/src/domain/acme/manager.rs +++ b/src/domain/acme/manager.rs @@ -18,11 +18,11 @@ pub enum GetHttp01ChallengeKeyError { #[mockall::automock] pub trait Manager { - fn sync_domain<'mgr>( - &'mgr self, + fn sync_domain( + &self, domain: domain::Name, external_config: Option, - ) -> util::BoxFuture<'mgr, unexpected::Result<()>>; + ) -> util::BoxFuture<'_, unexpected::Result<()>>; fn get_http01_challenge_key(&self, token: &str) -> Result; @@ -32,17 +32,19 @@ pub trait Manager { ) -> unexpected::Result>; } +type BoxExternalStoreFn = Box< + dyn Fn( + &domain::ConfigExternalDomain, + ) -> unexpected::Result> + + Send + + Sync, +>; + pub struct ManagerImpl { store: Box, token_store: Box, account: sync::Arc, - external_store_fn: Box< - dyn Fn( - &domain::ConfigExternalDomain, - ) -> unexpected::Result> - + Send - + Sync, - >, + external_store_fn: BoxExternalStoreFn, } impl ManagerImpl { @@ -112,11 +114,11 @@ impl ManagerImpl { } impl Manager for ManagerImpl { - fn sync_domain<'mgr>( - &'mgr self, + fn sync_domain( + &self, domain: domain::Name, external_config: Option, - ) -> util::BoxFuture<'mgr, Result<(), unexpected::Error>> { + ) -> util::BoxFuture<'_, Result<(), unexpected::Error>> { Box::pin(async move { let external_store; let store = match external_config { diff --git a/src/domain/acme/store/direct_fs_store.rs b/src/domain/acme/store/direct_fs_store.rs index c5be0c9..1ac2c97 100644 --- a/src/domain/acme/store/direct_fs_store.rs +++ b/src/domain/acme/store/direct_fs_store.rs @@ -10,7 +10,7 @@ pub struct DirectFSStore { } impl DirectFSStore { - pub fn new( key_file_path: &path::Path, cert_file_path: &path::Path,) -> Self { + pub fn new(key_file_path: &path::Path, cert_file_path: &path::Path) -> Self { Self { key_file_path: key_file_path.into(), cert_file_path: cert_file_path.into(), @@ -53,13 +53,10 @@ impl super::Store for DirectFSStore { ) })?; - if key.is_none() != certs.is_none() { - } - match (key, certs) { (None, None) => Ok(None), (Some(key), Some(certs)) => Ok(Some((key, certs))), - _ => + _ => Err(unexpected::Error::from(format!( "private key file {} and cert file {} are in inconsistent state, one exists but the other doesn't", &self.key_file_path.display(), diff --git a/src/domain/acme/store/json_fs_store.rs b/src/domain/acme/store/json_fs_store.rs index 34dd4f8..2ef2d65 100644 --- a/src/domain/acme/store/json_fs_store.rs +++ b/src/domain/acme/store/json_fs_store.rs @@ -73,6 +73,5 @@ impl super::Store for JSONFSStore { ))) } .map_unexpected_while(|| format!("path is {}", path.display())) - .map_err(|err| err.into()) } } diff --git a/src/domain/config/proxied_domain.rs b/src/domain/config/proxied_domain.rs index 33ba4c3..c024931 100644 --- a/src/domain/config/proxied_domain.rs +++ b/src/domain/config/proxied_domain.rs @@ -22,15 +22,13 @@ fn addr_from_url( if let Some(path_and_query) = parsed.path_and_query() { let path_and_query = path_and_query.as_str(); - if path_and_query != "" && path_and_query != "/" { - return Err(unexpected::Error::from( - format!("path must be empty").as_str(), - )); + if !path_and_query.is_empty() && path_and_query != "/" { + return Err(unexpected::Error::from("path must be empty")); } } match parsed.authority() { - None => Err(unexpected::Error::from(format!("host is missing").as_str())), + None => Err(unexpected::Error::from("host is missing")), Some(authority) => { let port = authority.port().map(|p| p.as_u16()).unwrap_or(default_port); Ok(format!("{}:{port}", authority.host())) @@ -90,15 +88,9 @@ pub struct HttpRequestHeader { value: String, } -#[derive(Clone)] +#[derive(Clone, Default)] pub struct HttpRequestHeaders(pub http::header::HeaderMap); -impl Default for HttpRequestHeaders { - fn default() -> Self { - Self(http::header::HeaderMap::default()) - } -} - impl TryFrom for Vec { type Error = http::header::ToStrError; diff --git a/src/domain/gemini.rs b/src/domain/gemini.rs index 927b109..80f5a5a 100644 --- a/src/domain/gemini.rs +++ b/src/domain/gemini.rs @@ -86,6 +86,6 @@ impl Store for FSStore { serde_json::to_writer(file, &stored).or_unexpected_while("writing cert to file")?; - return Ok(()); + Ok(()) } } diff --git a/src/domain/manager.rs b/src/domain/manager.rs index 31465f1..1334913 100644 --- a/src/domain/manager.rs +++ b/src/domain/manager.rs @@ -124,17 +124,17 @@ pub struct ManagedDomain { pub trait Manager: Sync + Send { fn get_settings(&self, domain: &domain::Name) -> Result; - fn get_file<'store>( - &'store self, + fn get_file( + &self, domain: &domain::Name, path: &str, ) -> Result; - fn sync_with_settings<'mgr>( - &'mgr self, + fn sync_with_settings( + &self, domain: domain::Name, settings: domain::Settings, - ) -> util::BoxFuture<'mgr, Result<(), SyncWithSettingsError>>; + ) -> util::BoxFuture<'_, Result<(), SyncWithSettingsError>>; fn get_acme_http01_challenge_key( &self, @@ -206,7 +206,11 @@ impl ManagerImpl { fn sync_domain_gemini_cert(&self, domain: &domain::Name) -> unexpected::Result<()> { if let Some(ref gemini_store) = self.gemini_store { log::info!("Syncing gemini certificate for domain {domain}"); - if let Some(_) = gemini_store.get_certificate(domain).or_unexpected()? { + if gemini_store + .get_certificate(domain) + .or_unexpected()? + .is_some() + { return Ok(()); } @@ -332,8 +336,8 @@ impl Manager for ManagerImpl { Ok(GetSettingsResult::Stored(self.domain_store.get(domain)?)) } - fn get_file<'store>( - &'store self, + fn get_file( + &self, domain: &domain::Name, path: &str, ) -> Result { @@ -364,11 +368,11 @@ impl Manager for ManagerImpl { Ok(f) } - fn sync_with_settings<'mgr>( - &'mgr self, + fn sync_with_settings( + &self, domain: domain::Name, settings: domain::Settings, - ) -> util::BoxFuture<'mgr, Result<(), SyncWithSettingsError>> { + ) -> util::BoxFuture<'_, Result<(), SyncWithSettingsError>> { Box::pin(async move { let is_interface = Some(&domain) == self.config.interface_domain.as_ref(); let is_builtin = self.config.builtin_domains.contains_key(&domain); @@ -440,8 +444,8 @@ impl Manager for ManagerImpl { self.config .proxied_domains - .iter() - .map(|(domain, _)| ManagedDomain { + .keys() + .map(|domain| ManagedDomain { domain: domain.clone(), public: false, }) @@ -456,8 +460,8 @@ impl Manager for ManagerImpl { self.config .external_domains - .iter() - .map(|(domain, _)| ManagedDomain { + .keys() + .map(|domain| ManagedDomain { domain: domain.clone(), public: false, }) diff --git a/src/domain/settings.rs b/src/domain/settings.rs index 18156ae..161b220 100644 --- a/src/domain/settings.rs +++ b/src/domain/settings.rs @@ -24,11 +24,11 @@ impl Settings { Ok(h.finalize().encode_hex::()) } - fn add_path_prefix<'path, 'prefix>( + fn add_path_prefix<'path>( path: borrow::Cow<'path, str>, - prefix: &'prefix str, + prefix: &str, ) -> borrow::Cow<'path, str> { - if prefix.len() == 0 { + if prefix.is_empty() { return path; } diff --git a/src/domain/store.rs b/src/domain/store.rs index 7e4744a..f37811f 100644 --- a/src/domain/store.rs +++ b/src/domain/store.rs @@ -81,9 +81,9 @@ impl Store for FSStore { "couldn't convert os string to &str", ))?; - Ok(domain + domain .parse() - .map_unexpected_while(|| format!("parsing {domain} as domain name"))?) + .map_unexpected_while(|| format!("parsing {domain} as domain name")) }, ) .try_collect() diff --git a/src/domain/tls/certificate.rs b/src/domain/tls/certificate.rs index bbba4eb..1d78a38 100644 --- a/src/domain/tls/certificate.rs +++ b/src/domain/tls/certificate.rs @@ -71,10 +71,9 @@ impl Certificate { let cert = builder.build(); - Ok(cert - .as_ref() + cert.as_ref() .try_into() - .or_unexpected_while("converting to cert")?) + .or_unexpected_while("converting to cert") } } diff --git a/src/main.rs b/src/main.rs index e13cbf2..625a33f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,7 +53,7 @@ async fn main() { }) }; - // inteface_cname is a CNAME record which points to the interface domain of the service. + // interface_cname is a CNAME record which points to the interface domain of the service. // Since the interface domain _must_ point to the service (otherwise it wouldn't work) it's // reasonable to assume that a CNAME on any domain would suffice to point that domain to // the service. @@ -74,7 +74,7 @@ async fn main() { } } - if config.domain.external_domains.len() > 0 && config.service.http.https_addr.is_none() { + if !config.domain.external_domains.is_empty() && config.service.http.https_addr.is_none() { panic!("https must be enabled to use external_domains") } diff --git a/src/origin.rs b/src/origin.rs index f817f18..527ef01 100644 --- a/src/origin.rs +++ b/src/origin.rs @@ -68,11 +68,7 @@ impl Store for sync::Arc> { self.lock().unwrap().all_descrs() } - fn get_file<'store>( - &'store self, - descr: &Descr, - path: &str, - ) -> Result { + fn get_file(&self, descr: &Descr, path: &str) -> Result { self.lock().unwrap().get_file(descr, path) } } diff --git a/src/origin/git.rs b/src/origin/git.rs index 090ad09..bd06f37 100644 --- a/src/origin/git.rs +++ b/src/origin/git.rs @@ -127,7 +127,7 @@ impl FSStore { .map_err(|e| match e { // it's not expected that the branch name is invalid at this point, it must have // existed for sync to have been successful. - CreateRepoSnapshotError::InvalidBranchName => e.into_unexpected().into(), + CreateRepoSnapshotError::InvalidBranchName => e.into_unexpected(), CreateRepoSnapshotError::Unexpected(e) => e, })?; @@ -293,8 +293,8 @@ impl super::Store for FSStore { ).try_collect() } - fn get_file<'store>( - &'store self, + fn get_file( + &self, descr: &origin::Descr, path: &str, ) -> Result { diff --git a/src/origin/mux.rs b/src/origin/mux.rs index 2ae26b0..fe3a2cf 100644 --- a/src/origin/mux.rs +++ b/src/origin/mux.rs @@ -41,8 +41,8 @@ where Ok(res) } - fn get_file<'store>( - &'store self, + fn get_file( + &self, descr: &origin::Descr, path: &str, ) -> Result { diff --git a/src/service.rs b/src/service.rs index 724137f..1950312 100644 --- a/src/service.rs +++ b/src/service.rs @@ -6,11 +6,8 @@ pub use config::*; use std::borrow; -fn append_index_to_path<'path, 'index>( - path: &'path str, - index: &'index str, -) -> borrow::Cow<'path, str> { - if path.len() == 0 { +fn append_index_to_path<'path>(path: &'path str, index: &str) -> borrow::Cow<'path, str> { + if path.is_empty() { let mut path = String::with_capacity(1 + index.len()); path.push('/'); path.push_str(index); diff --git a/src/service/http.rs b/src/service/http.rs index bd7a570..317fa0d 100644 --- a/src/service/http.rs +++ b/src/service/http.rs @@ -128,7 +128,7 @@ impl Service { if self.config.http.https_addr.is_some() { return "https"; } - return "http"; + "http" } fn render_error_page(&self, status_code: u16, e: &str) -> Response { @@ -212,7 +212,7 @@ impl Service { match self.domain_manager.get_file(&domain, &path) { Ok(f) => self.serve(200, &path, Body::wrap_stream(f)), Err(domain::manager::GetFileError::DomainNotFound) => { - return self.render_error_page(404, "Unknown domain name") + self.render_error_page(404, "Unknown domain name") } Err(domain::manager::GetFileError::FileNotFound) => { self.render_error_page(404, "File not found") @@ -267,7 +267,7 @@ impl Service { let settings = match self.domain_manager.get_settings(&args.domain) { Ok(domain::manager::GetSettingsResult::Stored(settings)) => Some(settings), Ok(domain::manager::GetSettingsResult::Builtin(config)) => { - config.public.then(|| config.settings) + config.public.then_some(config.settings) } Ok(domain::manager::GetSettingsResult::Proxied(_)) => None, Ok(domain::manager::GetSettingsResult::Interface) => None, @@ -318,10 +318,11 @@ impl Service { } }; - let dns_records_have_cname = self.config.dns_records.iter().any(|r| match r { - service::ConfigDNSRecord::CNAME { .. } => true, - _ => false, - }); + let dns_records_have_cname = self + .config + .dns_records + .iter() + .any(|r| matches!(r, service::ConfigDNSRecord::CNAME { .. })); let url_encoded_domain_settings = match settings.try_into() { Ok(s) => s, diff --git a/src/service/http/proxy.rs b/src/service/http/proxy.rs index ff5bfd2..f29a3d0 100644 --- a/src/service/http/proxy.rs +++ b/src/service/http/proxy.rs @@ -9,7 +9,7 @@ pub async fn serve_http_request( req_is_https: bool, ) -> unexpected::Result> { for (name, value) in headers { - if value == "" { + if value.is_empty() { req.headers_mut().remove(name); continue; } diff --git a/src/service/http/tasks.rs b/src/service/http/tasks.rs index a77e26f..f26376b 100644 --- a/src/service/http/tasks.rs +++ b/src/service/http/tasks.rs @@ -12,7 +12,7 @@ pub async fn listen_http( service: sync::Arc, canceller: CancellationToken, ) -> Result<(), unexpected::Error> { - let addr = service.config.http.http_addr.clone(); + let addr = service.config.http.http_addr; // only used for logging let listen_host = service @@ -51,7 +51,7 @@ pub async fn listen_https( canceller: CancellationToken, ) -> Result<(), unexpected::Error> { let cert_resolver = service.cert_resolver.clone(); - let addr = service.config.http.https_addr.unwrap().clone(); + let addr = service.config.http.https_addr.unwrap(); // only used for logging let listen_host = service diff --git a/src/token.rs b/src/token.rs index 7454297..7cd9bc3 100644 --- a/src/token.rs +++ b/src/token.rs @@ -20,6 +20,12 @@ impl MemStore { } } +impl Default for MemStore { + fn default() -> Self { + Self::new() + } +} + impl Store for MemStore { fn get(&self, key: &str) -> unexpected::Result> { Ok(self.m.lock().unwrap().get(key).map(|s| s.to_string())) diff --git a/src/util.rs b/src/util.rs index e354dbf..52b2f10 100644 --- a/src/util.rs +++ b/src/util.rs @@ -23,7 +23,7 @@ pub fn parse_file( path: &path::Path, ) -> Result, ParseFileError> { match fs::read_to_string(path) { - Ok(s) => Ok(Some(s.parse().map_err(|e| ParseFileError::FromStr(e))?)), + Ok(s) => Ok(Some(s.parse().map_err(ParseFileError::FromStr)?)), Err(err) => match err.kind() { io::ErrorKind::NotFound => Ok(None), _ => Err(ParseFileError::IO(err)),