Skip to content

Commit 993b23f

Browse files
ilonatommyilonatommy
authored
Limit PNSE by using Invariant HashCode in HybridGlobalization (dotnet#96354)
* Unblock all tests. * SortKey is Invariant now. * Missing SortKey changes + fix HashCode. * Typo * Revert unbocking tests connected with CompareOptions PNSE. * Hashing uses Invariant mode -these tests should be skipped. * Add active issue. * feedback * Feedback * Better documentation. * Add new tests + sanitize string before invariant comparison. * Comment + more cases. * Clean CI. * Missing change for clean CI commit. * `SortKey` not supported for non-invariant cultures, `HashCode` supported for non-invariant cultures only with `IgnoreCase` or `None` options. * Feedback. * Fix build, add docs. * Feedback @matouskozak @pavelsavara * Added tests + fixed algo. * Block failing tests for a follow-up PR. * Add more details to PNSE. * Feedback - correct comment
1 parent 26d8316 commit 993b23f

File tree

55 files changed

+416
-214
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+416
-214
lines changed

‎docs/design/features/globalization-hybrid-mode.md

+18-3
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,29 @@ For WebAssembly in Browser we are using Web API instead of some ICU data. Ideall
1717

1818
Hybrid has higher priority than sharding or custom modes, described in globalization-icu-wasm.md.
1919

20+
**HashCode**
21+
22+
Affected public APIs:
23+
- System.Globalization.CompareInfo.GetHashCode
24+
25+
For invariant culture all `CompareOptions` are available.
26+
27+
For non-invariant cultures following `CompareOptions` are available:
28+
- `CompareOption.None`
29+
- `CompareOption.IgnoreCase`
30+
31+
The remaining combinations for non-invariant cultures throw `PlatformNotSupportedException`.
32+
2033
**SortKey**
2134

2235
Affected public APIs:
2336
- System.Globalization.CompareInfo.GetSortKey
2437
- System.Globalization.CompareInfo.GetSortKeyLength
25-
- System.Globalization.CompareInfo.GetHashCode
38+
39+
For invariant culture all `CompareOptions` are available.
40+
41+
For non-invariant cultures `PlatformNotSupportedException` is thrown.
42+
2643
Indirectly affected APIs (the list might not be complete):
2744
- Microsoft.VisualBasic.Collection.Add
2845
- System.Collections.Hashtable.Add
@@ -43,8 +60,6 @@ Indirectly affected APIs (the list might not be complete):
4360
- System.Net.Mail.MailAddress.GetHashCode
4461
- System.Xml.Xsl.XslCompiledTransform.Transform
4562

46-
Web API does not have an equivalent, so they throw `PlatformNotSupportedException`.
47-
4863
**Case change**
4964

5065
Affected public APIs:

‎src/libraries/Microsoft.VisualBasic.Core/tests/CollectionsTests.cs

+6-6
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public static void Add_RelativeIndex()
7474
Assert.Equal(item1, coll[3]);
7575
}
7676

77-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
77+
[Fact]
7878
public static void Add_RelativeKey()
7979
{
8080
var coll = new Collection();
@@ -175,7 +175,7 @@ public static void RemoveAt_InvalidIndex_ThrowsArgumentOutOfRangeException()
175175
Assert.Throws<ArgumentOutOfRangeException>("Index", () => coll.RemoveAt(-1)); // Index < 0
176176
}
177177

178-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
178+
[Fact]
179179
public static void Remove_Key()
180180
{
181181
var coll = CreateKeyedCollection(10);
@@ -185,7 +185,7 @@ public static void Remove_Key()
185185
Assert.False(coll.Contains("Key3"));
186186
}
187187

188-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
188+
[Fact]
189189
public static void Remove_InvalidKey_ThrowsArgumentException()
190190
{
191191
var coll = CreateKeyedCollection(10);
@@ -242,7 +242,7 @@ public static void Contains()
242242
Assert.False(coll.Contains(new Foo()));
243243
}
244244

