Skip to content

Commit 3304d76

Browse files
committed
feat(core): reject an undefined NormalizationForm, and declare the no-op as .NET's invariant mode (#2386)
Split out of #2338 after measurement corrected that ticket's premise twice. THE FORM CHECK LANDS BECAUSE IT DEPENDS ON NO TABLE. CheckNormalizationForm (Normalization.cs:88-97) runs BEFORE the invariant shortcut, so .NET rejects an undefined form on every platform and in every mode. All four overloads now do too: ArgumentException with .NET's verbatim "Invalid or unsupported normalization form." (Strings.resx:1324-1326) and paramName "normalizationForm" -- nameof, not this port's own parameter spelling "form", because a caller reads ParamName. The four values are ENUMERATED, not range-checked, and that is load-bearing: 3 and 4 are HOLES (FormC=1, FormD=2, FormKC=5, FormKD=6), so `raw >= 1 && raw <= 6` accepts two undefined values. That mutation is caught. .NET's second clause -- PlatformNotSupportedException for FormKC/FormKD on Browser/WASI -- is deliberately NOT reproduced: it is conditioned on !GlobalizationMode.Invariant and so is unreachable in .NET under this port's own conditions. THE LARGER HALF IS A PREMISE CORRECTION. #2337 pinned three behaviours as "statements about the CURRENT stub", each of which "must be inverted by the repair (#2338)". Measured, .NET's own body is: // In Invariant mode we assume all characters are normalized because we don't // support any linguistic operations on strings. if (GlobalizationMode.Invariant || Ascii.IsValid(source)) { return true; } Normalization.cs:11-20, 27-40 So returning true and returning the argument unchanged is exactly what .NET does for a build with no globalization backend, which is what this runtime is. Those three pins are a DECLARED DEVIATION with the reference behind it, not a stub awaiting repair, and the class doc-comment now says so with the citation instead of describing itself as a stub. WHY #2338 IS needs_user RATHER THAN UNBLOCKED BY SA-4. Its recorded gate is Approval F, and SA-4 is Approval F -- but SA-4's source of record is CharUnicodeInfoData.cs, and that file contains ZERO normalization data (grep decompos|combiningclass|composition|quickcheck|nfc|nfd|nfkc|nfkd: 0 hits). .NET has no normalization tables at all. Using Perl unicore or Python unicodedata instead would promote a cross-check corpus to source of record at Unicode 15.0.0/15.1.0 rather than the 16.0 SA-4 pins. Three options are recorded on #2338: declare invariant-mode parity (recommended), grant a new data source plus UAX #15, or take an ICU dependency. Mutations: 5, all caught. The identity mutation is caught by FOUR cases, three of them PRE-EXISTING #2337 pins, which is what shows the identity behaviour is load-bearing. Gate: 17,376 run, 17,376 passed, 0 failed, 0 skipped across 38 executables (+1, in SharpRuntimeTests_Core_Base, 6,063 -> 6,064). Built in build/ with --parallel 2. Downstream: zero IsNormalized/Normalize call sites in cna and mobile-eggbert. docs/Migration-NormalizationFormValidation.md
1 parent 538174f commit 3304d76

5 files changed

