Change incorrect usage of ArgumentNullException (#48243)

* Change incorrect usage of ArgumentNullException

* Fix tests

* Split exception

* Try using shared source

* Fix argument helpers for netstandard and netfx

* Address feedback from review

* Fix nullability attributes in shared helpers

* Fix null or empty check for netfx and netstandard

* Use EmptyString again for checks

* Update check again

---------

Co-authored-by: mla-alm <[email protected]>

Author: captainsafia

src/DataProtection/Extensions/src/DataProtectionProvider.cs

@@ -24,10 +24,7 @@ public static class DataProtectionProvider
     /// applications on the machine.</param>
     public static IDataProtectionProvider Create(string applicationName)
     {
-        if (string.IsNullOrEmpty(applicationName))
-        {
-            throw new ArgumentNullException(nameof(applicationName));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(applicationName);
 
         return CreateProvider(
             keyDirectory: null,
@@ -74,10 +71,7 @@ public static IDataProtectionProvider Create(
     /// <param name="certificate">The <see cref="X509Certificate2"/> to be used for encryption.</param>
     public static IDataProtectionProvider Create(string applicationName, X509Certificate2 certificate)
     {
-        if (string.IsNullOrEmpty(applicationName))
-        {
-            throw new ArgumentNullException(nameof(applicationName));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(applicationName);
         ArgumentNullThrowHelper.ThrowIfNull(certificate);
 
         return CreateProvider(

src/DataProtection/Extensions/src/Microsoft.AspNetCore.DataProtection.Extensions.csproj

@@ -14,6 +14,7 @@
     <Compile Include="..\..\shared\src\*.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)TrimmingAttributes.cs" LinkBase="Shared"
       Condition="'$(TargetFramework)' != '$(DefaultNetCoreTargetFramework)'" />
+    <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentThrowHelper.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentNullThrowHelper.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)CallerArgument\CallerArgumentExpressionAttribute.cs" LinkBase="Shared" />
   </ItemGroup>

src/Http/Authentication.Abstractions/src/TokenExtensions.cs

@@ -36,7 +36,7 @@ public static void StoreTokens(this AuthenticationProperties properties, IEnumer
         {
             if (token.Name is null)
             {
-                throw new ArgumentNullException(nameof(tokens), "Token name cannot be null.");
+                throw new ArgumentException("Token name cannot be null for any token.", nameof(tokens));
             }
 
             // REVIEW: should probably check that there are no ; in the token name and throw or encode

src/Http/Http.Abstractions/src/Internal/ParsingHelpers.cs

@@ -46,11 +46,8 @@ public static StringValues GetHeaderUnmodified(IHeaderDictionary headers, string
     public static void SetHeaderJoined(IHeaderDictionary headers, string key, StringValues value)
     {
         ArgumentNullException.ThrowIfNull(headers);
+        ArgumentException.ThrowIfNullOrEmpty(key);
 
-        if (string.IsNullOrEmpty(key))
-        {
-            throw new ArgumentNullException(nameof(key));
-        }
         if (StringValues.IsNullOrEmpty(value))
         {
             headers.Remove(key);
@@ -87,11 +84,8 @@ public static void SetHeaderJoined(IHeaderDictionary headers, string key, String
     public static void SetHeaderUnmodified(IHeaderDictionary headers, string key, StringValues? values)
     {
         ArgumentNullException.ThrowIfNull(headers);
+        ArgumentException.ThrowIfNullOrEmpty(key);
 
-        if (string.IsNullOrEmpty(key))
-        {
-            throw new ArgumentNullException(nameof(key));
-        }
         if (!values.HasValue || StringValues.IsNullOrEmpty(values.GetValueOrDefault()))
         {
             headers.Remove(key);

src/Http/Routing/src/Patterns/RoutePatternFactory.cs

@@ -823,7 +823,7 @@ public static RoutePatternParameterPart ParameterPart(
 
         if (@default != null && parameterKind == RoutePatternParameterKind.Optional)
         {
-            throw new ArgumentNullException(nameof(parameterKind), Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
+            throw new ArgumentException(Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
         }
 
         return ParameterPartCore(
@@ -860,7 +860,7 @@ public static RoutePatternParameterPart ParameterPart(
 
         if (@default != null && parameterKind == RoutePatternParameterKind.Optional)
         {
-            throw new ArgumentNullException(nameof(parameterKind), Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
+            throw new ArgumentException(Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
         }
 
         ArgumentNullException.ThrowIfNull(parameterPolicies);
@@ -899,7 +899,7 @@ public static RoutePatternParameterPart ParameterPart(
 
         if (@default != null && parameterKind == RoutePatternParameterKind.Optional)
         {
-            throw new ArgumentNullException(nameof(parameterKind), Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
+            throw new ArgumentException(Resources.TemplateRoute_OptionalCannotHaveDefaultValue);
         }
 
         ArgumentNullException.ThrowIfNull(parameterPolicies);

src/Identity/EntityFrameworkCore/src/UserStore.cs

@@ -611,10 +611,7 @@ join user in Users on userclaims.UserId equals user.Id
     {
         cancellationToken.ThrowIfCancellationRequested();
         ThrowIfDisposed();
-        if (string.IsNullOrEmpty(normalizedRoleName))
-        {
-            throw new ArgumentNullException(nameof(normalizedRoleName));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(normalizedRoleName);
 
         var role = await FindRoleAsync(normalizedRoleName, cancellationToken);
 

src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs

@@ -28,7 +28,8 @@ public UserClaimsPrincipalFactory(
         ArgumentNullThrowHelper.ThrowIfNull(userManager);
         if (optionsAccessor == null || optionsAccessor.Value == null)
         {
-            throw new ArgumentNullException(nameof(optionsAccessor));
+            ArgumentNullThrowHelper.ThrowIfNull(optionsAccessor);
+            throw new ArgumentException($"{nameof(optionsAccessor)} cannot wrap a null value.", nameof(optionsAccessor));
         }
         UserManager = userManager;
         Options = optionsAccessor.Value;

src/Identity/test/Identity.Test/UserClaimsPrincipalFactoryTest.cs

@@ -15,7 +15,7 @@ public async Task CreateIdentityNullChecks()
         var userManager = MockHelpers.MockUserManager<PocoUser>().Object;
         var roleManager = MockHelpers.MockRoleManager<PocoRole>().Object;
         var options = new Mock<IOptions<IdentityOptions>>();
-        Assert.Throws<ArgumentNullException>("optionsAccessor",
+        Assert.Throws<ArgumentException>("optionsAccessor",
             () => new UserClaimsPrincipalFactory<PocoUser, PocoRole>(userManager, roleManager, options.Object));
         var identityOptions = new IdentityOptions();
         options.Setup(a => a.Value).Returns(identityOptions);

src/Identity/test/InMemory.Test/InMemoryStore.cs

@@ -57,10 +57,7 @@ public class InMemoryStore<TUser, TRole> :
     // RoleId == rolename for inmemory store tests
     public Task<IList<TUser>> GetUsersInRoleAsync(string roleName, CancellationToken cancellationToken = default(CancellationToken))
     {
-        if (String.IsNullOrEmpty(roleName))
-        {
-            throw new ArgumentNullException(nameof(roleName));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(roleName);
 
         var role = _roles.Values.Where(x => x.NormalizedName.Equals(roleName)).SingleOrDefault();
         if (role == null)

src/Localization/Localization/src/Microsoft.Extensions.Localization.csproj

@@ -23,6 +23,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentThrowHelper.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentNullThrowHelper.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)CallerArgument\CallerArgumentExpressionAttribute.cs" LinkBase="Shared" />
   </ItemGroup>

src/Localization/Localization/src/ResourceLocationAttribute.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using Microsoft.AspNetCore.Shared;
 
 namespace Microsoft.Extensions.Localization;
 
@@ -17,10 +18,7 @@ public class ResourceLocationAttribute : Attribute
     /// <param name="resourceLocation">The location of resources for this Assembly.</param>
     public ResourceLocationAttribute(string resourceLocation)
     {
-        if (string.IsNullOrEmpty(resourceLocation))
-        {
-            throw new ArgumentNullException(nameof(resourceLocation));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(resourceLocation);
 
         ResourceLocation = resourceLocation;
     }

src/Localization/Localization/src/ResourceManagerStringLocalizerFactory.cs

@@ -76,11 +76,7 @@ protected virtual string GetResourcePrefix(TypeInfo typeInfo)
     protected virtual string GetResourcePrefix(TypeInfo typeInfo, string? baseNamespace, string? resourcesRelativePath)
     {
         ArgumentNullThrowHelper.ThrowIfNull(typeInfo);
-
-        if (string.IsNullOrEmpty(baseNamespace))
-        {
-            throw new ArgumentNullException(nameof(baseNamespace));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(baseNamespace);
 
         if (string.IsNullOrEmpty(typeInfo.FullName))
         {
@@ -107,15 +103,8 @@ protected virtual string GetResourcePrefix(TypeInfo typeInfo, string? baseNamesp
     /// <returns>The prefix for resource lookup.</returns>
     protected virtual string GetResourcePrefix(string baseResourceName, string baseNamespace)
     {
-        if (string.IsNullOrEmpty(baseResourceName))
-        {
-            throw new ArgumentNullException(nameof(baseResourceName));
-        }
-
-        if (string.IsNullOrEmpty(baseNamespace))
-        {
-            throw new ArgumentNullException(nameof(baseNamespace));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(baseResourceName);
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(baseNamespace);
 
         var assemblyName = new AssemblyName(baseNamespace);
         var assembly = Assembly.Load(assemblyName);

src/Localization/Localization/src/RootNamespaceAttribute.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using Microsoft.AspNetCore.Shared;
 
 namespace Microsoft.Extensions.Localization;
 
@@ -18,10 +19,7 @@ public class RootNamespaceAttribute : Attribute
     /// <param name="rootNamespace">The RootNamespace for this Assembly.</param>
     public RootNamespaceAttribute(string rootNamespace)
     {
-        if (string.IsNullOrEmpty(rootNamespace))
-        {
-            throw new ArgumentNullException(nameof(rootNamespace));
-        }
+        ArgumentThrowHelper.ThrowIfNullOrEmpty(rootNamespace);
 
         RootNamespace = rootNamespace;
     }

src/Logging.AzureAppServices/src/AzureFileLoggerOptions.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using Microsoft.AspNetCore.Shared;
 
 namespace Microsoft.Extensions.Logging.AzureAppServices;
 
@@ -59,10 +60,8 @@ public string FileName
         get { return _fileName; }
         set
         {
-            if (string.IsNullOrEmpty(value))
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
+            ArgumentThrowHelper.ThrowIfNullOrEmpty(value);
+
             _fileName = value;
         }
     }

src/Logging.AzureAppServices/src/Microsoft.Extensions.Logging.AzureAppServices.csproj

@@ -28,4 +28,11 @@
   <ItemGroup>
     <InternalsVisibleTo Include="DynamicProxyGenAssembly2" Key="$(MoqPublicKey)" />
   </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentThrowHelper.cs" LinkBase="Shared" />
+    <Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentNullThrowHelper.cs" LinkBase="Shared" />
+    <Compile Include="$(SharedSourceRoot)CallerArgument\CallerArgumentExpressionAttribute.cs" LinkBase="Shared" />
+  </ItemGroup>
+
 </Project>

src/Middleware/HttpLogging/src/W3CLoggerOptions.cs

@@ -64,10 +64,7 @@ public string FileName
         get { return _fileName; }
         set
         {
-            if (string.IsNullOrEmpty(value))
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
+            ArgumentException.ThrowIfNullOrEmpty(value);
             _fileName = value;
         }
     }
@@ -83,10 +80,7 @@ public string LogDirectory
         get { return _logDirectory; }
         set
         {
-            if (string.IsNullOrEmpty(value))
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
+            ArgumentException.ThrowIfNullOrEmpty(value);
             _logDirectory = value;
         }
     }

src/Middleware/HttpLogging/test/W3CLoggerOptionsTests.cs

@@ -26,14 +26,14 @@ public void ThrowsOnNegativeFileSizeLimit()
     public void ThrowsOnEmptyFileName()
     {
         var options = new W3CLoggerOptions();
-        Assert.Throws<ArgumentNullException>(() => options.FileName = "");
+        Assert.Throws<ArgumentException>(() => options.FileName = "");
     }
 
     [Fact]
     public void ThrowsOnEmptyLogDirectory()
     {
         var options = new W3CLoggerOptions();
-        Assert.Throws<ArgumentNullException>(() => options.LogDirectory = "");
+        Assert.Throws<ArgumentException>(() => options.LogDirectory = "");
     }
 
     [Fact]

src/Middleware/Rewrite/src/ApacheModRewrite/CookieActionFactory.cs

@@ -16,10 +16,7 @@ internal sealed class CookieActionFactory
     /// <returns>The action</returns>
     public static ChangeCookieAction Create(string flagValue)
     {
-        if (string.IsNullOrEmpty(flagValue))
-        {
-            throw new ArgumentNullException(nameof(flagValue));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(flagValue);
 
         var i = 0;
         var separator = ':';

src/Middleware/Rewrite/src/IISUrlRewrite/IISRewriteMap.cs

@@ -9,10 +9,7 @@ internal sealed class IISRewriteMap
 
     public IISRewriteMap(string name)
     {
-        if (string.IsNullOrEmpty(name))
-        {
-            throw new ArgumentNullException(nameof(name));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(name);
         Name = name;
     }
 
@@ -26,14 +23,8 @@ public string? this[string key]
         }
         set
         {
-            if (string.IsNullOrEmpty(key))
-            {
-                throw new ArgumentNullException(nameof(key));
-            }
-            if (string.IsNullOrEmpty(value))
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
+            ArgumentException.ThrowIfNullOrEmpty(key);
+            ArgumentException.ThrowIfNullOrEmpty(value);
             _map[key] = value;
         }
     }

src/Middleware/Rewrite/src/RedirectRule.cs

@@ -16,15 +16,8 @@ internal sealed class RedirectRule : IRule
     public int StatusCode { get; }
     public RedirectRule(string regex, string replacement, int statusCode)
     {
-        if (string.IsNullOrEmpty(regex))
-        {
-            throw new ArgumentNullException(nameof(regex));
-        }
-
-        if (string.IsNullOrEmpty(replacement))
-        {
-            throw new ArgumentNullException(nameof(replacement));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(regex);
+        ArgumentException.ThrowIfNullOrEmpty(replacement);
 
         InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout);
         Replacement = replacement;

src/Middleware/Rewrite/src/RewriteRule.cs

@@ -16,15 +16,8 @@ internal sealed class RewriteRule : IRule
     public bool StopProcessing { get; }
     public RewriteRule(string regex, string replacement, bool stopProcessing)
     {
-        if (string.IsNullOrEmpty(regex))
-        {
-            throw new ArgumentNullException(nameof(regex));
-        }
-
-        if (string.IsNullOrEmpty(replacement))
-        {
-            throw new ArgumentNullException(nameof(replacement));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(regex);
+        ArgumentException.ThrowIfNullOrEmpty(replacement);
 
         InitialMatch = new Regex(regex, RegexOptions.Compiled | RegexOptions.CultureInvariant, _regexTimeout);
         Replacement = replacement;

src/Middleware/Rewrite/src/UrlActions/ChangeCookieAction.cs

@@ -18,13 +18,9 @@ public ChangeCookieAction(string name)
     // for testing
     internal ChangeCookieAction(string name, Func<DateTimeOffset> timeSource)
     {
-        _timeSource = timeSource;
-
-        if (string.IsNullOrEmpty(name))
-        {
-            throw new ArgumentNullException(nameof(name));
-        }
+        ArgumentException.ThrowIfNullOrEmpty(name);
 
+        _timeSource = timeSource;
         Name = name;
     }
 

src/Middleware/StaticFiles/src/StaticFileContext.cs

@@ -45,7 +45,7 @@ public StaticFileContext(HttpContext context, StaticFileOptions options, ILogger
     {
         if (subPath.Value == null)
         {
-            throw new ArgumentNullException(nameof(subPath));
+            throw new ArgumentException($"{nameof(subPath)} cannot wrap a null value.", nameof(subPath));
         }
 
         _context = context;