245-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
245+
[Fact]
246246
public static void Contains_ByKey()
247247
{
248248
var coll = CreateKeyedCollection(10);
@@ -275,7 +275,7 @@ public static void Item_Get_InvalidIndex_ThrowsIndexOutOfRangeException()
275275
Assert.Throws<ArgumentException>(() => coll[(object)Guid.Empty]); // Neither string nor int
276276
}
277277

278-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
278+
[Fact]
279279
public static void Item_GetByKey()
280280
{
281281
Collection coll = CreateKeyedCollection(10);
@@ -291,7 +291,7 @@ public static void Item_GetByKey()
291291
Assert.Equal(CreateValue(11), coll[(object)'X']);
292292
}
293293

294-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
294+
[Fact]
295295
public static void Item_GetByKey_InvalidIndex_ThrowsIndexOutOfRangeException()
296296
{
297297
Collection coll = CreateKeyedCollection(10);

‎src/libraries/Microsoft.VisualBasic.Core/tests/LateBindingTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public void LateSet(object obj, Type objType, string name, object[] args, string
3232
Assert.Equal(expected, getResult(obj));
3333
}
3434

35-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
35+
[Theory]
3636
[MemberData(nameof(LateSetComplex_TestData))]
3737
public void LateSetComplex(object obj, Type objType, string name, object[] args, string[] paramNames, bool missing, bool valueType)
3838
{
@@ -74,7 +74,7 @@ public void LateIndexSet(object obj, object[] args, string[] paramNames, Func<ob
7474
Assert.Equal(expected, getResult(obj));
7575
}
7676

77-
[Theory]
77+
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
7878
[MemberData(nameof(LateIndexSet_MissingMember_TestData))]
7979
public void LateIndexSet_MissingMember(object obj, object[] args, string[] paramNames)
8080
{

‎src/libraries/System.Collections.NonGeneric/tests/CaseInsensitiveHashCodeProviderTests.cs

+6-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace System.Collections.Tests
1313
{
1414
public class CaseInsensitiveHashCodeProviderTests
1515
{
16-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
16+
[Theory]
1717
[InlineData("hello", "HELLO", true)]
1818
[InlineData("hello", "hello", true)]
1919
[InlineData("HELLO", "HELLO", true)]
@@ -29,7 +29,7 @@ public void Ctor_Empty_GetHashCodeCompare(object a, object b, bool expected)
2929
Assert.Equal(expected, provider.GetHashCode(a) == provider.GetHashCode(b));
3030
}
3131

32-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
32+
[Theory]
3333
[InlineData("hello", "HELLO", true)]
3434
[InlineData("hello", "hello", true)]
3535
[InlineData("HELLO", "HELLO", true)]
@@ -63,7 +63,7 @@ public void Ctor_Empty_ChangeCurrentCulture_GetHashCodeCompare(object a, object
6363
}
6464
}
6565

66-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
66+
[Theory]
6767
[InlineData("hello", "HELLO", true)]
6868
[InlineData("hello", "hello", true)]
6969
[InlineData("HELLO", "HELLO", true)]
@@ -94,9 +94,10 @@ public void Ctor_CultureInfo_ChangeCurrentCulture_GetHashCodeCompare(object a, o
9494
}
9595
}
9696

97-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
97+
[Fact]
9898
[ActiveIssue("https://github.com/dotnet/runtime/issues/37069", TestPlatforms.Android | TestPlatforms.LinuxBionic)]
9999
[ActiveIssue("https://github.com/dotnet/runtime/issues/95338", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnApplePlatform))]
100+
[ActiveIssue("https://github.com/dotnet/runtime/issues/95503", typeof(PlatformDetection), nameof(PlatformDetection.IsHybridGlobalizationOnBrowser))]
100101
public void Ctor_CultureInfo_GetHashCodeCompare_TurkishI()
101102
{
102103
var cultureNames = Helpers.TestCultureNames;
@@ -135,7 +136,7 @@ public void GetHashCode_NullObj_ThrowsArgumentNullException()
135136
AssertExtensions.Throws<ArgumentNullException>("obj", () => new CaseInsensitiveHashCodeProvider().GetHashCode(null));
136137
}
137138

138-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
139+
[Theory]
139140
[InlineData("hello", "HELLO", true)]
140141
[InlineData("hello", "hello", true)]
141142
[InlineData("HELLO", "HELLO", true)]

‎src/libraries/System.Collections.NonGeneric/tests/CollectionsUtilTests.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace System.Collections.Tests
88
{
99
public static class CollectionsUtilTests
1010
{
11-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
11+
[Fact]
1212
public static void CreateCaseInsensitiveHashtable()
1313
{
1414
Hashtable hashtable = CollectionsUtil.CreateCaseInsensitiveHashtable();
@@ -20,7 +20,7 @@ public static void CreateCaseInsensitiveHashtable()
2020
AssertExtensions.Throws<ArgumentException>(null, () => hashtable.Add("key1", "value1"));
2121
}
2222

23-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
23+
[Fact]
2424
public static void CreateCaseInsensitiveHashtable_Capacity()
2525
{
2626
Hashtable hashtable = CollectionsUtil.CreateCaseInsensitiveHashtable(15);
@@ -33,7 +33,7 @@ public static void CreateCaseInsensitiveHashtable_Capacity()
3333
AssertExtensions.Throws<ArgumentException>(null, () => hashtable.Add("key1", "value1"));
3434
}
3535

36-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
36+
[Fact]
3737
public static void CreateCaseInsensitiveHashtable_IDictionary()
3838
{
3939
Hashtable hashtable1 = CollectionsUtil.CreateCaseInsensitiveHashtable();

‎src/libraries/System.Collections.NonGeneric/tests/HashtableTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ public void Values_ModifyingHashtable_ModifiesCollection()
779779
}
780780
}
781781

782-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
782+
[Fact]
783783
public void HashCodeProvider_Set_ImpactsSearch()
784784
{
785785
var hash = new ComparableHashtable(CaseInsensitiveHashCodeProvider.DefaultInvariant, StringComparer.OrdinalIgnoreCase);
@@ -834,7 +834,7 @@ public void HashCodeProvider_Comparer_IncompatibleGetSet_Throws()
834834
AssertExtensions.Throws<ArgumentException>(null, () => hash.Comparer = StringComparer.OrdinalIgnoreCase);
835835
}
836836

837-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
837+
[Fact]
838838
public void Comparer_Set_ImpactsSearch()
839839
{
840840
var hash = new ComparableHashtable(CaseInsensitiveHashCodeProvider.DefaultInvariant, StringComparer.OrdinalIgnoreCase);

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.ConstructorTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public void Constructor_Provider_Comparer()
2020
Assert.Equal(0, coll.Count);
2121
}
2222

23-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
23+
[Fact]
2424
public void Constructor_Int_Provider_Comparer()
2525
{
2626
#pragma warning disable CS0618 // Type or member is obsolete

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.CopyToTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseCopyToTests
99
{
10-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Theory]
1111
[InlineData(0, 0)]
1212
[InlineData(0, 5)]
1313
[InlineData(10, 0)]
@@ -39,7 +39,7 @@ public void CopyTo(int count, int index)
3939
Assert.Equal(previousCount, copyArray.Length);
4040
}
4141

42-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
42+
[Theory]
4343
[InlineData(0)]
4444
[InlineData(10)]
4545
public void CopyTo_Invalid(int count)

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.GetAllValuesTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class GetAllValuesTests
99
{
10-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Theory]
1111
[InlineData(0, typeof(object))]
1212
[InlineData(0, typeof(Foo))]
1313
[InlineData(10, typeof(object))]
@@ -33,7 +33,7 @@ private static void VerifyGetAllValues(NameObjectCollectionBase nameObjectCollec
3333
}
3434
}
3535

36-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
36+
[Fact]
3737
public static void GetAllValues_Invalid()
3838
{
3939
MyNameObjectCollection nameObjectCollection = new MyNameObjectCollection();

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.GetEnumeratorTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseGetEnumeratorTests
99
{
10-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Theory]
1111
[InlineData(0)]
1212
[InlineData(10)]
1313
public void GetEnumerator(int count)
@@ -29,7 +29,7 @@ public void GetEnumerator(int count)
2929
}
3030
}
3131

