Skip to content

Commit

Permalink
Add/use ArgumentException.ThrowIfNullOrEmpty (#64357)
Browse files Browse the repository at this point in the history 
Adds ArgumentException.ThrowIfNullOrEmpty and then uses it in a bunch of places around the tree.  Most of the replaced places used a resource string for the message, and I used it in places where it didn't feel like we were losing any meaningful information by centralizing on the single shared string, but I didn't use it in places where the resource string message used in the empty case conveyed something that seemed useful, e.g. a recommendation on what to use instead of an empty string.

There are a few places where I allowed for order of exception throws to change in the case where there were multiple user errors and the validation for a single argument was split, e.g. I changed a few cases of:
    ```C#
        if (arg1 is null) throw new ArgumentNullException(...);
        if (arg2 is null) throw new ArgumentNullException(...);
        if (arg1.Length == 0) throw new ArgumentException(...);
        if (arg2.Length == 0) throw new ArgumentException(...);
    ```
to:
    ```C#
        ArgumentException.ThrowIfNullOrEmpty(arg1);
        ArgumentException.ThrowIfNullOrEmpty(arg2);
    ```
even though it'll produce a different exception for `M("", null)` than it would previously.  Technically a breaking change, but given this is a case of multiple usage errors and all the exceptions are ArgumentException-based, I think it's acceptable.

I wanted to get this in before we do the massive !! conversion, as I expect !! auto-fixes will make it a bit harder to roll this out.  I explicitly did not do anything to roll out use of ArgumentNullException.ThrowIfNull, except in cases where I was modifying a method to use ArgumentException.ThrowIfNullOrEmpty, in which case for consistency I changed anything else in the function that could have been using ThrowIfNull to do so.
  • Loading branch information
@stephentoub
stephentoub authored Jan 28, 2022
1 parent f2538c6 commit 91da4ac
Show file tree
Hide file tree
Showing 136 changed files with 553 additions and 1,462 deletions.
18 changes: 2 additions & 16 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,7 @@ public ModuleBuilder DefineDynamicModule(string name)

private ModuleBuilder DefineDynamicModuleInternalNoLock(string name)
{
if (name == null)
{
throw new ArgumentNullException(nameof(name));
}
if (name.Length == 0)
{
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
}
ArgumentException.ThrowIfNullOrEmpty(name);
if (name[0] == '\0')
{
throw new ArgumentException(SR.Argument_InvalidName, nameof(name));
Expand Down Expand Up @@ -467,14 +460,7 @@ public override Assembly GetSatelliteAssembly(CultureInfo culture, Version? vers
/// <param name="name">The name of module for the look up.</param>
private ModuleBuilder? GetDynamicModuleNoLock(string name)
{
if (name == null)
{
throw new ArgumentNullException(nameof(name));
}
if (name.Length == 0)
{
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
}
ArgumentException.ThrowIfNullOrEmpty(name);

for (int i = 0; i < _assemblyData._moduleBuilderList.Count; i++)
{
Expand Down
9 changes: 2 additions & 7 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,12 @@ public sealed class FieldBuilder : FieldInfo
internal FieldBuilder(TypeBuilder typeBuilder, string fieldName, Type type,
Type[]? requiredCustomModifiers, Type[]? optionalCustomModifiers, FieldAttributes attributes)
{
if (fieldName == null)
throw new ArgumentNullException(nameof(fieldName));

if (fieldName.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(fieldName));
ArgumentException.ThrowIfNullOrEmpty(fieldName);

if (fieldName[0] == '\0')
throw new ArgumentException(SR.Argument_IllegalName, nameof(fieldName));

if (type == null)
throw new ArgumentNullException(nameof(type));
ArgumentNullException.ThrowIfNull(type);

if (type == typeof(void))
throw new ArgumentException(SR.Argument_BadFieldType);
Expand Down
6 changes: 1 addition & 5 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ILGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1273,11 +1273,7 @@ public virtual void UsingNamespace(string usingNamespace)
// Specifying the namespace to be used in evaluating locals and watches
// for the current active lexical scope.

if (usingNamespace == null)
throw new ArgumentNullException(nameof(usingNamespace));

if (usingNamespace.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(usingNamespace));
ArgumentException.ThrowIfNullOrEmpty(usingNamespace);

MethodBuilder? methodBuilder = m_methodBuilder as MethodBuilder;
if (methodBuilder == null)
Expand Down
12 changes: 3 additions & 9 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,18 @@ internal MethodBuilder(string name, MethodAttributes attributes, CallingConventi
Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers,
ModuleBuilder mod, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TypeBuilder type)
{
if (name == null)
throw new ArgumentNullException(nameof(name));

if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);

if (name[0] == '\0')
throw new ArgumentException(SR.Argument_IllegalName, nameof(name));

if (mod == null)
throw new ArgumentNullException(nameof(mod));
ArgumentNullException.ThrowIfNull(mod);

if (parameterTypes != null)
{
foreach (Type t in parameterTypes)
{
if (t == null)
throw new ArgumentNullException(nameof(parameterTypes));
ArgumentNullException.ThrowIfNull(t, nameof(parameterTypes));
}
}

Expand Down
23 changes: 3 additions & 20 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,14 +948,7 @@ private MethodBuilder DefineGlobalMethodNoLock(string name, MethodAttributes att
{
throw new InvalidOperationException(SR.InvalidOperation_GlobalsHaveBeenCreated);
}
if (name == null)
{
throw new ArgumentNullException(nameof(name));
}
if (name.Length == 0)
{
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
}
ArgumentException.ThrowIfNullOrEmpty(name);
if ((attributes & MethodAttributes.Static) == 0)
{
throw new ArgumentException(SR.Argument_GlobalFunctionHasToBeStatic);
Expand Down Expand Up @@ -1352,18 +1345,8 @@ internal int GetArrayMethodToken(Type arrayClass, string methodName, CallingConv
private int GetArrayMethodTokenNoLock(Type arrayClass, string methodName, CallingConventions callingConvention,
Type? returnType, Type[]? parameterTypes)
{
if (arrayClass == null)
{
throw new ArgumentNullException(nameof(arrayClass));
}
if (methodName == null)
{
throw new ArgumentNullException(nameof(methodName));
}
if (methodName.Length == 0)
{
throw new ArgumentException(SR.Argument_EmptyName, nameof(methodName));
}
ArgumentNullException.ThrowIfNull(arrayClass);
ArgumentException.ThrowIfNullOrEmpty(methodName);
if (!arrayClass.IsArray)
{
throw new ArgumentException(SR.Argument_HasToBeArrayClass);
Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ internal PropertyBuilder(
int prToken, // the metadata token for this property
TypeBuilder containingType) // the containing type
{
if (name == null)
throw new ArgumentNullException(nameof(name));
if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);
if (name[0] == '\0')
throw new ArgumentException(SR.Argument_IllegalName, nameof(name));

Expand Down
48 changes: 8 additions & 40 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,7 @@ internal TypeBuilder(
string fullname, TypeAttributes attr, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent, Type[]? interfaces, ModuleBuilder module,
PackingSize iPackingSize, int iTypeSize, TypeBuilder? enclosingType)
{
if (fullname == null)
throw new ArgumentNullException(nameof(fullname));

if (fullname.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(fullname));
ArgumentException.ThrowIfNullOrEmpty(fullname);

if (fullname[0] == '\0')
throw new ArgumentException(SR.Argument_IllegalName, nameof(fullname));
Expand Down Expand Up @@ -581,11 +577,7 @@ private FieldBuilder DefineDataHelper(string name, byte[]? data, int size, Field
FieldBuilder fdBuilder;
TypeAttributes typeAttributes;

if (name == null)
throw new ArgumentNullException(nameof(name));

if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);

if (size <= 0 || size >= 0x003f0000)
throw new ArgumentException(SR.Argument_BadSizeForData);
Expand Down Expand Up @@ -1282,11 +1274,7 @@ private MethodBuilder DefineMethodNoLock(string name, MethodAttributes attribute
Type? returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers,
Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers)
{
if (name == null)
throw new ArgumentNullException(nameof(name));

if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);

AssemblyBuilder.CheckContext(returnType);
AssemblyBuilder.CheckContext(returnTypeRequiredCustomModifiers, returnTypeOptionalCustomModifiers, parameterTypes);
Expand Down Expand Up @@ -1374,23 +1362,9 @@ private MethodBuilder DefinePInvokeMethodHelper(

lock (SyncRoot)
{
if (name == null)
throw new ArgumentNullException(nameof(name));

if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));

if (dllName == null)
throw new ArgumentNullException(nameof(dllName));

if (dllName.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(dllName));

if (importName == null)
throw new ArgumentNullException(nameof(importName));

if (importName.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(importName));
ArgumentException.ThrowIfNullOrEmpty(name);
ArgumentException.ThrowIfNullOrEmpty(dllName);
ArgumentException.ThrowIfNullOrEmpty(importName);

if ((attributes & MethodAttributes.Abstract) != 0)
throw new ArgumentException(SR.Argument_BadPInvokeMethod);
Expand Down Expand Up @@ -1792,10 +1766,7 @@ private PropertyBuilder DefinePropertyNoLock(string name, PropertyAttributes att
Type returnType, Type[]? returnTypeRequiredCustomModifiers, Type[]? returnTypeOptionalCustomModifiers,
Type[]? parameterTypes, Type[][]? parameterTypeRequiredCustomModifiers, Type[][]? parameterTypeOptionalCustomModifiers)
{
if (name == null)
throw new ArgumentNullException(nameof(name));
if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);

AssemblyBuilder.CheckContext(returnType);
AssemblyBuilder.CheckContext(returnTypeRequiredCustomModifiers, returnTypeOptionalCustomModifiers, parameterTypes);
Expand Down Expand Up @@ -1847,10 +1818,7 @@ public EventBuilder DefineEvent(string name, EventAttributes attributes, Type ev

private EventBuilder DefineEventNoLock(string name, EventAttributes attributes, Type eventtype)
{
if (name == null)
throw new ArgumentNullException(nameof(name));
if (name.Length == 0)
throw new ArgumentException(SR.Argument_EmptyName, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);
if (name[0] == '\0')
throw new ArgumentException(SR.Argument_IllegalName, nameof(name));

Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,7 @@ private static partial void GetTypeByName(string name, bool throwOnError, bool i

internal static RuntimeType GetTypeByNameUsingCARules(string name, RuntimeModule scope)
{
if (string.IsNullOrEmpty(name))
throw new ArgumentException(null, nameof(name));
ArgumentException.ThrowIfNullOrEmpty(name);

RuntimeType? type = null;
GetTypeByNameUsingCARules(name, new QCallModule(ref scope), ObjectHandleOnStack.Create(ref type));
Expand Down
12 changes: 4 additions & 8 deletions src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ public override byte[] DeriveKeyFromHash(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand All @@ -45,10 +43,8 @@ public override byte[] DeriveKeyFromHmac(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand Down
12 changes: 4 additions & 8 deletions src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ public override byte[] DeriveKeyFromHash(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

using (SafeNCryptSecretHandle secretAgreement = DeriveSecretAgreementHandle(otherPartyPublicKey))
{
Expand All @@ -99,10 +97,8 @@ public override byte[] DeriveKeyFromHmac(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

using (SafeNCryptSecretHandle secretAgreement = DeriveSecretAgreementHandle(otherPartyPublicKey))
{
Expand Down
12 changes: 4 additions & 8 deletions src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ public override byte[] DeriveKeyFromHash(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand All @@ -46,10 +44,8 @@ public override byte[] DeriveKeyFromHmac(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand Down
12 changes: 4 additions & 8 deletions src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,8 @@ public override byte[] DeriveKeyFromHash(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand All @@ -143,10 +141,8 @@ public override byte[] DeriveKeyFromHmac(
byte[]? secretPrepend,
byte[]? secretAppend)
{
if (otherPartyPublicKey == null)
throw new ArgumentNullException(nameof(otherPartyPublicKey));
if (string.IsNullOrEmpty(hashAlgorithm.Name))
throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm));
ArgumentNullException.ThrowIfNull(otherPartyPublicKey);
ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm));

ThrowIfDisposed();

Expand Down
Loading

0 comments on commit 91da4ac

Please sign in to comment.