Skip to content

Commit

Permalink
Some more automated C# modernization in corelib (#105151)
Browse files Browse the repository at this point in the history 
* Fix IDE0056 on corelib (indexing can be simplified)

* Fix IDE (null check can be simplified)

* Fix IDE0078 (use pattern matching)

* Fix IDE0019 (use pattern matching)

* Fix IDE0066 (use switch expression)

* Fix IDE0250 (struct can be made readonly)

* Fix nullability warning and address PR feedback

* Address PR feedback and revert a downlevel change

* Wrap any `?? throw new`s that go beyond 120 characters
  • Loading branch information
@stephentoub
stephentoub committed Jul 25, 2024
1 parent 72c9b85 commit 94c356d
Show file tree
Hide file tree
Showing 104 changed files with 551 additions and 1,038 deletions.
6 changes: 1 addition & 5 deletions src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,11 +842,7 @@ public string RequestLicKey(Type type)

((IDisposable?)parameters[2])?.Dispose();

var licenseKey = (string?)parameters[3];
if (licenseKey == null)
{
throw new COMException(); // E_FAIL
}
var licenseKey = (string?)parameters[3] ?? throw new COMException(); // E_FAIL

return licenseKey;
}
Expand Down
3 changes: 1 addition & 2 deletions ...clr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/InMemoryAssemblyLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ public static unsafe void LoadInMemoryAssemblyInContext(IntPtr moduleHandle, Int
[RequiresUnreferencedCode("C++/CLI is not trim-compatible", Url = "https://aka.ms/dotnet-illink/nativehost")]
private static void LoadInMemoryAssemblyInContextImpl(IntPtr moduleHandle, IntPtr assemblyPath, AssemblyLoadContext? alc = null)
{
string? assemblyPathString = Marshal.PtrToStringUni(assemblyPath);
if (assemblyPathString == null)
string assemblyPathString = Marshal.PtrToStringUni(assemblyPath) ??
throw new ArgumentOutOfRangeException(nameof(assemblyPath));

// We don't cache the ALCs or resolvers here since each IJW assembly will call this method at most once
Expand Down
123 changes: 64 additions & 59 deletions src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al

if (target.ContainsGenericParameters)
throw new ArgumentException(SR.Arg_UnboundGenParam, nameof(target));
if (!(target is RuntimeType rtTarget))
if (target is not RuntimeType rtTarget)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(target));

// This API existed in v1/v1.1 and only expected to create open
Expand All @@ -85,7 +85,6 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al
return invoke.Invoke(this, BindingFlags.Default, null, args, null);
}


public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj == null || !InternalEqualTypes(this, obj))
Expand All @@ -107,9 +106,11 @@ public override bool Equals([NotNullWhen(true)] object? obj)
{
if (d._methodPtrAux != IntPtr.Zero)
return false; // different delegate kind

// they are both closed over the first arg
if (_target != d._target)
return false;

// fall through method handle check
}
else
Expand All @@ -121,19 +122,20 @@ public override bool Equals([NotNullWhen(true)] object? obj)
/*
if (_methodPtr != d._methodPtr)
return false;
*/
*/

if (_methodPtrAux == d._methodPtrAux)
return true;

// fall through method handle check
}

// method ptrs don't match, go down long path
//
if (_methodBase == null || d._methodBase == null || !(_methodBase is MethodInfo) || !(d._methodBase is MethodInfo))
return InternalEqualMethodHandles(this, d);
else

if (_methodBase is MethodInfo && d._methodBase is MethodInfo)
return _methodBase.Equals(d._methodBase);
else
return InternalEqualMethodHandles(this, d);
}

public override int GetHashCode()
Expand All @@ -156,60 +158,63 @@ public override int GetHashCode()

protected virtual MethodInfo GetMethodImpl()
{
if ((_methodBase == null) || !(_methodBase is MethodInfo))
if (_methodBase is MethodInfo methodInfo)
{
IRuntimeMethodInfo method = FindMethodHandle();
RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);
// need a proper declaring type instance method on a generic type
if (declaringType.IsGenericType)
return methodInfo;
}

IRuntimeMethodInfo method = FindMethodHandle();
RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);

