Merge pull request #898 from AArnott/IsNull

Apply `is null` syntax pattern and analyzer
microsoft · Aug 23, 2021 · 79735d7 · 79735d7
2 parents 51ddca0 + 05c193b
commit 79735d7
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 20 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -186,5 +186,11 @@ dotnet_diagnostic.SA1133.severity = none
 # SA1108: Block statements should not contain embedded comments
 dotnet_diagnostic.SA1108.severity = suggestion
 
+# CSIsNull001: Use `is null` for null checks
+dotnet_diagnostic.CSIsNull001.severity = warning
+
+# CSIsNull002: Use `is object` for non-null checks
+dotnet_diagnostic.CSIsNull002.severity = warning
+
 [*.sln]
 indent_style = tab
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -31,6 +31,7 @@
     <PackageReference Include="Nerdbank.GitVersioning" Version="3.4.231" PrivateAssets="all" />
     <PackageReference Include="Microsoft.Net.Compilers.Toolset" Version="3.11.0" PrivateAssets="all" />
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.354" PrivateAssets="all" />
+    <PackageReference Include="CSharpIsNullAnalyzer" Version="0.1.288-beta" PrivateAssets="all" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs
@@ -115,7 +115,7 @@ internal static bool ShouldIgnoreContext(SyntaxNodeAnalysisContext context)
 
         private static SyntaxNode? GetEnclosingBlock(SyntaxNode node)
         {
-            while (node != null)
+            while (node is not null)
             {
                 if (node.IsKind(SyntaxKind.Block))
                 {
@@ -131,7 +131,7 @@ internal static bool ShouldIgnoreContext(SyntaxNodeAnalysisContext context)
         private static bool IsVariablePassedToInvocation(InvocationExpressionSyntax invocationExpr, string variableName, bool byRef)
         {
             ArgumentListSyntax? argList = invocationExpr.ChildNodes().OfType<ArgumentListSyntax>().FirstOrDefault();
-            if (argList == null)
+            if (argList is null)
             {
                 return false;
             }
@@ -145,7 +145,7 @@ private static bool IsVariablePassedToInvocation(InvocationExpressionSyntax invo
                 }
 
                 IdentifierNameSyntax identiferName = arg.ChildNodes().OfType<IdentifierNameSyntax>().FirstOrDefault();
-                if (identiferName == null)
+                if (identiferName is null)
                 {
                     return false;
                 }
@@ -200,7 +200,7 @@ private static bool IsTaskCompletedWithWhenAll(SyntaxNodeAnalysisContext context
         private static bool HasTaskCompleted(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax)
         {
             SyntaxNode? enclosingBlock = GetEnclosingBlock(memberAccessSyntax);
-            if (enclosingBlock == null)
+            if (enclosingBlock is null)
             {
                 return false;
             }
@@ -212,7 +212,7 @@ private static bool HasTaskCompleted(SyntaxNodeAnalysisContext context, MemberAc
             //   task2.GetAwaiter().GetResult();
             string? taskVariableName = null;
             ExpressionSyntax parentExpr = memberAccessSyntax.Expression;
-            while (parentExpr != null)
+            while (parentExpr is not null)
             {
                 if (parentExpr is IdentifierNameSyntax identifierExpr)
                 {
@@ -233,7 +233,7 @@ private static bool HasTaskCompleted(SyntaxNodeAnalysisContext context, MemberAc
                 }
             }
 
-            if (taskVariableName == null)
+            if (taskVariableName is null)
             {
                 return false;
             }

diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpUtils.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpUtils.cs
@@ -206,7 +206,7 @@ internal override SyntaxNode IsolateMethodName(IObjectCreationOperation objectCr
         internal override bool MethodReturnsNullableReferenceType(IMethodSymbol methodSymbol)
         {
             SyntaxReference? syntaxReference = methodSymbol.DeclaringSyntaxReferences.FirstOrDefault();
-            if (syntaxReference == null)
+            if (syntaxReference is null)
             {
                 return false;
             }
@@ -223,7 +223,7 @@ internal override bool MethodReturnsNullableReferenceType(IMethodSymbol methodSy
                 returnType = localFunc.ReturnType;
             }
 
-            return returnType != null && returnType.IsKind(SyntaxKind.NullableType);
+            return returnType is not null && returnType.IsKind(SyntaxKind.NullableType);
         }
 
         internal readonly struct ContainingFunctionData

diff --git a/src/Microsoft.VisualStudio.Threading/AsyncReaderWriterLock.cs b/src/Microsoft.VisualStudio.Threading/AsyncReaderWriterLock.cs
@@ -1663,8 +1663,8 @@ private void TryInvokeAllDependentReadersIfAppropriate()
                 if (this.issuedWriteLocks.Count == 0 && this.waitingWriters.Count > 0 && this.waitingReaders.Count > 0 && (this.issuedReadLocks.Count > 0 || this.issuedUpgradeableReadLocks.Count > 0))
                 {
                     HashSet<JoinableTask>? dependentTasks = JoinableTaskDependencyGraph.GetDependentTasksFromCandidates(
-                        this.issuedReadLocks.Concat(this.issuedUpgradeableReadLocks).Where(w => w.AmbientJoinableTask != null).Select(w => w.AmbientJoinableTask!),
-                        this.waitingReaders.Where(w => w.AmbientJoinableTask != null).Select(w => w.AmbientJoinableTask!));
+                        this.issuedReadLocks.Concat(this.issuedUpgradeableReadLocks).Where(w => w.AmbientJoinableTask is not null).Select(w => w.AmbientJoinableTask!),
+                        this.waitingReaders.Where(w => w.AmbientJoinableTask is not null).Select(w => w.AmbientJoinableTask!));
 
                     if (dependentTasks.Count > 0)
                     {
@@ -1673,7 +1673,7 @@ private void TryInvokeAllDependentReadersIfAppropriate()
                         {
                             Awaiter pendingReader = this.waitingReaders.Dequeue();
                             JoinableTask? readerContext = pendingReader.AmbientJoinableTask;
-                            if (readerContext != null && dependentTasks.Contains(readerContext))
+                            if (readerContext is not null && dependentTasks.Contains(readerContext))
                             {
                                 this.IssueAndExecute(pendingReader);
                             }
@@ -1821,8 +1821,8 @@ private void PendAwaiter(Awaiter awaiter)
 
         private void StartPendingWriterDeadlockTimerIfNecessary()
         {
-            if (this.joinableTaskContext != null &&
-                this.pendingWriterLockDeadlockCheckTimer == null &&
+            if (this.joinableTaskContext is not null &&
+                this.pendingWriterLockDeadlockCheckTimer is null &&
                 this.waitingWriters.Count > 0 &&
                 (this.issuedReadLocks.Count > 0 || this.issuedUpgradeableReadLocks.Count > 0))
             {
@@ -1832,7 +1832,7 @@ private void StartPendingWriterDeadlockTimerIfNecessary()
 
         private void StopPendingWriterLockDeadlockWatching()
         {
-            if (this.pendingWriterLockDeadlockCheckTimer != null)
+            if (this.pendingWriterLockDeadlockCheckTimer is not null)
             {
                 this.pendingWriterLockDeadlockCheckTimer.Dispose();
                 this.pendingWriterLockDeadlockCheckTimer = null;
@@ -2285,7 +2285,7 @@ internal Awaiter(AsyncReaderWriterLock lck, LockKind kind, LockFlags options, Ca
                 this.cancellationToken = cancellationToken;
                 this.nestingLock = lck.GetFirstActiveSelfOrAncestor(lck.topAwaiter.Value);
                 this.requestingStackTrace = lck.captureDiagnostics ? new StackTrace(2, true) : null;
-                this.AmbientJoinableTask = (this.nestingLock == null && this.kind != LockKind.Write) ? this.lck.joinableTaskContext?.AmbientTask : null;
+                this.AmbientJoinableTask = (this.nestingLock is null && this.kind != LockKind.Write) ? this.lck.joinableTaskContext?.AmbientTask : null;
             }
 
             /// <summary>

diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTask.cs b/src/Microsoft.VisualStudio.Threading/JoinableTask.cs
@@ -1144,7 +1144,7 @@ private bool TryDequeueSelfOrDependencies(bool onMainThread, ref HashSet<IJoinab
                         bool foundWork = TryDequeueSelfOrDependencies(this, onMainThread, visited, out work);
 
                         HashSet<IJoinableTaskDependent>? visitedNodes = visited;
-                        if (visitedNodes != null && this.HasPotentialUnreachableDependents)
+                        if (visitedNodes is not null && this.HasPotentialUnreachableDependents)
                         {
                             // We walked the dependencies tree and use this information to update the PotentialUnreachableDependents list.
                             this.PotentialUnreachableDependents!.RemoveWhere(n => visitedNodes.Contains(n));

diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs b/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs
@@ -202,7 +202,7 @@ internal static HashSet<JoinableTask> GetDependentTasksFromCandidates(IEnumerabl
             var queue = new Queue<IJoinableTaskDependent>();
             foreach (JoinableTask task in sourceTasks)
             {
-                if (task != null && visited.Add(task))
+                if (task is not null && visited.Add(task))
                 {
                     queue.Enqueue(task);
                 }

diff --git a/src/SosThreadingTools/DumpAsyncCommand.cs b/src/SosThreadingTools/DumpAsyncCommand.cs
@@ -57,7 +57,7 @@ private static void GetAllStateMachines(DebuggerContext context, ClrHeap heap, L
                                         }
 
                                         ClrValueType? nextAsyncBuilder = asyncBuilder.TryGetValueClassField("m_builder");
-                                        if (nextAsyncBuilder == null)
+                                        if (nextAsyncBuilder is null)
                                         {
                                             asyncBuilder = asyncBuilder.TryGetValueClassField("_methodBuilder");
                                         }
@@ -77,7 +77,7 @@ private static void GetAllStateMachines(DebuggerContext context, ClrHeap heap, L
                                     // and remember references encounted, and we skip them when we go through GC references.
                                     // Note: we can do better by going through other value structures, and extract references from them here, which we can consider when we have a real scenario.
                                     var previousReferences = new Dictionary<ulong, int>();
-                                    if (stateMachine.Type?.GetFieldByName("<>t__builder") != null)
+                                    if (stateMachine.Type?.GetFieldByName("<>t__builder") is not null)
                                     {
                                         foreach (ClrInstanceField field in stateMachine.Type.Fields)
                                         {

diff --git a/test/Microsoft.VisualStudio.Threading.Tests/AsyncReaderWriterLockTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/AsyncReaderWriterLockTests.cs
@@ -2406,7 +2406,7 @@ JoinableTask CreateReaderLockTask(JoinableTaskFactory taskFactory, AsyncReaderWr
                     this.Logger.WriteLine($"Releasing related read lock {sequence}.");
                 }
 
-                if (previousTask != null)
+                if (previousTask is not null)
                 {
                     await previousTask;
                 }