32-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
32+
[Theory]
3333
[InlineData(0)]
3434
[InlineData(10)]
3535
public void GetEnumerator_Invalid(int count)

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.KeysTests.cs

+6-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseKeysTests
99
{
10-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Theory]
1111
[InlineData(0)]
1212
[InlineData(10)]
1313
public void Keys_PreservesInstance(int count)
@@ -16,7 +16,7 @@ public void Keys_PreservesInstance(int count)
1616
Assert.Same(nameObjectCollection.Keys, nameObjectCollection.Keys);
1717
}
1818

19-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
19+
[Theory]
2020
[InlineData(0)]
2121
[InlineData(10)]
2222
public void Keys_GetEnumerator(int count)
@@ -40,7 +40,7 @@ public void Keys_GetEnumerator(int count)
4040
}
4141
}
4242

43-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
43+
[Theory]
4444
[InlineData(0)]
4545
[InlineData(10)]
4646
public void Keys_GetEnumerator_Invalid(int count)
@@ -85,7 +85,7 @@ public void Keys_GetEnumerator_Invalid(int count)
8585
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
8686
}
8787

88-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
88+
[Theory]
8989
[InlineData(0)]
9090
[InlineData(10)]
9191
public void Keys_Properties(int count)
@@ -97,7 +97,7 @@ public void Keys_Properties(int count)
9797
Assert.False(keysCollection.IsSynchronized);
9898
}
9999