// need a proper declaring type instance method on a generic type
if (declaringType.IsGenericType)
{
bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
if (!isStatic)
{
bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
if (!isStatic)
if (_methodPtrAux == IntPtr.Zero)
{
if (_methodPtrAux == IntPtr.Zero)
// The target may be of a derived type that doesn't have visibility onto the
// target method. We don't want to call RuntimeType.GetMethodBase below with that
// or reflection can end up generating a MethodInfo where the ReflectedType cannot
// see the MethodInfo itself and that breaks an important invariant. But the
// target type could include important generic type information we need in order
// to work out what the exact instantiation of the method's declaring type is. So
// we'll walk up the inheritance chain (which will yield exactly instantiated
// types at each step) until we find the declaring type. Since the declaring type
// we get from the method is probably shared and those in the hierarchy we're
// walking won't be we compare using the generic type definition forms instead.
Type targetType = declaringType.GetGenericTypeDefinition();
Type? currentType;
for (currentType = _target!.GetType(); currentType != null; currentType = currentType.BaseType)
{
// The target may be of a derived type that doesn't have visibility onto the
// target method. We don't want to call RuntimeType.GetMethodBase below with that
// or reflection can end up generating a MethodInfo where the ReflectedType cannot
// see the MethodInfo itself and that breaks an important invariant. But the
// target type could include important generic type information we need in order
// to work out what the exact instantiation of the method's declaring type is. So
// we'll walk up the inheritance chain (which will yield exactly instantiated
// types at each step) until we find the declaring type. Since the declaring type
// we get from the method is probably shared and those in the hierarchy we're
// walking won't be we compare using the generic type definition forms instead.
Type? currentType = _target!.GetType();
Type targetType = declaringType.GetGenericTypeDefinition();
while (currentType != null)
if (currentType.IsGenericType &&
currentType.GetGenericTypeDefinition() == targetType)
{
if (currentType.IsGenericType &&
currentType.GetGenericTypeDefinition() == targetType)
{
declaringType = currentType as RuntimeType;
break;
}
currentType = currentType.BaseType;
declaringType = currentType as RuntimeType;
break;
}

// RCWs don't need to be "strongly-typed" in which case we don't find a base type
// that matches the declaring type of the method. This is fine because interop needs
// to work with exact methods anyway so declaringType is never shared at this point.
// The targetType may also be an interface with a Default interface method (DIM).
Debug.Assert(
currentType != null
|| _target.GetType().IsCOMObject
|| targetType.IsInterface, "The class hierarchy should declare the method or be a DIM");
}
else
{
// it's an open one, need to fetch the first arg of the instantiation
MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
}

// RCWs don't need to be "strongly-typed" in which case we don't find a base type
// that matches the declaring type of the method. This is fine because interop needs
// to work with exact methods anyway so declaringType is never shared at this point.
// The targetType may also be an interface with a Default interface method (DIM).
Debug.Assert(
currentType != null
|| _target.GetType().IsCOMObject
|| targetType.IsInterface, "The class hierarchy should declare the method or be a DIM");
}
else
{
// it's an open one, need to fetch the first arg of the instantiation
MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
}
}
_methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
}

_methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
return (MethodInfo)_methodBase;
}

Expand All @@ -223,7 +228,7 @@ protected virtual MethodInfo GetMethodImpl()
ArgumentNullException.ThrowIfNull(target);
ArgumentNullException.ThrowIfNull(method);

if (!(type is RuntimeType rtType))
if (type is not RuntimeType rtType)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(type));
if (!rtType.IsDelegate())
throw new ArgumentException(SR.Arg_MustBeDelegate, nameof(type));
Expand Down Expand Up @@ -260,9 +265,9 @@ protected virtual MethodInfo GetMethodImpl()

if (target.ContainsGenericParameters)
throw new ArgumentException(SR.Arg_UnboundGenParam, nameof(target));
if (!(type is RuntimeType rtType))
if (type is not RuntimeType rtType)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(type));
if (!(target is RuntimeType rtTarget))
if (target is not RuntimeType rtTarget)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(target));

if (!rtType.IsDelegate())
Expand Down Expand Up @@ -293,10 +298,10 @@ protected virtual MethodInfo GetMethodImpl()
ArgumentNullException.ThrowIfNull(type);
ArgumentNullException.ThrowIfNull(method);

