[ISSUE #1477]🚀derive RequestHeaderCodec support Struct🔥#1487
[ISSUE #1477]🚀derive RequestHeaderCodec support Struct🔥#1487rocketmq-rust-bot merged 1 commit intomainfrom
Conversation
WalkthroughThe pull request introduces several new functions and modifies existing ones in the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
rocketmq-macros/src/request_header_custom.rs (1)
100-107: Refactor duplicated code when handling flattened structsThe logic for handling fields that are structs with the
serde(flatten)attribute is duplicated in multiple places within theto_mapmethod. Consider extracting this repeated code into a helper function or macro to improve maintainability and reduce code redundancy.Also applies to: 132-138
rocketmq-macros/src/lib.rs (1)
81-114: Add documentation tois_struct_typefunction for clarityConsider adding a doc comment to the
is_struct_typefunction to explain its purpose and logic. This will improve code readability and assist future contributors in understanding the functionality.
| fn has_serde_flatten_attribute(field: &Field) -> bool { | ||
| for attr in &field.attrs { | ||
| if let Some(ident) = attr.path().get_ident() { | ||
| let mut has_serde_flatten_attribute = false; | ||
| if ident == "serde" { | ||
| let _ = attr.parse_nested_meta(|meta| { | ||
| has_serde_flatten_attribute = meta | ||
| .path | ||
| .segments | ||
| .iter() | ||
| .any(|segment| segment.ident == "flatten"); | ||
| Ok(()) | ||
| }); | ||
| } | ||
| return has_serde_flatten_attribute; | ||
| } | ||
| } | ||
| false |
There was a problem hiding this comment.
Fix premature return in has_serde_flatten_attribute function
The has_serde_flatten_attribute function currently returns after checking the first attribute. This may cause the function to miss the #[serde(flatten)] attribute if it's not the first in the list. To ensure all attributes are checked, modify the function to continue iterating through all attributes.
Apply this diff to fix the issue:
fn has_serde_flatten_attribute(field: &Field) -> bool {
for attr in &field.attrs {
if let Some(ident) = attr.path().get_ident() {
let mut has_serde_flatten_attribute = false;
if ident == "serde" {
let _ = attr.parse_nested_meta(|meta| {
has_serde_flatten_attribute = meta
.path
.segments
.iter()
.any(|segment| segment.ident == "flatten");
Ok(())
});
- return has_serde_flatten_attribute;
+ if has_serde_flatten_attribute {
+ return true;
+ }
}
}
}
false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn has_serde_flatten_attribute(field: &Field) -> bool { | |
| for attr in &field.attrs { | |
| if let Some(ident) = attr.path().get_ident() { | |
| let mut has_serde_flatten_attribute = false; | |
| if ident == "serde" { | |
| let _ = attr.parse_nested_meta(|meta| { | |
| has_serde_flatten_attribute = meta | |
| .path | |
| .segments | |
| .iter() | |
| .any(|segment| segment.ident == "flatten"); | |
| Ok(()) | |
| }); | |
| } | |
| return has_serde_flatten_attribute; | |
| } | |
| } | |
| false | |
| fn has_serde_flatten_attribute(field: &Field) -> bool { | |
| for attr in &field.attrs { | |
| if let Some(ident) = attr.path().get_ident() { | |
| let mut has_serde_flatten_attribute = false; | |
| if ident == "serde" { | |
| let _ = attr.parse_nested_meta(|meta| { | |
| has_serde_flatten_attribute = meta | |
| .path | |
| .segments | |
| .iter() | |
| .any(|segment| segment.ident == "flatten"); | |
| Ok(()) | |
| }); | |
| if has_serde_flatten_attribute { | |
| return true; | |
| } | |
| } | |
| } | |
| } | |
| false | |
| } |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1487 +/- ##
==========================================
+ Coverage 22.53% 22.55% +0.02%
==========================================
Files 450 450
Lines 58412 58500 +88
==========================================
+ Hits 13162 13196 +34
- Misses 45250 45304 +54 ☔ View full report in Codecov by Sentry. |
Which Issue(s) This PR Fixes(Closes)
Fixes #1477
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
New Features
snake_to_camel_case,is_struct_type,is_basic_or_string_type, andhas_serde_flatten_attribute.serde(flatten)attribute.Bug Fixes