100-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
100+
[Theory]
101101
[InlineData(0, 0)]
102102
[InlineData(0, 5)]
103103
[InlineData(10, 0)]
@@ -145,7 +145,7 @@ private static void Keys_CopyTo_Helper(MyNameObjectCollection nameObjectCollecti
145145
Assert.Equal(previousCount, keysArray.Length);
146146
}
147147

148-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
148+
[Theory]
149149
[InlineData(0)]
150150
[InlineData(10)]
151151
public void Keys_CopyTo_Invalid(int count)

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.ReadOnlyTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseReadOnlyTests
99
{
10-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Fact]
1111
public void IsReadOnly_Set()
1212
{
1313
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(10);

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.RemoveAtTests.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseRemoveAtTests
99
{
10-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Fact]
1111
public void RemoveAt()
1212
{
1313
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(10);
@@ -59,7 +59,7 @@ public void RemoveAt()
5959
}
6060
}
6161

62-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
62+
[Theory]
6363
[InlineData(0)]
6464
[InlineData(10)]
6565
public void RemoveAt_InvalidIndex_ThrowsArgumentOutOfRangeException(int count)
@@ -69,7 +69,7 @@ public void RemoveAt_InvalidIndex_ThrowsArgumentOutOfRangeException(int count)
6969
AssertExtensions.Throws<ArgumentOutOfRangeException>("index", () => nameObjectCollection.RemoveAt(count));
7070
}
7171

72-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
72+
[Fact]
7373
public void RemoveAt_ReadOnly_ThrowsNotSupportedException()
7474
{
7575
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(1);

‎src/libraries/System.Collections.Specialized/tests/NameObjectCollectionBase/NameObjectCollectionBase.SetItemTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
77
{
88
public class NameObjectCollectionBaseSetItemTests
99
{
10-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
10+
[Fact]
1111
public void Set_ObjectAtIndex_ModifiesCollection()
1212
{
1313
var noc = new MyNameObjectCollection();

0 commit comments

Comments
 (0)