if (!(type is RuntimeType rtType))
if (type is not RuntimeType rtType)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(type));

if (!(method is RuntimeMethodInfo rmi))
if (method is not RuntimeMethodInfo rmi)
throw new ArgumentException(SR.Argument_MustBeRuntimeMethodInfo, nameof(method));

if (!rtType.IsDelegate())
Expand Down Expand Up @@ -328,10 +333,10 @@ protected virtual MethodInfo GetMethodImpl()
ArgumentNullException.ThrowIfNull(type);
ArgumentNullException.ThrowIfNull(method);

if (!(type is RuntimeType rtType))
if (type is not RuntimeType rtType)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(type));

if (!(method is RuntimeMethodInfo rmi))
if (method is not RuntimeMethodInfo rmi)
throw new ArgumentException(SR.Argument_MustBeRuntimeMethodInfo, nameof(method));

if (!rtType.IsDelegate())
Expand Down Expand Up @@ -366,7 +371,7 @@ internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target
if (method.IsNullHandle())
throw new ArgumentNullException(nameof(method));

if (!(type is RuntimeType rtType))
if (type is not RuntimeType rtType)
throw new ArgumentException(SR.Argument_MustBeRuntimeType, nameof(type));

if (!rtType.IsDelegate())
Expand Down
39 changes: 21 additions & 18 deletions src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ protected sealed override Delegate CombineImpl(Delegate? follow)
followCount = (int)dFollow._invocationCount;

int resultCount;
if (!(_invocationList is object[] invocationList))
if (_invocationList is not object[] invocationList)
{
resultCount = 1 + followCount;
resultList = new object[resultCount];
Expand Down Expand Up @@ -335,9 +335,9 @@ private static bool EqualInvocationLists(object[] a, object[] b, int start, int

if (v == null)
return this;
if (!(v._invocationList is object[]))
if (v._invocationList is not object[])
{
if (!(_invocationList is object[] invocationList))
if (_invocationList is not object[] invocationList)
{
// they are both not real Multicast
if (this.Equals(value))
Expand Down Expand Up @@ -401,7 +401,7 @@ private static bool EqualInvocationLists(object[] a, object[] b, int start, int
public sealed override Delegate[] GetInvocationList()
{
Delegate[] del;
if (!(_invocationList is object[] invocationList))
if (_invocationList is not object[] invocationList)
{
del = new Delegate[1];
del[0] = this;
Expand All @@ -418,12 +418,12 @@ public sealed override Delegate[] GetInvocationList()
return del;
}

internal new bool HasSingleTarget => !(_invocationList is object[]);
internal new bool HasSingleTarget => _invocationList is not object[];

// Used by delegate invocation list enumerator
internal object? /* Delegate? */ TryGetAt(int index)
{
if (!(_invocationList is object[] invocationList))
if (_invocationList is not object[] invocationList)
{
return (index == 0) ? this : null;
}
Expand All @@ -447,7 +447,7 @@ public sealed override int GetHashCode()
}
}

if (!(_invocationList is object[] invocationList))
if (_invocationList is not object[] invocationList)
{
return base.GetHashCode();
}
Expand Down Expand Up @@ -515,20 +515,23 @@ protected override MethodInfo GetMethodImpl()
{
// we handle unmanaged function pointers here because the generic ones (used for WinRT) would otherwise
// be treated as open delegates by the base implementation, resulting in failure to get the MethodInfo
if ((_methodBase == null) || !(_methodBase is MethodInfo))
if (_methodBase is MethodInfo methodInfo)
{
IRuntimeMethodInfo method = FindMethodHandle();
RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);
return methodInfo;
}

// need a proper declaring type instance method on a generic type
if (declaringType.IsGenericType)
{
// we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
RuntimeType reflectedType = (RuntimeType)GetType();
declaringType = reflectedType;
}
_methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
IRuntimeMethodInfo method = FindMethodHandle();
RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);

// need a proper declaring type instance method on a generic type
if (declaringType.IsGenericType)
{
// we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
RuntimeType reflectedType = (RuntimeType)GetType();
declaringType = reflectedType;
}

_methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
return (MethodInfo)_methodBase;
}

Expand Down
Loading

0 comments on commit 94c356d

Please sign in to comment.