Merge pull request #5744 from tmat/EncExceptionRegionFix

Calculate exception regions when an active statement is deleted

src/EditorFeatures/CSharpTest/EditAndContinue/ActiveStatementTests.cs

@@ -5088,6 +5088,169 @@ static void Foo()
             edits.VerifyRudeDiagnostics(active);
         }
 
+        [Fact]
+        public void TryFinally_DeleteStatement_Inner()
+        {
+            string src1 = @"
+class C
+{
+    static void Main()
+    {
+        <AS:0>Console.WriteLine(0);</AS:0>
+
+        try
+        {
+            <AS:1>Console.WriteLine(1);</AS:1>
+        }
+        <ER:1.0>finally
+        {
+            Console.WriteLine(2);
+        }</ER:1.0>
+    }
+}";
+            string src2 = @"
+class C
+{
+    static void Main()
+    {
+        <AS:0>Console.WriteLine(0);</AS:0>
+     
+        try
+        {
+        <AS:1>}</AS:1>
+        finally
+        {
+            Console.WriteLine(2);
+        }
+    }
+}";
+            var edits = GetTopEdits(src1, src2);
+            var active = GetActiveStatements(src1, src2);
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.DeleteActiveStatement, "{"));
+        }
+
+        [Fact]
+        public void TryFinally_DeleteStatement_Leaf()
+        {
+            string src1 = @"
+class C
+{
+    static void Main(string[] args)
+    {
+        <ER:0.0>try
+        {
+            Console.WriteLine(0);
+        }
+        finally
+        {
+            <AS:0>Console.WriteLine(1);</AS:0>
+        }</ER:0.0>
+    }
+}";
+            string src2 = @"
+class C
+{
+    static void Main(string[] args)
+    {
+        try
+        {
+            Console.WriteLine(0);
+        }
+        finally
+        {
+        <AS:0>}</AS:0>
+    }
+}";
+            var edits = GetTopEdits(src1, src2);
+            var active = GetActiveStatements(src1, src2);
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.UpdateAroundActiveStatement, "finally", CSharpFeaturesResources.FinallyClause));
+        }
+
+        [Fact]
+        public void Try_DeleteStatement_Inner()
+        {
+            string src1 = @"
+class C
+{
+    static void Main()
+    {
+        <AS:0>Console.WriteLine(0);</AS:0>
+        
+        try
+        {
+            <AS:1>Console.WriteLine(1);</AS:1>
+        }
+        finally
+        {
+            Console.WriteLine(2);
+        }
+    }
+}";
+            string src2 = @"
+class C
+{
+    static void Main()
+    {
+        <AS:0>Console.WriteLine(0);</AS:0>
+        
+        try
+        {
+        <AS:1>}</AS:1>
+        finally
+        {
+            Console.WriteLine(2);
+        }
+    }
+}";
+            var edits = GetTopEdits(src1, src2);
+            var active = GetActiveStatements(src1, src2);
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.DeleteActiveStatement, "{"));
+        }
+
+        [Fact]
+        public void Try_DeleteStatement_Leaf()
+        {
+            string src1 = @"
+class C
+{
+    static void Main()
+    {
+        try
+        {
+            <AS:0>Console.WriteLine(1);</AS:0>
+        }
+        finally
+        {
+            Console.WriteLine(2);
+        }
+    }
+}";
+            string src2 = @"
+class C
+{
+    static void Main()
+    {
+        try
+        {
+        <AS:0>}</AS:0>
+        finally
+        {
+            Console.WriteLine(2);
+        }
+    }
+}";
+            var edits = GetTopEdits(src1, src2);
+            var active = GetActiveStatements(src1, src2);
+
+            edits.VerifyRudeDiagnostics(active);
+        }
+
         #endregion
 
         #region Catch

src/EditorFeatures/VisualBasicTest/EditAndContinue/ActiveStatementTests.vb

@@ -3447,6 +3447,135 @@ End Class
             edits.VerifyRudeDiagnostics(active)
         End Sub
 
