Fix a couple of issues in UseExceptionThrowHelpers (#6396)

* Fix a couple of issues in UseExceptionThrowHelpers

Based on additional false positives discovered while enabling the rules in dotnet/runtime:
- We don't want to warn to use ObjectDisposedException.ThrowIf when inside of a struct, as doing so is likely to lead to additional costs (e.g. boxing).
- We don't want to warn to use ArgumentXxException.ThrowIfXx when the code to compute the argument name is more complicated than just a literal / nameof, e.g. a method call that determines what parameter name should be employed.

* Address PR feedback

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseExceptionThrowHelpers.cs

@@ -181,7 +181,8 @@ aooreThrowIfGreaterThan is not null || aooreThrowIfGreaterThanOrEqual is not nul
                     {
                         if (aneThrowIfNull is not null &&
                             IsParameterNullCheck(condition.Condition, out IParameterReferenceOperation? nullCheckParameter) &&
-                            nullCheckParameter.Type.IsReferenceType)
+                            nullCheckParameter.Type.IsReferenceType &&
+                            HasReplaceableArgumentName(objectCreationOperation, 0))
                         {
                             context.ReportDiagnostic(condition.CreateDiagnostic(
                                 UseArgumentNullExceptionThrowIfNullRule,
@@ -197,7 +198,8 @@ aooreThrowIfGreaterThan is not null || aooreThrowIfGreaterThanOrEqual is not nul
                     if (SymbolEqualityComparer.Default.Equals(objectCreationOperation.Type, ae))
                     {
                         if (aeThrowIfNullOrEmpty is not null &&
-                            IsNullOrEmptyCheck(stringIsNullOrEmpty, stringLength, stringEmpty, condition.Condition, out IParameterReferenceOperation? nullOrEmptyCheckParameter))
+                            IsNullOrEmptyCheck(stringIsNullOrEmpty, stringLength, stringEmpty, condition.Condition, out IParameterReferenceOperation? nullOrEmptyCheckParameter) &&
+                            HasReplaceableArgumentName(objectCreationOperation, 1))
                         {
                             context.ReportDiagnostic(condition.CreateDiagnostic(
                                 UseArgumentExceptionThrowIfNullOrEmptyRule,
@@ -218,7 +220,8 @@ aooreThrowIfGreaterThan is not null || aooreThrowIfGreaterThanOrEqual is not nul
                     // Handle ArgumentOutOfRangeException.ThrowIfLessThanOrEqual
                     if (SymbolEqualityComparer.Default.Equals(objectCreationOperation.Type, aoore))
                     {
-                        if (hasAnyAooreThrow)
+                        if (hasAnyAooreThrow &&
+                            HasReplaceableArgumentName(objectCreationOperation, 0))
                         {
                             ImmutableArray<Location> additionalLocations = ImmutableArray<Location>.Empty;
 
@@ -251,7 +254,12 @@ static bool AvoidComparing(IParameterReferenceOperation p) =>
                     // Handle ObjectDisposedException.ThrowIf.
                     if (SymbolEqualityComparer.Default.Equals(objectCreationOperation.Type, ode))
                     {
-                        if (odeThrowIf is not null)
+                        // If we have ObjectDisposedException.ThrowIf and if this operation is in a reference type, issue a diagnostic.
+                        // We check whether the containing type is a reference type because we want to avoid passing `this` at the call
+                        // site to ThrowIf for a struct as that will box, and we want to avoid using `GetType()` at the call site as
+                        // that adds additional cost prior to the guard check.
+                        if (odeThrowIf is not null &&
+                            context.ContainingSymbol.ContainingType.IsReferenceType)
                         {
                             // We always report a diagnostic. However, the fixer is only currently provided in the case
                             // of the argument to the ObjectDisposedException constructor containing a call to {something.}GetType().{Full}Name,
@@ -282,6 +290,19 @@ nameReference.Instance is IInvocationOperation getTypeCall &&
                     }
                 }, OperationKind.Throw);
 
+                // As a heuristic, we only want to replace throws with ThrowIfNull if either there isn't currently
+                // a specified parameter name, e.g. the parameterless constructor was used, or if it's specified as a
+                // constant, e.g. a nameof or a literal string.  This is primarily to avoid false positives
+                // with complicated expressions for computing the parameter name to use, which with ThrowIfNull would
+                // need to be done prior to the guard check, and thus something we want to avoid.
+                bool HasReplaceableArgumentName(IObjectCreationOperation creationOperation, int argumentIndex)
+                {
+                    ImmutableArray<IArgumentOperation> args = creationOperation.Arguments;
+                    return
+                        argumentIndex >= args.Length ||
+                        args.GetArgumentForParameterAtIndex(argumentIndex).Value.ConstantValue.HasValue;
+                }
+
                 // As a heuristic, we avoid issuing diagnostics if there are additional arguments (e.g. message)
                 // to the exception that could be useful.
                 bool HasPossiblyMeaningfulAdditionalArguments(IObjectCreationOperation objectCreationOperation)

src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/UseExceptionThrowHelpersTests.cs

@@ -100,12 +100,12 @@ void M(string arg)
         {|CA1510:if (arg is null)
             throw new ArgumentNullException(nameof(arg));|}
         {|CA1510:if (arg == null)
-            throw new ArgumentNullException(nameof(arg), (string)null);|}
+            throw new ArgumentNullException(""arg"", (string)null);|}
         {|CA1510:if (null == arg)
             throw new ArgumentNullException(nameof(arg));|}
         {|CA1510:if (arg is null)
         {
-            throw new ArgumentNullException(nameof(arg));
+            throw new ArgumentNullException(""arg"");
         }|}
         {|CA1510:if (arg == null)
         {
@@ -165,9 +165,20 @@ void M(string arg)
             Console.WriteLine(arg);
         }
 
+        if (arg is null)
+            throw new ArgumentNullException(ComputeName(nameof(arg)));
+
+        if (arg is null)
+            throw new ArgumentNullException(innerException: null, paramName: ComputeName(nameof(arg)));
+
+        if (arg is null)
+            throw new ArgumentNullException(IntPtr.Size == 8 ? ""arg"" : ""arg"");
+
         throw new ArgumentNullException(nameof(arg)); // no guard
     }
 
+    static string ComputeName(string arg) => arg;
+
     string this[string name]
     {
         get
@@ -282,9 +293,20 @@ void M(string arg)
             Console.WriteLine(arg);
         }
 
+        if (arg is null)
+            throw new ArgumentNullException(ComputeName(nameof(arg)));
+
+        if (arg is null)
+            throw new ArgumentNullException(innerException: null, paramName: ComputeName(nameof(arg)));
+
+        if (arg is null)
+            throw new ArgumentNullException(IntPtr.Size == 8 ? ""arg"" : ""arg"");
+
         throw new ArgumentNullException(nameof(arg)); // no guard
     }
 
+    static string ComputeName(string arg) => arg;
+
     string this[string name]
     {
         get
@@ -980,6 +1002,17 @@ string Prop
         }
     }
 }
+
+struct S
+{
+    private bool IsDisposed { get; set; }
+
+    void M()
+    {
+        if (IsDisposed) throw new ObjectDisposedException(null);
+        if (IsDisposed) throw new ObjectDisposedException(this.GetType().FullName);
+    }
+}
 ",
                 FixedCode =
 @"
@@ -1033,6 +1066,17 @@ string Prop
         }
     }
 }
+
+struct S
+{
+    private bool IsDisposed { get; set; }
+
+    void M()
+    {
+        if (IsDisposed) throw new ObjectDisposedException(null);
+        if (IsDisposed) throw new ObjectDisposedException(this.GetType().FullName);
+    }
+}
 "
             };
             test.FixedState.MarkupHandling = CodeAnalysis.Testing.MarkupMode.Allow;