fix(common): errors with clippy pedantic (manual_assert)#3204
fix(common): errors with clippy pedantic (manual_assert)#3204chammika wants to merge 3 commits intoapache:masterfrom
Conversation
cargo clippy -- -W clippy::manual_assert
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3204 +/- ##
============================================
- Coverage 74.09% 71.51% -2.59%
Complexity 943 943
============================================
Files 1164 1164
Lines 103048 98355 -4693
Branches 80083 75405 -4678
============================================
- Hits 76353 70337 -6016
- Misses 24006 25137 +1131
- Partials 2689 2881 +192
🚀 New features to boost your workflow:
|
chammika
changed the title
| if bytes.is_empty() { | ||
| panic!("Can not parse empty string as u32"); | ||
| } | ||
| assert!(!bytes.is_empty(), "End index out of bounds"); |
There was a problem hiding this comment.
the message "End index out of bounds" doesn't describe the !bytes.is_empty() check - it describes a different condition (matching the assertion at line 96 about end <= bytes.len()). this is a diagnostic regression: original message was "Can not parse empty string as u32", which actually matched the check. restore the original message.
| assert!( | ||
| !((username.is_ok() && password.is_err()) || (username.is_err() && password.is_ok())), | ||
| "When providing the custom root user credentials, both username and password must be set." | ||
| ); |
There was a problem hiding this comment.
double-negated XOR is hard to parse. equivalent and clearer:
assert_eq!(
username.is_ok(),
password.is_ok(),
"When providing the custom root user credentials, both username and password must be set."
);| assert!( | ||
| jwt_manager.load_revoked_tokens().await.is_ok(), | ||
| "Failed to load revoked access tokens" | ||
| ); |
There was a problem hiding this comment.
assert!(... .is_ok(), "...") drops the underlying error from load_revoked_tokens(). the original if .is_err() { panic!(...) } had the same issue, so this isn't a regression from this PR, but worth fixing while you're in here:
if let Err(error) = jwt_manager.load_revoked_tokens().await {
panic!("Failed to load revoked access tokens: {error}");
}
There was a problem hiding this comment.
Replacing panic with assert in many places is not appropriate. It would be better to return proper error types with clear error messages, rather than using assert to validate configuration. cc @hubcio WDYT?
3 participants
Which issue does this PR close?
This PR fixes some issues raised by clippy pedantic mode
Closes #3205
Rationale
This was suggested by Clippy as better practice
https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#manual_assert
What changed?
Fix all
clippy::manual_assertcases indicated by clippyLocal Execution
AI Usage
None