feat: ensure_and_debug_assert{,_eq,_ne} macros combining debug_assert* and anyhow::ensure (#6907)

We have some debug assertions already, but we also want the corresponding errors in the release
configuration so that it's not less reliable than non-optimized one. This doesn't change any
function signatures, only debug assertions in functions returning `Result` are replaced.

Co-authored-by: l <link2xt@testrun.org>
This commit is contained in:
iequidoo
2025-07-11 14:59:49 -03:00
committed by GitHub
parent 6406f305b8
commit 0359481ba4
4 changed files with 58 additions and 16 deletions

View File

@@ -37,7 +37,7 @@ use crate::mimeparser::AvatarAction;
use crate::param::{Param, Params}; use crate::param::{Param, Params};
use crate::sync::{self, Sync::*}; use crate::sync::{self, Sync::*};
use crate::tools::{SystemTime, duration_to_str, get_abs_path, time}; use crate::tools::{SystemTime, duration_to_str, get_abs_path, time};
use crate::{chat, chatlist_events, stock_str}; use crate::{chat, chatlist_events, ensure_and_debug_assert_ne, stock_str};
/// Time during which a contact is considered as seen recently. /// Time during which a contact is considered as seen recently.
const SEEN_RECENTLY_SECONDS: i64 = 600; const SEEN_RECENTLY_SECONDS: i64 = 600;
@@ -1922,9 +1922,10 @@ pub(crate) async fn mark_contact_id_as_verified(
contact_id: ContactId, contact_id: ContactId,
verifier_id: ContactId, verifier_id: ContactId,
) -> Result<()> { ) -> Result<()> {
debug_assert_ne!( ensure_and_debug_assert_ne!(
contact_id, verifier_id, contact_id,
"Contact cannot be verified by self" verifier_id,
"Contact cannot be verified by self",
); );
context context
.sql .sql

View File

@@ -20,6 +20,7 @@ use crate::constants::{Chattype, DC_FROM_HANDSHAKE};
use crate::contact::{Contact, ContactId, Origin}; use crate::contact::{Contact, ContactId, Origin};
use crate::context::Context; use crate::context::Context;
use crate::e2ee::EncryptHelper; use crate::e2ee::EncryptHelper;
use crate::ensure_and_debug_assert;
use crate::ephemeral::Timer as EphemeralTimer; use crate::ephemeral::Timer as EphemeralTimer;
use crate::key::self_fingerprint; use crate::key::self_fingerprint;
use crate::key::{DcKey, SignedPublicKey}; use crate::key::{DcKey, SignedPublicKey};
@@ -308,7 +309,7 @@ impl MimeFactory {
} else if id == ContactId::SELF { } else if id == ContactId::SELF {
member_fingerprints.push(self_fingerprint.to_string()); member_fingerprints.push(self_fingerprint.to_string());
} else { } else {
debug_assert!(member_fingerprints.is_empty(), "If some past member is a key-contact, all other past members should be key-contacts too"); ensure_and_debug_assert!(member_fingerprints.is_empty(), "If some past member is a key-contact, all other past members should be key-contacts too");
} }
} }
member_timestamps.push(add_timestamp); member_timestamps.push(add_timestamp);
@@ -359,7 +360,7 @@ impl MimeFactory {
// if we are leaving the group. // if we are leaving the group.
past_member_fingerprints.push(self_fingerprint.to_string()); past_member_fingerprints.push(self_fingerprint.to_string());
} else { } else {
debug_assert!(past_member_fingerprints.is_empty(), "If some past member is a key-contact, all other past members should be key-contacts too"); ensure_and_debug_assert!(past_member_fingerprints.is_empty(), "If some past member is a key-contact, all other past members should be key-contacts too");
} }
} }
} }
@@ -367,8 +368,8 @@ impl MimeFactory {
} }
} }
debug_assert!(member_timestamps.len() >= to.len()); ensure_and_debug_assert!(member_timestamps.len() >= to.len());
debug_assert!(member_fingerprints.is_empty() || member_fingerprints.len() >= to.len()); ensure_and_debug_assert!(member_fingerprints.is_empty() || member_fingerprints.len() >= to.len());
if to.len() > 1 { if to.len() > 1 {
if let Some(position) = to.iter().position(|(_, x)| x == &from_addr) { if let Some(position) = to.iter().position(|(_, x)| x == &from_addr) {
@@ -445,7 +446,7 @@ impl MimeFactory {
}; };
let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await; let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await;
debug_assert!( ensure_and_debug_assert!(
member_timestamps.is_empty() member_timestamps.is_empty()
|| to.len() + past_members.len() == member_timestamps.len() || to.len() + past_members.len() == member_timestamps.len()
); );
@@ -668,7 +669,7 @@ impl MimeFactory {
)); ));
} }
debug_assert!( ensure_and_debug_assert!(
self.member_timestamps.is_empty() self.member_timestamps.is_empty()
|| to.len() + past_members.len() == self.member_timestamps.len() || to.len() + past_members.len() == self.member_timestamps.len()
); );

