Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix argument exception warnings on runtime #35717

Merged
merged 9 commits into from
May 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ csharp_space_between_square_brackets = false

# Analyzers
dotnet_code_quality.ca1802.api_surface = private, internal
dotnet_code_quality.CA2208.api_surface = public

# C++ Files
[*.{cpp,h,in}]
Expand Down
2 changes: 1 addition & 1 deletion eng/CodeAnalysis.ruleset
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
<Rule Id="CA2200" Action="Warning" /> <!-- Rethrow to preserve stack details. -->
<Rule Id="CA2201" Action="None" /> <!-- Do not raise reserved exception types -->
<Rule Id="CA2207" Action="Warning" /> <!-- Initialize value type static fields inline -->
<Rule Id="CA2208" Action="None" /> <!-- Instantiate argument exceptions correctly -->
<Rule Id="CA2208" Action="Info" /> <!-- Instantiate argument exceptions correctly -->
<Rule Id="CA2211" Action="None" /> <!-- Non-constant fields should not be visible -->
<Rule Id="CA2213" Action="None" /> <!-- Disposable fields should be disposed -->
<Rule Id="CA2214" Action="None" /> <!-- Do not call overridable methods in constructors -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public static object GetClassFactoryForType(ComActivationContext cxt)

if (!Path.IsPathRooted(cxt.AssemblyPath))
{
throw new ArgumentException();
throw new ArgumentException(null, nameof(cxt));
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
}

