Skip to content

Fix swapcase()#7788

Merged
youknowone merged 1 commit into
RustPython:mainfrom
joshuamegnauth54:swapcase-fixes
May 13, 2026
Merged

Fix swapcase()#7788
youknowone merged 1 commit into
RustPython:mainfrom
joshuamegnauth54:swapcase-fixes

Conversation

@joshuamegnauth54
Copy link
Copy Markdown
Contributor

@joshuamegnauth54 joshuamegnauth54 commented May 5, 2026 •
edited by coderabbitai Bot
Loading

The tests for swapcase() were failing for two reasons. The first is '𐐧' casing which should be fixed with modern Unicode tables. The second failure is due to CPython's sigma override, which I implemented in #7117.

This should work after rebasing, so I'm opening it as a placeholder for now. 😸 Ready since #7117 is merged.

Summary by CodeRabbit

  • Refactor
    • Optimized text case operations: faster ASCII fast paths and a new UTF‑8 swapcase routine for improved performance.
    • Byte-level case swapping now uses a streamlined ASCII path; invalid or non-UTF‑8 bytes are preserved during UTF‑8 processing.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 3fb1ce6e-d430-41e3-9be8-308086cd8a47

📥 Commits

Reviewing files that changed from the base of the PR and between e4ba68f and b7ad7c4.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_str.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/bytes_inner.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/bytes_inner.rs
📝 Walkthrough

Walkthrough

Adds a crate-local ASCII swapcase helper, makes bytes swapcase delegate to it, imports helpers into str.rs, and refactors PyStr::swapcase to dispatch by encoding kind with a new UTF‑8-aware swapcase routine.

Changes

Swapcase refactoring with per-kind helpers

Layer / File(s) Summary
Shared ASCII swapcase helper and bytes integration
crates/vm/src/bytes_inner.rs		 pub(crate) fn swapcase_ascii(bytes: &[u8]) -> Vec<u8> flips ASCII letters via XOR 0x20. PyBytesInner::swapcase now delegates to this helper.
str imports and title ASCII call
crates/vm/src/builtins/str.rs		 Import list updated to bring swapcase_ascii and title_ascii into scope. PyStr::title() ASCII branch now calls title_ascii.
PyStr swapcase dispatch and UTF-8 implementation
crates/vm/src/builtins/str.rs		 PyStr::swapcase() now dispatches on StrKind: ASCII -> swapcase_ascii, UTF-8 -> swapcase_utf8, WTF-8 -> apply swapcase_utf8 to valid chunks and preserve invalid/surrogate bytes. Adds swapcase_utf8 that iterates char_indices() and uses lowercase_or_sigma / to_uppercase with correct multi-char expansion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ShaharNaveh
  • youknowone

Poem

🐰 I hop through bytes and strings tonight,
XOR flips ASCII with nimble might,
UTF‑8 letters bloom and spread,
Surrogate bytes stay safe, unfed,
Swapcase done — a rabbit’s delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix swapcase()' directly refers to the main change: refactoring and fixing the swapcase() method implementation across multiple files to use optimized helpers and handle Unicode properly.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026 •
edited
Loading

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_str.py (TODO: 7)
[x] test: cpython/Lib/test/test_fstring.py (TODO: 19)
[x] test: cpython/Lib/test/test_string_literals.py (TODO: 4)

dependencies:

dependent tests: (no tests depend on str)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@joshuamegnauth54 joshuamegnauth54 marked this pull request as ready for review May 13, 2026 02:21
coderabbitai[bot]
coderabbitai Bot reviewed May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/vm/src/builtins/str.rs`:
- Around line 1664-1676: In swapcase_utf8, don’t treat every non-uppercase char
as lowercase; instead preserve titlecase and other uncased characters. Change
the branching so that if ch.is_uppercase() you call lowercase_or_sigma(ch, s, i,
out); else if ch.is_lowercase() you perform the to_uppercase() path and write
the uppercase bytes to out. For the remaining case (neither is_uppercase nor
is_lowercase, e.g. titlecase, digits, punctuation) copy the original char’s
UTF‑8 bytes into out unchanged. This touches swapcase_utf8 and reuses
lowercase_or_sigma for the uppercase->lowercase work.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 06ccac61-207a-4696-bbb2-271a962b34f6

📥 Commits

Reviewing files that changed from the base of the PR and between 5ef91c2 and e4ba68f.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_str.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/bytes_inner.rs

Comment thread crates/vm/src/builtins/str.rs
@joshuamegnauth54
Fix swapcase()
b7ad7c4 
The tests for swapcase() were failing for two reasons. The first is
'𐐧' casing which should be fixed with modern Unicode tables. The second
failure is due to CPython's sigma override, which I implemented in
PR RustPython#7717.
youknowone
youknowone approved these changes May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thanks!

@youknowone youknowone merged commit ef375be into RustPython:main May 13, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@joshuamegnauth54 @youknowone