Skip to content

Commit cffea23

Browse files
stephentoubstephentoub
committed
Revert "Use ArgumentNullException.ThrowIfNull in more places (dotnet#105380)"
Specifically the changes to System.Collections.Concurrent (ConcurrentDictionary)
1 parent a562a9f commit cffea23

File tree

4 files changed

+127
-29
lines changed

4 files changed

+127
-29
lines changed

‎src/libraries/System.Collections.Concurrent/src/Resources/Strings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@
126126
<data name="ConcurrentDictionary_KeyAlreadyExisted" xml:space="preserve">
127127
<value>The key already existed in the dictionary.</value>
128128
</data>
129+
<data name="ConcurrentDictionary_ItemKeyIsNull" xml:space="preserve">
130+
<value>TKey is a reference type and item.Key is null.</value>
131+
</data>
129132
<data name="ConcurrentDictionary_TypeOfKeyIncorrect" xml:space="preserve">
130133
<value>The key was of an incorrect type for this dictionary.</value>
131134
</data>

‎src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs‎

Lines changed: 116 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,10 @@ private void InitializeFromCollection(IEnumerable<KeyValuePair<TKey, TValue>> co
329329
{
330330
foreach (KeyValuePair<TKey, TValue> pair in collection)
331331
{
332-
ArgumentNullException.ThrowIfNull(pair.Key, "key");
332+
if (pair.Key is null)
333+
{
334+
ThrowHelper.ThrowKeyNullException();
335+
}
333336

334337
if (!TryAddInternal(_tables, pair.Key, null, pair.Value, updateIfExists: false, acquireLock: false, out _))
335338
{
@@ -357,7 +360,10 @@ private void InitializeFromCollection(IEnumerable<KeyValuePair<TKey, TValue>> co
357360
/// <exception cref="OverflowException">The <see cref="ConcurrentDictionary{TKey, TValue}"/> contains too many elements.</exception>
358361
public bool TryAdd(TKey key, TValue value)
359362
{
360-
ArgumentNullException.ThrowIfNull(key);
363+
if (key is null)
364+
{
365+
ThrowHelper.ThrowKeyNullException();
366+
}
361367

362368
return TryAddInternal(_tables, key, null, value, updateIfExists: false, acquireLock: true, out _);
363369
}
@@ -383,7 +389,10 @@ public bool TryAdd(TKey key, TValue value)
383389
/// <exception cref="ArgumentNullException"><paramref name="key"/> is a null reference (Nothing in Visual Basic).</exception>
384390
public bool TryRemove(TKey key, [MaybeNullWhen(false)] out TValue value)
385391
{
386-
ArgumentNullException.ThrowIfNull(key);
392+
if (key is null)
393+
{
394+
ThrowHelper.ThrowKeyNullException();
395+
}
387396

388397
return TryRemoveInternal(key, out value, matchValue: false, default);
389398
}
@@ -405,7 +414,10 @@ public bool TryRemove(TKey key, [MaybeNullWhen(false)] out TValue value)
405414
/// </exception>
406415
public bool TryRemove(KeyValuePair<TKey, TValue> item)
407416
{
408-
ArgumentNullException.ThrowIfNull(item.Key);
417+
if (item.Key is null)
418+
{
419+
ThrowHelper.ThrowArgumentNullException(nameof(item), SR.ConcurrentDictionary_ItemKeyIsNull);
420+
}
409421

410422
return TryRemoveInternal(item.Key, out _, matchValue: true, item.Value);
411423
}
@@ -503,7 +515,10 @@ private bool TryRemoveInternal(TKey key, [MaybeNullWhen(false)] out TValue value
503515
/// <exception cref="ArgumentNullException"><paramref name="key"/> is a null reference (Nothing in Visual Basic).</exception>
504516
public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value)
505517
{
506-
ArgumentNullException.ThrowIfNull(key);
518+
if (key is null)
519+
{
520+
ThrowHelper.ThrowKeyNullException();
521+
}
507522

508523
Tables tables = _tables;
509524

@@ -589,7 +604,10 @@ private static bool TryGetValueInternal(Tables tables, TKey key, int hashcode, [
589604
/// <exception cref="ArgumentNullException"><paramref name="key"/> is a null reference.</exception>
590605
public bool TryUpdate(TKey key, TValue newValue, TValue comparisonValue)
591606
{
592-
ArgumentNullException.ThrowIfNull(key);
607+
if (key is null)
608+
{
609+
ThrowHelper.ThrowKeyNullException();
610+
}
593611

594612
return TryUpdateInternal(_tables, key, null, newValue, comparisonValue);
595613
}
@@ -1080,7 +1098,10 @@ public TValue this[TKey key]
10801098
}
10811099
set
10821100
{
1083-
ArgumentNullException.ThrowIfNull(key);
1101+
if (key is null)
1102+
{
1103+
ThrowHelper.ThrowKeyNullException();
1104+
}
10841105

10851106
TryAddInternal(_tables, key, null, value, updateIfExists: true, acquireLock: true, out _);
10861107
}
@@ -1184,8 +1205,15 @@ private int GetCountNoLocks()
11841205
/// if the key was not in the dictionary.</returns>
11851206
public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory)
11861207
{
1187-
ArgumentNullException.ThrowIfNull(key);
1188-
ArgumentNullException.ThrowIfNull(valueFactory);
1208+
if (key is null)
1209+
{
1210+
ThrowHelper.ThrowKeyNullException();
1211+
}
1212+
1213+
if (valueFactory is null)
1214+
{
1215+
ThrowHelper.ThrowArgumentNullException(nameof(valueFactory));
1216+
}
11891217

11901218
Tables tables = _tables;
11911219

@@ -1219,8 +1247,15 @@ public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory)
12191247
public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TArg factoryArgument)
12201248
where TArg : allows ref struct
12211249
{
1222-
ArgumentNullException.ThrowIfNull(key);
1223-
ArgumentNullException.ThrowIfNull(valueFactory);
1250+
if (key is null)
1251+
{
1252+
ThrowHelper.ThrowKeyNullException();
1253+
}
1254+
1255+
if (valueFactory is null)
1256+
{
1257+
ThrowHelper.ThrowArgumentNullException(nameof(valueFactory));
1258+
}
12241259

12251260
Tables tables = _tables;
12261261

@@ -1249,7 +1284,10 @@ public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TA
12491284
/// key is already in the dictionary, or the new value if the key was not in the dictionary.</returns>
12501285
public TValue GetOrAdd(TKey key, TValue value)
12511286
{
1252-
ArgumentNullException.ThrowIfNull(key);
1287+
if (key is null)
1288+
{
1289+
ThrowHelper.ThrowKeyNullException();
1290+
}
12531291

12541292
Tables tables = _tables;
12551293

@@ -1288,9 +1326,20 @@ public TValue AddOrUpdate<TArg>(
12881326
TKey key, Func<TKey, TArg, TValue> addValueFactory, Func<TKey, TValue, TArg, TValue> updateValueFactory, TArg factoryArgument)
12891327
where TArg : allows ref struct
12901328
{
1291-
ArgumentNullException.ThrowIfNull(key);
1292-
ArgumentNullException.ThrowIfNull(addValueFactory);
1293-
ArgumentNullException.ThrowIfNull(updateValueFactory);
1329+
if (key is null)
1330+
{
1331+
ThrowHelper.ThrowKeyNullException();
1332+
}
1333+
1334+
if (addValueFactory is null)
1335+
{
1336+
ThrowHelper.ThrowArgumentNullException(nameof(addValueFactory));
1337+
}
1338+
1339+
if (updateValueFactory is null)
1340+
{
1341+
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
1342+
}
12941343

12951344
Tables tables = _tables;
12961345

@@ -1350,9 +1399,20 @@ public TValue AddOrUpdate<TArg>(
13501399
/// absent) or the result of updateValueFactory (if the key was present).</returns>
13511400
public TValue AddOrUpdate(TKey key, Func<TKey, TValue> addValueFactory, Func<TKey, TValue, TValue> updateValueFactory)
13521401
{
1353-
ArgumentNullException.ThrowIfNull(key);
1354-
ArgumentNullException.ThrowIfNull(addValueFactory);
1355-
ArgumentNullException.ThrowIfNull(updateValueFactory);
1402+
if (key is null)
1403+
{
1404+
ThrowHelper.ThrowKeyNullException();
1405+
}
1406+
1407+
if (addValueFactory is null)
1408+
{
1409+
ThrowHelper.ThrowArgumentNullException(nameof(addValueFactory));
1410+
}
1411+
1412+
if (updateValueFactory is null)
1413+
{
1414+
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
1415+
}
13561416

13571417
Tables tables = _tables;
13581418

@@ -1410,8 +1470,15 @@ public TValue AddOrUpdate(TKey key, Func<TKey, TValue> addValueFactory, Func<TKe
14101470
/// absent) or the result of updateValueFactory (if the key was present).</returns>
14111471
public TValue AddOrUpdate(TKey key, TValue addValue, Func<TKey, TValue, TValue> updateValueFactory)
14121472
{
1413-
ArgumentNullException.ThrowIfNull(key);
1414-
ArgumentNullException.ThrowIfNull(updateValueFactory);
1473+
if (key is null)
1474+
{
1475+
ThrowHelper.ThrowKeyNullException();
1476+
}
1477+
1478+
if (updateValueFactory is null)
1479+
{
1480+
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
1481+
}
14151482

14161483
Tables tables = _tables;
14171484

@@ -1632,7 +1699,11 @@ bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> k
16321699
/// </exception>
16331700
void IDictionary.Add(object key, object? value)
16341701
{
1635-
ArgumentNullException.ThrowIfNull(key);
1702+
if (key is null)
1703+
{
1704+
ThrowHelper.ThrowKeyNullException();
1705+
}
1706+
16361707
if (!(key is TKey))
16371708
{
16381709
throw new ArgumentException(SR.ConcurrentDictionary_TypeOfKeyIncorrect);
@@ -1655,7 +1726,10 @@ void IDictionary.Add(object key, object? value)
16551726
/// (Nothing in Visual Basic).</exception>
16561727
bool IDictionary.Contains(object key)
16571728
{
1658-
ArgumentNullException.ThrowIfNull(key);
1729+
if (key is null)
1730+
{
1731+
ThrowHelper.ThrowKeyNullException();
1732+
}
16591733

16601734
return key is TKey tkey && ContainsKey(tkey);
16611735
}
@@ -1703,7 +1777,10 @@ bool IDictionary.Contains(object key)
17031777
/// (Nothing in Visual Basic).</exception>
17041778
void IDictionary.Remove(object key)
17051779
{
1706-
ArgumentNullException.ThrowIfNull(key);
1780+
if (key is null)
1781+
{
1782+
ThrowHelper.ThrowKeyNullException();
1783+
}
17071784

17081785
if (key is TKey tkey)
17091786
{
@@ -1741,7 +1818,10 @@ void IDictionary.Remove(object key)
17411818
{
17421819
get
17431820
{
1744-
ArgumentNullException.ThrowIfNull(key);
1821+
if (key is null)
1822+
{
1823+
ThrowHelper.ThrowKeyNullException();
1824+
}
17451825

17461826
if (key is TKey tkey && TryGetValue(tkey, out TValue? value))
17471827
{
@@ -1752,7 +1832,10 @@ void IDictionary.Remove(object key)
17521832
}
17531833
set
17541834
{
1755-
ArgumentNullException.ThrowIfNull(key);
1835+
if (key is null)
1836+
{
1837+
ThrowHelper.ThrowKeyNullException();
1838+
}
17561839

17571840
if (!(key is TKey))
17581841
{
@@ -2408,7 +2491,10 @@ private bool TryAdd(TAlternateKey key, TValue value, bool updateIfExists, out TV
24082491
}
24092492

24102493
TKey actualKey = comparer.Create(key);
2411-
ArgumentNullException.ThrowIfNull(actualKey, nameof(key));
2494+
if (actualKey is null)
2495+
{
2496+
ThrowHelper.ThrowKeyNullException();
2497+
}
24122498

24132499
// The key was not found in the bucket. Insert the key-value pair.
24142500
var resultNode = new Node(actualKey, value, hashcode, bucket);
@@ -2638,7 +2724,10 @@ internal sealed class IDictionaryDebugView<TKey, TValue> where TKey : notnull
26382724

26392725
public IDictionaryDebugView(IDictionary<TKey, TValue> dictionary)
26402726
{
2641-
ArgumentNullException.ThrowIfNull(dictionary);
2727+
if (dictionary is null)
2728+
{
2729+
ThrowHelper.ThrowArgumentNullException(nameof(dictionary));
2730+
}
26422731

26432732
_dictionary = dictionary;
26442733
}

‎src/libraries/System.Collections.Concurrent/src/System/ThrowHelper.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,13 @@ namespace System
88
internal static class ThrowHelper
99
{
1010
[DoesNotReturn]
11-
internal static void ThrowKeyNullException() => throw new ArgumentNullException("key");
11+
internal static void ThrowKeyNullException() => ThrowArgumentNullException("key");
12+
13+
[DoesNotReturn]
14+
internal static void ThrowArgumentNullException(string name) => throw new ArgumentNullException(name);
15+
16+
[DoesNotReturn]
17+
internal static void ThrowArgumentNullException(string name, string message) => throw new ArgumentNullException(name, message);
1218

1319
[DoesNotReturn]
1420
internal static void ThrowValueNullException() => throw new ArgumentException(SR.ConcurrentDictionary_TypeOfValueIncorrect);

‎src/libraries/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionaryTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ public static void TestRemove3()
449449
[Fact]
450450
public static void TryRemove_KeyValuePair_ArgumentValidation()
451451
{
452-
AssertExtensions.Throws<ArgumentNullException>("item.Key", () => new ConcurrentDictionary<string, int>().TryRemove(new KeyValuePair<string, int>(null, 42)));
452+
AssertExtensions.Throws<ArgumentNullException>("item", () => new ConcurrentDictionary<string, int>().TryRemove(new KeyValuePair<string, int>(null, 42)));
453453
new ConcurrentDictionary<int, int>().TryRemove(new KeyValuePair<int, int>(0, 0)); // no error when using default value type
454454
new ConcurrentDictionary<int?, int>().TryRemove(new KeyValuePair<int?, int>(0, 0)); // or nullable
455455
}

0 commit comments

Comments
 (0)