+        <Fact>
+        Public Sub TryFinally_DeleteStatement_Inner()
+            Dim src1 = "
+Class C
+    Sub Main()
+        <AS:0>Console.WriteLine(0)</AS:0>
+
+        Try
+            <AS:1>Console.WriteLine(1)</AS:1>
+        <ER:1.0>Finally
+            Console.WriteLine(2)
+        End Try</ER:1.0>
+    End Sub
+End Class
+"
+            Dim src2 = "
+Class C
+    Sub Main()
+        <AS:0>Console.WriteLine(0)</AS:0>
+
+        <AS:1>Try</AS:1>
+        Finally
+            Console.WriteLine(2)
+        End Try
+    End Sub
+End Class
+"
+            Dim edits = GetTopEdits(src1, src2)
+            Dim active = GetActiveStatements(src1, src2)
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.DeleteActiveStatement, "Try"))
+        End Sub
+
+        <Fact>
+        Public Sub TryFinally_DeleteStatement_Leaf()
+            Dim src1 = "
+Class C
+    Sub Main()
+        <ER:0.0>Try
+            Console.WriteLine(0)
+        Finally
+            <AS:0>Console.WriteLine(1)</AS:0>
+        End Try</ER:0.0>
+    End Sub
+End Class
+"
+            Dim src2 = "
+Class C
+    Sub Main()
+        Try
+            Console.WriteLine(0)
+        <AS:0>Finally</AS:0>
+        End Try
+    End Sub
+End Class
+"
+            Dim edits = GetTopEdits(src1, src2)
+            Dim active = GetActiveStatements(src1, src2)
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.UpdateAroundActiveStatement, "Finally", VBFeaturesResources.FinallyClause))
+        End Sub
+
+        <Fact>
+        Public Sub Try_DeleteStatement_Inner()
+            Dim src1 = "
+Class C
+    Sub Main()
+        <AS:0>Console.WriteLine(0)</AS:0>
+
+        Try
+            <AS:1>Console.WriteLine(1)</AS:1>
+        Finally
+            Console.WriteLine(2)
+        End Try
+    End Sub
+End Class
+"
+            Dim src2 = "
+Class C
+    Sub Main()
+        <AS:0>Console.WriteLine(0)</AS:0>
+
+        <AS:1>Try</AS:1>
+        Finally
+            Console.WriteLine(2)
+        End Try
+    End Sub
+End Class
+"
+            Dim edits = GetTopEdits(src1, src2)
+            Dim active = GetActiveStatements(src1, src2)
+
+            edits.VerifyRudeDiagnostics(active,
+                Diagnostic(RudeEditKind.DeleteActiveStatement, "Try"))
+        End Sub
+
+        <Fact>
+        Public Sub Try_DeleteStatement_Leaf()
+            Dim src1 = "
+Class C
+    Sub Main()
+
+        Try
+            <AS:0>Console.WriteLine(1)</AS:0>
+        Finally
+            Console.WriteLine(2)
+        End Try
+    End Sub
+End Class
+"
+            Dim src2 = "
+Class C
+    Sub Main()
+
+        <AS:0>Try</AS:0>
+        Finally
+            Console.WriteLine(2)
+        End Try
+    End Sub
+End Class
+"
+            Dim edits = GetTopEdits(src1, src2)
+            Dim active = GetActiveStatements(src1, src2)
+
+            edits.VerifyRudeDiagnostics(active)
+        End Sub
+
         <Fact>
         Public Sub Catch_Add_Inner()
             Dim src1 = "

src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs

