From 2d782606fe984cb2ffebe7b98807a58494441a4c Mon Sep 17 00:00:00 2001
From: Tamir Duberstein <tamird@gmail.com>
Date: Thu, 27 Feb 2025 17:44:11 -0500
Subject: [PATCH] *: avoid Result::is_{ok,err}
These methods discard information. Discarding information is bad.
---
aya-log-parser/Cargo.toml | 3 +++
aya-log-parser/src/lib.rs | 13 ++++++-------
aya/src/maps/bloom_filter.rs | 12 +++++++-----
aya/src/maps/hash_map/hash_map.rs | 14 +++++++-------
aya/src/maps/hash_map/per_cpu_hash_map.rs | 6 +++---
aya/src/maps/lpm_trie.rs | 8 ++++----
aya/src/programs/links.rs | 8 ++++----
aya/src/programs/uprobe.rs | 2 +-
aya/src/util.rs | 6 +++---
9 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/aya-log-parser/Cargo.toml b/aya-log-parser/Cargo.toml
index 38a30d33..670c816e 100644
--- a/aya-log-parser/Cargo.toml
+++ b/aya-log-parser/Cargo.toml
@@ -11,5 +11,8 @@ edition.workspace = true
[dependencies]
aya-log-common = { path = "../aya-log-common", version = "^0.1.14", default-features = false }
+[dev-dependencies]
+assert_matches = { workspace = true }
+
[lib]
path = "src/lib.rs"
diff --git a/aya-log-parser/src/lib.rs b/aya-log-parser/src/lib.rs
index 1e760333..d01a17ed 100644
--- a/aya-log-parser/src/lib.rs
+++ b/aya-log-parser/src/lib.rs
@@ -128,11 +128,10 @@ pub fn parse(format_string: &str) -> Result<Vec<Fragment>, String> {
#[cfg(test)]
mod test {
+ use assert_matches::assert_matches;
+
use super::*;
- // TODO(https://github.com/rust-lang/rust-clippy/issues/13885): narrow this to just the specific
- // strings when that doesn't trip the lint.
- #[allow(clippy::literal_string_with_formatting_args)]
#[test]
fn test_parse() {
assert_eq!(
@@ -160,9 +159,9 @@ mod test {
}),
])
);
- assert!(parse("foo {:}").is_err());
- assert!(parse("foo { bar").is_err());
- assert!(parse("foo } bar").is_err());
- assert!(parse("foo { bar }").is_err());
+ assert_matches!(parse("foo {:}"), Err(_));
+ assert_matches!(parse("foo { bar"), Err(_));
+ assert_matches!(parse("foo } bar"), Err(_));
+ assert_matches!(parse("foo { bar }"), Err(_));
}
}
diff --git a/aya/src/maps/bloom_filter.rs b/aya/src/maps/bloom_filter.rs
index 08c45360..2f0d0d18 100644
--- a/aya/src/maps/bloom_filter.rs
+++ b/aya/src/maps/bloom_filter.rs
@@ -20,15 +20,17 @@ use crate::{
/// # Examples
///
/// ```no_run
+/// # use assert_matches::assert_matches;
/// # let mut bpf = aya::Ebpf::load(&[])?;
+/// use aya::maps::MapError;
/// use aya::maps::bloom_filter::BloomFilter;
///
/// let mut bloom_filter = BloomFilter::try_from(bpf.map_mut("BLOOM_FILTER").unwrap())?;
///
/// bloom_filter.insert(1, 0)?;
///
-/// assert!(bloom_filter.contains(&1, 0).is_ok());
-/// assert!(bloom_filter.contains(&2, 0).is_err());
+/// assert_matches!(bloom_filter.contains(&1, 0), Ok(()));
+/// assert_matches!(bloom_filter.contains(&2, 0), Err(MapError::ElementNotFound));
///
/// # Ok::<(), aya::EbpfError>(())
/// ```
@@ -131,7 +133,7 @@ mod tests {
fn test_new_ok() {
let map = new_map(new_obj_map());
- assert!(BloomFilter::<_, u32>::new(&map).is_ok());
+ let _: BloomFilter<_, u32> = BloomFilter::new(&map).unwrap();
}
#[test]
@@ -139,7 +141,7 @@ mod tests {
let map = new_map(new_obj_map());
let map = Map::BloomFilter(map);
- assert!(BloomFilter::<_, u32>::try_from(&map).is_ok())
+ let _: BloomFilter<_, u32> = map.try_into().unwrap();
}
#[test]
@@ -168,7 +170,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(bloom_filter.insert(0, 42).is_ok());
+ assert_matches!(bloom_filter.insert(0, 42), Ok(()));
}
#[test]
diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs
index 952cd48c..dc138fe6 100644
--- a/aya/src/maps/hash_map/hash_map.rs
+++ b/aya/src/maps/hash_map/hash_map.rs
@@ -179,23 +179,23 @@ mod tests {
#[test]
fn test_new_ok() {
let map = new_map(new_obj_map());
- assert!(HashMap::<_, u32, u32>::new(&map).is_ok());
+ let _: HashMap<_, u32, u32> = HashMap::new(&map).unwrap();
}
#[test]
fn test_try_from_ok() {
let map = new_map(new_obj_map());
let map = Map::HashMap(map);
- assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok())
+ let _: HashMap<_, u32, u32> = map.try_into().unwrap();
}
#[test]
fn test_try_from_ok_lru() {
let map_data = || new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_HASH));
let map = Map::HashMap(map_data());
- assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok());
+ let _: HashMap<_, u32, u32> = map.try_into().unwrap();
let map = Map::LruHashMap(map_data());
- assert!(HashMap::<_, u32, u32>::try_from(&map).is_ok())
+ let _: HashMap<_, u32, u32> = map.try_into().unwrap();
}
#[test]
@@ -224,7 +224,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(hm.insert(1, 42, 0).is_ok());
+ assert_matches!(hm.insert(1, 42, 0), Ok(()));
}
#[test]
@@ -240,7 +240,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(hm.insert(Box::new(1), Box::new(42), 0).is_ok());
+ assert_matches!(hm.insert(Box::new(1), Box::new(42), 0), Ok(()));
}
#[test]
@@ -269,7 +269,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(hm.remove(&1).is_ok());
+ assert_matches!(hm.remove(&1), Ok(()));
}
#[test]
diff --git a/aya/src/maps/hash_map/per_cpu_hash_map.rs b/aya/src/maps/hash_map/per_cpu_hash_map.rs
index afb76620..022b64c9 100644
--- a/aya/src/maps/hash_map/per_cpu_hash_map.rs
+++ b/aya/src/maps/hash_map/per_cpu_hash_map.rs
@@ -174,16 +174,16 @@ mod tests {
let map = Map::PerCpuHashMap(test_utils::new_map(test_utils::new_obj_map::<u32>(
BPF_MAP_TYPE_PERCPU_HASH,
)));
- assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok())
+ let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
}
#[test]
fn test_try_from_ok_lru() {
let map_data =
|| test_utils::new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_LRU_PERCPU_HASH));
let map = Map::PerCpuHashMap(map_data());
- assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok());
+ let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
let map = Map::PerCpuLruHashMap(map_data());
- assert!(PerCpuHashMap::<_, u32, u32>::try_from(&map).is_ok())
+ let _: PerCpuHashMap<_, u32, u32> = map.try_into().unwrap();
}
#[test]
fn test_get_not_found() {
diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs
index b247d38d..2e7c1788 100644
--- a/aya/src/maps/lpm_trie.rs
+++ b/aya/src/maps/lpm_trie.rs
@@ -260,7 +260,7 @@ mod tests {
fn test_new_ok() {
let map = new_map(new_obj_map());
- assert!(LpmTrie::<_, u32, u32>::new(&map).is_ok());
+ let _: LpmTrie<_, u32, u32> = LpmTrie::new(&map).unwrap();
}
#[test]
@@ -268,7 +268,7 @@ mod tests {
let map = new_map(new_obj_map());
let map = Map::LpmTrie(map);
- assert!(LpmTrie::<_, u32, u32>::try_from(&map).is_ok())
+ let _: LpmTrie<_, u32, u32> = map.try_into().unwrap();
}
#[test]
@@ -301,7 +301,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(trie.insert(&key, 1, 0).is_ok());
+ assert_matches!(trie.insert(&key, 1, 0), Ok(()));
}
#[test]
@@ -334,7 +334,7 @@ mod tests {
_ => sys_error(EFAULT),
});
- assert!(trie.remove(&key).is_ok());
+ assert_matches!(trie.remove(&key), Ok(()));
}
#[test]
diff --git a/aya/src/programs/links.rs b/aya/src/programs/links.rs
index 3ea90936..9442ed0a 100644
--- a/aya/src/programs/links.rs
+++ b/aya/src/programs/links.rs
@@ -634,11 +634,11 @@ mod tests {
assert_eq!(*l1_detached.borrow(), 0);
assert_eq!(*l2_detached.borrow(), 0);
- assert!(links.remove(id1).is_ok());
+ links.remove(id1).unwrap();
assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 0);
- assert!(links.remove(id2).is_ok());
+ links.remove(id2).unwrap();
assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 1);
}
@@ -678,7 +678,7 @@ mod tests {
let id1 = links.insert(l1).unwrap();
links.insert(l2).unwrap();
// manually remove one link
- assert!(links.remove(id1).is_ok());
+ links.remove(id1).unwrap();
assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 0);
}
@@ -710,7 +710,7 @@ mod tests {
assert_eq!(*l2_detached.borrow(), 1);
// manually detach l1
- assert!(owned_l1.detach().is_ok());
+ owned_l1.detach().unwrap();
assert_eq!(*l1_detached.borrow(), 1);
assert_eq!(*l2_detached.borrow(), 1);
}
diff --git a/aya/src/programs/uprobe.rs b/aya/src/programs/uprobe.rs
index 1e68cd27..0743fa92 100644
--- a/aya/src/programs/uprobe.rs
+++ b/aya/src/programs/uprobe.rs
@@ -716,7 +716,7 @@ mod tests {
let align_bytes = aligned_slice(&mut debug_bytes);
let debug_obj = object::File::parse(&*align_bytes).expect("got debug obj");
- assert!(verify_build_ids(&main_obj, &debug_obj, "symbol_name").is_ok());
+ verify_build_ids(&main_obj, &debug_obj, "symbol_name").unwrap();
}
#[test]
diff --git a/aya/src/util.rs b/aya/src/util.rs
index fbb9307f..8a5155e3 100644
--- a/aya/src/util.rs
+++ b/aya/src/util.rs
@@ -441,9 +441,9 @@ mod tests {
parse_cpu_ranges("0-5,6,7").unwrap(),
(0..=7).collect::<Vec<_>>()
);
- assert!(parse_cpu_ranges("").is_err());
- assert!(parse_cpu_ranges("0-1,2-").is_err());
- assert!(parse_cpu_ranges("foo").is_err());
+ assert_matches!(parse_cpu_ranges(""), Err(_));
+ assert_matches!(parse_cpu_ranges("0-1,2-"), Err(_));
+ assert_matches!(parse_cpu_ranges("foo"), Err(_));
}
#[test]