fix: Python-Rust combining char diff in isalnum#7612

joshuamegnauth54

Closes: #7518

Rust and Python differ on alphanumeric characters. Rust follows the Unicode standard closer than Python. This means that is_alphanumeric (char function in Rust) is different from isalnum (Python). To fix the discrepancy, RustPython needs to mimic Python by rejecting certain characters. Some classes of combining characters count as alphanumeric in Rust but not Python. Combining characters are accent marks that are combined with other characters to create a single grapheme.

It's possible that this PR is not exhaustive. I fixed the combining character issue BUT I don't know the full range of discrepancies.

~~This doesn't actually fix #7518, but it fixes a similar issue based on that report.~~ Actually, CodeRabbit pointed me in the right direction so now it does fix #7518.

assert!('\u{006e}'.is_alphanumeric());
assert!(!'\u{0303}'.is_alphanumeric());
assert!('\u{00f1}'.is_alphanumeric());
assert!('\u{0345}'.is_alphanumeric());

for raw in 0x0363..=0x036f {
    let c = char::from_u32(raw).unwrap();
    assert!(c.is_alphanumeric());
}

assert '\u006e'.isalnum()
assert not '\u0303'.isalnum()
assert '\u00f1'.isalnum()
assert not '\u0345'.isalnum()
assert not '\u0363'.isalnum()

for raw in range(0x0363, 0x036f):
    assert not chr(raw).isalnum()

^Note the differences which are accounted for by the new tests.

Summary by CodeRabbit

Bug Fixes
- Corrected str.isalnum() so standalone Unicode combining marks are not treated as alphanumeric, while base letters remain valid.
Tests
- Expanded tests for str.isalnum() covering combining-character cases.
- Added regex test ensuring standalone combining marks are not matched as word characters.

coderabbitai

📝 Walkthrough

Walkthrough

Updated alphanumeric checks to consult ICU canonical combining class data so combining marks are excluded from isalnum(); corresponding tests and a dependency on icu_properties were added.

Changes

Cohort / File(s)	Summary
VM builtin (isalnum) `crates/vm/src/builtins/str.rs`	Rewrote `PyStr::isalnum()` to require both `char::is_alphanumeric()` and that the code point's `CanonicalCombiningClass` is `NotReordered` via ICU lookup.
sre_engine: string predicate `crates/sre_engine/src/string.rs`	`is_uni_alnum` now consults `CodePointMapData<CanonicalCombiningClass>` and excludes combining marks from alphanumeric classification.
sre_engine: dependency `crates/sre_engine/Cargo.toml`	Added `icu_properties` workspace dependency.
Tests: builtin & stdlib `extra_tests/snippets/builtin_str.py`, `extra_tests/snippets/stdlib_re.py`	Added assertions ensuring combining marks (e.g., U+0345 and ranges U+0363..U+036E) are not treated as alphanumeric and that `\w` does not match such marks.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Python test / Caller
    participant VM as VM (`PyStr::isalnum`)
    participant SRE as sre_engine (`is_uni_alnum`)
    participant ICU as ICU properties (CodePointMapData<CanonicalCombiningClass>)

    Test->>VM: call isalnum() on string
    VM->>SRE: check code point alnum predicate
    SRE->>ICU: lookup CanonicalCombiningClass.for_char(code_point)
    ICU-->>SRE: return CanonicalCombiningClass
    SRE-->>VM: return (is_alnum && CCC == NotReordered)
    VM-->>Test: final boolean result

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Replace unmaintained unic crates #7555 — Similar migration to icu_properties and ICU-based Unicode property lookups affecting VM builtins.

Suggested reviewers

ShaharNaveh
youknowone

Poem

🐰 I hopped through code with whiskers bright,
Marked the marks that hid from sight.
ICU maps helped set things right,
Alnum now skips the trailing bite—
A tiny patch, a rabbit's delight.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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: fixing the discrepancy between Python and Rust implementations regarding combining character handling in isalnum().
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue `#7518` by excluding combining marks (Mn category) from isalnum() via CanonicalCombiningClass checks in both Python and regex implementations.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing combining character handling in isalnum() as specified in the linked issue; no unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@extra_tests/snippets/stdlib_re.py`:
- Around line 82-84: The commented-out test shows that is_uni_word() currently
inherits is_uni_alnum() which uses Rust's char::is_alphanumeric() and therefore
wrongly treats combining marks (e.g. U+0345, category Mn) as word characters;
update is_uni_alnum() (and thus is_uni_word()) to exclude characters whose
Unicode General Category is a Mark (Mn, Mc, Me) rather than relying solely on
char::is_alphanumeric(), for example by querying the character's general
category and returning false for Mark categories so the test re.match(r"\w",
"\u0345") will not match; alternatively, if you intend to keep this limitation,
uncomment the test and mark it as an expected failure with a comment referencing
issue `#7518` and the functions is_uni_alnum() / is_uni_word().