Type classType = FindClassType(cxt.ClassId, cxt.AssemblyPath, cxt.AssemblyName, cxt.TypeName);
Expand Down Expand Up @@ -156,7 +156,7 @@ public static void ClassRegistrationScenarioForType(ComActivationContext cxt, bo

if (!Path.IsPathRooted(cxt.AssemblyPath))
{
throw new ArgumentException();
throw new ArgumentException(null, nameof(cxt));
}

Type classType = FindClassType(cxt.ClassId, cxt.AssemblyPath, cxt.AssemblyName, cxt.TypeName);
Expand Down Expand Up @@ -288,7 +288,7 @@ public static unsafe int RegisterClassForTypeInternal(ComActivationContextIntern
if (cxtInt.InterfaceId != Guid.Empty
|| cxtInt.ClassFactoryDest != IntPtr.Zero)
{
throw new ArgumentException();
throw new ArgumentException(null, nameof(pCxtInt));
}

try
Expand Down Expand Up @@ -328,7 +328,7 @@ public static unsafe int UnregisterClassForTypeInternal(ComActivationContextInte
if (cxtInt.InterfaceId != Guid.Empty
|| cxtInt.ClassFactoryDest != IntPtr.Zero)
{
throw new ArgumentException();
throw new ArgumentException(null, nameof(pCxtInt));
}

try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ public TypedReference GetNextArg(RuntimeTypeHandle rth)
// malicious caller to increment the pointer to an arbitrary
// location in memory and read the contents.
if (ArgPtr == IntPtr.Zero)
#pragma warning disable CA2208 // Instantiate argument exceptions correctly, the argument not applicable
Copy link
Contributor Author

@buyaa-n buyaa-n May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is not related to argument, might better to use InvalidOperationException, suppressing for now

throw new ArgumentNullException();
#pragma warning restore CA2208

TypedReference result = default;
// reference to TypedReference is banned, so have to pass result as pointer
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/System.Private.CoreLib/src/System/GC.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public static void AddMemoryPressure(long bytesAllocated)

if ((4 == IntPtr.Size) && (bytesAllocated > int.MaxValue))
{
throw new ArgumentOutOfRangeException("pressure",
throw new ArgumentOutOfRangeException(nameof(bytesAllocated),
SR.ArgumentOutOfRange_MustBeNonNegInt32);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public IntPtr GetOrCreateComInterfaceForObject(object instance, CreateComInterfa
{
IntPtr ptr;
if (!TryGetOrCreateComInterfaceForObjectInternal(this, instance, flags, out ptr))
throw new ArgumentException();
throw new ArgumentException(null, nameof(instance));

return ptr;
}
Expand Down Expand Up @@ -187,7 +187,7 @@ public object GetOrCreateObjectForComInstance(IntPtr externalComObject, CreateOb
{
object? obj;
if (!TryGetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, null, out obj))
throw new ArgumentNullException();
throw new ArgumentNullException(nameof(externalComObject));

return obj!;
}
Expand Down Expand Up @@ -230,7 +230,7 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create

object? obj;
if (!TryGetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, wrapper, out obj))
throw new ArgumentNullException();
throw new ArgumentNullException(nameof(externalComObject));

return obj!;
}
Expand Down Expand Up @@ -319,4 +319,4 @@ internal static int CallICustomQueryInterface(object customQueryInterfaceMaybe,
return (int)customQueryInterface.GetInterface(ref iid, out ppObject);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2752,7 +2752,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)
protected override PropertyInfo? GetPropertyImpl(
string name, BindingFlags bindingAttr, Binder? binder, Type? returnType, Type[]? types, ParameterModifier[]? modifiers)
{
if (name == null) throw new ArgumentNullException();
if (name == null) throw new ArgumentNullException(nameof(name));

ListBuilder<PropertyInfo> candidates = GetPropertyCandidates(name, bindingAttr, types, false);

Expand Down Expand Up @@ -2788,7 +2788,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)

public override EventInfo? GetEvent(string name, BindingFlags bindingAttr)
{
if (name is null) throw new ArgumentNullException();
if (name is null) throw new ArgumentNullException(nameof(name));

FilterHelper(bindingAttr, ref name, out _, out MemberListType listType);

Expand All @@ -2814,7 +2814,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)

public override FieldInfo? GetField(string name, BindingFlags bindingAttr)
{
if (name is null) throw new ArgumentNullException();
if (name is null) throw new ArgumentNullException(nameof(name));

FilterHelper(bindingAttr, ref name, out _, out MemberListType listType);

Expand Down Expand Up @@ -2851,7 +2851,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)

public override Type? GetInterface(string fullname, bool ignoreCase)
{
if (fullname is null) throw new ArgumentNullException();
if (fullname is null) throw new ArgumentNullException(nameof(fullname));

BindingFlags bindingAttr = BindingFlags.Public | BindingFlags.NonPublic;

Expand Down Expand Up @@ -2885,7 +2885,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)

public override Type? GetNestedType(string fullname, BindingFlags bindingAttr)
{
if (fullname is null) throw new ArgumentNullException();
if (fullname is null) throw new ArgumentNullException(nameof(fullname));

bindingAttr &= ~BindingFlags.Static;
string name, ns;
Expand Down Expand Up @@ -2913,7 +2913,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)

public override MemberInfo[] GetMember(string name, MemberTypes type, BindingFlags bindingAttr)
{
if (name is null) throw new ArgumentNullException();
if (name is null) throw new ArgumentNullException(nameof(name));

ListBuilder<MethodInfo> methods = default;
ListBuilder<ConstructorInfo> constructors = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ private static extern long PendingUnmanagedWorkItemCount
get;
}

private static RegisteredWaitHandle RegisterWaitForSingleObject( // throws RegisterWaitException
private static RegisteredWaitHandle RegisterWaitForSingleObject(
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
WaitHandle waitObject,
WaitOrTimerCallback callBack,
object? state,
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public ArrayBufferWriter()
public ArrayBufferWriter(int initialCapacity)
{
if (initialCapacity <= 0)
throw new ArgumentException(nameof(initialCapacity));
throw new ArgumentException(null, nameof(initialCapacity));

_buffer = new T[initialCapacity];
_index = 0;
Expand Down Expand Up @@ -101,7 +101,7 @@ public void Clear()
public void Advance(int count)
{
if (count < 0)
throw new ArgumentException(nameof(count));
throw new ArgumentException(null, nameof(count));

if (_index > _buffer.Length - count)
ThrowInvalidOperationException_AdvancedTooFar(_buffer.Length);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/System/Threading/Tasks/TaskToApm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static void End(IAsyncResult asyncResult)
return;
}

throw new ArgumentNullException();
throw new ArgumentNullException(nameof(asyncResult));
}

/// <summary>Processes an IAsyncResult returned by Begin.</summary>
Expand All @@ -55,7 +55,7 @@ public static TResult End<TResult>(IAsyncResult asyncResult)
return task.GetAwaiter().GetResult();
}

throw new ArgumentNullException();
throw new ArgumentNullException(nameof(asyncResult));
}

/// <summary>Provides a simple IAsyncResult that wraps a Task.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ public ChainedConfigurationProvider(ChainedConfigurationSource source)
}
if (source.Configuration == null)
{
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentNullException(nameof(source.Configuration));
#pragma warning restore CA2208
}

_config = source.Configuration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ public Dependency(string name, string version)
{
if (string.IsNullOrEmpty(name))
{
throw new ArgumentException(nameof(name));
throw new ArgumentException(null, nameof(name));
}
if (string.IsNullOrEmpty(version))
{
throw new ArgumentException(nameof(version));
throw new ArgumentException(null, nameof(version));
}
Name = name;
Version = version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ public Library(string type,
{
if (string.IsNullOrEmpty(type))
{
throw new ArgumentException(nameof(type));
throw new ArgumentException(null, nameof(type));
}
if (string.IsNullOrEmpty(name))
{
throw new ArgumentException(nameof(name));
throw new ArgumentException(null, nameof(name));
}
if (string.IsNullOrEmpty(version))
{
throw new ArgumentException(nameof(version));
throw new ArgumentException(null, nameof(version));
}
if (dependencies == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ public ResourceAssembly(string path, string locale)
{
if (string.IsNullOrEmpty(path))
{
throw new ArgumentException(nameof(path));
throw new ArgumentException(null, nameof(path));
}
if (string.IsNullOrEmpty(locale))
{
throw new ArgumentException(nameof(locale));
throw new ArgumentException(null, nameof(locale));
}
Locale = locale;
Path = path;
Expand All @@ -27,4 +27,4 @@ public ResourceAssembly(string path, string locale)
public string Path { get; set; }

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ public RuntimeAssembly(string assemblyName, string path)
{
if (string.IsNullOrEmpty(assemblyName))
{
throw new ArgumentException(nameof(assemblyName));
throw new ArgumentException(null, nameof(assemblyName));
}
if (string.IsNullOrEmpty(path))
{
throw new ArgumentException(nameof(path));
throw new ArgumentException(null, nameof(path));
}
_assemblyName = assemblyName;
Path = path;
Expand All @@ -45,4 +45,4 @@ public static RuntimeAssembly Create(string path)
return new RuntimeAssembly(assemblyName, path);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public RuntimeFallbacks(string runtime, IEnumerable<string> fallbacks)
{
if (string.IsNullOrEmpty(runtime))
{
throw new ArgumentException(nameof(runtime));
throw new ArgumentException(null, nameof(runtime));
}
if (fallbacks == null)
{
Expand All @@ -28,4 +28,4 @@ public RuntimeFallbacks(string runtime, IEnumerable<string> fallbacks)
Fallbacks = fallbacks.ToArray();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public RuntimeFile(string path, string assemblyVersion, string fileVersion)
{
if (string.IsNullOrEmpty(path))
{
throw new ArgumentException(nameof(path));
throw new ArgumentException(null, nameof(path));
}

Path = path;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public TargetInfo(string framework,
{
if (string.IsNullOrEmpty(framework))
{
throw new ArgumentException(nameof(framework));
throw new ArgumentException(null, nameof(framework));
}

Framework = framework;
Expand All @@ -32,4 +32,4 @@ public TargetInfo(string framework,
public bool IsPortable { get; }

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected DesignerOptionCollection CreateOptionCollection(DesignerOptionCollecti

if (name.Length == 0)
{
throw new ArgumentException(SR.Format(SR.InvalidArgumentValue, name.Length.ToString(), "0"), "name.Length");
throw new ArgumentException(SR.Format(SR.InvalidArgumentValue, "name.Length", "0"), nameof(name));
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
}

return new DesignerOptionCollection(this, parent, name, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ public MemberRelationship this[MemberRelationship source]
// and not the other as the main constructor performs argument validation.
if (source.Owner == null)
{
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentNullException(nameof(MemberRelationship.Owner));
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
#pragma warning restore CA2208
}

Debug.Assert(source.Member != null);
Expand All @@ -57,7 +59,9 @@ public MemberRelationship this[MemberRelationship source]
// and not the other as the main constructor performs argument validation.
if (source.Owner == null)
{
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
throw new ArgumentNullException(nameof(MemberRelationship.Owner));
#pragma warning restore CA2208
}

Debug.Assert(source.Member != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void CreateOptionCollection_NullName_ThrowsArgumentNullException()
public void CreateOptionCollection_EmptyName_ThrowsArgumentException()
{
var service = new TestDesignerOptionService();
AssertExtensions.Throws<ArgumentException>("name.Length", () => service.DoCreateOptionCollection(service.Options, string.Empty, "value"));
AssertExtensions.Throws<ArgumentException>("name", () => service.DoCreateOptionCollection(service.Options, string.Empty, "value"));
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public CounterSetInstance CreateCounterSetInstance(string instanceName)
{
if (_provider == null)
{
throw new ArgumentException(SR.Format(SR.Perflib_Argument_ProviderNotFound, _providerGuid), "ProviderGuid");
throw new ArgumentException(SR.Format(SR.Perflib_Argument_ProviderNotFound, _providerGuid), nameof(instanceName));
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
}
if (_provider._hProvider.IsInvalid)
{
Expand Down Expand Up @@ -256,7 +256,7 @@ public CounterSetInstance CreateCounterSetInstance(string instanceName)
{
throw Status switch
{
(uint)Interop.Errors.ERROR_ALREADY_EXISTS => new ArgumentException(SR.Format(SR.Perflib_Argument_CounterSetAlreadyRegister, _counterSet), "CounterSetGuid"),
(uint)Interop.Errors.ERROR_ALREADY_EXISTS => new ArgumentException(SR.Format(SR.Perflib_Argument_CounterSetAlreadyRegister, _counterSet), nameof(instanceName)),
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved

_ => new Win32Exception((int)Status),
};
Expand Down
Loading