Skip to content

Commit

Permalink
Revert "Use ArgumentNullException.ThrowIfNull in more places (#105380)…
Browse files Browse the repository at this point in the history 
…" (#106108)

Specifically the changes to System.Collections.Concurrent (ConcurrentDictionary)
  • Loading branch information
@stephentoub
stephentoub authored Aug 8, 2024
1 parent 2c237cb commit 8a894b5
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 29 deletions.
3 changes: 3 additions & 0 deletions src/libraries/System.Collections.Concurrent/src/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@
<data name="ConcurrentDictionary_KeyAlreadyExisted" xml:space="preserve">
<value>The key already existed in the dictionary.</value>
</data>
<data name="ConcurrentDictionary_ItemKeyIsNull" xml:space="preserve">
<value>TKey is a reference type and item.Key is null.</value>
</data>
<data name="ConcurrentDictionary_TypeOfKeyIncorrect" xml:space="preserve">
<value>The key was of an incorrect type for this dictionary.</value>
</data>
Expand Down
143 changes: 116 additions & 27 deletions ...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ private void InitializeFromCollection(IEnumerable<KeyValuePair<TKey, TValue>> co
{
foreach (KeyValuePair<TKey, TValue> pair in collection)
{
ArgumentNullException.ThrowIfNull(pair.Key, "key");
if (pair.Key is null)
{
ThrowHelper.ThrowKeyNullException();
}

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

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

return TryRemoveInternal(key, out value, matchValue: false, default);
}
Expand All @@ -405,7 +414,10 @@ public bool TryRemove(TKey key, [MaybeNullWhen(false)] out TValue value)
/// </exception>
public bool TryRemove(KeyValuePair<TKey, TValue> item)
{
ArgumentNullException.ThrowIfNull(item.Key);
if (item.Key is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(item), SR.ConcurrentDictionary_ItemKeyIsNull);
}

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

Tables tables = _tables;

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

return TryUpdateInternal(_tables, key, null, newValue, comparisonValue);
}
Expand Down Expand Up @@ -1080,7 +1098,10 @@ public TValue this[TKey key]
}
set
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

TryAddInternal(_tables, key, null, value, updateIfExists: true, acquireLock: true, out _);
}
Expand Down Expand Up @@ -1184,8 +1205,15 @@ private int GetCountNoLocks()
/// if the key was not in the dictionary.</returns>
public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory)
{
ArgumentNullException.ThrowIfNull(key);
ArgumentNullException.ThrowIfNull(valueFactory);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (valueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(valueFactory));
}

Tables tables = _tables;

Expand Down Expand Up @@ -1219,8 +1247,15 @@ public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory)
public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TArg factoryArgument)
where TArg : allows ref struct
{
ArgumentNullException.ThrowIfNull(key);
ArgumentNullException.ThrowIfNull(valueFactory);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (valueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(valueFactory));
}

Tables tables = _tables;

Expand Down Expand Up @@ -1249,7 +1284,10 @@ public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TA
/// key is already in the dictionary, or the new value if the key was not in the dictionary.</returns>
public TValue GetOrAdd(TKey key, TValue value)
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

Tables tables = _tables;

Expand Down Expand Up @@ -1288,9 +1326,20 @@ public TValue AddOrUpdate<TArg>(
TKey key, Func<TKey, TArg, TValue> addValueFactory, Func<TKey, TValue, TArg, TValue> updateValueFactory, TArg factoryArgument)
where TArg : allows ref struct
{
ArgumentNullException.ThrowIfNull(key);
ArgumentNullException.ThrowIfNull(addValueFactory);
ArgumentNullException.ThrowIfNull(updateValueFactory);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (addValueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(addValueFactory));
}

if (updateValueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
}

Tables tables = _tables;