🪄 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: 04cb8ffa-9709-4846-a2ad-187862235904

📥 Commits

Reviewing files that changed from the base of the PR and between 898da7f and cd8b11d.

📒 Files selected for processing (3)

crates/vm/src/builtins/str.rs
extra_tests/snippets/builtin_str.py
extra_tests/snippets/stdlib_re.py

Related to: RustPython#7518 Rust and Python differ on alphanumeric characters. Rust follows the Unicode standard closer than Python. This means that is_alphanumeric (char function in Rust) is different from isalnum (Python). To fix the discrepancy, RustPython needs to mimic Python by rejecting certain characters. Some classes of combining characters count as alphanumeric in Rust but not Python. Combining characters are accent marks that are combined with other characters to create a single grapheme. It's possible that this PR is not exhaustive. I fixed the combining character issue BUT I don't know the full range of discrepancies.

Closes: RustPython#7518

joshuamegnauth54

I fixed the issues and force pushed. 😁

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/builtins/str.rs`:
- Around line 949-954: Replace the current isalnum implementation (which uses
char::is_alphanumeric plus CanonicalCombiningClass check) with a
CPython-compatible predicate that accepts only Unicode letter categories
(Lu/Ll/Lt/Lm/Lo) or number categories (Nd/Nl/Np) — i.e., check the character's
general category starts with 'L' or 'N' — so spacing/combing marks like U+093F
(Mc) are rejected; update the isalnum method (and the char_all predicate it
uses) to use this category test, update the mirror predicate used for \w in the
sre_engine string predicate to the same logic so they stay aligned, and add a
regression test asserting str.isalnum() returns false for U+093F.

🪄 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: 509798df-be24-4b49-ba31-08eb4f8b25c1

📥 Commits

Reviewing files that changed from the base of the PR and between 64dd760 and f79d6df.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

crates/sre_engine/Cargo.toml
crates/sre_engine/src/string.rs
crates/vm/src/builtins/str.rs
extra_tests/snippets/builtin_str.py
extra_tests/snippets/stdlib_re.py

✅ Files skipped from review due to trivial changes (3)

crates/sre_engine/Cargo.toml
extra_tests/snippets/stdlib_re.py
extra_tests/snippets/builtin_str.py

🚧 Files skipped from review as they are similar to previous changes (1)

crates/sre_engine/src/string.rs

ShaharNaveh

Looks great!
tysm:)

joshuamegnauth54

CodeRabbit's comment is interesting. 🤔 I assumed there were more edge cases. I didn't look at isalpha() and the rest yet, but there are issues open for them too. I'll try to fix those in a future patch in a more robust manner.

youknowone

Rust and Python differ on alphanumeric characters. Rust follows the Unicode standard closer than Python.

If it does, isn't it better to patch CPython to follow unicode standard better?

joshuamegnauth54

Sorry, I'm not sure if I'm supposed to respond here to you two after PRs have been merged. 😅

I don't fully understand the issue, but I think that the differences are due to Unicode version and how each project chooses to define "alphanumeric." I'm trying to figure out if I can just combine Unicode properties to mimic cpython.

coderabbitai Bot reviewed Apr 15, 2026

View reviewed changes

ShaharNaveh reviewed Apr 15, 2026

View reviewed changes

joshuamegnauth54 added 2 commits April 15, 2026 21:55

fix: Ignore combining characters in SRE …

f79d6df

Closes: RustPython#7518

joshuamegnauth54 force-pushed the isalnum-combining-character branch from 64dd760 to f79d6df Compare April 16, 2026 03:23

coderabbitai Bot reviewed Apr 16, 2026

View reviewed changes

ShaharNaveh approved these changes Apr 16, 2026

View reviewed changes

ShaharNaveh reviewed Apr 16, 2026

View reviewed changes

youknowone approved these changes Apr 17, 2026

View reviewed changes

joshuamegnauth54 deleted the isalnum-combining-character branch April 17, 2026 17:22

coderabbitai Bot mentioned this pull request Apr 18, 2026

Use Unicode properties for alnum, alpha, etc. #7626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Python-Rust combining char diff in isalnum#7612