Merge pull request #8424 from DustinCampbell/issue-8423

Code Model: Don't crash when adding element, deleting its text and then adding it again

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel.cs

@@ -199,6 +199,27 @@ internal void OnCodeElementCreated(SyntaxNodeKey nodeKey, EnvDTE.CodeElement ele
             _codeElementTable.Add(nodeKey, element);
         }
 
+        internal void OnBeforeCodeElementCreated(SyntaxNode node)
+        {
+            // It's conceivable that a consumer is creating a code element with the same node key as a "dead" element
+            // that hasn't been removed from the cache yet. For example, the element could have been "deleted" by
+            // simply replacing its text in the underlying buffer. To handle this situation, we test to see if the
+            // element is "dead" by checking whether it's underlying node is invalid (that is, it can't be found by
+            // its node key). If the element is "dead", we'll go ahead and remove it from the cache here to avoid a
+            // collision with the new element.
+
+            var nodeKey = CodeModelService.TryGetNodeKey(node);
+            EnvDTE.CodeElement codeElement;
+            if (!nodeKey.IsEmpty && _codeElementTable.TryGetValue(nodeKey, out codeElement))
+            {
+                var managedElement = ComAggregate.GetManagedObject<AbstractKeyedCodeElement>(codeElement);
+                if (managedElement?.IsValidNode() != true)
+                {
+                    _codeElementTable.Remove(nodeKey);
+                }
+            }
+        }
+
         internal void OnCodeElementDeleted(SyntaxNodeKey nodeKey)
         {
             _codeElementTable.Remove(nodeKey);

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel_CodeGen.cs

@@ -62,6 +62,8 @@ private SyntaxNode InsertMember(SyntaxNode containerNode, SyntaxNode memberNode,
                 var resultNode = CodeModelService.InsertMember(
                     document, IsBatchOpen, insertionIndex, containerNode, memberNode, CancellationToken.None, out newDocument);
 
+                OnBeforeCodeElementCreated(resultNode);
+
                 return Tuple.Create(resultNode, newDocument);
             });
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/AbstractCodeElement.cs

@@ -56,9 +56,8 @@ protected ProjectId GetProjectId()
 
         internal bool IsValidNode()
         {
-            var node = LookupNode();
-
-            if (node == null)
+            SyntaxNode node;
+            if (!TryLookupNode(out node))
             {
                 return false;
             }
@@ -72,7 +71,18 @@ internal bool IsValidNode()
             return true;
         }
 
-        internal abstract SyntaxNode LookupNode();
+        internal virtual SyntaxNode LookupNode()
+        {
+            SyntaxNode node;
+            if (!TryLookupNode(out node))
+            {
+                throw Exceptions.ThrowEFail();
+            }
+
+            return node;
+        }
+
+        internal abstract bool TryLookupNode(out SyntaxNode node);
 
         internal virtual ISymbol LookupSymbol()
         {

src/VisualStudio/Core/Impl/CodeModel/InternalElements/AbstractKeyedCodeElement.cs

@@ -55,7 +55,7 @@ internal override SyntaxNode LookupNode()
             return CodeModelService.LookupNode(_nodeKey, GetSyntaxTree());
         }
 
-        internal bool TryLookupNode(out SyntaxNode node)
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
             return CodeModelService.TryLookupNode(_nodeKey, GetSyntaxTree(), out node);
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeAccessorFunction.cs