Expand Down Expand Up @@ -1350,9 +1399,20 @@ public TValue AddOrUpdate<TArg>(
/// absent) or the result of updateValueFactory (if the key was present).</returns>
public TValue AddOrUpdate(TKey key, Func<TKey, TValue> addValueFactory, Func<TKey, TValue, TValue> updateValueFactory)
{
ArgumentNullException.ThrowIfNull(key);
ArgumentNullException.ThrowIfNull(addValueFactory);
ArgumentNullException.ThrowIfNull(updateValueFactory);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (addValueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(addValueFactory));
}

if (updateValueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
}

Tables tables = _tables;

Expand Down Expand Up @@ -1410,8 +1470,15 @@ public TValue AddOrUpdate(TKey key, Func<TKey, TValue> addValueFactory, Func<TKe
/// absent) or the result of updateValueFactory (if the key was present).</returns>
public TValue AddOrUpdate(TKey key, TValue addValue, Func<TKey, TValue, TValue> updateValueFactory)
{
ArgumentNullException.ThrowIfNull(key);
ArgumentNullException.ThrowIfNull(updateValueFactory);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (updateValueFactory is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(updateValueFactory));
}

Tables tables = _tables;

Expand Down Expand Up @@ -1632,7 +1699,11 @@ bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> k
/// </exception>
void IDictionary.Add(object key, object? value)
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (!(key is TKey))
{
throw new ArgumentException(SR.ConcurrentDictionary_TypeOfKeyIncorrect);
Expand All @@ -1655,7 +1726,10 @@ void IDictionary.Add(object key, object? value)
/// (Nothing in Visual Basic).</exception>
bool IDictionary.Contains(object key)
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

return key is TKey tkey && ContainsKey(tkey);
}
Expand Down Expand Up @@ -1703,7 +1777,10 @@ bool IDictionary.Contains(object key)
/// (Nothing in Visual Basic).</exception>
void IDictionary.Remove(object key)
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (key is TKey tkey)
{
Expand Down Expand Up @@ -1741,7 +1818,10 @@ void IDictionary.Remove(object key)
{
get
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (key is TKey tkey && TryGetValue(tkey, out TValue? value))
{
Expand All @@ -1752,7 +1832,10 @@ void IDictionary.Remove(object key)
}
set
{
ArgumentNullException.ThrowIfNull(key);
if (key is null)
{
ThrowHelper.ThrowKeyNullException();
}

if (!(key is TKey))
{
Expand Down Expand Up @@ -2408,7 +2491,10 @@ private bool TryAdd(TAlternateKey key, TValue value, bool updateIfExists, out TV
}

TKey actualKey = comparer.Create(key);
ArgumentNullException.ThrowIfNull(actualKey, nameof(key));
if (actualKey is null)
{
ThrowHelper.ThrowKeyNullException();
}

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

public IDictionaryDebugView(IDictionary<TKey, TValue> dictionary)
{
ArgumentNullException.ThrowIfNull(dictionary);
if (dictionary is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(dictionary));
}

_dictionary = dictionary;
}
Expand Down
8 changes: 7 additions & 1 deletion src/libraries/System.Collections.Concurrent/src/System/ThrowHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ namespace System
internal static class ThrowHelper
{
[DoesNotReturn]
internal static void ThrowKeyNullException() => throw new ArgumentNullException("key");
internal static void ThrowKeyNullException() => ThrowArgumentNullException("key");

[DoesNotReturn]
internal static void ThrowArgumentNullException(string name) => throw new ArgumentNullException(name);

[DoesNotReturn]
internal static void ThrowArgumentNullException(string name, string message) => throw new ArgumentNullException(name, message);

[DoesNotReturn]
internal static void ThrowValueNullException() => throw new ArgumentException(SR.ConcurrentDictionary_TypeOfValueIncorrect);
Expand Down
2 changes: 1 addition & 1 deletion ...ies/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionaryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ public static void TestRemove3()
[Fact]
public static void TryRemove_KeyValuePair_ArgumentValidation()
{
AssertExtensions.Throws<ArgumentNullException>("item.Key", () => new ConcurrentDictionary<string, int>().TryRemove(new KeyValuePair<string, int>(null, 42)));
AssertExtensions.Throws<ArgumentNullException>("item", () => new ConcurrentDictionary<string, int>().TryRemove(new KeyValuePair<string, int>(null, 42)));
new ConcurrentDictionary<int, int>().TryRemove(new KeyValuePair<int, int>(0, 0)); // no error when using default value type
new ConcurrentDictionary<int?, int>().TryRemove(new KeyValuePair<int?, int>(0, 0)); // or nullable
}
Expand Down

0 comments on commit 8a894b5

Please sign in to comment.