@@ -2780,7 +2780,7 @@ protected override List<SyntaxNode> GetExceptionHandlingAncestors(SyntaxNode nod
             List<RudeEditDiagnostic> diagnostics,
             IEnumerable<Edit<SyntaxNode>> exceptionHandlingEdits,
             SyntaxNode oldStatement,
-            SyntaxNode newStatement)
+            TextSpan newStatementSpan)
         {
             foreach (var edit in exceptionHandlingEdits)
             {
@@ -2789,7 +2789,7 @@ protected override List<SyntaxNode> GetExceptionHandlingAncestors(SyntaxNode nod
 
                 if (edit.Kind != EditKind.Update || !AreExceptionClausesEquivalent(edit.OldNode, edit.NewNode))
                 {
-                    AddRudeDiagnostic(diagnostics, edit.OldNode, edit.NewNode, newStatement);
+                    AddRudeDiagnostic(diagnostics, edit.OldNode, edit.NewNode, newStatementSpan);
                 }
             }
         }
@@ -3053,7 +3053,7 @@ private static bool IsSimpleAwaitAssignment(SyntaxNode node, SyntaxNode awaitExp
 
             if (isRude)
             {
-                AddRudeDiagnostic(diagnostics, oldCheckedStatement, newCheckedStatement, newActiveStatement);
+                AddRudeDiagnostic(diagnostics, oldCheckedStatement, newCheckedStatement, newActiveStatement.Span);
             }
         }
 

src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs

@@ -233,7 +233,7 @@ private SyntaxNode FindStatement(SyntaxNode declarationBody, int position, out i
         protected abstract TextSpan GetExceptionHandlingRegion(SyntaxNode node, out bool coversAllChildren);
 
         internal abstract void ReportSyntacticRudeEdits(List<RudeEditDiagnostic> diagnostics, Match<SyntaxNode> match, Edit<SyntaxNode> edit, Dictionary<SyntaxNode, EditKind> editMap);
-        internal abstract void ReportEnclosingExceptionHandlingRudeEdits(List<RudeEditDiagnostic> diagnostics, IEnumerable<Edit<SyntaxNode>> exceptionHandlingEdits, SyntaxNode oldStatement, SyntaxNode newStatement);
+        internal abstract void ReportEnclosingExceptionHandlingRudeEdits(List<RudeEditDiagnostic> diagnostics, IEnumerable<Edit<SyntaxNode>> exceptionHandlingEdits, SyntaxNode oldStatement, TextSpan newStatementSpan);
         internal abstract void ReportOtherRudeEditsAroundActiveStatement(List<RudeEditDiagnostic> diagnostics, Match<SyntaxNode> match, SyntaxNode oldStatement, SyntaxNode newStatement, bool isLeaf);
         internal abstract void ReportMemberUpdateRudeEdits(List<RudeEditDiagnostic> diagnostics, SyntaxNode newMember, TextSpan? span);
         internal abstract void ReportInsertedMemberSymbolRudeEdits(List<RudeEditDiagnostic> diagnostics, ISymbol newSymbol);
@@ -999,15 +999,15 @@ internal struct UpdatedMemberInfo
                 newExceptionRegions[ordinal] = ImmutableArray.Create<LinePositionSpan>();
 
                 TextSpan newSpan;
-                SyntaxNode newStatementSyntax;
+                SyntaxNode newStatementSyntaxOpt;
                 Match<SyntaxNode> match;
 
                 if (oldEnclosingLambdaBody == null)
                 {
                     match = bodyMatch;
 
-                    hasMatching = TryMatchActiveStatement(oldStatementSyntax, statementPart, oldBody, newBody, out newStatementSyntax) ||
-                                  match.TryGetNewNode(oldStatementSyntax, out newStatementSyntax);
+                    hasMatching = TryMatchActiveStatement(oldStatementSyntax, statementPart, oldBody, newBody, out newStatementSyntaxOpt) ||
+                                  match.TryGetNewNode(oldStatementSyntax, out newStatementSyntaxOpt);
                 }
                 else
                 {
@@ -1017,52 +1017,34 @@ internal struct UpdatedMemberInfo
 
                     if (match != null)
                     {
-                        hasMatching = TryMatchActiveStatement(oldStatementSyntax, statementPart, oldEnclosingLambdaBody, newEnclosingLambdaBody, out newStatementSyntax) ||
-                                      match.TryGetNewNode(oldStatementSyntax, out newStatementSyntax);
+                        hasMatching = TryMatchActiveStatement(oldStatementSyntax, statementPart, oldEnclosingLambdaBody, newEnclosingLambdaBody, out newStatementSyntaxOpt) ||
+                                      match.TryGetNewNode(oldStatementSyntax, out newStatementSyntaxOpt);
                     }
                     else
                     {
                         // Lambda match is null if lambdas can't be matched, 
                         // in such case we won't have active statement matched either.
                         hasMatching = false;
-                        newStatementSyntax = null;
+                        newStatementSyntaxOpt = null;
                     }
                 }
 
                 if (hasMatching)
                 {
-                    Debug.Assert(newStatementSyntax != null);
+                    Debug.Assert(newStatementSyntaxOpt != null);
 
                     // The matching node doesn't produce sequence points.
                     // E.g. "const" keyword is inserted into a local variable declaration with an initializer.
-                    newSpan = FindClosestActiveSpan(newStatementSyntax, statementPart);
+                    newSpan = FindClosestActiveSpan(newStatementSyntaxOpt, statementPart);
 
-                    if ((!isLeaf || isPartiallyExecuted) && !AreEquivalentActiveStatements(oldStatementSyntax, newStatementSyntax, statementPart))
+                    if ((!isLeaf || isPartiallyExecuted) && !AreEquivalentActiveStatements(oldStatementSyntax, newStatementSyntaxOpt, statementPart))
                     {
                         // rude edit: internal active statement changed
                         diagnostics.Add(new RudeEditDiagnostic(isLeaf ? RudeEditKind.PartiallyExecutedActiveStatementUpdate : RudeEditKind.ActiveStatementUpdate, newSpan));
                     }
 
-                    // exception handling around the statement:
-
-                    var oldAncestors = GetExceptionHandlingAncestors(oldStatementSyntax, isLeaf);
-                    var newAncestors = GetExceptionHandlingAncestors(newStatementSyntax, isLeaf);
-
-                    if (oldAncestors.Count > 0 || newAncestors.Count > 0)
-                    {
-                        var edits = match.GetSequenceEdits(oldAncestors, newAncestors);
-                        ReportEnclosingExceptionHandlingRudeEdits(diagnostics, edits, oldStatementSyntax, newStatementSyntax);
-
-                        // Exception regions are not needed in presence of errors.
-                        if (diagnostics.Count == 0)
-                        {
-                            Debug.Assert(oldAncestors.Count == newAncestors.Count);
-                            newExceptionRegions[ordinal] = GetExceptionRegions(newAncestors, newText);
-                        }
-                    }
-
                     // other statements around active statement:
-                    ReportOtherRudeEditsAroundActiveStatement(diagnostics, match, oldStatementSyntax, newStatementSyntax, isLeaf);
+                    ReportOtherRudeEditsAroundActiveStatement(diagnostics, match, oldStatementSyntax, newStatementSyntaxOpt, isLeaf);
                 }
                 else if (match == null)
                 {
@@ -1088,6 +1070,18 @@ internal struct UpdatedMemberInfo
                     }
                 }
 
+                // exception handling around the statement:
+                CalculateExceptionRegionsAroundActiveStatement(
+                    bodyMatch,
+                    oldStatementSyntax,
+                    newStatementSyntaxOpt,
+                    newSpan,
+                    ordinal,
+                    newText,
+                    isLeaf,
+                    newExceptionRegions,
+                    diagnostics);
+
                 Debug.Assert(newActiveStatements[ordinal] == default(LinePositionSpan) && newSpan != default(TextSpan));
                 newActiveStatements[ordinal] = newText.Lines.GetLinePositionSpan(newSpan);
 
@@ -1100,6 +1094,44 @@ internal struct UpdatedMemberInfo
             }
         }
 
