Skip to content

Conversation

@bug-ops
Copy link
Owner

@bug-ops bug-ops commented Dec 15, 2025

Summary

This PR implements comprehensive DRY (Don't Repeat Yourself) refactoring across the feedparser-rs codebase, introducing reusable abstractions and builder-style helper methods.

Changes

DRY Refactoring (Phases 1-4)

  • Add builder-style helper methods for FeedMeta and Entry types:
    • set_title(), set_subtitle(), set_rights(), set_generator()
    • set_author(), set_publisher(), set_summary()
  • Add NamespacePrefix abstraction for efficient XML namespace tag detection
  • Add buffer factory functions (new_event_buffer, new_text_buffer) for consistent capacity allocation

Performance Optimizations

  • Use std::mem::take() instead of .clone() in helper methods for zero-cost string moves
  • All helpers marked #[inline] for compiler optimization

Security Improvements

  • Add comprehensive SAFETY documentation for unsafe block in prefix() method
  • Document all safety invariants (compile-time UTF-8 guarantee, immutable private field)

Testing Improvements

  • Add ignore attribute to doc tests for private modules
  • Add unit tests for HTTP insert_header helper
  • All 315 tests pass

CI/CD Improvements

  • Add SSRF protection tests
  • Add namespace integration tests
  • Fix various CI issues (Windows glob, permissions, coverage)

Test plan

  • cargo nextest run - 315 tests pass
  • cargo clippy --all-targets -- -D warnings - No warnings
  • cargo test --doc - 50 doc tests pass, 10 ignored (private modules)
  • Code review completed by rust-code-reviewer agent

Files changed

  • crates/feedparser-rs-core/src/types/feed.rs - FeedMeta helper methods
  • crates/feedparser-rs-core/src/types/entry.rs - Entry helper methods
  • crates/feedparser-rs-core/src/parser/namespace_detection.rs - NamespacePrefix abstraction
  • crates/feedparser-rs-core/src/parser/common.rs - Buffer factory functions

This commit implements the low-hanging fruit refactorings from the DRY
analysis report, focusing on eliminating code duplication with minimal risk.

Changes:

1. Unify bytes_to_string() function
   - Moved implementation to util/text.rs as single source of truth
   - Removed duplicate implementations from parser/common.rs and types/common.rs
   - Added comprehensive documentation with examples
   - Re-exported from parser/common for backward compatibility
   - Lines saved: ~8

2. Add Person::from_name() helper method
   - Added convenience constructor for creating Person from just a name
   - Updated all 4 occurrences in namespace/dublin_core.rs to use helper
   - Reduces boilerplate and ensures consistent Person construction
   - Lines saved: ~10

3. Use Tag::new() builder method consistently
   - Tag::new() already existed but wasn't used everywhere
   - Updated 4 occurrences across namespace modules to use builder
   - Replaced verbose struct literal construction with concise builder
   - Lines saved: ~12

4. Add HTTP header helper method
   - Added insert_header() helper to reduce boilerplate in FeedHttpClient
   - Centralizes error handling for header value conversion
   - Updated get() method to use helper for User-Agent, ETag, and Last-Modified
   - Consistent error messages across all header insertions
   - Lines saved: ~15

Total impact:
- Lines reduced: ~45
- Files modified: 6
- Test coverage: All 302 tests pass
- Clippy: Clean with -D warnings
- Risk level: Low (backward compatible changes only)

These changes improve code maintainability and reduce the risk of
inconsistent behavior when similar operations are performed in different
parts of the codebase.
Add 3 tests for the new insert_header() helper method:
- test_insert_header_valid: verifies basic functionality
- test_insert_header_invalid_value: verifies error handling for invalid headers
- test_insert_header_multiple_headers: verifies multiple header insertions
Phase 2: Feed/Entry builder helper methods
- Added 6 FeedMeta helpers: set_title, set_subtitle, set_rights, set_generator, set_author, set_publisher
- Added 4 Entry helpers: set_title, set_summary, set_author, set_publisher
- Updated atom.rs and json.rs to use helpers
- Net reduction: 11 lines in parsers

Phase 3: Infrastructure for namespace consolidation
- Created parser/namespace_detection.rs (216 lines)
- Implemented NamespacePrefix struct with const construction
- Added 5 namespace constants: DC, CONTENT, MEDIA, ITUNES, PODCAST
- Added 6 unit tests for namespace detection

Phase 4: Buffer factory functions
- Added new_event_buffer() factory in common.rs
- Added new_text_buffer() factory in common.rs
- Semantic naming for consistent buffer creation

Impact:
- 315/315 tests passing
- Zero clippy warnings
- 300+ lines of infrastructure added
- Foundation for future namespace consolidation
- All changes backward compatible
- Use std::mem::take() instead of clone() for zero-cost string moves
- Enhance unsafe block documentation with detailed safety invariants
- Add ignore attribute to doc tests for private modules
- Add explicit fn main() wrapper to set_generator doc test

All 315 tests pass, zero clippy warnings.
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 72.87234% with 51 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/feedparser-rs-core/src/types/feed.rs 56.41% 17 Missing ⚠️
crates/feedparser-rs-core/src/http/client.rs 83.67% 8 Missing ⚠️
crates/feedparser-rs-core/src/types/entry.rs 50.00% 8 Missing ⚠️
...edparser-rs-core/src/parser/namespace_detection.rs 84.78% 7 Missing ⚠️
crates/feedparser-rs-core/src/parser/common.rs 0.00% 6 Missing ⚠️
crates/feedparser-rs-core/src/parser/atom.rs 66.66% 3 Missing ⚠️
...es/feedparser-rs-core/src/namespace/dublin_core.rs 83.33% 1 Missing ⚠️
crates/feedparser-rs-core/src/parser/json.rs 66.66% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (72.87%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   81.07%   81.18%   +0.10%     
==========================================
  Files          25       27       +2     
  Lines        4053     4145      +92     
==========================================
+ Hits         3286     3365      +79     
- Misses        767      780      +13     
Flag Coverage Δ
rust-core 81.18% <72.87%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ates/feedparser-rs-core/src/namespace/media_rss.rs 94.11% <100.00%> (-0.51%) ⬇️
crates/feedparser-rs-core/src/parser/mod.rs 76.19% <ø> (ø)
crates/feedparser-rs-core/src/parser/rss.rs 69.04% <100.00%> (+0.53%) ⬆️
crates/feedparser-rs-core/src/types/common.rs 87.45% <100.00%> (+0.45%) ⬆️
crates/feedparser-rs-core/src/util/text.rs 100.00% <100.00%> (ø)
...es/feedparser-rs-core/src/namespace/dublin_core.rs 83.67% <83.33%> (+0.04%) ⬆️
crates/feedparser-rs-core/src/parser/json.rs 91.62% <66.66%> (+0.19%) ⬆️
crates/feedparser-rs-core/src/parser/atom.rs 65.70% <66.66%> (+0.66%) ⬆️
crates/feedparser-rs-core/src/parser/common.rs 59.17% <0.00%> (-3.11%) ⬇️
...edparser-rs-core/src/parser/namespace_detection.rs 84.78% <84.78%> (ø)
... and 3 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bug-ops bug-ops merged commit 3ce7789 into main Dec 15, 2025
29 checks passed
@bug-ops bug-ops deleted the refactor/dry-phase1 branch December 15, 2025 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants