Merge pull request #168 from Integral-Tech/apply-clippy

refactor: apply clippy lints

build.rs

@@ -3,9 +3,8 @@ use std::{env, path::PathBuf};
 fn main() {
     let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap();
 
-    match target_os.as_str() {
-        "macos" => compile_macos(),
-        _ => {}
+    if target_os.as_str() == "macos" {
+        compile_macos()
     }
 }
 

flake.nix

@@ -35,17 +35,22 @@
         lib = pkgs.lib;
 
         crossHelpers = self.lib.crossHelpers system;
+        crossHelpersSelf = crossHelpers.forTarget "x86_64-linux";
       in {
         checks = import ./checks inputs system;
 
         packages = {
           default = self.packages."${system}".caligula;
 
-          lint-script = pkgs.writeScriptBin "lint.sh" ''
-            #!/bin/sh
-            export PATH=${lib.makeBinPath [ crossHelpers.baseToolchain ]}
-            ${./scripts/lint.sh}
-          '';
+          lint-script = with pkgs;
+            writeShellScriptBin "lint.sh" ''
+              #!/bin/sh
+              export PATH=${
+                lib.makeBinPath ([ bash crossHelpers.baseToolchain ]
+                  ++ crossHelpersSelf.buildInputs)
+              }
+              ${./scripts/lint.sh}
+            '';
 
           caligula = self.packages."${system}"."caligula-${system}";
         }

nix/cross-helpers.nix

@@ -124,7 +124,7 @@ in rec {
 
   crossCompileDevShell = let
     rust = baseToolchain.override {
-      extensions = [ "rust-src" "rust-analyzer" ];
+      extensions = [ "rust-src" "rust-analyzer" "clippy" ];
       targets = (map (target: (forTarget target).rustTarget) supportedSystems);
     };
 

scripts/lint.sh

@@ -1,3 +1,6 @@
-#!/bin/sh
+#!/usr/bin/env bash
+
+set -euxo pipefail
 
 cargo fmt --check
+cargo clippy

src/compression.rs

@@ -193,12 +193,8 @@ generate! {
 
 impl CompressionFormat {
     pub fn detect_from_path(path: impl AsRef<Path>) -> Option<CompressionFormat> {
-        if let Some(ext) = path.as_ref().extension() {
-            Some(CompressionFormat::detect_from_extension(
-                &ext.to_string_lossy(),
-            ))
-        } else {
-            None
-        }
+        path.as_ref()
+            .extension()
+            .map(|ext| CompressionFormat::detect_from_extension(&ext.to_string_lossy()))
     }
 }

src/device.rs

@@ -210,7 +210,7 @@ impl WriteTarget {
 
 impl PartialOrd for WriteTarget {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        self.devnode.partial_cmp(&other.devnode)
+        Some(self.cmp(other))
     }
 }
 
@@ -227,7 +227,7 @@ impl TryFrom<&Path> for WriteTarget {
         #[cfg(target_os = "linux")]
         if value.starts_with("/sys/class/block") || value.starts_with("/dev") {
             if let Some(n) = value.file_name() {
-                return Ok(Self::from_dev_name(n)?);
+                return Self::from_dev_name(n);
             }
         }
 
@@ -238,7 +238,7 @@ impl TryFrom<&Path> for WriteTarget {
             }
         }
 
-        Ok(Self::from_normal_file(value.to_owned())?)
+        Self::from_normal_file(value.to_owned())
     }
 }
 

src/escalation/mod.rs

@@ -19,7 +19,7 @@ pub enum Error {
 
 pub async fn run_escalate(
     cmd: &Command<'_>,
-    modify: impl FnOnce(&mut tokio::process::Command) -> (),
+    modify: impl FnOnce(&mut tokio::process::Command),
 ) -> anyhow::Result<tokio::process::Child> {
     #[cfg(target_os = "linux")]
     {

src/escalation/unix.rs

@@ -1,4 +1,4 @@
-use std::borrow::Cow;
+use std::{borrow::Cow, fmt};
 
 use itertools::Itertools;
 use shell_words::{join, quote};
@@ -53,7 +53,7 @@ impl EscalationMethod {
         }
     }
 
-    pub fn wrap_command<'a>(&self, cmd: &Command) -> Command {
+    pub fn wrap_command(&self, cmd: &Command) -> Command {
         let raw = cmd.to_string();
 
         match self {
@@ -87,18 +87,18 @@ impl EscalationMethod {
     }
 }
 
-impl ToString for Command<'_> {
-    fn to_string(&self) -> String {
+impl fmt::Display for Command<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let args = join([&self.proc].into_iter().chain(self.args.iter()));
 
         if self.envs.is_empty() {
-            args
+            write!(f, "{args}")
         } else {
             let envs: String = (self.envs.iter())
                 .map(|(k, v)| format!("{}={}", quote(k), quote(v)))
                 .join(" ");
 
-            format!("{envs} {args}")
+            write!(f, "{envs} {args}")
         }
     }
 }
@@ -203,7 +203,7 @@ mod tests {
     fn test_run0() {
         let result = EscalationMethod::Run0.wrap_command(&get_test_command());
 
-        let printed = format!("result:?");
+        let printed = "result:?".to_string();
         assert_eq!(
             result.to_string(),
             "run0 sh -c 'asdf=foo some/proc two --three '\\''\"four\"'\\'''"

src/hash.rs

@@ -286,8 +286,8 @@ where
 
 pub fn parse_base16_or_base64(s: &str) -> Option<Vec<u8>> {
     base16::decode(s)
-        .or_else(|_| base64::engine::general_purpose::URL_SAFE.decode(&s))
-        .or_else(|_| base64::engine::general_purpose::STANDARD.decode(&s))
+        .or_else(|_| base64::engine::general_purpose::URL_SAFE.decode(s))
+        .or_else(|_| base64::engine::general_purpose::STANDARD.decode(s))
         .ok()
 }
 
@@ -299,8 +299,8 @@ pub fn parse_hash_input(h: &str) -> Result<(Vec<HashAlg>, Vec<u8>), HashParseErr
     if let Some((alg, hash)) = h.split_once('-') {
         let alg =
             HashAlg::from_sri_alg(alg).ok_or_else(|| HashParseError::UnknownAlg(alg.into()))?;
-        let expected_hash = parse_base16_or_base64(hash)
-            .ok_or_else(|| HashParseError::SRIValueNotBase16OrBase64)?;
+        let expected_hash =
+            parse_base16_or_base64(hash).ok_or(HashParseError::SRIValueNotBase16OrBase64)?;
 
         let expected_bytes = alg.digest_bytes();
         let actual_bytes = expected_hash.len();

src/hashfile.rs

@@ -3,7 +3,7 @@ use anyhow::anyhow;
 use std::{
     fs::File,
     io::{BufRead, BufReader},
-    path::PathBuf,
+    path::Path,
 };
 
 /// Common filenames of hash files.
@@ -34,7 +34,7 @@ const HASH_FILES: [(HashAlg, &str); 24] = [
     (HashAlg::Sha512, "SHA512SUMS"),
 ];
 
-pub fn find_hash(input: &PathBuf) -> Option<(HashAlg, &str, Vec<u8>)> {
+pub fn find_hash(input: &Path) -> Option<(HashAlg, &str, Vec<u8>)> {
     for (alg, hash_file) in HASH_FILES {
         let hash_filepath = input.parent()?.join(hash_file);
         match File::open(&hash_filepath) {

src/logging.rs

@@ -22,9 +22,7 @@ impl LogPaths {
             state_dir.as_ref().join("log")
         };
         create_dir_all(&log_dir).unwrap();
-        Self {
-            log_dir: log_dir.into(),
-        }
+        Self { log_dir }
     }
 
     pub fn main(&self) -> PathBuf {

src/run_mode.rs

@@ -79,8 +79,8 @@ pub fn make_spawn_command<'a, C: Serialize + Debug>(
         envs: vec![(RUN_MODE_ENV_NAME.into(), run_mode.as_str().into())],
         // Arg order is documented in childproc_common.
         args: vec![
-            log_path.into(),
-            socket.into(),
+            log_path,
+            socket,
             serde_json::to_string(&init_config).unwrap().into(),
         ],
     }

src/ui/cli.rs

@@ -128,7 +128,7 @@ pub enum UseSudo {
 fn parse_path_exists(p: &str) -> Result<PathBuf, String> {
     let path = PathBuf::from(p);
     if !path.exists() {
-        return Err(format!("path does not exist"));
+        return Err("path does not exist".to_string());
     }
     Ok(path)
 }

src/ui/fancy_ui/display.rs

@@ -47,7 +47,7 @@ where
             terminal,
             handle: Some(handle),
             events: EventStream::new(),
-            state: State::initial(Instant::now(), &params, input_file_bytes),
+            state: State::initial(Instant::now(), params, input_file_bytes),
             log_paths,
         }
     }
@@ -81,7 +81,7 @@ where
             self.handle = None;
         }
 
-        draw(&mut self.state, &mut self.terminal, &self.log_paths)?;
+        draw(&mut self.state, self.terminal, &self.log_paths)?;
         Ok(self)
     }
 }

src/ui/fancy_ui/widgets.rs

@@ -45,7 +45,6 @@ impl StatefulWidget for SpeedChart<'_> {
         let verify_speeds: Option<Vec<(f64, f64)>> = self.state.verify_hist().map(|verify_data| {
             verify_data
                 .speeds(window)
-                .into_iter()
                 .map(|(x, y)| (x + write_data.last_datapoint().0, y))
                 .collect()
         });
@@ -76,7 +75,7 @@ impl StatefulWidget for SpeedChart<'_> {
                 dataset_style
                     .name("Verify")
                     .style(Style::default().fg(Color::Blue))
-                    .data(&vdata),
+                    .data(vdata),
             );
         }
 

src/ui/herder/handle.rs

@@ -30,7 +30,7 @@ impl ChildHandle {
     }
 
     pub async fn next_message<T: DeserializeOwned>(&mut self) -> anyhow::Result<T> {
-        Ok(read_msg_async::<T>(&mut self.rx).await?)
+        read_msg_async::<T>(&mut self.rx).await
     }
 }
 

src/ui/main.rs

@@ -54,9 +54,7 @@ fn handle_toplevel_error(err: anyhow::Error) {
 
 async fn inner_main(state_dir: PathBuf, log_paths: LogPaths) -> anyhow::Result<()> {
     let args = Args::parse();
-    let args = match args.command {
-        Command::Burn(a) => a,
-    };
+    let Command::Burn(args) = args.command;
 
     let log_paths = Arc::new(log_paths);
 

src/ui/simple_ui/ask_hash.rs

@@ -39,7 +39,7 @@ pub fn ask_hash(args: &BurnArgs, cf: CompressionFormat) -> anyhow::Result<Option
         }
         HashArg::Hash { alg, expected_hash } => Some(BeginHashParams {
             expected_hash: expected_hash.clone(),
-            alg: alg.clone(),
+            alg: *alg,
             hasher_compression: ask_hasher_compression(cf, args.hash_of)?,
         }),
     };

src/ui/writer_tracking.rs

@@ -124,10 +124,7 @@ impl WriterState {
     }
 
     pub fn is_finished(&self) -> bool {
-        match self {
-            WriterState::Finished { .. } => true,
-            _ => false,
-        }
+        matches!(self, WriterState::Finished { .. })
     }
 }
 

src/writer_process/mod.rs

@@ -12,7 +12,7 @@ use std::{
 
 use aligned_vec::avec_rt;
 use interprocess::local_socket::{prelude::*, GenericFilePath};
-use tracing::{debug, info};
+use tracing::{debug, info, trace};
 use tracing_unwrap::ResultExt;
 
 use crate::childproc_common::child_init;
@@ -106,6 +106,7 @@ fn run(mut tx: impl FnMut(StatusMessage), args: &WriterProcessConfig) -> Result<
             .read(true)
             .write(true)
             .create(true)
+            .truncate(true)
             .open(&args.dest)?,
         device::Type::Disk | device::Type::Partition => {
             open_blockdev(&args.dest, args.compression)?
@@ -297,15 +298,16 @@ impl<S: Read, D: Read> VerifyOp<S, D> {
 
         loop {
             for _ in 0..self.checkpoint_period {
-                let read_bytes = file.read(&mut file_buf)?;
-                if read_bytes == 0 {
+                let file_read_bytes = try_read_exact(&mut file, &mut file_buf)?;
+                if file_read_bytes == 0 {
                     checkpoint!();
                     return Ok(());
                 }
 
-                disk.read(&mut disk_buf)?;
+                try_read_exact(&mut disk, &mut disk_buf)?;
 
-                if &file_buf[..read_bytes] != &disk_buf[..read_bytes] {
+                if file_buf[..file_read_bytes] != disk_buf[..file_read_bytes] {
+                    trace!(file_read_bytes, "verification failed");
                     return Err(ErrorType::VerificationFailed);
                 }
             }

src/writer_process/tests.rs

@@ -307,7 +307,7 @@ mod helpers {
     impl<'a> MockRead<'a> {
         pub fn new(data: &'a [u8], enforced_block_size: Option<usize>) -> Self {
             Self {
-                cursor: Cursor::new(&data),
+                cursor: Cursor::new(data),
                 requested_reads: vec![],
                 enforced_block_size,
             }