Skip to content

Refactor text stdlib to use generic helper functions - #346

Merged
logbie merged 1 commit into
mainfrom
refactor/text-stdlib-helpers-6276029503791946529
Feb 13, 2026
Merged

Refactor text stdlib to use generic helper functions#346
logbie merged 1 commit into
mainfrom
refactor/text-stdlib-helpers-6276029503791946529

Conversation

@logbie

@logbie logbie commented Feb 13, 2026

Copy link
Copy Markdown
Collaborator

Summary of Changes

  • The Issue: Found significant code duplication in src/stdlib/text.rs where multiple native functions (native_touppercase, native_tolowercase, native_trim, native_starts_with, native_ends_with, native_padleft, native_padright, native_capitalize, native_reverse_text) implemented identical argument validation and type checking logic.
  • The Rational: This redundancy increases the maintenance burden and the risk of inconsistent error handling or behavior across similar functions. Refactoring this into shared helpers adheres to the DRY principle and improves code readability.
  • The Solution: Implemented three generic helper functions in src/stdlib/text.rs:
    • unary_text_op: Handles single-argument text-to-text transformations.
    • binary_text_predicate: Handles two-argument text-to-bool predicates.
    • pad_helper: Unified the logic for padleft and padright which shared identical validation and padding calculation logic.
      Refactored the corresponding native functions to use these helpers.

Verification Checklist

  • cargo fmt executed and passed.
  • cargo clippy returned no warnings or errors.
  • All cargo test suites passed (100% success rate).

PR created automatically by Jules for task 6276029503791946529 started by @logbie

Summary by CodeRabbit

  • Refactor
    • Internal restructuring of text utility implementations, including case conversion, string trimming, padding, and comparison operations, to improve code organization and maintainability. All public text functions maintain unchanged signatures and behavior with full backward compatibility.

- Introduced `unary_text_op` for single-argument text transformations.
- Introduced `binary_text_predicate` for two-argument text predicates.
- Introduced `pad_helper` for text padding operations.
- Refactored `native_touppercase`, `native_tolowercase`, `native_trim`, `native_starts_with`, `native_ends_with`, `native_padleft`, `native_padright`, `native_capitalize`, and `native_reverse_text` to use these helpers.
- Reduced code duplication and improved maintainability.

Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 13, 2026 10:06
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@claude

claude Bot commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @logbie's task —— View job


Code Review Complete ✅

Excellent refactoring! This PR successfully reduces code duplication while maintaining functionality. Here's my detailed review:

Overall Assessment

Strong positive recommendation. This refactoring follows solid software engineering principles, reduces technical debt, and maintains backward compatibility.


Code Quality & Best Practices

Grade: A

The three helper functions are well-designed:

  1. unary_text_op (lines 76-83): Clean generic wrapper for single-argument text transformations
  2. binary_text_predicate (lines 85-93): Elegant solution for two-argument text-to-bool operations
  3. pad_helper (lines 249-266): Smart consolidation of shared padding logic

Strengths:

  • DRY principle: Eliminates duplicate argument validation across 9 functions
  • Type safety: Preserves existing error handling patterns
  • Consistent naming: Functions clearly indicate their purpose and return types
  • Generic design: Uses closure parameters effectively for reusable logic
  • Performance: Zero-copy approach with Arc<str> where possible (line 256)

Technical Implementation

Grade: A-

✅ Excellent:

  • Function signatures are well-designed with appropriate generic constraints
  • Error handling is consistent and preserved from original implementations
  • The pad_helper correctly handles Unicode character counting (line 254)
  • Input validation (especially validated_pad_width at lines 238-247) is robust

⚠️ Minor Observations:

  • Consider adding documentation comments for the helper functions since they're now core utilities
  • The pad_helper function could benefit from a brief comment explaining the is_left parameter

Security & Safety

Grade: A

✅ No concerns identified:

  • Input validation is preserved and actually strengthened through centralization
  • No new attack vectors introduced
  • Memory safety maintained through existing patterns
  • The padding width limits (MAX_PAD_WIDTH = 1024) prevent potential DoS attacks

Performance Considerations

Grade: A

✅ Performance improvements:

  • Reduced code size: Net -10 lines reduces binary size slightly
  • Better optimization: Compiler can better optimize the consolidated logic
  • Memory efficiency: Preserved Arc cloning avoidance in padding (line 256)
  • No regressions: All existing optimizations maintained (e.g., substring early bounds check)

Test Coverage

Grade: B+

✅ Existing tests:

  • Comprehensive test suite covers all refactored functions (lines 386-536)
  • Edge cases well covered (Unicode, empty strings, error conditions)
  • Helper functions are implicitly tested through their callers

📝 Suggestion:
While not required, consider adding direct unit tests for the helper functions themselves to make future maintenance easier.


Backward Compatibility

Grade: A

✅ Perfect compatibility:

  • All public APIs unchanged
  • Function behavior identical
  • Error messages and types preserved
  • Registration logic unchanged (lines 293-384)

Final Verdict

This is a textbook example of good refactoring:

  • ✅ Reduces duplication without changing behavior
  • ✅ Improves maintainability and readability
  • ✅ No performance regressions
  • ✅ No security concerns
  • ✅ Comprehensive test coverage already exists

Recommendation: Approve and merge 🚀

The code is production-ready and represents a meaningful improvement to the codebase's maintainability.


@coderabbitai

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR refactors text utility functions in src/stdlib/text.rs by extracting three private helper functions (unary_text_op, binary_text_predicate, pad_helper) to consolidate repetitive argument validation and string transformation logic. Existing text functions are rewritten to delegate to these helpers while maintaining their public signatures and behavior.

Changes

Cohort / File(s) Summary
Text Utilities Refactoring
src/stdlib/text.rs
Introduces three private helper functions for unified handling of unary text operations, binary text predicates, and padding logic. Refactors eight existing functions (touppercase, tolowercase, trim, starts_with, ends_with, capitalize, reverse_text, padleft/padright) to use these helpers, eliminating duplicate argument validation and string manipulation code.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • Expands stdlib with list, math, and text functions #345: Directly refactors the same text functions in src/stdlib/text.rs (capitalize, reverse_text, padleft/padright, starts_with/ends_with, trim, touppercase/tolowercase) using the same generic helper approach, modifying identical code regions.

Poem

🐰 Helpers hop and consolidate,
Duplicate code meets its fate,
Unary, binary, padding neat,
Text utilities now complete! 🌿✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring text stdlib functions to use generic helper functions, which is the core objective of the PR.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/text-stdlib-helpers-6276029503791946529

No actionable comments were generated in the recent review. 🎉


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the text stdlib module to eliminate code duplication by introducing three generic helper functions. The refactoring consolidates repetitive argument validation and type checking logic that was previously duplicated across nine native text manipulation functions.

Changes:

  • Introduced unary_text_op helper for single-argument text-to-text transformations
  • Introduced binary_text_predicate helper for two-argument text-to-boolean predicates
  • Introduced pad_helper to unify padding logic for left and right padding operations
  • Refactored nine existing native functions to use these helpers while preserving behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants