feat(api): #[non_exhaustive] on public value enums + drop dead privacy enums - #736
Merged
Merged
Conversation
…y enums Continues the 1.0 forward-compat freeze started for error enums (#735), now on the public value enums, and removes a dead duplicate type cluster. - EventKind and ReceiptType get #[non_exhaustive] so adding a variant stays additive after 1.0 (these are server-driven sets that already grew). The sibling Event enum already carried it; this closes the asymmetry. Same-crate matches are unaffected. - EventKind also gains a documented CAPACITY = 64 and a build-time assert: it is used as a bit index into EventInterest's u64 mask, so a future variant that would overflow the mask now fails compilation instead of corrupting it at runtime. - Removes the dead PrivacySetting / PrivacySettingType / PrivacySettings types from wacore/src/types/user.rs. They have zero references anywhere (superseded by the WireEnum-based wacore::iq::privacy::{PrivacyCategory, PrivacyValue, PrivacySetting}), were a confusing second public PrivacySetting on the surface, and violated the repo's WireEnum convention. The orphaned serde import goes with them. Verified: clippy --all-targets -- -D warnings clean; cargo test --workspace --exclude e2e-tests green.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR hardens the public API contracts of core type enums by marking ChangesType System API Hardening
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes involve breaking API modifications—non-exhaustive markers on widely-used public enums require careful downstream impact assessment. The privacy type removals are straightforward but need verification that no internal or external code depends on those exports. The capacity constraint is a critical safety assertion that needs careful verification against Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Benchmark Results67 unchanged benchmark(s)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/types/events.rs`:
- Around line 262-264: Add a unit test that programmatically counts EventKind
variants and asserts the count is <= EventKind::CAPACITY to catch forgotten
updates automatically; implement the test (e.g., using a derived iterator like
strum::EnumVariantNames/EnumIter or by constructing a list of EventKind::...
variants) and assert!(count <= EventKind::CAPACITY as usize), referencing
EventKind and EventInterest/CAPACITY so the CI fails if the enum grows beyond
the u64 bitmask capacity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b06c8cff-d0bd-4d62-aea2-eb00bca177bb
📒 Files selected for processing (3)
wacore/src/types/events.rswacore/src/types/presence.rswacore/src/types/user.rs
💤 Files with no reviewable changes (1)
- wacore/src/types/user.rs
Comment on lines
+262
to
+264
| // Build-time tripwire: a new variant that would overflow EventInterest's bitmask | ||
| // fails compilation instead of silently corrupting the mask at runtime. | ||
| const _: () = assert!((EventKind::MexNotification as u8) < EventKind::CAPACITY); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider adding an automated variant count check.
Listen, this assertion is smart, but it relies on developers remembering to update it manually whenever they add a variant. If someone adds EventKind::NewVariant but forgets to change MexNotification to NewVariant in this assertion, the check won't catch overflow until we hit 64+ variants.
The comment tells people what to do, and that's good. But we can do better. Consider adding a unit test that programmatically counts EventKind variants and asserts the total is ≤ 64. That way we get automated enforcement without relying on human discipline.
Something like:
#[test]
fn event_kind_capacity_enforced() {
// If this test fails, we've exceeded EventInterest's u64 bitmask capacity.
// Solution: either remove variants or switch EventInterest to u128.
let count = /* derive variant count or maintain a manual list in test */;
assert!(count <= EventKind::CAPACITY as usize);
}This gives you defense in depth. The const assertion catches it if the last variant is updated correctly; the test catches it if someone forgets.
We need WhatsApp to scale. Manual processes don't scale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wacore/src/types/events.rs` around lines 262 - 264, Add a unit test that
programmatically counts EventKind variants and asserts the count is <=
EventKind::CAPACITY to catch forgotten updates automatically; implement the test
(e.g., using a derived iterator like strum::EnumVariantNames/EnumIter or by
constructing a list of EventKind::... variants) and assert!(count <=
EventKind::CAPACITY as usize), referencing EventKind and EventInterest/CAPACITY
so the CI fails if the enum grows beyond the u64 bitmask capacity.
|
Docs PR opened: oxidezap/whatsapp-rust-docs#271 Documented that EventKind and ReceiptType are now non_exhaustive and surfaced the new EventKind::CAPACITY constant in the events guide. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Continues the 1.0 forward-compat freeze started for error enums in #735, now on the public value enums, plus removes a dead duplicate type cluster.
Changes
EventKindandReceiptTypeget#[non_exhaustive]. Both are server-driven sets that already grew over time (ReceiptType addedEncRekeyRetry,ReadSelf,PlayedSelf,PeerMsg,HistorySync); without the attribute, a downstream exhaustivematchbreaks the moment a variant is added. The siblingEventenum already carried it, so this closes the asymmetry. Same-crate matches are unaffected.EventKindgains a documentedCAPACITY = 64and a build-timeassert!. It is used as a bit index intoEventInterest'su64mask, so a future variant that would overflow the mask now fails compilation instead of silently corrupting it at runtime.PrivacySetting/PrivacySettingType/PrivacySettingstypes fromwacore/src/types/user.rs. They have zero references anywhere in the workspace, superseded by the WireEnum-basedwacore::iq::privacy::{PrivacyCategory, PrivacyValue, PrivacySetting}(the live privacy feature). They were also a confusing second publicPrivacySettingon the surface and violated the repo's WireEnum convention. The now-orphanedserdeimport goes with them.Verification
cargo clippy --all-targets -- -D warningscleancargo test --workspace --exclude e2e-testsgreenResolves gap-analysis api-07, api-16, api-17. Breaking change, acceptable pre-1.0.