Skip to content

Commit

Permalink
Enable several more IDE analyzer warnings (#42692)
Browse files Browse the repository at this point in the history
* Enable IDE0062: Make local functions static

* Enable IDE0079: Remove unnecessary suppression

* Enable IDE0082: Convert typeof to nameof

* Enable IDE0043: Validate format string

* Enable IDE0041: Use is null check
  • Loading branch information
stephentoub committed Sep 25, 2020
1 parent e17d818 commit 6b5b9f7
Show file tree
Hide file tree
Showing 54 changed files with 171 additions and 168 deletions.
10 changes: 5 additions & 5 deletions eng/CodeAnalysis.ruleset
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,9 @@
<Rule Id="IDE0038" Action="Hidden" /> <!-- InlineIsTypeWithoutNameCheck -->
<Rule Id="IDE0039" Action="Hidden" /> <!-- UseLocalFunction -->
<Rule Id="IDE0040" Action="Hidden" /> <!-- AddAccessibilityModifiers -->
<Rule Id="IDE0041" Action="Hidden" /> <!-- UseIsNullCheck -->
<Rule Id="IDE0041" Action="Warning" /> <!-- UseIsNullCheck -->
<Rule Id="IDE0042" Action="Hidden" /> <!-- UseDeconstruction -->
<Rule Id="IDE0043" Action="Hidden" /> <!-- ValidateFormatString -->
<Rule Id="IDE0043" Action="Warning" /> <!-- ValidateFormatString -->
<Rule Id="IDE0044" Action="Hidden" /> <!-- MakeFieldReadonly -->
<Rule Id="IDE0045" Action="Hidden" /> <!-- UseConditionalExpressionForAssignment -->
<Rule Id="IDE0046" Action="Hidden" /> <!-- UseConditionalExpressionForReturn -->
Expand All @@ -437,7 +437,7 @@
<Rule Id="IDE0059" Action="Hidden" /> <!-- ValueAssignedIsUnused -->
<Rule Id="IDE0060" Action="Hidden" /> <!-- UnusedParameter -->
<Rule Id="IDE0061" Action="Hidden" /> <!-- UseExpressionBodyForLocalFunctions -->
<Rule Id="IDE0062" Action="Hidden" /> <!-- MakeLocalFunctionStatic -->
<Rule Id="IDE0062" Action="Warning" /> <!-- MakeLocalFunctionStatic -->
<Rule Id="IDE0063" Action="Hidden" /> <!-- UseSimpleUsingStatement -->
<Rule Id="IDE0064" Action="Hidden" /> <!-- MakeStructFieldsWritable -->
<Rule Id="IDE0065" Action="Hidden" /> <!-- MoveMisplacedUsingDirectives -->
Expand All @@ -454,10 +454,10 @@
<Rule Id="IDE0076" Action="Hidden" /> <!-- InvalidSuppressMessageAttribute -->
<Rule Id="IDE0077" Action="Hidden" /> <!-- LegacyFormatSuppressMessageAttribute -->
<Rule Id="IDE0078" Action="Hidden" /> <!-- UsePatternCombinators -->
<Rule Id="IDE0079" Action="Hidden" /> <!-- RemoveUnnecessarySuppression -->
<Rule Id="IDE0079" Action="Warning" /> <!-- RemoveUnnecessarySuppression -->
<Rule Id="IDE0080" Action="Hidden" /> <!-- RemoveConfusingSuppressionForIsExpression -->
<Rule Id="IDE0081" Action="Hidden" /> <!-- RemoveUnnecessaryByVal -->
<Rule Id="IDE0082" Action="Hidden" /> <!-- ConvertTypeOfToNameOf -->
<Rule Id="IDE0082" Action="Warning" /> <!-- ConvertTypeOfToNameOf -->
<Rule Id="IDE0083" Action="Hidden" /> <!-- UseNotPattern -->
<Rule Id="IDE0084" Action="Hidden" /> <!-- UseIsNotExpression -->
<Rule Id="IDE1001" Action="Hidden" /> <!-- AnalyzerChanged -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ internal void StoreDynamicMethod(MethodInfo dynamicMethod)
// to form a new delegate.
protected sealed override Delegate CombineImpl(Delegate? follow)
{
if ((object?)follow == null) // cast to object for a more efficient test
if (follow is null)
return this;

// Verify that the types are the same...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public override MethodInfo[] GetAccessors(bool nonPublic)
if (Associates.IncludeAccessor(m_setterMethod, nonPublic))
accessorList.Add(m_setterMethod!);

if ((object?)m_otherMethod != null)
if (m_otherMethod is not null)
{
for (int i = 0; i < m_otherMethod.Length; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2203,7 +2203,7 @@ private static bool FilterApplyBase(
private static bool FilterApplyType(
Type type, BindingFlags bindingFlags, string name, bool prefixLookup, string? ns)
{
Debug.Assert((object)type != null);
Debug.Assert(type is not null);
Debug.Assert(type is RuntimeType);

bool isPublic = type.IsNestedPublic || type.IsPublic;
Expand Down Expand Up @@ -2341,7 +2341,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 ((object)argumentTypes[i] != null && !argumentTypes[i].MatchesParameterTypeExactly(parameterInfos[i]))
if (argumentTypes[i] is Type t && !t.MatchesParameterTypeExactly(parameterInfos[i]))
return false;
}
}
Expand Down Expand Up @@ -2834,7 +2834,7 @@ public override InterfaceMapping GetInterfaceMap(Type ifaceType)
{
PropertyInfo firstCandidate = candidates[0];

if ((object?)returnType != null && !returnType.IsEquivalentTo(firstCandidate.PropertyType))
if (returnType is not null && !returnType.IsEquivalentTo(firstCandidate.PropertyType))
return null;

return firstCandidate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private static int IntFromString(string text, CultureInfo culture)
}
catch (Exception e)
{
throw new ArgumentException(SR.Format(SR.ConvertInvalidPrimitive, text, typeof(int).Name), e);
throw new ArgumentException(SR.Format(SR.ConvertInvalidPrimitive, text, nameof(Int32)), e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ private static bool IsTypeParameterEquivalentToTypeInst(this Type typeParam, Typ
// See if MetadataToken property is available.
PropertyInfo property = memberInfo.GetProperty("MetadataToken", typeof(int), Array.Empty<Type>());
if ((object)property != null && property.CanRead)
if (property is not null && property.CanRead)
{
// (parameter1, parameter2) => parameter1.MetadataToken == parameter2.MetadataToken
var parameter1 = Expression.Parameter(memberInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ public Symbol Sym
{
return true;
}
else if (ReferenceEquals(swt1, null))
else if (swt1 is null)
{
return swt2._sym == null;
}
else if (ReferenceEquals(swt2, null))
else if (swt2 is null)
{
return swt1._sym == null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public bool Equals(Dependency other)

public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj)) return false;
if (obj is null) return false;
return obj is Dependency && Equals((Dependency) obj);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public bool Equals(EventId other)
/// <inheritdoc />
public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
if (obj is null)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public static int Compare(StringSegment a, StringSegment b, StringComparison com
/// <inheritdoc />
public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
if (obj is null)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ internal ImportDefinition(string? contractName, ImportCardinality cardinality, b
(cardinality != ImportCardinality.ZeroOrOne)
)
{
throw new ArgumentException(SR.Format(SR.ArgumentOutOfRange_InvalidEnum, nameof(cardinality), cardinality, typeof(ImportCardinality).Name), nameof(cardinality));
throw new ArgumentException(SR.Format(SR.ArgumentOutOfRange_InvalidEnum, nameof(cardinality), cardinality, nameof(ImportCardinality)), nameof(cardinality));
}

_contractName = contractName ?? EmptyContractName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public override IEnumerable<ExportDescriptorPromise> GetExportDescriptors(Compos

return new[] { new ExportDescriptorPromise(
contract,
typeof(CompositionContext).Name,
nameof(CompositionContext),
true,
NoDependencies,
_ => ExportDescriptor.Create((c, o) => c, NoMetadata)) };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ internal static bool AreEqual(object? a, object? b)
// same reference or (null, null) or (DBNull.Value, DBNull.Value)
return true;
}
if (ReferenceEquals(a, null) || ReferenceEquals(a, DBNull.Value) ||
ReferenceEquals(b, null) || ReferenceEquals(b, DBNull.Value))
if (a is null || ReferenceEquals(a, DBNull.Value) ||
b is null || ReferenceEquals(b, DBNull.Value))
{
// (null, non-null) or (null, DBNull.Value) or vice versa
return false;
Expand All @@ -40,8 +40,8 @@ private static bool AreElementEqual(object? a, object? b)
// same reference or (null, null) or (DBNull.Value, DBNull.Value)
return true;
}
if (ReferenceEquals(a, null) || ReferenceEquals(a, DBNull.Value) ||
ReferenceEquals(b, null) || ReferenceEquals(b, DBNull.Value))
if (a is null || ReferenceEquals(a, DBNull.Value) ||
b is null || ReferenceEquals(b, DBNull.Value))
{
// (null, non-null) or (null, DBNull.Value) or vice versa
return false;
Expand Down Expand Up @@ -143,8 +143,8 @@ public bool Equals(TRow? leftRow, TRow? rightRow)
return true;
}

if (ReferenceEquals(leftRow, null) ||
ReferenceEquals(rightRow, null))
if (leftRow is null ||
rightRow is null)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public static void Animate(Image image, EventHandler onFrameChangedHandler)
if (s_animationThread == null)
{
s_animationThread = new Thread(new ThreadStart(AnimateImages50ms));
s_animationThread.Name = typeof(ImageAnimator).Name;
s_animationThread.Name = nameof(ImageAnimator);
s_animationThread.IsBackground = true;
s_animationThread.Start();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static unsafe MemoryMappedView CreateView(
}
if (memMappedFileHandle.IsClosed)
{
throw new ObjectDisposedException(typeof(MemoryMappedFile).Name);
throw new ObjectDisposedException(nameof(MemoryMappedFile));
}

if (requestedSize == MemoryMappedFile.DefaultSize)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void SetUncompleted()
public void OnCompleted(Action<object?> continuation, object? state, ValueTaskSourceOnCompletedFlags flags, out CompletionData completionData, out bool doubleCompletion)
{
completionData = default;
doubleCompletion = !ReferenceEquals(_completion, null);
doubleCompletion = _completion is not null;

if (IsCompleted || doubleCompletion)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ private static bool IsImplicitNullableConversion(Type source, Type destination)

public static Type? FindGenericType(Type definition, Type? type)
{
while ((object?)type != null && type != typeof(object))
while (type is not null && type != typeof(object))
{
if (type.IsConstructedGenericType && AreEquivalent(type.GetGenericTypeDefinition(), definition))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal static void Emit(this ILGenerator il, OpCode opcode, MethodBase methodB
Debug.Assert(methodBase is MethodInfo || methodBase is ConstructorInfo);

var ctor = methodBase as ConstructorInfo;
if ((object?)ctor != null)
if (ctor is not null)
{
il.Emit(opcode, ctor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private void AddressOf(MemberExpression node, Type type)
private void EmitMemberAddress(MemberInfo member, Type? objectType)
{
FieldInfo? field = member as FieldInfo;
if ((object?)field != null)
if (field is not null)
{
// Verifiable code may not take the address of an init-only field.
// If we are asked to do so then get the value out of the field, stuff it
Expand Down Expand Up @@ -260,7 +260,7 @@ private void EmitExpressionAddress(Expression node, Type type)
private WriteBack? AddressOfWriteBack(MemberExpression node)
{
var property = node.Member as PropertyInfo;
if ((object?)property == null || !property.CanWrite)
if (property is null || !property.CanWrite)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ private void EmitMemberAssignment(AssignBinaryExpression node, CompilationFlags
}

var fld = member as FieldInfo;
if ((object?)fld != null)
if (fld is not null)
{
_ilg.EmitFieldSet((FieldInfo)member);
}
Expand Down Expand Up @@ -840,7 +840,7 @@ private void EmitMemberExpression(Expression expr)
private void EmitMemberGet(MemberInfo member, Type? objectType)
{
var fi = member as FieldInfo;
if ((object?)fi != null)
if (fi is not null)
{
if (fi.IsLiteral)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ private static CallInstruction GetArrayAccessor(MethodInfo info, int argumentCou
break;
}

if ((object?)alternativeMethod == null)
if (alternativeMethod is null)
{
return new MethodInfoCallInstruction(info, argumentCount);
}
Expand Down Expand Up @@ -261,10 +261,10 @@ private static CallInstruction SlowCreate(MethodInfo info, ParameterInfo[] pis)
protected static bool TryGetLightLambdaTarget(object? instance, [NotNullWhen(true)] out LightLambda? lightLambda)
{
var del = instance as Delegate;
if ((object?)del != null)
if (del is not null)
{
var thunk = del.Target as Func<object[], object>;
if ((object?)thunk != null)
if (thunk is not null)
{
lightLambda = thunk.Target as LightLambda;
if (lightLambda != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@ private void CompileConvertToType(Type typeFrom, Type typeTo, bool isChecked, bo

if (from == to)
{
if ((object?)enumTypeTo != null)
if (enumTypeTo is not null)
{
// If casting between enums of the same underlying type or to enum from the underlying
// type, there's no need for the numeric conversion, so just include a null-check if
Expand Down Expand Up @@ -1189,7 +1189,7 @@ private void CompileConvertToType(Type typeFrom, Type typeTo, bool isChecked, bo
}
}

if ((object?)enumTypeTo != null)
if (enumTypeTo is not null)
{
// Convert from underlying to the enum
_instructions.EmitCastToEnum(enumTypeTo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private HttpMethod(string method, int? http3StaticTableIndex)

public bool Equals(HttpMethod? other)
{
if ((object?)other == null)
if (other is null)
{
return false;
}
Expand Down Expand Up @@ -159,7 +159,7 @@ public override string ToString()

public static bool operator ==(HttpMethod? left, HttpMethod? right)
{
return (object?)left == null || (object?)right == null ?
return left is null || right is null ?
ReferenceEquals(left, right) :
left.Equals(right);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
if (response.Headers.ConnectionClose.GetValueOrDefault())
{
// Server is closing the connection and asking us to authenticate on a new connection.
#pragma warning disable CS8600 // expression returns null connection on error, was not able to use '!' for the expression
// expression returns null connection on error, was not able to use '!' for the expression
#pragma warning disable CS8600
(connection, response) = await connectionPool.CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false);
#pragma warning restore CS8600
if (response != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -806,8 +806,8 @@ private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus,
{
throw new WebSocketException(WebSocketError.InvalidMessageType,
SR.Format(SR.net_WebSockets_InvalidMessageType,
typeof(WebSocket).Name + "." + nameof(CloseAsync),
typeof(WebSocket).Name + "." + nameof(CloseOutputAsync),
nameof(WebSocket) + "." + nameof(CloseAsync),
nameof(WebSocket) + "." + nameof(CloseOutputAsync),
receiveResult.MessageType));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/System/Lazy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ private void PublicationOnlyViaFactory(LazyHelper initializer)
private void PublicationOnlyWaitForOtherThreadToPublish()
{
SpinWait spinWait = default;
while (!ReferenceEquals(_state, null))
while (_state is not null)
{
// We get here when PublicationOnly temporarily sets _state to LazyHelper.PublicationOnlyWaitForOtherThreadToPublish.
// This temporary state should be quickly followed by _state being set to null.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public static bool Equals<T>(Nullable<T> n1, Nullable<T> n2) where T : struct
// Otherwise, returns the underlying type of the Nullable type
public static Type? GetUnderlyingType(Type nullableType)
{
if ((object)nullableType == null)
if (nullableType is null)
{
throw new ArgumentNullException(nameof(nullableType));
}
Expand Down
Loading

0 comments on commit 6b5b9f7

Please sign in to comment.