Skip to content

Commit 69dfe3e

Browse files
committed
refactor: document allocation requirements and add error constant
- Add MALFORMED_ATTRIBUTES_ERROR constant to avoid string duplication - Document why tag.to_vec() allocation is necessary (borrow checker) - Document why attribute keys must be cloned to Vec<u8> - Note: allocations cannot be avoided without architectural refactoring Addresses Copilot review comments about: - Duplicated error message (lines 145, 169) - tag.to_vec() allocation documentation (lines 142, 508) - collect_attributes key cloning documentation (line 33)
1 parent dfc1c62 commit 69dfe3e

File tree

1 file changed

+15
-2
lines changed
  • crates/feedparser-rs-core/src/parser

1 file changed

+15
-2
lines changed

crates/feedparser-rs-core/src/parser/rss.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,16 @@ use super::common::{
1919
is_itunes_tag, is_media_tag, read_text, skip_element,
2020
};
2121

22+
/// Error message for malformed XML attributes (shared constant)
23+
const MALFORMED_ATTRIBUTES_ERROR: &str = "Malformed XML attributes";
24+
2225
/// Extract attributes as owned key-value pairs
2326
/// Returns (attributes, `has_errors`) tuple where `has_errors` indicates
2427
/// if any attribute parsing errors occurred (for bozo flag)
28+
///
29+
/// Note: Keys are cloned to `Vec<u8>` because `quick_xml::Attribute` owns the key
30+
/// data only for the lifetime of the event, but we need to store attributes across
31+
/// multiple parsing calls in `parse_enclosure` and other functions.
2532
#[inline]
2633
fn collect_attributes(e: &quick_xml::events::BytesStart) -> (Vec<(Vec<u8>, String)>, bool) {
2734
let mut has_errors = false;
@@ -138,11 +145,14 @@ fn parse_channel(
138145
*depth += 1;
139146
check_depth(*depth, limits.max_nesting_depth)?;
140147

148+
// NOTE: Allocation here is necessary due to borrow checker constraints.
149+
// We need owned tag data to pass &mut buf to helper functions simultaneously.
150+
// Potential future optimization: restructure helpers to avoid this allocation.
141151
let tag = e.name().as_ref().to_vec();
142152
let (attrs, has_attr_errors) = collect_attributes(&e);
143153
if has_attr_errors {
144154
feed.bozo = true;
145-
feed.bozo_exception = Some("Malformed XML attributes".to_string());
155+
feed.bozo_exception = Some(MALFORMED_ATTRIBUTES_ERROR.to_string());
146156
}
147157

148158
// Use full qualified name to distinguish standard RSS tags from namespaced tags
@@ -166,7 +176,7 @@ fn parse_channel(
166176
if has_attr_errors {
167177
feed.bozo = true;
168178
feed.bozo_exception =
169-
Some("Malformed XML attributes".to_string());
179+
Some(MALFORMED_ATTRIBUTES_ERROR.to_string());
170180
}
171181
feed.entries.push(entry);
172182
}
@@ -504,6 +514,9 @@ fn parse_item(
504514
*depth += 1;
505515
check_depth(*depth, limits.max_nesting_depth)?;
506516

517+
// NOTE: Allocation here is necessary due to borrow checker constraints.
518+
// We need owned tag data to pass &mut buf to helper functions simultaneously.
519+
// Potential future optimization: restructure helpers to avoid this allocation.
507520
let tag = e.name().as_ref().to_vec();
508521
let (attrs, attr_error) = collect_attributes(e);
509522
if attr_error {

0 commit comments

Comments
 (0)