Skip to content

fix: match canonically equivalent NFC and NFD text [patch] - #72

Merged
matt-edmondson merged 2 commits into
mainfrom
claude/nice-davinci-6fem2r
Sep 14, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
claude/nice-davinci-6fem2r

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #70

The problem

Fuzzy.Contains and CalculateScore compared subject and pattern per UTF-16 char with no Unicode normalization. Precomposed text (café, with U+00E9) and its canonically equivalent decomposed form (cafe + U+0301 COMBINING ACUTE ACCENT) are the same visible text but different code-unit sequences, so they never matched:

Fuzzy.Contains("café", "café");   // true
Fuzzy.Contains("café", "café");  // false — same visible text

NFD is the normal form for filenames on HFS+/APFS, so a consumer matching against such paths silently missed matches.

The fix

Both inputs are normalized to NormalizationForm.FormC before matching. Contains and CalculateScore became thin normalizing wrappers over ContainsCore/CalculateScoreCore; the matching loop itself is untouched.

Two details worth review:

  • The hot path stays allocation-free for ASCII. ASCII-only text is by definition already in FormC, so NormalizeForComparison detects it with a plain scan and returns the original span. Only text containing a non-ASCII character pays for ToString().Normalize(...). This addresses the allocation concern raised in the issue's triage comment.
  • Malformed Unicode no longer becomes an exception. string.Normalize throws ArgumentException on input that is not well-formed (a lone surrogate — see Matching operates on UTF-16 code units, so a lone surrogate can spuriously match inside an unrelated emoji #71). That is caught and the text is compared as given, so these methods keep their existing "never throws" contract.

Invariant globalization caveat

Normalization needs the runtime's globalization data. Under invariant globalization string.Normalize is a no-op, and the test project inherits InvariantGlobalization=true from ktsu.Sdk — which is why the new tests failed until the test project opted out. The test project now sets InvariantGlobalization=false, and the limitation is documented in the Fuzzy XML remarks.

Worth flagging separately: any consumer app that runs with invariant globalization (the ktsu.Sdk default) gets no normalization and will still see this behaviour. Whether that default should change is out of scope here.

Testing

Five tests added under a new Unicode Normalization Tests region: NFD subject vs NFC pattern, the reverse, equal scores across the two forms, a non-match that stays a non-match, and the lone-surrogate case.

Verified by reverting FuzzySearch/Fuzzy.cs to its pre-fix state and re-running with the test changes in place — exactly the three normalization tests failed (3 failed / 31 passed), and all 34 pass with the fix. Full suite green, Debug and Release both build with 0 warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01W78KwkEpp9G2ZNXfm6VVqL


Generated by Claude Code

Fuzzy.Contains and CalculateScore compared subject and pattern per UTF-16
char with no Unicode normalization, so precomposed text (e.g. "café" with
U+00E9) never matched its canonically equivalent decomposed form ("cafe" +
U+0301). NFD is the normal form for filenames on HFS+/APFS, so consumers
matching against such paths silently missed matches for visibly identical
text.

Both inputs are now normalized to FormC before matching. The common case
costs nothing: ASCII-only text is already in FormC, so it is detected by an
allocation-free scan and passed through unchanged. Only text containing a
non-ASCII character pays for the conversion. Text that is not well-formed
Unicode (a lone surrogate) cannot be normalized, so it is compared as given
rather than throwing out of a method that previously never threw.

Normalization needs the runtime's globalization data, which invariant mode
does not provide, so the test project opts out of invariant globalization
and the limitation is documented on the type.

Fixes #70

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W78KwkEpp9G2ZNXfm6VVqL
Introducing ContainsCore between the two public Contains overloads split
them apart, which SonarQube flags as S4136. Move ContainsCore below both.
No behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W78KwkEpp9G2ZNXfm6VVqL

Copy link
Copy Markdown
Contributor Author

On SonarQube's 2 new issues (gate passed, but for the record):

  • S4136 — "All 'Contains' method overloads should be adjacent." Genuinely introduced by this PR: ContainsCore landed between the two public Contains overloads. Fixed in cc4ae2c by moving it below both. No behaviour change.
  • S3776 — cognitive complexity 32 in the scoring loop. Pre-existing, not introduced here: it is already open on main at FuzzySearch/Fuzzy.cs:99, same rule and same score of 32. This PR only renamed that method to CalculateScoreCore, which moved it to line 142 and made Sonar re-attribute it to new code; the body is byte-identical. Reducing it means restructuring the scoring loop, which is a behaviour-risk refactor well outside a normalization fix, so I've left it alone rather than widening this PR.

Generated by Claude Code

@sonarqubecloud

Copy link
Copy Markdown

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.

Combining-character (NFD) text never matches its precomposed (NFC) equivalent

2 participants