Lines changed: 267 additions & 22 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — an undefined `NormalizationForm` is rejected, and the no-op is declared (ticket #2386)
5+
6+
*2026-08-19.* `System::StringNormalizationExtensions`'s four overloads now validate their
7+
`NormalizationForm`, and the class documentation states what the identity behaviour actually is.
8+
No signature, layout or vtable change.
9+
10+
This ticket exists because measurement corrected **#2338**'s premise twice. §2 is the more
11+
important half.
12+
13+
---
14+
15+
## 1. What changed
16+
17+
| Call | Was | Is |
18+
|---|---|---|
19+
| `Normalize(s, (NormalizationForm)0x1234)` | returns `s` | `ArgumentException` |
20+
| `IsNormalized(s, (NormalizationForm)3)` | `true` | `ArgumentException` |
21+
| `IsNormalized(s, FormC \| FormD \| FormKC \| FormKD)` | `true` | **unchanged** |
22+
| `Normalize(s, <any defined form>)` | `s` | **unchanged** |
23+
24+
The exception is `System::ArgumentException` with .NET's verbatim message — *"Invalid or
25+
unsupported normalization form."* (`Strings.resx:1324-1326`) — and `paramName`
26+
**`normalizationForm`**, which is `nameof(normalizationForm)` in `Normalization.cs:95`, **not** this
27+
port's own parameter spelling `form`. A caller catching `ArgumentException` reads `ParamName`, and
28+
.NET's answer is the one worth matching.
29+
30+
**The four values are enumerated, not range-checked**, because `3` and `4` are **holes**:
31+
`FormC = 1`, `FormD = 2`, `FormKC = 5`, `FormKD = 6`. A `raw >= 1 && raw <= 6` check accepts two
32+
undefined values, and that mutation is caught.
33+
34+
.NET's second clause — `PlatformNotSupportedException` for `FormKC`/`FormKD` on Browser and WASI,
35+
where ICU ships without compatibility data — is deliberately **not** reproduced. It is conditioned
36+
on `!GlobalizationMode.Invariant`, so it is unreachable in .NET under this port's own conditions
37+
(§2), and adding it would invent a failure the reference does not have here.
38+
39+
## 2. The premise correction: this was never a stub
40+
41+
#2337 pinned four behaviours as *"statements about the CURRENT stub"*, each of which *"must be
42+
inverted by the repair (#2338)"*. Measured against `/rv/tmp/runtime`, that is wrong for three of
43+
the four. .NET's own body is:
44+
45+
```csharp
46+
// In Invariant mode we assume all characters are normalized because we don't
47+
// support any linguistic operations on strings.
48+
if (GlobalizationMode.Invariant || Ascii.IsValid(source)) { return true; }
49+
// Normalization.cs:11-20, 27-40
50+
```
51+
52+
Returning `true` and returning the argument unchanged is **exactly what .NET does** for a build
53+
with no globalization backend — which is what this runtime is. So those three pins are a **declared
54+
deviation with the reference behind it**, not a stub awaiting repair, and the class doc-comment now
55+
says so with the citation rather than describing itself as a stub *"correct for ASCII-only
56+
strings"*.
57+
58+
The **fourth** pin is genuinely inverted, and its half never depended on any of that:
59+
`CheckNormalizationForm` runs *before* the invariant shortcut (`Normalization.cs:13,29`), so .NET
60+
rejects an undefined form on every platform and in every mode.
61+
62+
**What a caller should take from `IsNormalized` returning `true`:** this runtime performs no
63+
linguistic normalization, exactly as for a .NET application built with
64+
`InvariantGlobalization=true`. It is not a claim that the string is in the requested form.
65+
66+
## 3. Why the rest is not implementable under SA-4
67+
68+
SA-4 grants deriving Unicode tables *"from .NET's own generated data in `/rv`"*, naming
69+
`CharUnicodeInfoData.cs`. That file contains **zero** normalization data — measured, a
70+
case-insensitive grep for `decompos|combiningclass|composition|quickcheck|nfc|nfd|nfkc|nfkd`
71+
returns 0 hits. .NET has no normalization tables at all; `Normalization.cs` dispatches to ICU on
72+
Unix and NLS on Windows.
73+
74+
So SA-4 unblocked #2315, #2336 and #2018 and **does not reach #2338**: there is nothing in the
75+
source of record to derive from. Using Perl's `unicore` or Python's `unicodedata` instead would
76+
promote a *cross-check corpus* to source of record, at Unicode 15.0.0 or 15.1.0 rather than the
77+
16.0 SA-4 pins, leaving the port's Unicode data inconsistent across two versions.
78+
79+
**#2338 is therefore `needs_user`**, with three options recorded on the ticket: declare
80+
invariant-mode parity as the end state (recommended), grant a new Unicode data source plus a UAX
81+
#15 implementation, or take an ICU dependency — the last being a reversal of the standing
82+
cryptography-style decision against large external dependencies rather than a new one.
83+
84+
## 4. Evidence
85+
86+
Five mutations, **all caught**:
87+
88+
| Mutation | Caught by |
89+
|---|---|
90+
| the four-way test becomes a bounds check (accepts holes 3 and 4) | the enum-holes rows |
91+
| validation dropped from `IsNormalized` | the inverted pin |
92+
| validation dropped from `Normalize` | the inverted pin |
93+
| `paramName` becomes `"form"` | the message/paramName rows |
94+
| `Normalize` stops being the identity for non-ASCII | four cases, three of them **pre-existing** |
95+
96+
The last is worth noting: three of the four tests that catch it are #2337's own content pins, which
97+
is what shows the identity behaviour is load-bearing rather than incidental. `Normalize` is the
98+
identity **byte for byte**, including for input that is not valid UTF-8, because .NET's invariant
99+
path is `return strInput;` with no inspection — a decoder here would invent a failure mode the
100+
reference does not have.
101+
102+
## 5. Downstream, measured
103+
104+
`cna` and `mobile-eggbert` call `IsNormalized`/`Normalize` in **zero** places (every `Normalize`
105+
match in either repository is `Vector`/`Plane`/`Matrix`/`Quaternion` geometry). The only
106+
in-repository uses are this type's own tests. Neither repository was modified.

modules/core/include/System/StringNormalizationExtensions.hpp

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
44
#pragma once
55
#include <string>
6+
#include "System/ArgumentException.hpp"
67
#include "System/Text/NormalizationForm.hpp"
78

89
namespace System {
@@ -12,38 +13,65 @@ namespace System {
1213
*
1314
* C++ counterpart of .NET System.StringNormalizationExtensions.
1415
*
15-
* @note Full Unicode normalization (NFC/NFD/NFKC/NFKD) requires external ICU or
16-
* similar tables and is outside the scope of sharp-runtime. These stubs preserve
17-
* the API surface: IsNormalized always returns true and Normalize returns the
18-
* input unchanged, which is correct for ASCII-only strings.
16+
* @note <b>`IsNormalized` returns true and `Normalize` returns its argument unchanged for
17+
* every input, and that is .NET's own behaviour in invariant globalization mode</b>
18+
* rather than a stub this port invented. Measured on ticket #2386 against
19+
* `Normalization.cs:11-40`:
20+
*
21+
* @code
22+
* // In Invariant mode we assume all characters are normalized because we don't
23+
* // support any linguistic operations on strings.
24+
* if (GlobalizationMode.Invariant || Ascii.IsValid(source)) { return true; }
25+
* @endcode
26+
*
27+
* .NET has <b>no normalization tables of its own</b> — it delegates to ICU on Unix and
28+
* NLS on Windows (`Normalization.Icu.cs`, `Normalization.Nls.cs`), and
29+
* `CharUnicodeInfoData.cs`, the source of record `docs/StandingApprovals.md` SA-4
30+
* names, contains zero decomposition, combining-class, composition-exclusion or
31+
* quick-check data. So reproducing .NET's <i>non</i>-invariant behaviour needs a
32+
* Unicode data source SA-4 does not grant plus a UAX #15 implementation; that decision
33+
* is ticket <b>#2338</b> and is open.
34+
*
35+
* What this means for a caller today: a true from `IsNormalized` means "this runtime
36+
* performs no linguistic normalization", exactly as it does for a .NET application
37+
* built with `InvariantGlobalization=true`. It is <b>not</b> a claim that the string
38+
* is in the requested form.
39+
*
40+
* @note <b>The normalization form is validated, and that half is not invariant-mode
41+
* dependent.</b> `CheckNormalizationForm` runs <i>before</i> the invariant shortcut
42+
* (`Normalization.cs:13,29`), so .NET rejects an undefined form on every platform and
43+
* in every mode. This port now does the same (#2386).
1944
*/
2045
struct StringNormalizationExtensions {
2146
StringNormalizationExtensions() = delete;
2247

2348
/**
2449
* @brief Determines whether the string is in Unicode NFC form.
2550
* @param str The string to check.
26-
* @return true (stub — ASCII strings are always NFC).
51+
* @return true — see the class note.
2752
*/
2853
static bool IsNormalized(const std::string& str) {
2954
return IsNormalized(str, System::Text::NormalizationForm::FormC);
3055
}
3156

3257
/**
3358
* @brief Determines whether the string is in the specified normalization form.
34-
* @param str The string to check.
59+
* @param str The string to check.
3560
* @param form The normalization form.
36-
* @return true (stub — ASCII strings satisfy all normalization forms).
61+
* @return true — see the class note.
62+
* @throws System::ArgumentException with `paramName == "normalizationForm"` if @p form
63+
* is not one of the four defined values (#2386).
3764
*/
3865
static bool IsNormalized(const std::string& /*str*/,
39-
System::Text::NormalizationForm /*form*/) {
66+
System::Text::NormalizationForm form) {
67+
CheckNormalizationForm(form);
4068
return true;
4169
}
4270

4371
/**
4472
* @brief Returns the string normalized to NFC.
4573
* @param str The string to normalize.
46-
* @return The input string unchanged (stub — correct for ASCII input).
74+
* @return The input string unchanged — see the class note.
4775
*/
4876
static std::string Normalize(const std::string& str) {
4977
return Normalize(str, System::Text::NormalizationForm::FormC);
@@ -53,12 +81,44 @@ namespace System {
5381
* @brief Returns the string normalized to the specified form.
5482
* @param str The string to normalize.
5583
* @param form The normalization form.
56-
* @return The input string unchanged (stub — correct for ASCII input).
84+
* @return The input string unchanged — see the class note.
85+
* @throws System::ArgumentException with `paramName == "normalizationForm"` if @p form
86+
* is not one of the four defined values (#2386).
5787
*/
5888
static std::string Normalize(const std::string& str,
59-
System::Text::NormalizationForm /*form*/) {
89+
System::Text::NormalizationForm form) {
90+
CheckNormalizationForm(form);
6091
return str;
6192
}
93+
94+
private:
95+
/**
96+
* @brief `Normalization.CheckNormalizationForm` (`Normalization.cs:88-97`), transcribed.
97+
*
98+
* The enum's four values are 1, 2, 5 and 6 — <b>3 and 4 are holes</b>, so an undefined
99+
* value is not merely one outside the range and a bounds check would accept two of them.
100+
* .NET enumerates the four, and so does this.
101+
*
102+
* The message is .NET's verbatim (`Strings.resx:1324-1326`,
103+
* `Argument_InvalidNormalizationForm`), and the parameter name is `nameof(
104+
* normalizationForm)` — <b>not</b> this port's own parameter spelling `form`, because a
105+
* caller catching `ArgumentException` reads `ParamName` and .NET's answer is the one
106+
* worth matching.
107+
*
108+
* .NET's second clause — a `PlatformNotSupportedException` for FormKC/FormKD on Browser
109+
* and WASI, where ICU ships without compatibility data — is deliberately NOT reproduced.
110+
* It is conditioned on `!GlobalizationMode.Invariant`, and this runtime is always in the
111+
* invariant case, so the branch is unreachable in .NET under this port's own conditions.
112+
*/
113+
static void CheckNormalizationForm(System::Text::NormalizationForm form) {
114+
if (form != System::Text::NormalizationForm::FormC
115+
&& form != System::Text::NormalizationForm::FormD
116+
&& form != System::Text::NormalizationForm::FormKC
117+
&& form != System::Text::NormalizationForm::FormKD) {
118+
throw System::ArgumentException("Invalid or unsupported normalization form.",
119+
"normalizationForm");
120+
}
121+
}
62122
};
63123

64124
} // namespace System

modules/core/tests/System/StringTests.cpp

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -897,11 +897,27 @@ TEST(StringNormalizationExtensionsTests, Normalize_EmptyString) {
897897
EXPECT_EQ(System::StringNormalizationExtensions::Normalize(""), "");
898898
}
899899

900-
// --- SR-AUD-182 gated-behaviour pins (ticket #2337). Every test above uses ASCII or
901-
// the empty string, so all of them pass identically before and after real Unicode
902-
// normalization lands: the divergence had no pin at all. These four record it, so a
903-
// future normalization implementation cannot change it silently. They are statements
904-
// about the CURRENT stub, and each must be inverted by the repair (#2338).
900+
// --- SR-AUD-182 pins (ticket #2337), THREE OF THEM RE-FRAMED AND ONE INVERTED by #2386.
901+
//
902+
// #2337 wrote these as statements about "the CURRENT stub", each of which "must be inverted
903+
// by the repair (#2338)". Measured against /rv/tmp/runtime, that framing is wrong for three
904+
// of the four: returning true and returning the argument unchanged is exactly what .NET does
905+
// in invariant globalization mode, and says so in a comment --
906+
//
907+
// // In Invariant mode we assume all characters are normalized because we don't
908+
// // support any linguistic operations on strings.
909+
// if (GlobalizationMode.Invariant || Ascii.IsValid(source)) { return true; }
910+
// Normalization.cs:11-20, 27-40
911+
//
912+
// -- so the three below are a DECLARED DEVIATION with the reference behind it, not a stub
913+
// awaiting repair. .NET has no normalization tables at all; it delegates to ICU or NLS, and
914+
// SA-4's source of record contains zero normalization data. Whether this port should acquire
915+
// a data source SA-4 does not grant, plus a UAX #15 implementation, is ticket #2338 and is a
916+
// user decision.
917+
//
918+
// The fourth pin IS inverted, because its half never depended on any of that:
919+
// CheckNormalizationForm runs BEFORE the invariant shortcut, so .NET rejects an undefined
920+
// form on every platform and in every mode.
905921

906922
TEST(StringNormalizationExtensionsTests, Pin_DecomposedTextIsReportedNormalizedForFormC) {
907923
// "e" + U+0301 COMBINING ACUTE ACCENT, UTF-8 65 CC 81. .NET answers false for FormC
@@ -936,12 +952,75 @@ TEST(StringNormalizationExtensionsTests, Pin_CompatibilityFormsAreNoOpsToo) {
936952
}
937953
}
938954

939-
TEST(StringNormalizationExtensionsTests, Pin_AnUndefinedNormalizationFormIsAccepted) {
940-
// .NET reports an argument error for a form value that is not one of the four; the
941-
// stub ignores the argument entirely, so an undefined value succeeds like any other.
955+
TEST(StringNormalizationExtensionsTests, Fix2386_AnUndefinedNormalizationFormIsRejected) {
956+
// INVERTED by #2386. The form check is Normalization.cs:88-97 and runs before anything
957+
// else, so it holds in invariant mode too -- which is why this half could land while the
958+
// normalization itself stays open.
942959
const auto bogus = static_cast<System::Text::NormalizationForm>(0x1234);
943-
EXPECT_TRUE(System::StringNormalizationExtensions::IsNormalized("abc", bogus));
944-
EXPECT_EQ(System::StringNormalizationExtensions::Normalize("abc", bogus), "abc");
960+
EXPECT_THROW((void)System::StringNormalizationExtensions::IsNormalized("abc", bogus),
961+
System::ArgumentException);
962+
EXPECT_THROW((void)System::StringNormalizationExtensions::Normalize("abc", bogus),
963+
System::ArgumentException);
964+
965+
// THE VALUES 3 AND 4 ARE HOLES IN THE ENUM (FormC=1, FormD=2, FormKC=5, FormKD=6), so an
966+
// undefined value is not merely one outside the range and a bounds check would accept two
967+
// of them. This is the row that fails if the four-way test becomes `form >= 1 && form <= 6`.
968+
for (int raw : {0, 3, 4, 7, -1, 0x7FFFFFFF}) {
969+
const auto form = static_cast<System::Text::NormalizationForm>(raw);
970+
EXPECT_THROW((void)System::StringNormalizationExtensions::IsNormalized("abc", form),
971+
System::ArgumentException) << "raw form value " << raw;
972+
EXPECT_THROW((void)System::StringNormalizationExtensions::Normalize("abc", form),
973+
System::ArgumentException) << "raw form value " << raw;
974+
}
975+
976+
// All four defined values are accepted, or the check above would be satisfied by a
977+
// function that rejects everything.
978+
for (auto form : {System::Text::NormalizationForm::FormC,
979+
System::Text::NormalizationForm::FormD,
980+
System::Text::NormalizationForm::FormKC,
981+
System::Text::NormalizationForm::FormKD}) {
982+
EXPECT_NO_THROW((void)System::StringNormalizationExtensions::IsNormalized("abc", form));
983+
EXPECT_NO_THROW((void)System::StringNormalizationExtensions::Normalize("abc", form));
984+
}
985+
986+
// The message and parameter name are .NET's, transcribed rather than paraphrased.
987+
try {
988+
(void)System::StringNormalizationExtensions::Normalize("abc", bogus);
989+
ADD_FAILURE() << "an undefined form was accepted";
990+
} catch (const System::ArgumentException& e) {
991+
const std::string what = e.what();
992+
EXPECT_NE(what.find("Invalid or unsupported normalization form."), std::string::npos)
993+
<< what;
994+
// nameof(normalizationForm), NOT this port's own parameter spelling "form".
995+
EXPECT_NE(what.find("Parameter 'normalizationForm'"), std::string::npos) << what;
996+
}
997+
}
998+
999+
TEST(StringNormalizationExtensionsTests, Decl2386_TheNoOpBehaviourIsDotNetsInvariantModeNotAStub) {
1000+
// The declaration, asserted rather than left in prose. The default overloads must reach
1001+
// the same answers as the explicit FormC ones, which is what says the two-argument form is
1002+
// the whole implementation and the one-argument one adds nothing.
1003+
const std::string decomposed = "\x65\xCC\x81"; // "e" + U+0301
1004+
EXPECT_EQ(System::StringNormalizationExtensions::IsNormalized(decomposed),
1005+
System::StringNormalizationExtensions::IsNormalized(
1006+
decomposed, System::Text::NormalizationForm::FormC));
1007+
EXPECT_EQ(System::StringNormalizationExtensions::Normalize(decomposed),
1008+
System::StringNormalizationExtensions::Normalize(
1009+
decomposed, System::Text::NormalizationForm::FormC));
1010+
1011+
// Normalize is the IDENTITY, byte for byte, including for input that is not valid UTF-8 --
1012+
// .NET's invariant path is `return strInput;` with no inspection at all, so a decoder here
1013+
// would be inventing a failure mode the reference does not have.
1014+
for (const std::string& input : {std::string("hello"), std::string(""), decomposed,
1015+
std::string("\xFF\xFE"), std::string("\x00\x01", 2)}) {
1016+
for (auto form : {System::Text::NormalizationForm::FormC,
1017+
System::Text::NormalizationForm::FormD,
1018+
System::Text::NormalizationForm::FormKC,
1019+
System::Text::NormalizationForm::FormKD}) {
1020+
EXPECT_EQ(System::StringNormalizationExtensions::Normalize(input, form), input);
1021+
EXPECT_TRUE(System::StringNormalizationExtensions::IsNormalized(input, form));
1022+
}
1023+
}
9451024
}
9461025

9471026
// ---------------------------------------------------------------------------

plan.sqlite3

8 KB
Binary file not shown.

0 commit comments

Comments
 (0)