View File

@@ -44,7 +44,7 @@ use crate::simplify;
use crate::stock_str; use crate::stock_str;
use crate::sync::Sync::*; use crate::sync::Sync::*;
use crate::tools::{self, buf_compress, remove_subject_prefix}; use crate::tools::{self, buf_compress, remove_subject_prefix};
use crate::{chatlist_events, location}; use crate::{chatlist_events, ensure_and_debug_assert, ensure_and_debug_assert_eq, location};
use crate::{contact, imap}; use crate::{contact, imap};
/// This is the struct that is returned after receiving one email (aka MIME message). /// This is the struct that is returned after receiving one email (aka MIME message).
@@ -1456,7 +1456,7 @@ async fn do_chat_assignment(
false => None, false => None,
}; };
if let Some(chat) = chat { if let Some(chat) = chat {
debug_assert!(chat.typ == Chattype::Single); ensure_and_debug_assert!(chat.typ == Chattype::Single);
let mut new_protection = match verified_encryption { let mut new_protection = match verified_encryption {
VerifiedEncryption::Verified => ProtectionStatus::Protected, VerifiedEncryption::Verified => ProtectionStatus::Protected,
VerifiedEncryption::NotVerified(_) => ProtectionStatus::Unprotected, VerifiedEncryption::NotVerified(_) => ProtectionStatus::Unprotected,
@@ -2141,7 +2141,7 @@ RETURNING id
// afterwards insert additional parts. // afterwards insert additional parts.
replace_msg_id = None; replace_msg_id = None;
debug_assert!(!row_id.is_special()); ensure_and_debug_assert!(!row_id.is_special());
created_db_entries.push(row_id); created_db_entries.push(row_id);
} }
@@ -2404,7 +2404,9 @@ async fn lookup_chat_by_reply(
// lookup by reply should never be needed // lookup by reply should never be needed
// as we can directly assign the message to the chat // as we can directly assign the message to the chat
// by its group ID. // by its group ID.
debug_assert!(mime_parser.get_chat_group_id().is_none() || !mime_parser.was_encrypted()); ensure_and_debug_assert!(
mime_parser.get_chat_group_id().is_none() || !mime_parser.was_encrypted()
);
// Try to assign message to the same chat as the parent message. // Try to assign message to the same chat as the parent message.
let Some(parent_chat_id) = ChatId::lookup_by_message(parent) else { let Some(parent_chat_id) = ChatId::lookup_by_message(parent) else {
@@ -3763,7 +3765,7 @@ async fn add_or_lookup_key_contacts_by_address_list(
} }
} }
debug_assert_eq!(contact_ids.len(), address_list.len()); ensure_and_debug_assert_eq!(contact_ids.len(), address_list.len(),);
Ok(contact_ids) Ok(contact_ids)
} }
@@ -3917,7 +3919,7 @@ async fn lookup_key_contacts_by_address_list(
contact_ids.push(contact_id); contact_ids.push(contact_id);
} }
} }
debug_assert_eq!(address_list.len(), contact_ids.len()); ensure_and_debug_assert_eq!(address_list.len(), contact_ids.len(),);
Ok(contact_ids) Ok(contact_ids)
} }

View File

@@ -763,5 +763,43 @@ pub(crate) fn inc_and_check<T: PrimInt + AddAssign + std::fmt::Debug>(
Ok(()) Ok(())
} }
/// Returns early with an error if a condition is not satisfied.
/// In non-optimized builds, panics instead if so.
#[macro_export]
macro_rules! ensure_and_debug_assert {
($($arg:tt)*) => {
debug_assert!($($arg)*);
anyhow::ensure!($($arg)*);
};
}
/// Returns early with an error on two expressions inequality.
/// In non-optimized builds, panics instead if so.
#[macro_export]
macro_rules! ensure_and_debug_assert_eq {
($left:expr, $right:expr, $($arg:tt)*) => {
match (&$left, &$right) {
(left_val, right_val) => {
debug_assert_eq!(left_val, right_val, $($arg)*);
anyhow::ensure!(left_val == right_val, $($arg)*);
}
}
};
}
/// Returns early with an error on two expressions equality.
/// In non-optimized builds, panics instead if so.
#[macro_export]
macro_rules! ensure_and_debug_assert_ne {
($left:expr, $right:expr, $($arg:tt)*) => {
match (&$left, &$right) {
(left_val, right_val) => {
debug_assert_ne!(left_val, right_val, $($arg)*);
anyhow::ensure!(left_val != right_val, $($arg)*);
}
}
};
}
#[cfg(test)] #[cfg(test)]
mod tools_tests; mod tools_tests;