+        private void CalculateExceptionRegionsAroundActiveStatement(
+            Match<SyntaxNode> bodyMatch,
+            SyntaxNode oldStatementSyntax,
+            SyntaxNode newStatementSyntaxOpt,
+            TextSpan newStatementSyntaxSpan,
+            int ordinal,
+            SourceText newText,
+            bool isLeaf,
+            ImmutableArray<LinePositionSpan>[] newExceptionRegions,
+            List<RudeEditDiagnostic> diagnostics)
+        {
+            if (newStatementSyntaxOpt == null && bodyMatch.NewRoot.Span.Contains(newStatementSyntaxSpan.Start))
+            {
+                newStatementSyntaxOpt = bodyMatch.NewRoot.FindToken(newStatementSyntaxSpan.Start).Parent;
+            }
+
+            if (newStatementSyntaxOpt == null)
+            {
+                return;
+            }
+
+            var oldAncestors = GetExceptionHandlingAncestors(oldStatementSyntax, isLeaf);
+            var newAncestors = GetExceptionHandlingAncestors(newStatementSyntaxOpt, isLeaf);
+
+            if (oldAncestors.Count > 0 || newAncestors.Count > 0)
+            {
+                var edits = bodyMatch.GetSequenceEdits(oldAncestors, newAncestors);
+                ReportEnclosingExceptionHandlingRudeEdits(diagnostics, edits, oldStatementSyntax, newStatementSyntaxSpan);
+
+                // Exception regions are not needed in presence of errors.
+                if (diagnostics.Count == 0)
+                {
+                    Debug.Assert(oldAncestors.Count == newAncestors.Count);
+                    newExceptionRegions[ordinal] = GetExceptionRegions(newAncestors, newText);
+                }
+            }
+        }
+
         /// <summary>
         /// Calculates a syntax map of the entire method body including all lambda bodies it contains (recursively).
         /// Internal for testing.
