From a6bf7527918178821e080db10e65b057f427200d Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 18 Apr 2020 14:36:54 -0700 Subject: [PATCH] Use `invariant` instead of `unwrap` and `expect` Add the `Invariant` trait, which provides `Invariant::invariant` and `Invariant::invariant_unwrap` methods, and use them instead of unwrap and expect. I think these methods are a bit clearer than `unwrap` and `expect`, since they more clearly document intent, i.e. that the thing passed to `invariant` should be a description of an invariant that should always be true, and should provide better error messages. Replace uses of `unwrap` and `expect` with `invariant`. type: reform fixes: - https://github.com/casey/intermodal/issues/167 --- CHANGELOG.md | 3 ++- src/common.rs | 5 +++-- src/error.rs | 5 ++++- src/host_port.rs | 13 ++++++++++--- src/invariant.rs | 22 ++++++++++++++++++++++ src/magnet_link.rs | 2 +- src/main.rs | 4 +--- src/piece_list.rs | 8 +++++++- src/platform.rs | 7 ++++++- src/subcommand/torrent/stats.rs | 2 +- src/subcommand/torrent/verify.rs | 2 +- src/torrent_summary.rs | 2 +- src/verifier.rs | 2 +- 13 files changed, 60 insertions(+), 17 deletions(-) create mode 100644 src/invariant.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index bb6bdad..93fdd08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ Changelog UNRELEASED - 2020-04-18 ----------------------- -- :white_check_mark: [`xxxxxxxxxxxx`](https://github.com/casey/intermodal/commits/master) Test that globs match torrent contents - Fixes [#377](https://github.com/casey/intermodal/issues/377) - _Casey Rodarmor _ +- :art: [`xxxxxxxxxxxx`](https://github.com/casey/intermodal/commits/master) Use `invariant` instead of `unwrap` and `expect` - Fixes [#167](https://github.com/casey/intermodal/issues/167) - _Casey Rodarmor _ +- :white_check_mark: [`faf46c0f0e6f`](https://github.com/casey/intermodal/commit/faf46c0f0e6fd4e4f8b504d414a3bf02d7d68e4a) Test that globs match torrent contents - Fixes [#377](https://github.com/casey/intermodal/issues/377) - _Casey Rodarmor _ - :books: [`0a754d0bcfcf`](https://github.com/casey/intermodal/commit/0a754d0bcfcfd65127d7b6e78d41852df78d3ea2) Add manual Arch install link - Fixes [#373](https://github.com/casey/intermodal/issues/373) - _Casey Rodarmor _ - :art: [`0a870ed2ee2c`](https://github.com/casey/intermodal/commit/0a870ed2ee2cca79fddb9940fb879354468deb4d) Get current time early when creating torrents - Fixes [#207](https://github.com/casey/intermodal/issues/207) - _Casey Rodarmor _ - :books: [`9098d3684032`](https://github.com/casey/intermodal/commit/9098d368403232a07684cae8c0b9b1f1383dd2ce) Readme improvements - _Casey Rodarmor _ diff --git a/src/common.rs b/src/common.rs index 55c4ba7..d868223 100644 --- a/src/common.rs +++ b/src/common.rs @@ -55,8 +55,9 @@ pub(crate) use crate::{consts, error, host_port_parse_error}; // traits pub(crate) use crate::{ - input_stream::InputStream, into_u64::IntoU64, into_usize::IntoUsize, path_ext::PathExt, - platform_interface::PlatformInterface, print::Print, reckoner::Reckoner, step::Step, + input_stream::InputStream, into_u64::IntoU64, into_usize::IntoUsize, invariant::Invariant, + path_ext::PathExt, platform_interface::PlatformInterface, print::Print, reckoner::Reckoner, + step::Step, }; // structs and enums diff --git a/src/error.rs b/src/error.rs index c869798..6f9889a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -173,7 +173,10 @@ impl From for Error { impl From for Error { fn from(walkdir_error: walkdir::Error) -> Self { - let path = walkdir_error.path().unwrap().to_owned(); + let path = walkdir_error + .path() + .invariant_unwrap("Walkdir errors always have path") + .to_owned(); if let Some(source) = walkdir_error.into_io_error() { Self::Filesystem { source, path } diff --git a/src/host_port.rs b/src/host_port.rs index cc63b8d..48223b9 100644 --- a/src/host_port.rs +++ b/src/host_port.rs @@ -19,11 +19,18 @@ impl FromStr for HostPort { $ ", ) - .unwrap(); + .invariant_unwrap("regex is valid"); if let Some(captures) = socket_address_re.captures(text) { - let host_text = captures.name("host").unwrap().as_str(); - let port_text = captures.name("port").unwrap().as_str(); + let host_text = captures + .name("host") + .invariant_unwrap("Capture group `host` always present") + .as_str(); + + let port_text = captures + .name("port") + .invariant_unwrap("Capture group `port` always present") + .as_str(); let host = Host::parse(&host_text).context(host_port_parse_error::Host { text: text.to_owned(), diff --git a/src/invariant.rs b/src/invariant.rs new file mode 100644 index 0000000..817972f --- /dev/null +++ b/src/invariant.rs @@ -0,0 +1,22 @@ +use crate::common::*; + +pub(crate) trait Invariant: Sized { + fn invariant(self, invariant: D) -> Result; + + fn invariant_unwrap(self, invariant: D) -> T { + #![allow(clippy::result_unwrap_used)] + self.invariant(invariant).unwrap() + } +} + +impl Invariant for Option { + fn invariant(self, invariant: D) -> Result { + self.ok_or_else(|| Error::internal(format!("Invariant violated: {}", invariant))) + } +} + +impl Invariant for Result { + fn invariant(self, invariant: D) -> Result { + self.map_err(|err| Error::internal(format!("Invariant `{}` violated: {}", invariant, err))) + } +} diff --git a/src/magnet_link.rs b/src/magnet_link.rs index 7939414..40fbdbd 100644 --- a/src/magnet_link.rs +++ b/src/magnet_link.rs @@ -50,7 +50,7 @@ impl MagnetLink { } pub(crate) fn to_url(&self) -> Url { - let mut url = Url::parse("magnet:").unwrap(); + let mut url = Url::parse("magnet:").invariant_unwrap("`magnet:` is valid URL"); let mut query = format!("xt=urn:btih:{}", self.infohash); diff --git a/src/main.rs b/src/main.rs index 4b8159d..ec1529b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,9 +13,6 @@ clippy::needless_pass_by_value, clippy::non_ascii_literal, clippy::option_map_unwrap_or_else, - clippy::option_unwrap_used, - clippy::result_expect_used, - clippy::result_unwrap_used, clippy::shadow_reuse, clippy::too_many_lines, clippy::unseparated_literal_suffix, @@ -71,6 +68,7 @@ mod input_stream; mod input_target; mod into_u64; mod into_usize; +mod invariant; mod lint; mod linter; mod magnet_link; diff --git a/src/piece_list.rs b/src/piece_list.rs index 05a82e1..dce2bbb 100644 --- a/src/piece_list.rs +++ b/src/piece_list.rs @@ -67,7 +67,13 @@ impl<'de> Deserialize<'de> for PieceList { let piece_hashes = bytes .chunks_exact(Sha1Digest::LENGTH) - .map(|chunk| Sha1Digest::from_bytes(chunk.try_into().unwrap())) + .map(|chunk| { + Sha1Digest::from_bytes( + chunk + .try_into() + .invariant_unwrap("chunks are all Sha1Digest::LENGTH"), + ) + }) .collect(); Ok(Self { piece_hashes }) diff --git a/src/platform.rs b/src/platform.rs index 690c414..0b9c49d 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -23,7 +23,12 @@ impl PlatformInterface for Platform { let mut stat: libc::stat = unsafe { mem::zeroed() }; - let cpath = CString::new(path.as_os_str().as_bytes()).expect("Path contained null character."); + let cpath = if let Ok(cstr) = CString::new(path.as_os_str().as_bytes()) { + cstr + } else { + // Consider paths containing null bytes to be hidden + return Ok(true); + }; let error_code = unsafe { libc::stat(cpath.as_ptr(), &mut stat) }; diff --git a/src/subcommand/torrent/stats.rs b/src/subcommand/torrent/stats.rs index c8a3964..59e0041 100644 --- a/src/subcommand/torrent/stats.rs +++ b/src/subcommand/torrent/stats.rs @@ -128,7 +128,7 @@ struct Extractor { impl Extractor { fn new(print: bool, regexes: &[Regex]) -> Self { let regex_set = RegexSet::new(regexes.iter().map(Regex::as_str)) - .expect("Validated regex pattern failed to recompile in regex set"); + .invariant_unwrap("Regexes already validated by compilation"); Self { bencode_decode_errors: 0, diff --git a/src/subcommand/torrent/verify.rs b/src/subcommand/torrent/verify.rs index 244e401..39e1fd1 100644 --- a/src/subcommand/torrent/verify.rs +++ b/src/subcommand/torrent/verify.rs @@ -47,7 +47,7 @@ impl Verify { content.clone() } else { match &self.metainfo { - InputTarget::Path(path) => path.parent().unwrap().join(&metainfo.info.name), + InputTarget::Path(path) => path.join("..").join(&metainfo.info.name).clean(), InputTarget::Stdin => PathBuf::from(&metainfo.info.name), } }; diff --git a/src/torrent_summary.rs b/src/torrent_summary.rs index 0aa3e2e..ec79d35 100644 --- a/src/torrent_summary.rs +++ b/src/torrent_summary.rs @@ -62,7 +62,7 @@ impl TorrentSummary { creation_date .min(i64::max_value() as u64) .try_into() - .unwrap(), + .invariant_unwrap("min with i64 is always valid i64"), 0, ), ); diff --git a/src/verifier.rs b/src/verifier.rs index eed9f18..51d8450 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -79,7 +79,7 @@ impl<'a> Verifier<'a> { let to_buffer: usize = remaining .min(self.buffer.len().into_u64()) .try_into() - .unwrap(); + .invariant_unwrap("min with usize should fit in usize"); let buffer = &mut self.buffer[0..to_buffer];