From 87dc2a5681685d3c24884f249a5cfba649e91056 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 13:13:36 +1300 Subject: [PATCH 01/18] Add explicit anonymous lifetime parameters Implicit elision of type lifetime parameters is deprecated, as per the rust-2018-idioms warning. --- src/actions.rs | 2 +- src/handlebars_helpers.rs | 8 ++++++-- src/hooks.rs | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 05d4a69..4dd9888 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -40,7 +40,7 @@ pub struct RealActionRunner<'a> { impl<'a> RealActionRunner<'a> { pub fn new( fs: &'a mut dyn Filesystem, - handlebars: &'a Handlebars, + handlebars: &'a Handlebars<'_>, variables: &'a Variables, force: bool, diff_context_lines: usize, diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 98d80f9..663e0bc 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -31,7 +31,7 @@ pub fn create_new_handlebars<'b>(config: &mut Configuration) -> Result, variables: &Variables, files: &mut Files, ) -> Result<()> { @@ -57,7 +57,11 @@ fn filter_files_condition( Ok(()) } -fn eval_condition(handlebars: &Handlebars, variables: &Variables, condition: &str) -> Result { +fn eval_condition( + handlebars: &Handlebars<'_>, + variables: &Variables, + condition: &str, +) -> Result { // extra { for format!() let condition = format!("{{{{#if {} }}}}true{{{{/if}}}}", condition); let rendered = handlebars diff --git a/src/hooks.rs b/src/hooks.rs index 6abd4e9..6a1f401 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -10,7 +10,7 @@ use crate::filesystem::{Filesystem, RealFilesystem}; pub(crate) fn run_hook( location: &Path, cache_dir: &Path, - handlebars: &Handlebars, + handlebars: &Handlebars<'_>, variables: &crate::config::Variables, ) -> Result<()> { if !location.exists() { From 9567ba0c45a951b7cf15c30632bd0314e43ab624 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 13:56:07 +1300 Subject: [PATCH 02/18] Take some args by reference Clippy's needless_pass_by_value lint shows a couple functions that don't need to own their arguments. undeploy() in particular takes a 240-byte Options struct by value but, like deploy(), doesn't need mutable access. --- src/actions.rs | 2 +- src/deploy.rs | 2 +- src/difference.rs | 6 +++--- src/main.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 4dd9888..002b698 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -544,7 +544,7 @@ pub fn update_template( ); if log_enabled!(log::Level::Info) { info!("Refusing because of the following changes in target location: "); - print_diff(diff, diff_context_lines); + print_diff(&diff, diff_context_lines); } Ok(false) } else { diff --git a/src/deploy.rs b/src/deploy.rs index f9a4eb5..9ba5f50 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -157,7 +157,7 @@ Proceeding by copying instead of symlinking." Ok(error_occurred) } -pub fn undeploy(opt: Options) -> Result { +pub fn undeploy(opt: &Options) -> Result { // === Load configuration === let mut config = config::load_configuration(&opt.local_config, &opt.global_config, None) .context("get a configuration")?; diff --git a/src/difference.rs b/src/difference.rs index 3d217f8..b553a16 100644 --- a/src/difference.rs +++ b/src/difference.rs @@ -28,7 +28,7 @@ pub fn print_template_diff( source, target.target ); - print_diff(diff, diff_context_lines); + print_diff(&diff, diff_context_lines); } } Err(e) => { @@ -86,7 +86,7 @@ pub fn diff_nonempty(diff: &[diff::Result]) -> bool { false } -fn hunkify_diff(diff: Diff, extra_lines: usize) -> HunkDiff { +fn hunkify_diff(diff: &[diff::Result], extra_lines: usize) -> HunkDiff { let mut hunks = vec![]; let mut left_line_number: usize = 1; @@ -191,7 +191,7 @@ fn print_hunk(mut left_line: usize, mut right_line: usize, hunk: Diff, max_digit } } -pub fn print_diff(diff: Diff, extra_lines: usize) { +pub fn print_diff(diff: &[diff::Result], extra_lines: usize) { let mut diff = hunkify_diff(diff, extra_lines); let last_hunk = diff.pop().expect("at least one hunk"); diff --git a/src/main.rs b/src/main.rs index 2a2e7c1..7882761 100644 --- a/src/main.rs +++ b/src/main.rs @@ -100,7 +100,7 @@ Otherwise, run `dotter undeploy` as root, remove cache.toml and cache/ folders, } args::Action::Undeploy => { debug!("Un-Deploying..."); - if deploy::undeploy(opt).context("undeploy")? { + if deploy::undeploy(&opt).context("undeploy")? { // An error occurred return Ok(false); } From f2aac815789fa18b02a38b4d091b4b02ea61f807 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 14:08:23 +1300 Subject: [PATCH 03/18] Remove redundant closure in handlebars_helpers.rs --- src/handlebars_helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 663e0bc..55463e5 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -16,7 +16,7 @@ use crate::config::{Configuration, Files, Variables}; pub fn create_new_handlebars<'b>(config: &mut Configuration) -> Result> { debug!("Creating Handlebars instance..."); let mut handlebars = Handlebars::new(); - handlebars.register_escape_fn(|s| s.to_string()); // Disable html-escaping + handlebars.register_escape_fn(str::to_string); // Disable html-escaping handlebars.set_strict_mode(true); // Report missing variables as errors register_rust_helpers(&mut handlebars); From 9edf99eec5aad736416ddbd7883e2057f3ec12e0 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 14:15:05 +1300 Subject: [PATCH 04/18] Simplify @ binding in DryRunFilesystem::copy_file() Clippy's unnested-or-patterns lint caught this. --- src/filesystem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index fb43c72..eee339b 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -670,7 +670,7 @@ impl Filesystem for DryRunFilesystem { } Ok(()) } - s @ FileState::SymbolicLink(_) | s @ FileState::Directory | s @ FileState::Missing => { + s @ (FileState::SymbolicLink(_) | FileState::Directory | FileState::Missing) => { anyhow::bail!("file is not regular file but is a {:?}", s); } } From 8273e0df4d101ae788d678bb6e92e1d6dffa2b29 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 14:22:40 +1300 Subject: [PATCH 05/18] Merge identical match arms in FileTarget::condition() Clippy's match-same-arms lint caught this. The previous two functions already used or-patterns like this, but condition() was overlooked. --- src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index 5ce299b..f326a16 100644 --- a/src/config.rs +++ b/src/config.rs @@ -412,8 +412,8 @@ impl FileTarget { pub fn condition(&self) -> Option<&String> { match self { FileTarget::Automatic(_) => None, - FileTarget::Symbolic(SymbolicTarget { condition, .. }) => condition.as_ref(), - FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) => condition.as_ref(), + FileTarget::Symbolic(SymbolicTarget { condition, .. }) + | FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) => condition.as_ref(), } } } From b5ca7a8fb099690b92cd50904dd78bfd97bcfdc4 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 14:48:01 +1300 Subject: [PATCH 06/18] Remove unused &self from is_owned_by_user() Clippy's unused-self lint caught this. --- src/filesystem.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index eee339b..9a908b5 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -257,7 +257,7 @@ impl RealFilesystem { Command::new("sudo") } - fn is_owned_by_user(&self, path: &Path) -> Result { + fn is_owned_by_user(path: &Path) -> Result { use std::os::unix::fs::MetadataExt; let file_uid = path.metadata().context("get file metadata")?.uid(); let process_uid = unsafe { libc::geteuid() }; @@ -460,9 +460,7 @@ impl Filesystem for RealFilesystem { } fn set_owner(&mut self, file: &Path, owner: &Option) -> Result<()> { - if self - .is_owned_by_user(file) - .context("detect if file is owned by the current user")? + if Self::is_owned_by_user(file).context("detect if file is owned by the current user")? && owner.is_none() { // Nothing to do, no need to elevate From afc4b692b62f3ee66f129733cf1d4e6245be0b88 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 14:51:58 +1300 Subject: [PATCH 07/18] Add backtick formatting to set_owner() docs Clippy's doc-markdown lint caught this. --- src/filesystem.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index 9a908b5..41e93a9 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -72,8 +72,8 @@ pub trait Filesystem { /// Target file will be owned by the selected user. Privileges elevated as needed. fn copy_file(&mut self, source: &Path, target: &Path, owner: &Option) -> Result<()>; - /// If owner.is_some, elevates privileges and sets file to that owner - /// If owner.is_none, ensures file is owned by the current user (elevating privileges if needed) + /// If `owner.is_some`, elevates privileges and sets file to that owner + /// If `owner.is_none`, ensures file is owned by the current user (elevating privileges if needed) fn set_owner(&mut self, file: &Path, owner: &Option) -> Result<()>; /// Copy file mode, elevating privileges as needed. (Does not change owner) From e4ae24b69cbb66787729a81989d642c3c092da5f Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sat, 16 Dec 2023 20:04:23 +1300 Subject: [PATCH 08/18] Remove pointless clone from watch.rs Due to being in a move closure, this clone captures opt by value and then clones it. This wouldn't work even if opt were used later in the enclosing scope and serves no purpose as it is. --- src/watch.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/watch.rs b/src/watch.rs index 8a17fe8..e6132b9 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -51,7 +51,6 @@ pub(crate) async fn watch(opt: Options) -> Result<()> { config.filterer(filter); config.on_action(move |mut action| { - let opt = opt.clone(); if action.signals().next().is_some() { action.quit(); return action; From 1e086c17b87f859ee7afaa051c841344dec1dd74 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 22 Dec 2023 14:05:45 +1300 Subject: [PATCH 09/18] Update to 2021 Rust edition --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ddc9d86..b285e3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ name = "dotter" version = "0.13.2" authors = ["SuperCuber "] description = "A dotfile manager and templater written in rust" -edition = "2018" +edition = "2021" repository = "https://github.com/SuperCuber/dotter" readme = "README.md" keywords = ["dotter", "dotfiles", "manager"] From b9acebbac0ed57049742cf62a2a41f30604204d3 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 22 Dec 2023 14:17:43 +1300 Subject: [PATCH 10/18] Use inline format args Clippy's uninlined-format-args lint caught this. --- src/config.rs | 24 ++++++++++++------------ src/deploy.rs | 18 +++++++++--------- src/filesystem.rs | 25 ++++++++++--------------- src/handlebars_helpers.rs | 5 ++--- src/main.rs | 2 +- 5 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/config.rs b/src/config.rs index f326a16..bc3bb06 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,8 +18,8 @@ pub enum UnixUser { impl fmt::Display for UnixUser { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - UnixUser::Uid(uid) => write!(f, "{}", uid), - UnixUser::Name(name) => write!(f, "{}", name), + UnixUser::Uid(uid) => write!(f, "{uid}"), + UnixUser::Name(name) => write!(f, "{name}"), } } } @@ -133,7 +133,7 @@ pub fn load_configuration( ) -> Result { let global: GlobalConfig = filesystem::load_file(global_config) .and_then(|c| c.ok_or_else(|| anyhow::anyhow!("file not found"))) - .with_context(|| format!("load global config {:?}", global_config))?; + .with_context(|| format!("load global config {global_config:?}"))?; trace!("Global config: {:#?}", global); // If local.toml can't be found, look for a file named .toml instead @@ -147,12 +147,12 @@ pub fn load_configuration( "{:?} not found, using {}.toml instead (based on hostname)", local_config, hostname ); - local_config_buf.set_file_name(&format!("{}.toml", hostname)); + local_config_buf.set_file_name(&format!("{hostname}.toml")); } let local: LocalConfig = filesystem::load_file(local_config_buf.as_path()) .and_then(|c| c.ok_or_else(|| anyhow::anyhow!("file not found"))) - .with_context(|| format!("load local config {:?}", local_config))?; + .with_context(|| format!("load local config {local_config:?}"))?; trace!("Local config: {:#?}", local); let mut merged_config = @@ -290,7 +290,7 @@ fn merge_configuration_files( Ok(()) }() - .with_context(|| format!("including file {:?}", included_path))?; + .with_context(|| format!("including file {included_path:?}"))?; } // Enable depended packages @@ -305,7 +305,7 @@ fn merge_configuration_files( global .packages .get(package) - .with_context(|| format!("get info of package {}", package))? + .with_context(|| format!("get info of package {package}"))? .depends .clone(), ); @@ -369,7 +369,7 @@ fn merge_configuration_files( Ok(()) }() - .with_context(|| format!("merge package {:?}", package_name))?; + .with_context(|| format!("merge package {package_name:?}"))?; } output.files = first_package.files; output.variables = first_package.variables; @@ -500,7 +500,7 @@ fn expand_directories(config: &Configuration) -> Result { .files .iter() .map(|(source, target)| { - expand_directory(source, target, config).context(format!("expand file {:?}", source)) + expand_directory(source, target, config).context(format!("expand file {source:?}")) }) .collect::>>()?; Ok(expanded.into_iter().flatten().collect::()) @@ -539,7 +539,7 @@ fn expand_directory(source: &Path, target: &FileTarget, config: &Configuration) let mut child_target = target.clone(); child_target.set_path(child_target.path().join(&child)); expand_directory(&child_source, &child_target, config) - .context(format!("expand file {:?}", child_source)) + .context(format!("expand file {child_source:?}")) }) .collect::>>()?; // Use transposition of Iterator> -> Result, E> Ok(expanded.into_iter().flatten().collect()) @@ -551,14 +551,14 @@ impl UnixUser { pub fn as_sudo_arg(&self) -> String { match self { UnixUser::Name(n) => n.clone(), - UnixUser::Uid(id) => format!("#{}", id), + UnixUser::Uid(id) => format!("#{id}"), } } pub fn as_chown_arg(&self) -> String { match self { UnixUser::Name(n) => n.clone(), - UnixUser::Uid(id) => format!("{}", id), + UnixUser::Uid(id) => format!("{id}"), } } } diff --git a/src/deploy.rs b/src/deploy.rs index 9ba5f50..f9971fc 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -85,7 +85,7 @@ Proceeding by copying instead of symlinking." match target { FileTarget::Automatic(target) => { if filesystem::is_template(&source) - .context(format!("check whether {:?} is a template", source))? + .context(format!("check whether {source:?} is a template"))? { desired_templates.insert(source, target.into()); } else { @@ -198,7 +198,7 @@ pub fn undeploy(opt: &Options) -> Result { execute_action( actions::delete_symlink(&deleted_symlink, &target, fs, opt.force), || cache.symlinks.remove(&deleted_symlink), - || format!("delete symlink {:?} -> {:?}", deleted_symlink, target), + || format!("delete symlink {deleted_symlink:?} -> {target:?}"), &mut suggest_force, &mut error_occurred, ); @@ -214,7 +214,7 @@ pub fn undeploy(opt: &Options) -> Result { opt.force, ), || cache.templates.remove(&deleted_template), - || format!("delete template {:?} -> {:?}", deleted_template, target), + || format!("delete template {deleted_template:?} -> {target:?}"), &mut suggest_force, &mut error_occurred, ); @@ -287,7 +287,7 @@ fn run_deploy( execute_action( runner.delete_symlink(source, target), || resulting_cache.symlinks.remove(source), - || format!("delete symlink {:?} -> {:?}", source, target), + || format!("delete symlink {source:?} -> {target:?}"), &mut suggest_force, &mut error_occurred, ); @@ -299,7 +299,7 @@ fn run_deploy( execute_action( runner.delete_template(source, &opt.cache_directory.join(source), target), || resulting_cache.templates.remove(source), - || format!("delete template {:?} -> {:?}", source, target), + || format!("delete template {source:?} -> {target:?}"), &mut suggest_force, &mut error_occurred, ); @@ -321,7 +321,7 @@ fn run_deploy( .symlinks .insert(source.clone(), target_path.clone()) }, - || format!("create symlink {:?} -> {:?}", source, target_path), + || format!("create symlink {source:?} -> {target_path:?}"), &mut suggest_force, &mut error_occurred, ); @@ -343,7 +343,7 @@ fn run_deploy( .templates .insert(source.clone(), target_path.clone()) }, - || format!("create template {:?} -> {:?}", source, target_path), + || format!("create template {source:?} -> {target_path:?}"), &mut suggest_force, &mut error_occurred, ); @@ -358,7 +358,7 @@ fn run_deploy( execute_action( runner.update_symlink(source, target), || (), - || format!("update symlink {:?} -> {:?}", source, target_path), + || format!("update symlink {source:?} -> {target_path:?}"), &mut suggest_force, &mut error_occurred, ); @@ -373,7 +373,7 @@ fn run_deploy( execute_action( runner.update_template(source, &opt.cache_directory.join(source), target), || (), - || format!("update template {:?} -> {:?}", source, target_path), + || format!("update template {source:?} -> {target_path:?}"), &mut suggest_force, &mut error_occurred, ); diff --git a/src/filesystem.rs b/src/filesystem.rs index 41e93a9..3ade72e 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -292,7 +292,7 @@ impl Filesystem for RealFilesystem { Ok(()) => Ok(()), Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { let success = self - .sudo(format!("removing file {:?} as root", path)) + .sudo(format!("removing file {path:?} as root")) .arg("rm") .arg("-r") .arg(path) @@ -328,15 +328,14 @@ impl Filesystem for RealFilesystem { { if (self.noconfirm || no_ask) || ask_boolean(&format!( - "Directory at {:?} is now empty. Delete [y/N]? ", - path + "Directory at {path:?} is now empty. Delete [y/N]? " )) { match std::fs::remove_dir(path) { Ok(()) => {} Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { let success = self - .sudo(format!("removing directory {:?}", path)) + .sudo(format!("removing directory {path:?}")) .arg("rmdir") .arg(path) .spawn() @@ -352,7 +351,7 @@ impl Filesystem for RealFilesystem { } } } - path = path.parent().context(format!("get parent of {:?}", path))?; + path = path.parent().context(format!("get parent of {path:?}"))?; } Ok(()) } @@ -363,8 +362,7 @@ impl Filesystem for RealFilesystem { if let Some(owner) = owner { let success = self .sudo(format!( - "creating symlink {:?} -> {:?} from user {:?}", - link, target, owner + "creating symlink {link:?} -> {target:?} from user {owner:?}" )) .arg("-u") .arg(owner.as_sudo_arg()) @@ -397,8 +395,7 @@ impl Filesystem for RealFilesystem { if let Some(owner) = owner { let success = self .sudo(format!( - "Creating directory {:?} from user {:?}...", - path, owner + "Creating directory {path:?} from user {owner:?}..." )) .arg("-u") .arg(owner.as_sudo_arg()) @@ -427,8 +424,7 @@ impl Filesystem for RealFilesystem { .context("read source file contents as current user")?; let mut child = self .sudo(format!( - "Copying {:?} -> {:?} as user {:?}", - source, target, owner + "Copying {source:?} -> {target:?} as user {owner:?}" )) .arg("-u") .arg(owner.as_sudo_arg()) @@ -472,7 +468,7 @@ impl Filesystem for RealFilesystem { )); let success = self - .sudo(format!("setting owner of {:?} to user \"{}\"", file, owner)) + .sudo(format!("setting owner of {file:?} to user \"{owner}\"")) .arg("chown") .arg(owner.as_chown_arg()) .arg("-h") // no-dereference @@ -496,8 +492,7 @@ impl Filesystem for RealFilesystem { if let Some(owner) = owner { let success = self .sudo(format!( - "Copying permissions {:?} -> {:?} as user {:?}", - source, target, owner + "Copying permissions {source:?} -> {target:?} as user {owner:?}" )) .arg("chmod") .arg("--reference") @@ -811,7 +806,7 @@ pub fn ask_boolean(prompt: &str) -> bool { || buf.to_lowercase().starts_with('n') || buf.is_empty()) { - eprintln!("{}", prompt); + eprintln!("{prompt}"); buf.clear(); io::stdin() .read_line(&mut buf) diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 55463e5..2856931 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -63,7 +63,7 @@ fn eval_condition( condition: &str, ) -> Result { // extra { for format!() - let condition = format!("{{{{#if {} }}}}true{{{{/if}}}}", condition); + let condition = format!("{{{{#if {condition} }}}}true{{{{/if}}}}"); let rendered = handlebars .render_template(&condition, variables) .context("")?; @@ -88,8 +88,7 @@ fn math_helper( &evalexpr::eval(&expression) .map_err(|e| { RenderErrorReason::Other(format!( - "Cannot evaluate expression {} because {}", - expression, e + "Cannot evaluate expression {expression} because {e}" )) })? .to_string(), diff --git a/src/main.rs b/src/main.rs index 7882761..8bff64e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,7 +36,7 @@ pub(crate) fn display_error(error: anyhow::Error) { let mut error_message = format!("Failed to {}\nCaused by:\n", chain.next().unwrap()); for e in chain { - writeln!(error_message, " {}", e).unwrap(); + writeln!(error_message, " {e}").unwrap(); } // Remove last \n error_message.pop(); From 6f22f15075d234d65118df0d6ed6f4da31a82074 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 15:43:11 +1200 Subject: [PATCH 11/18] Hoist else branch after bail!() Clippy's redundant-else lint caught this. --- src/config.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index bc3bb06..aac989b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -343,9 +343,8 @@ fn merge_configuration_files( for (file_name, file_target) in package.files { if first_package.files.contains_key(&file_name) { anyhow::bail!("file {:?} already encountered", file_name); - } else { - first_package.files.insert(file_name, file_target); } + first_package.files.insert(file_name, file_target); } for (variable_name, variable_value) in package.variables { From dada9ca4ca7df708195ee3962b0d68a6dd16be2f Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 15:55:20 +1200 Subject: [PATCH 12/18] Add ; after assignment in FileTarget::set_path() Rustfmt wants this broken due to line length and requires {} for multiline match arms, but doesn't insert a ; because it only sees syntax, not types. Clippy then complains (when pedantic) about the implicit () return. Clippy's clippy::semicolon-if-nothing-returned lint caught this. --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index aac989b..d6cf6d6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -403,7 +403,7 @@ impl FileTarget { FileTarget::Automatic(ref mut path) => *path = new_path.into(), FileTarget::Symbolic(SymbolicTarget { target, .. }) | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => { - *target = new_path.into() + *target = new_path.into(); } } } From d1df781e72352a41c6e5e9b3dd86cbc7f995c1df Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:10:54 +1200 Subject: [PATCH 13/18] Remove redundant closure in config::tests Clippy's redundant-closure lint caught this. --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index d6cf6d6..54c438c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -573,7 +573,7 @@ mod tests { file: FileTarget, } - let parse = |s| toml::from_str::(s); + let parse = toml::from_str::; assert_eq!( parse( From a64e6541c5e16a054d042cb1caf02ab6ebd134c3 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:44:47 +1200 Subject: [PATCH 14/18] Simplify asserts in tests Replaces `assert_eq!(x, [true|false])` with `assert!([x|!x])`. Clippy's bool-assert-comparison lint caught this. --- src/config.rs | 21 +++++++-------- src/deploy.rs | 20 +++++++------- src/handlebars_helpers.rs | 55 ++++++++++++++------------------------- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/src/config.rs b/src/config.rs index 54c438c..11eadbf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -622,17 +622,14 @@ mod tests { .file, FileTarget::ComplexTemplate(PathBuf::from("~/.QuarticCat").into()), ); - assert_eq!( - parse( - r#" - [file] - target = '~/.QuarticCat' - type = 'symbolic' - append = 'whatever' - "#, - ) - .is_err(), - true - ); + assert!(parse( + r#" + [file] + target = '~/.QuarticCat' + type = 'symbolic' + append = 'whatever' + "#, + ) + .is_err()); } } diff --git a/src/deploy.rs b/src/deploy.rs index f9971fc..b940dfa 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -468,8 +468,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert!(cache.symlinks.contains_key(&PathBuf::from("a_in"))); assert!(cache.templates.contains_key(&PathBuf::from("b_in"))); @@ -525,8 +525,8 @@ mod test { }, ); - assert_eq!(suggest_force, true); - assert_eq!(error_occurred, true); + assert!(suggest_force); + assert!(error_occurred); assert_eq!(cache.symlinks.len(), 0); assert_eq!(cache.templates.len(), 0); @@ -577,8 +577,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); @@ -633,8 +633,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); @@ -682,8 +682,8 @@ mod test { }, ); - assert_eq!(suggest_force, false); - assert_eq!(error_occurred, false); + assert!(!suggest_force); + assert!(!error_occurred); assert_eq!(cache.symlinks.len(), 1); assert_eq!(cache.templates.len(), 0); diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 2856931..15bca98 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -364,31 +364,18 @@ mod test { }; let handlebars = create_new_handlebars(&mut config).unwrap(); - assert_eq!( - eval_condition(&handlebars, &config.variables, "foo").unwrap(), - true - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "bar").unwrap(), - false - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "dotter.packages.default").unwrap(), - true - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "dotter.packages.nonexist").unwrap(), - false - ); - assert_eq!( - eval_condition( - &handlebars, - &config.variables, - "(and true dotter.packages.disabled)" - ) - .unwrap(), - false + assert!(eval_condition(&handlebars, &config.variables, "foo").unwrap()); + assert!(!eval_condition(&handlebars, &config.variables, "bar").unwrap()); + assert!(eval_condition(&handlebars, &config.variables, "dotter.packages.default").unwrap()); + assert!( + !eval_condition(&handlebars, &config.variables, "dotter.packages.nonexist").unwrap() ); + assert!(!eval_condition( + &handlebars, + &config.variables, + "(and true dotter.packages.disabled)" + ) + .unwrap()); } #[test] @@ -402,18 +389,14 @@ mod test { }; let handlebars = create_new_handlebars(&mut config).unwrap(); - assert_eq!( - eval_condition( - &handlebars, - &config.variables, - "(is_executable \"no_such_executable_please\")" - ) - .unwrap(), - false - ); - assert_eq!( - eval_condition(&handlebars, &config.variables, "(eq (math \"5+5\") \"10\")").unwrap(), - true + assert!(!eval_condition( + &handlebars, + &config.variables, + "(is_executable \"no_such_executable_please\")" + ) + .unwrap()); + assert!( + eval_condition(&handlebars, &config.variables, "(eq (math \"5+5\") \"10\")").unwrap() ); } } From 7f942c109757dcca2f677335cde688bc5b34a00f Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:52:04 +1200 Subject: [PATCH 15/18] Fix compile error in handlebars_helpers tests When testing without the `scripting` feature, compilation would fail because a `Configuration` was constructed assuming the `helpers` field always exists, but it only exists when `scripting` is enabled. --- src/handlebars_helpers.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/handlebars_helpers.rs b/src/handlebars_helpers.rs index 15bca98..38af55e 100644 --- a/src/handlebars_helpers.rs +++ b/src/handlebars_helpers.rs @@ -358,6 +358,7 @@ mod test { let mut config = Configuration { files: Files::new(), variables: maplit::btreemap! { "foo".into() => 2.into() }, + #[cfg(feature = "scripting")] helpers: Helpers::new(), packages: maplit::btreemap! { "default".into() => true, "disabled".into() => false }, recurse: true, @@ -383,6 +384,7 @@ mod test { let mut config = Configuration { files: Files::new(), variables: Variables::new(), + #[cfg(feature = "scripting")] helpers: Helpers::new(), packages: BTreeMap::new(), recurse: true, From 7b9995568f22f3bef2bdc857323796d3068e8992 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 12 Apr 2024 17:11:28 +1200 Subject: [PATCH 16/18] Avoid mysterious `Default::default()` In `deploy::test::{low_level_simple, low_level_skip}`, `Default::default()` was used to construct a `BTreeMap`, with the only indication of the variable's type being the signature of `RealActionRunner::new()`, which it was passed by reference to. Clippy's default-trait-access lint caught this. --- src/deploy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deploy.rs b/src/deploy.rs index b940dfa..87c604c 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -697,7 +697,7 @@ mod test { let opt = Options::default(); let handlebars = handlebars::Handlebars::new(); - let variables = Default::default(); + let variables = BTreeMap::new(); // Expectation: // create_symlink @@ -800,7 +800,7 @@ mod test { let opt = Options::default(); let handlebars = handlebars::Handlebars::new(); - let variables = Default::default(); + let variables = BTreeMap::new(); // Expectation: // create_symlink From 348166796de723d427bf99127d078751d5184bbc Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sun, 14 Apr 2024 01:13:30 +1200 Subject: [PATCH 17/18] Use `unwrap_err()` instead of `assert!(x.is_err())` This is less verbose and provides a more detailed panic message. --- src/config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config.rs b/src/config.rs index 11eadbf..2bd997a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -568,7 +568,7 @@ mod tests { #[test] fn deserialize_file_target() { - #[derive(Deserialize)] + #[derive(Debug, Deserialize)] struct Helper { file: FileTarget, } @@ -622,7 +622,7 @@ mod tests { .file, FileTarget::ComplexTemplate(PathBuf::from("~/.QuarticCat").into()), ); - assert!(parse( + parse( r#" [file] target = '~/.QuarticCat' @@ -630,6 +630,6 @@ mod tests { append = 'whatever' "#, ) - .is_err()); + .unwrap_err(); } } From 8ae4a9bccd1c347fe941f0a782887e5bb41d243c Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Sun, 14 Apr 2024 01:16:03 +1200 Subject: [PATCH 18/18] Rename config.rs test module for consistency `config::tests` was the only test module not named `test`. --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 2bd997a..9ccc777 100644 --- a/src/config.rs +++ b/src/config.rs @@ -563,7 +563,7 @@ impl UnixUser { } #[cfg(test)] -mod tests { +mod test { use super::*; #[test]