@@ -1601,7 +1633,7 @@ protected static bool HasEdit(Dictionary<SyntaxNode, EditKind> editMap, SyntaxNo
 
         #region Rude Edits around Active Statement 
 
-        protected void AddRudeDiagnostic(List<RudeEditDiagnostic> diagnostics, SyntaxNode oldNode, SyntaxNode newNode, SyntaxNode newActiveStatement)
+        protected void AddRudeDiagnostic(List<RudeEditDiagnostic> diagnostics, SyntaxNode oldNode, SyntaxNode newNode, TextSpan newActiveStatementSpan)
         {
             if (oldNode == null)
             {
@@ -1609,7 +1641,7 @@ protected void AddRudeDiagnostic(List<RudeEditDiagnostic> diagnostics, SyntaxNod
             }
             else if (newNode == null)
             {
-                AddRudeDeleteAroundActiveStatement(diagnostics, oldNode, newActiveStatement);
+                AddRudeDeleteAroundActiveStatement(diagnostics, oldNode, newActiveStatementSpan);
             }
             else
             {
@@ -1635,11 +1667,11 @@ protected void AddRudeInsertAroundActiveStatement(List<RudeEditDiagnostic> diagn
                 new[] { GetStatementDisplayName(newNode, EditKind.Insert) }));
         }
 
-        protected void AddRudeDeleteAroundActiveStatement(List<RudeEditDiagnostic> diagnostics, SyntaxNode oldNode, SyntaxNode newActiveStatement)
+        protected void AddRudeDeleteAroundActiveStatement(List<RudeEditDiagnostic> diagnostics, SyntaxNode oldNode, TextSpan newActiveStatementSpan)
         {
             diagnostics.Add(new RudeEditDiagnostic(
                 RudeEditKind.DeleteAroundActiveStatement,
-                newActiveStatement.Span,
+                newActiveStatementSpan,
                 oldNode,
                 new[] { GetStatementDisplayName(oldNode, EditKind.Delete) }));
         }

src/Features/Core/Portable/EditAndContinue/EditSession.cs

@@ -64,6 +64,11 @@ public override int GetHashCode()
         private readonly Dictionary<DocumentId, Analysis> _analyses;
 
         // A document id is added whenever any analysis reports rude edits.
+        // We collect a set of document ids that contained a rude edit
+        // at some point in time during the lifespan of an edit session.
+        // At the end of the session we aks the diagnostic analyzer to reanalyze 
+        // the documents to clean up the diagnostics.
+        // An id may be present in this set even if the document doesn't have a rude edit anymore.
         private readonly object _documentsWithReportedRudeEditsGuard = new object();
         private readonly HashSet<DocumentId> _documentsWithReportedRudeEdits;
 

src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb

@@ -2912,12 +2912,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue
         Friend Overrides Sub ReportEnclosingExceptionHandlingRudeEdits(diagnostics As List(Of RudeEditDiagnostic),
                                                                        exceptionHandlingEdits As IEnumerable(Of Edit(Of SyntaxNode)),
                                                                        oldStatement As SyntaxNode,
-                                                                       newStatement As SyntaxNode)
+                                                                       newStatementSpan As TextSpan)
             For Each edit In exceptionHandlingEdits
                 Debug.Assert(edit.Kind <> EditKind.Update OrElse edit.OldNode.RawKind = edit.NewNode.RawKind)
 
                 If edit.Kind <> EditKind.Update OrElse Not AreExceptionHandlingPartsEquivalent(edit.OldNode, edit.NewNode) Then
-                    AddRudeDiagnostic(diagnostics, edit.OldNode, edit.NewNode, newStatement)
+                    AddRudeDiagnostic(diagnostics, edit.OldNode, edit.NewNode, newStatementSpan)
                 End If
             Next
         End Sub
@@ -3112,7 +3112,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue
 
             Dim onErrorOrResumeStatement = FindOnErrorOrResumeStatement(match.NewRoot)
             If onErrorOrResumeStatement IsNot Nothing Then
-                AddRudeDiagnostic(diagnostics, oldActiveStatement, onErrorOrResumeStatement, newActiveStatement)
+                AddRudeDiagnostic(diagnostics, oldActiveStatement, onErrorOrResumeStatement, newActiveStatement.Span)
             End If
 
             ReportRudeEditsForAncestorsDeclaringInterStatementTemps(diagnostics, match, oldActiveStatement, newActiveStatement, isLeaf)

src/VisualStudio/Core/Def/Implementation/EditAndContinue/VsENCRebuildableProjectImpl.cs

@@ -1118,7 +1118,12 @@ public int EncApplySucceeded(int hrApplyResult)
         /// <summary>
         /// Called when changes are being applied.
         /// </summary>
-        public int GetCurrentExceptionSpanPosition(uint id, VsTextSpan[] ptsNewPosition)
+        /// <param name="exceptionRegionId">
+        /// The value of <see cref="ShellInterop.ENC_EXCEPTION_SPAN.id"/>. 
+        /// Set by <see cref="GetExceptionSpans(uint, ShellInterop.ENC_EXCEPTION_SPAN[], ref uint)"/> to the index into <see cref="_exceptionRegions"/>. 
+        /// </param>
+        /// <param name="ptsNewPosition">Output value holder.</param>
+        public int GetCurrentExceptionSpanPosition(uint exceptionRegionId, VsTextSpan[] ptsNewPosition)
         {
             try
             {
@@ -1129,7 +1134,7 @@ public int GetCurrentExceptionSpanPosition(uint id, VsTextSpan[] ptsNewPosition)
                     Debug.Assert(!_encService.EditSession.StoppedAtException);
                     Debug.Assert(ptsNewPosition.Length == 1);
 
-                    var exceptionRegion = _exceptionRegions[(int)id];
+                    var exceptionRegion = _exceptionRegions[(int)exceptionRegionId];
 
                     var session = _encService.EditSession;
                     var asid = _activeStatementIds[exceptionRegion.ActiveStatementId];
@@ -1142,7 +1147,7 @@ public int GetCurrentExceptionSpanPosition(uint id, VsTextSpan[] ptsNewPosition)
                     Debug.Assert(!analysis.HasChangesAndErrors);
                     Debug.Assert(!regions.IsDefault);
 
-                    // Absence of rude edits guarantees that the exception regions around AS hasn't semantically changed.
+                    // Absence of rude edits guarantees that the exception regions around AS haven't semantically changed.
                     // Only their spans might have changed.
                     ptsNewPosition[0] = regions[asid.Ordinal][exceptionRegion.Ordinal].ToVsTextSpan();
                 }