Add conservative reset of analysis data in presence of escaped lambda and local function delegates

Fixes #5684

We were already performing escape analysis since #4113. However, we were not propertly resetting all the analysis data on encountering such escapes.
In future, we can consider making the analysis more aggressive for such escapes by not resetting the entire analysis data on escapes, but instead just resetting the analysis data pertaining to capture variables and any points to locations reachable from these variables.

Author: mavasani

src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/DataFlowOperationVisitor.cs

@@ -2278,7 +2278,7 @@ private TAbstractAnalysisValue PerformInterproceduralAnalysis(
                     // Implement Non-context sensitive interprocedural analysis to
                     // merge the relevant data from invoked method's analysis result into CurrentAnalysisData.
                     // For now, retain the original logic of resetting the analysis data.
-                    ResetAnalysisData();
+                    ResetAnalysisData(hasEscapedLambdaOrLocalFunctions: false);
                 }
             }
             finally
@@ -2308,12 +2308,12 @@ private TAbstractAnalysisValue PerformInterproceduralAnalysis(
             // Local functions
             TAbstractAnalysisValue ResetAnalysisDataAndReturnDefaultValue()
             {
-                ResetAnalysisData();
-                MarkEscapedLambdasAndLocalFunctionsFromArguments();
+                var hasEscapes = MarkEscapedLambdasAndLocalFunctionsFromArguments();
+                ResetAnalysisData(hasEscapes);
                 return defaultValue;
             }
 
-            void ResetAnalysisData()
+            void ResetAnalysisData(bool hasEscapedLambdaOrLocalFunctions)
             {
                 // Interprocedural analysis did not succeed, so we need to conservatively reset relevant analysis data.
                 if (!PessimisticAnalysis)
@@ -2322,7 +2322,7 @@ void ResetAnalysisData()
                     return;
                 }
 
-                if (isLambdaOrLocalFunction)
+                if (isLambdaOrLocalFunction || hasEscapedLambdaOrLocalFunctions)
                 {
                     // For local/lambda cases, we reset all analysis data.
                     ResetCurrentAnalysisData();
@@ -2336,23 +2336,36 @@ void ResetAnalysisData()
                 }
             }
 
-            void MarkEscapedLambdasAndLocalFunctionsFromArguments()
+            bool MarkEscapedLambdasAndLocalFunctionsFromArguments()
             {
-                if (!IsPointsToAnalysis)
-                {
-                    // Only mark escaped lambdas and local functions for points to analysis.
-                    return;
-                }
-
+                var hasEscapes = false;
                 foreach (var argument in arguments)
                 {
                     if (argument.Parameter?.Type.TypeKind == TypeKind.Delegate ||
                         argument.Parameter?.Type.SpecialType == SpecialType.System_Object)
                     {
-                        var pointsToValue = GetPointsToAbstractValue(argument);
-                        MarkEscapedLambdasAndLocalFunctions(pointsToValue);
+                        if (!IsPointsToAnalysis)
+                        {
+                            // For non-points to analysis, pessimistically assume delegate arguments
+                            // lead to escaped lambda or local function target which may get invoked.
+                            if (argument.Parameter.Type.TypeKind == TypeKind.Delegate)
+                            {
+                                hasEscapes = true;
+                                break;
+                            }
+                        }
+                        else
+                        {
+                            // For points to analysis, we try to compute the target lambda or local function
+                            // to determine if we have an escape.
+                            var pointsToValue = GetPointsToAbstractValue(argument);
+                            if (MarkEscapedLambdasAndLocalFunctions(pointsToValue))
+                                hasEscapes = true;
+                        }
                     }
                 }
+
+                return hasEscapes;
             }
 
             InterproceduralAnalysisData<TAnalysisData, TAnalysisContext, TAbstractAnalysisValue> ComputeInterproceduralAnalysisData()
@@ -3206,10 +3219,11 @@ private TAbstractAnalysisValue VisitInvocation_NonLambdaOrDelegateOrLocalFunctio
                 invokedAsDelegate: false, originalOperation: operation, defaultValue: value);
         }
 