@@ -45,21 +45,24 @@ private bool IsPropertyAccessor()
             return _kind == MethodKind.PropertyGet || _kind == MethodKind.PropertySet;
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parentHandle.Value.LookupNode();
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode accessorNode;
             if (!CodeModelService.TryGetAccessorNode(parentNode, _kind, out accessorNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return accessorNode;
+            node = accessorNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeAttribute.cs

@@ -47,24 +47,27 @@ protected override EnvDTE.CodeElements GetCollection()
             return GetCollection<CodeAttribute>(Parent);
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parent != null
                 ? _parent.LookupNode()
                 : FileCodeModel.GetSyntaxRoot();
 
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode attributeNode;
             if (!CodeModelService.TryGetAttributeNode(parentNode, _name, _ordinal, out attributeNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return attributeNode;
+            node = attributeNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeAttributeArgument.cs

@@ -40,19 +40,33 @@ protected override EnvDTE.CodeElements GetCollection()
 
         internal override SyntaxNode LookupNode()
         {
+            SyntaxNode node;
+            if (!TryLookupNode(out node))
+            {
+                throw Exceptions.ThrowEUnexpected();
+            }
+
+            return node;
+        }
+
+        internal override bool TryLookupNode(out SyntaxNode node)
+        {
+            node = null;
+            
             var attributeNode = _parentHandle.Value.LookupNode();
             if (attributeNode == null)
             {
-                throw Exceptions.ThrowEUnexpected();
+                return false;
             }
 
             SyntaxNode attributeArgumentNode;
             if (!CodeModelService.TryGetAttributeArgumentNode(attributeNode, _index, out attributeArgumentNode))
             {
-                throw Exceptions.ThrowEUnexpected();
+                return false;
             }
 
-            return attributeArgumentNode;
+            node = attributeArgumentNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeImplementsStatement.cs

@@ -60,21 +60,24 @@ public sealed class CodeImplementsStatement : AbstractCodeElement
             _namespaceName = name;
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parentHandle.Value.LookupNode();
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode implementsNode;
             if (!CodeModelService.TryGetImplementsNode(parentNode, _namespaceName, _ordinal, out implementsNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return implementsNode;
+            node = implementsNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeImport.cs

@@ -60,24 +60,27 @@ public sealed class CodeImport : AbstractCodeElement, EnvDTE80.CodeImport
             _dottedName = dottedName;
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parentHandle.Value != null
                 ? _parentHandle.Value.LookupNode()
                 : FileCodeModel.GetSyntaxRoot();
 
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode importNode;
             if (!CodeModelService.TryGetImportNode(parentNode, _dottedName, out importNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return importNode;
+            node = importNode;
+            return node != null;
         }
 
         internal override ISymbol LookupSymbol()

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeInheritsStatement.cs

@@ -60,21 +60,24 @@ public sealed class CodeInheritsStatement : AbstractCodeElement
             _namespaceName = name;
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parentHandle.Value.LookupNode();
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode inheritsNode;
             if (!CodeModelService.TryGetInheritsNode(parentNode, _namespaceName, _ordinal, out inheritsNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return inheritsNode;
+            node = inheritsNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeOptionsStatement.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System;
 using System.Runtime.InteropServices;
 using Microsoft.CodeAnalysis;
 using Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel.Collections;
@@ -59,21 +58,24 @@ public sealed class CodeOptionsStatement : AbstractCodeElement
             _name = name;
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = this.FileCodeModel.GetSyntaxRoot();
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode optionNode;
             if (!CodeModelService.TryGetOptionNode(parentNode, _name, _ordinal, out optionNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return optionNode;
+            node = optionNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeParameter.cs

@@ -78,21 +78,24 @@ protected override string GetFullName()
             return CodeModelService.GetParameterFullName(node);
         }
 
-        internal override SyntaxNode LookupNode()
+        internal override bool TryLookupNode(out SyntaxNode node)
         {
+            node = null;
+
             var parentNode = _parentHandle.Value.LookupNode();
             if (parentNode == null)
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
             SyntaxNode parameterNode;
             if (!CodeModelService.TryGetParameterNode(parentNode, _name, out parameterNode))
             {
-                throw Exceptions.ThrowEFail();
+                return false;
             }
 
-            return parameterNode;
+            node = parameterNode;
+            return node != null;
         }
 
         public override EnvDTE.vsCMElement Kind

src/VisualStudio/Core/Test/CodeModel/CSharp/CodeClassTests.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+Imports System.Threading
 Imports System.Threading.Tasks
 Imports Microsoft.CodeAnalysis
+Imports Microsoft.CodeAnalysis.Text
 Imports Roslyn.Test.Utilities
 
 Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel.CSharp
@@ -3830,6 +3832,35 @@ class C$$
                 End Sub)
         End Function
 
+        <WorkItem(8423, "https://github.com/dotnet/roslyn/issues/8423")>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestAddAndRemoveViaTextChangeManyTimes() As Task
+            Dim code =
+<Code>
+class C$$
+{
+}
+</Code>
+
+            Await TestElement(code,
+                Sub(state, codeClass)
+                    For i = 1 To 100
+                        Dim variable = codeClass.AddVariable("x", "System.Int32")
+
+                        ' Now, delete the variable that we just added.
+                        Dim startPoint = variable.StartPoint
+                        Dim document = state.FileCodeModelObject.GetDocument()
+                        Dim text = document.GetTextAsync(CancellationToken.None).Result
+                        Dim textLine = text.Lines(startPoint.Line - 1)
+                        text = text.Replace(textLine.SpanIncludingLineBreak, "")
+                        document = document.WithText(text)
+
+                        Dim result = state.VisualStudioWorkspace.TryApplyChanges(document.Project.Solution)
+                        Assert.True(result)
+                    Next
+                End Sub)
+        End Function
+
         <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
         Public Async Function TestTypeDescriptor_GetProperties() As Task
             Dim code =

src/VisualStudio/Core/Test/CodeModel/VisualBasic/CodeClassTests.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 Imports System.Runtime.InteropServices
+Imports System.Threading
 Imports System.Threading.Tasks
 Imports Microsoft.CodeAnalysis
 Imports Microsoft.VisualStudio.LanguageServices.VisualBasic.CodeModel.Extenders
@@ -3141,6 +3142,34 @@ End Class
                 End Sub)
         End Function
 
+        <WorkItem(8423, "https://github.com/dotnet/roslyn/issues/8423")>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestAddAndRemoveViaTextChangeManyTimes() As Task
+            Dim code =
+<Code>
+Class C$$
+End Class
+</Code>
+
+            Await TestElement(code,
+                Sub(state, codeClass)
+                    For i = 1 To 100
+                        Dim variable = codeClass.AddVariable("x", "System.Int32")
+
+                        ' Now, delete the variable that we just added.
+                        Dim startPoint = variable.StartPoint
+                        Dim document = state.FileCodeModelObject.GetDocument()
+                        Dim text = document.GetTextAsync(CancellationToken.None).Result
+                        Dim textLine = text.Lines(startPoint.Line - 1)
+                        text = text.Replace(textLine.SpanIncludingLineBreak, "")
+                        document = document.WithText(text)
+
+                        Dim result = state.VisualStudioWorkspace.TryApplyChanges(document.Project.Solution)
+                        Assert.True(result)
+                    Next
+                End Sub)
+        End Function
+
         <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
         Public Async Function TestExternalClass_ImplementedInterfaces() As Task
             Dim code =

src/VisualStudio/Core/Test/MockVisualStudioWorkspace.vb

@@ -59,7 +59,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests
         End Function
 
         Public Overrides Function GetFileCodeModel(documentId As DocumentId) As EnvDTE.FileCodeModel
-            Return CType(_fileCodeModels(documentId).Handle, EnvDTE.FileCodeModel)
+            Return _fileCodeModels(documentId).Handle
         End Function
 
         Public Overrides Function TryGoToDefinition(symbol As ISymbol, project As Project, cancellationToken As CancellationToken) As Boolean