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 ConvertTypeCheckPatternToNullCheck #110973

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ private static Attribute[] InternalParamGetCustomAttributes(ParameterInfo param,
count = 0;
for (int i = 0; i < objAttr.Length; i++)
{
if (objAttr[i] is object attr)
if (objAttr[i] is { } attr)
Copy link
Member

Choose a reason for hiding this comment

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

This is not readable. In the initial version of pattern matching before is not null, is { } is rejected by many projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot use is not null with declaration pattern, i.e. if (objAttr[i] is not null attr) is invalid C#.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
if (objAttr[i] is { } attr)
if (objAttr[i] is not null and var attr)

Copy link
Member

@jkotas jkotas Dec 29, 2024

Choose a reason for hiding this comment

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

Yes, it is unfortunate that there are number of more or less cryptic ways one can express a null check in modern C#. The choice between them is a matter of personal preference. My personal preference is the old fashioned:

object? attr = objAttr[i];
if (attr != null)

I believe that it is the best balance of readable and succinct. (Except for the few types that overload operator==. It is fine to use more verbose is not null in those cases for performance reasons.)

{
attributes[count] = (Attribute)attr;
count++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ private static bool TrySetSlot(object?[] a, int index, object o)

// The slot may be already set because we have added and removed the same method before.
// Optimize this case, because it's cheaper than copying the array.
if (a[index] is object ai)
if (a[index] is { } ai)
{
MulticastDelegate d = (MulticastDelegate)o;
MulticastDelegate dd = (MulticastDelegate)ai;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public static void Run(Action action, CancellationToken cancellationToken)
// an OperationCanceledException, preserving stack trace details from the ThreadAbortException in
// order to aid in diagnostics and debugging.
OperationCanceledException e = cancellationToken.IsCancellationRequested ? new(cancellationToken) : new();
if (tae.StackTrace is string stackTrace)
if (tae.StackTrace is { } stackTrace)
{
ExceptionDispatchInfo.SetRemoteStackTrace(e, stackTrace);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ internal static object CreateInstanceForAnotherGenericParameter(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] RuntimeType type,
RuntimeType genericParameter)
{
Debug.Assert(type.GetConstructor(Type.EmptyTypes) is ConstructorInfo c && c.IsPublic,
Debug.Assert(type.GetConstructor(Type.EmptyTypes) is { } c && c.IsPublic,
$"CreateInstanceForAnotherGenericParameter requires {nameof(type)} to have a public parameterless constructor so it can be annotated for trimming without preserving private constructors.");

object? instantiatedObject = null;
Expand All @@ -239,7 +239,7 @@ internal static object CreateInstanceForAnotherGenericParameter(
RuntimeType genericParameter1,
RuntimeType genericParameter2)
{
Debug.Assert(type.GetConstructor(Type.EmptyTypes) is ConstructorInfo c && c.IsPublic,
Debug.Assert(type.GetConstructor(Type.EmptyTypes) is { } c && c.IsPublic,
$"CreateInstanceForAnotherGenericParameter requires {nameof(type)} to have a public parameterless constructor so it can be annotated for trimming without preserving private constructors.");

object? instantiatedObject = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2383,7 +2383,7 @@ private static bool FilterApplyMethodBase(
for (int i = 0; i < parameterInfos.Length; i++)
{
// a null argument type implies a null arg which is always a perfect match
if (argumentTypes[i] is Type t && !t.MatchesParameterTypeExactly(parameterInfos[i]))
if (argumentTypes[i] is { } t && !t.MatchesParameterTypeExactly(parameterInfos[i]))
return false;
}
}
Expand Down Expand Up @@ -3899,7 +3899,7 @@ private void CreateInstanceCheckThis()
Type[] argsType = args.Length != 0 ? new Type[args.Length] : EmptyTypes;
for (int i = 0; i < args.Length; i++)
{
if (args[i] is object arg)
if (args[i] is { } arg)
{
argsType[i] = arg.GetType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal static class RoslynExtensions
continue;
}

if (type is object)
if (type is not null)
{
// Multiple visible types with the same metadata name are present
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public static Task Unwrap(IAsyncResult asyncResult)
}
#endif

if ((asyncResult as TaskAsyncResult)?._task is not Task task)
if ((asyncResult as TaskAsyncResult)?._task is not { } task)
{
throw new ArgumentException(null, nameof(asyncResult));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ private static INamedTypeSymbol[][] DecomposePropertySymbolForIsSupportedGroups_
if (propertyDefiningSyntax != null)
{
if (propertyDefiningSyntax is PropertyDeclarationSyntax propertyDeclaration
&& propertyDeclaration.ExpressionBody is ArrowExpressionClauseSyntax arrowExpression)
&& propertyDeclaration.ExpressionBody is { } arrowExpression)
{
if (model.SyntaxTree != arrowExpression.SyntaxTree)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int
return handle;
}

if (createOpenException?.Invoke(error, flags, path) is Exception ex)
if (createOpenException?.Invoke(error, flags, path) is { } ex)
{
throw ex;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ private static bool TryGetInt32EnvironmentVariable(string variable, out int resu
{
// Avoid globalization stack, as it might in turn be using ArrayPool.

if (Environment.GetEnvironmentVariableCore_NoArrayPool(variable) is string envVar &&
if (Environment.GetEnvironmentVariableCore_NoArrayPool(variable) is { } envVar &&
envVar.Length is > 0 and <= 32) // arbitrary limit that allows for some spaces around the maximum length of a non-negative Int32 (10 digits)
{
ReadOnlySpan<char> value = envVar.AsSpan().Trim(' ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ public override int IndexOf(object? value, int startIndex, int count)
else
{
for (int i = startIndex; i < endIndex; i++)
if (_list[i] is object o && o.Equals(value))
if (_list[i] is { } o && o.Equals(value))
return i;
return -1;
}
Expand Down Expand Up @@ -944,7 +944,7 @@ public override int LastIndexOf(object? value, int startIndex, int count)
else
{
for (int i = startIndex; i >= endIndex; i--)
if (_list[i] is object o && o.Equals(value))
if (_list[i] is { } o && o.Equals(value))
return i;
return -1;
}
Expand Down Expand Up @@ -2268,7 +2268,7 @@ public override bool Contains(object? item)
else
{
for (int i = 0; i < _baseSize; i++)
if (_baseList[_baseIndex + i] is object o && o.Equals(item))
if (_baseList[_baseIndex + i] is { } o && o.Equals(item))
return true;
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public Dictionary(int capacity, IEqualityComparer<TKey>? comparer)
// We use a non-randomized comparer for improved perf, falling back to a randomized comparer if the
// hash buckets become unbalanced.
if (typeof(TKey) == typeof(string) &&
NonRandomizedStringEqualityComparer.GetStringComparer(_comparer!) is IEqualityComparer<string> stringComparer)
NonRandomizedStringEqualityComparer.GetStringComparer(_comparer!) is { } stringComparer)
{
_comparer = (IEqualityComparer<TKey>)stringComparer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public HashSet(IEqualityComparer<T>? comparer)
// We use a non-randomized comparer for improved perf, falling back to a randomized comparer if the
// hash buckets become unbalanced.
if (typeof(T) == typeof(string) &&
NonRandomizedStringEqualityComparer.GetStringComparer(_comparer) is IEqualityComparer<string> stringComparer)
NonRandomizedStringEqualityComparer.GetStringComparer(_comparer) is { } stringComparer)
{
_comparer = (IEqualityComparer<T>)stringComparer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public AssertInterpolatedStringHandler(int literalLength, int formattedCount, bo
/// <summary>Extracts the built string from the handler.</summary>
internal string ToStringAndClear()
{
string s = _stringBuilderHandler._stringBuilder is StringBuilder sb ?
string s = _stringBuilderHandler._stringBuilder is { } sb ?
sb.ToString() :
string.Empty;
_stringBuilderHandler = default;
Expand Down Expand Up @@ -402,7 +402,7 @@ public WriteIfInterpolatedStringHandler(int literalLength, int formattedCount, b
/// <summary>Extracts the built string from the handler.</summary>
internal string ToStringAndClear()
{
string s = _stringBuilderHandler._stringBuilder is StringBuilder sb ?
string s = _stringBuilderHandler._stringBuilder is { } sb ?
StringBuilderCache.GetStringAndRelease(sb) :
string.Empty;
_stringBuilderHandler = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3376,7 +3376,7 @@ private static bool AttributeTypeNamesMatch(Type attributeType, Type reflectedAt
if (manifest.HasResources)
{
string eventKey = "event_" + eventName;
if (manifest.GetLocalizedMessage(eventKey, CultureInfo.CurrentUICulture, etwFormat: false) is string msg)
if (manifest.GetLocalizedMessage(eventKey, CultureInfo.CurrentUICulture, etwFormat: false) is { } msg)
{
// overwrite inline message with the localized message
eventAttribute.Message = msg;
Expand Down Expand Up @@ -5856,7 +5856,7 @@ private void WriteMessageAttrib(StringBuilder? stringBuilder, string elementName
{
// resource fallback: strings in the neutral culture will take precedence over inline strings
key = elementName + "_" + name;
if (resources.GetString(key, CultureInfo.InvariantCulture) is string localizedString)
if (resources.GetString(key, CultureInfo.InvariantCulture) is { } localizedString)
value = localizedString;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Enum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,7 @@ private static bool TryFormatPrimitiveDefault<TUnderlying, TStorage>(RuntimeType

if (!enumInfo.HasFlagsAttribute)
{
if (GetNameInlined(enumInfo, Unsafe.BitCast<TUnderlying, TStorage>(value)) is string enumName)
if (GetNameInlined(enumInfo, Unsafe.BitCast<TUnderlying, TStorage>(value)) is { } enumName)
{
if (enumName.TryCopyTo(destination))
{
Expand Down Expand Up @@ -1967,7 +1967,7 @@ private static bool TryFormatFlagNames<TStorage>(EnumInfo<TStorage> enumInfo, TS
TStorage[] values = enumInfo.Values;
Debug.Assert(names.Length == values.Length);

if (GetSingleFlagsEnumNameForValue(resultValue, names, values, out int index) is string singleEnumFlagsFormat)
if (GetSingleFlagsEnumNameForValue(resultValue, names, values, out int index) is { } singleEnumFlagsFormat)
{
if (singleEnumFlagsFormat.TryCopyTo(destination))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ private void InternalDispose(bool disposing)

CloseDirectoryHandle();

if (_pathBuffer is char[] pathBuffer)
if (_pathBuffer is { } pathBuffer)
{
_pathBuffer = null;
ArrayPool<char>.Shared.Return(pathBuffer);
}

if (_entryBuffer is byte[] entryBuffer)
if (_entryBuffer is { } entryBuffer)
{
_entryBuffer = null;
ArrayPool<byte>.Shared.Return(entryBuffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3987,7 +3987,7 @@ private static bool TryWrite<TArg0, TArg1, TArg2>(Span<char> destination, IForma
foreach ((string? Literal, int ArgIndex, int Alignment, string? Format) segment in format._segments)
{
bool appended;
if (segment.Literal is string literal)
if (segment.Literal is { } literal)
{
appended = handler.AppendLiteral(literal);
}
Expand Down Expand Up @@ -4548,7 +4548,7 @@ private bool AppendCustomFormatter<T>(T value, string? format)
ICustomFormatter? formatter = (ICustomFormatter?)_provider.GetFormat(typeof(ICustomFormatter));
Debug.Assert(formatter != null, "An incorrectly written provider said it implemented ICustomFormatter, and then didn't");

if (formatter is not null && formatter.Format(format, value, _provider) is string customFormatted)
if (formatter is not null && formatter.Format(format, value, _provider) is { } customFormatted)
{
return AppendLiteral(customFormatted);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private static MemberInfo GetMemberMetadataDefinition(MemberInfo member)

private static Type GetPropertyMetaType(PropertyInfo property)
{
if (property.GetGetMethod(true) is MethodInfo method)
if (property.GetGetMethod(true) is { } method)
{
return method.ReturnType;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public ref ExecutionContext? Context
{
get
{
Debug.Assert(m_stateObject is null or ExecutionContext, $"Expected {nameof(m_stateObject)} to be null or an ExecutionContext but was {(m_stateObject is object o ? o.GetType().ToString() : "(null)")}.");
Debug.Assert(m_stateObject is null or ExecutionContext, $"Expected {nameof(m_stateObject)} to be null or an ExecutionContext but was {(m_stateObject is { } o ? o.GetType().ToString() : "(null)")}.");
return ref Unsafe.As<object?, ExecutionContext?>(ref m_stateObject);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ private void AppendCustomFormatter<T>(T value, string? format)
ICustomFormatter? formatter = (ICustomFormatter?)_provider.GetFormat(typeof(ICustomFormatter));
Debug.Assert(formatter != null, "An incorrectly written provider said it implemented ICustomFormatter, and then didn't");

if (formatter is not null && formatter.Format(format, value, _provider) is string customFormatted)
if (formatter is not null && formatter.Format(format, value, _provider) is { } customFormatted)
{
AppendLiteral(customFormatted);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private void Unregister()
{
lock (s_registrations)
{
if (_token is Token token)
if (_token is { } token)
{
_token = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public RabinKarp(ReadOnlySpan<string> values)

// Start with a bucket containing 1 element and reallocate larger ones if needed.
// As MaxValues is similar to BucketCount, we will have 1 value per bucket on average.
if (buckets[bucket] is string[] existingBucket)
if (buckets[bucket] is { } existingBucket)
{
newBucket = new string[existingBucket.Length + 1];
existingBucket.AsSpan().CopyTo(newBucket);
Expand Down Expand Up @@ -132,7 +132,7 @@ private readonly int IndexOfAnyCore<TCaseSensitivity>(ReadOnlySpan<char> span)
{
ValidateReadPosition(span, ref current);

if (Unsafe.Add(ref bucketsRef, hash % BucketCount) is string[] bucket)
if (Unsafe.Add(ref bucketsRef, hash % BucketCount) is { } bucket)
{
int startOffset = (int)((nuint)Unsafe.ByteOffset(ref MemoryMarshal.GetReference(span), ref current) / sizeof(char));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ private static string Format<TArg0, TArg1, TArg2>(IFormatProvider? provider, Com
// Format each segment.
foreach ((string? Literal, int ArgIndex, int Alignment, string? Format) segment in format._segments)
{
if (segment.Literal is string literal)
if (segment.Literal is { } literal)
{
handler.AppendLiteral(literal);
}
Expand Down Expand Up @@ -1078,7 +1078,7 @@ private static string JoinCore(ReadOnlySpan<char> separator, ReadOnlySpan<string

// We range check again to avoid buffer overflows if this happens.

if (values[i] is string value)
if (values[i] is { } value)
{
int valueLen = value.Length;
if (valueLen > totalLength - copiedLength)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ private CompositeFormat(string format, (string? Literal, int ArgIndex, int Align
{
Debug.Assert((segment.Literal is not null) ^ (segment.ArgIndex >= 0), "The segment should represent a literal or a format hole, but not both.");

if (segment.Literal is string literal)
if (segment.Literal is { } literal)
{
literalLength += literal.Length; // no concern about overflow as these were parsed out of a single string
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1897,7 +1897,7 @@ private StringBuilder AppendFormat<TArg0, TArg1, TArg2>(IFormatProvider? provide
// Append each segment.
foreach ((string? Literal, int ArgIndex, int Alignment, string? Format) segment in format._segments)
{
if (segment.Literal is string literal)
if (segment.Literal is { } literal)
{
handler.AppendLiteral(literal);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ private bool AppendCustomFormatter<T>(T value, string? format)
Debug.Assert(formatter is not null, "An incorrectly written provider said it implemented ICustomFormatter, and then didn't");

if (formatter is not null &&
formatter.Format(format, value, _provider) is string customFormatted)
formatter.Format(format, value, _provider) is { } customFormatted)
{
return AppendFormatted(customFormatted.AsSpan());
}
Expand Down
Loading
Loading