-        private protected void MarkEscapedLambdasAndLocalFunctions(PointsToAbstractValue pointsToAbstractValue)
+        private protected bool MarkEscapedLambdasAndLocalFunctions(PointsToAbstractValue pointsToAbstractValue)
         {
             Debug.Assert(IsPointsToAnalysis);
 
+            var hasEscapes = false;
             using var methodTargetsOptBuilder = PooledHashSet<(IMethodSymbol method, IOperation? instance)>.GetInstance();
             using var lambdaTargets = PooledHashSet<IFlowAnonymousFunctionOperation>.GetInstance();
             if (ResolveLambdaOrDelegateOrLocalFunctionTargets(pointsToAbstractValue, methodTargetsOptBuilder, lambdaTargets))
@@ -3219,14 +3233,18 @@ private protected void MarkEscapedLambdasAndLocalFunctions(PointsToAbstractValue
                     if (targetMethod.MethodKind == MethodKind.LocalFunction)
                     {
                         _escapedLocalFunctions.Add(targetMethod);
+                        hasEscapes = true;
                     }
                 }
 
                 foreach (var flowAnonymousFunctionOperation in lambdaTargets)
                 {
                     _escapedLambdas.Add(flowAnonymousFunctionOperation);
+                    hasEscapes = true;
                 }
             }
+
+            return hasEscapes;
         }
 
         private bool ResolveLambdaOrDelegateOrLocalFunctionTargets(

src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs

@@ -7579,5 +7579,122 @@ void M(A a)
                 LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8,
             }.RunAsync();
         }
+
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.PointsToAnalysis)]
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)]
+        [Theory, CombinatorialData, WorkItem(5684, "https://github.com/dotnet/roslyn-analyzers/issues/5684")]
+        public async Task EscapedLambdaWithMethodCall_AssignsCapturedVariable_NoDiagnosticsAsync(bool enableInterproceduralAnalysis)
+        {
+            var interproceduralValue = enableInterproceduralAnalysis ? "ContextSensitive" : "None";
+            var editorConfigText = $"dotnet_code_quality.interprocedural_analysis_kind = {interproceduralValue}";
+
+            var test = new VerifyCS.Test
+            {
+                TestState =
+                {
+                    Sources =
+                    {
+                        @"
+#nullable enable
+
+using System;
+
+internal class C
+{
+    static void M1(Action a)
+    {
+        a();
+    }
+
+    static void M2()
+    {
+        Exception? exception = null;
+
+        M1(() =>
+        {
+            exception = new Exception();
+        });
+
+        if (exception != null)
+        {
+        }
+    }
+}"
+                    },
+                    AnalyzerConfigFiles = { ("/.editorconfig", $@"root = true
+
+[*]
+{editorConfigText}
+"),
+
+                    },
+                },
+                LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8
+            };
+
+            if (enableInterproceduralAnalysis)
+            {
+                test.ExpectedDiagnostics.Add(
+                    // /0/Test0.cs(22,13): warning CA1508: 'exception != null' is always 'true'. Remove or refactor the condition(s) to avoid dead code.
+                    GetCSharpResultAt(22, 13, "exception != null", "true"));
+            }
+
+            await test.RunAsync();
+        }
+
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.PointsToAnalysis)]
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)]
+        [Theory, CombinatorialData, WorkItem(5684, "https://github.com/dotnet/roslyn-analyzers/issues/5684")]
+        public async Task EscapedLambdaInTaskRun_AssignsCapturedVariable_NoDiagnosticsAsync(bool enableInterproceduralAnalysis)
+        {
+            var interproceduralValue = enableInterproceduralAnalysis ? "ContextSensitive" : "None";
+            var editorConfigText = $"dotnet_code_quality.interprocedural_analysis_kind = {interproceduralValue}";
+
+            var test = new VerifyCS.Test
+            {
+                TestState =
+                {
+                    Sources =
+                    {
+                        @"
+#nullable enable
+
+using System;
+using System.Threading.Tasks;
+
+internal class C
+{
+    static async Task M()
+    {
+        Exception? exception = null;
+
+        await Task.Run(() =>
+        {
+            exception = new Exception();
+        }).ConfigureAwait(false);
+
+        if (exception != null)
+        {
+        }
+    }
+}"
+                    },
+                    AnalyzerConfigFiles = { ("/.editorconfig", $@"root = true
+
+[*]
+{editorConfigText}
+"),
+
+                    },
+                },
+                LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8
+            };
+
+            // NOTE: Even with interprocedural analysis enabled, we do not have flow analysis logic
+            // to perform interprocedural analysis for a delegate passed to Task.Run.
+            // Hence the results are identical with interprocedural analysis enabled and disabled.
+
+            await test.RunAsync();
+        }
     }
 }