Watson: Fix crash when accessing CodeProperty.OverrideKind property for...

Watson: Fix crash when accessing CodeProperty.OverrideKind property for property not contained by a type

The root cause is based on analysis of a Watson crash dump with ~300 hits. The call stack looks like this:

```
microsoft_visualstudio_languageservices_csharp_ni!Microsoft.VisualStudio.LanguageServices.CSharp.CodeModel.CSharpCodeModelService.GetOverrideKind
microsoft_visualstudio_languageservices_implementation_ni!Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel.InternalElements.AbstractCodeMember.get_OverrideKind
envdte80!EnvDTE80.CodeProperty2.get_OverrideKind
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeController+AllLanguages+AttributeIsNew.GetAttribute
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeController.GetAttribute
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeController.GetProperties
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.FireAddEvent
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.AddElement
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler.WalkCodeElement
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.AddElementWithChildren
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.RebindChildren
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.ChangeElementUnknown
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler+CodeModelCache.ChangeElement
microsoft_visualstudio_modeling_artifactmapper_vshost!Microsoft.VisualStudio.Modeling.ArtifactMapper.VSHost.CodeHandler.codeModelEvents_Changed
```

After analysis of the dump, I was able to reproduce the call stack using the following steps:

1. Create new Class Library
2. In the Solution Explorer, right-click on Class1.cs and choose View->View Class Diagram from the context menu.
3. Add a new property inside Class1.
4. Copy and paste that property *twice* inside the enclosing namespace of Class1.

This causes an "Unknown" code model event to fire which the Class Designer responds to by digging through its elements and accessing various properties. One of those properties (`OverrideKind`) retrieves the containing type of the element, but that's null for the properties contained in a namespace and the call throws. So, the fix is to add a couple of simple null checks. Note that this affects all members, so I added tests for fields, events and methods as well.

src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs

@@ -2417,12 +2417,12 @@ public override EnvDTE80.vsCMOverrideKind GetOverrideKind(SyntaxNode memberNode)
 
             var result = EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone;
 
-            if ((flags & ModifierFlags.Abstract) != 0 || containingType.Kind() == SyntaxKind.InterfaceDeclaration)
+            if ((flags & ModifierFlags.Abstract) != 0 || containingType?.Kind() == SyntaxKind.InterfaceDeclaration)
             {
                 result |= EnvDTE80.vsCMOverrideKind.vsCMOverrideKindAbstract;
             }
 
-            if ((flags & ModifierFlags.Virtual) != 0 || containingType.Kind() == SyntaxKind.InterfaceDeclaration)
+            if ((flags & ModifierFlags.Virtual) != 0 || containingType?.Kind() == SyntaxKind.InterfaceDeclaration)
             {
                 result |= EnvDTE80.vsCMOverrideKind.vsCMOverrideKindVirtual;
             }

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

@@ -197,7 +197,7 @@ public void RemoveParameter(object element)
 
                 if (codeElement == null)
                 {
-                    throw new ArgumentException(ServicesVSResources.ElementIsNotValid, "element");
+                    throw new ArgumentException(ServicesVSResources.ElementIsNotValid, nameof(element));
                 }
 
                 codeElement.Delete();

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

@@ -184,7 +184,7 @@ public void Remove(object element)
 
             if (codeElement == null)
             {
-                throw new ArgumentException(ServicesVSResources.ElementIsNotValid, "element");
+                throw new ArgumentException(ServicesVSResources.ElementIsNotValid, nameof(element));
             }
 
             codeElement.Delete();

src/VisualStudio/Core/Test/CodeModel/AbstractCodeEventTests.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 EnvDTE80
+
 Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
     Public MustInherit Class AbstractCodeEventTests
         Inherits AbstractCodeElementTests(Of EnvDTE80.CodeEvent)
@@ -52,6 +54,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
             Return Sub(name) codeElement.Name = name
         End Function
 
+        Protected Overrides Function GetOverrideKind(codeElement As CodeEvent) As vsCMOverrideKind
+            Return codeElement.OverrideKind
+        End Function
+
         Protected Overrides Function GetParent(codeElement As EnvDTE80.CodeEvent) As Object
             Return codeElement.Parent
         End Function

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

@@ -552,6 +552,42 @@ class C1 : I1
 
 #End Region
 
+#Region "OverrideKind tests"
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForEventOutsideClass1() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    event System.EventHandler $$E;
+}
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForEventOutsideClass2() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    event System.EventHandler $$E
+    {
+        add { }
+        remove { }
+    }
+}
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
+#End Region
+
 #Region "Type tests"
 
         <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>

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

@@ -841,6 +841,22 @@ class C : B
             Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNew)
         End Function
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForMethodOutsideClass() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    void $$M()
+    {
+    }
+}
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
 #End Region
 
 #Region "Prototype tests"

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

@@ -1038,6 +1038,20 @@ abstract class C
             Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindAbstract)
         End Function
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForPropertyOutsideClass() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    int $$P { get { return 42; } }
+}
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
 #End Region
 
 #Region "ReadWrite tests"

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

@@ -875,6 +875,122 @@ class Program
                  Remove("System.CLSCompliant", "bar"))
         End Function
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedMethodsInNamespace() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    void M()
+    {
+    }
+}
+</Code>
+
+            Dim changedCode =
+<Code>
+namespace N
+{
+    void M()
+    {
+    }
+
+    void M()
+    {
+    }
+}
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedPropertiesInNamespace() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    int P { get { return 42; } }
+}
+</Code>
+
+            Dim changedCode =
+<Code>
+namespace N
+{
+    int P { get { return 42; } }
+    int P { get { return 42; } }
+}
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedEventsInNamespace1() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    event System.EventHandler E;
+}
+</Code>
+
+            Dim changedCode =
+<Code>
+namespace N
+{
+    event System.EventHandler E;
+    event System.EventHandler E;
+}
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedEventsInNamespace2() As Task
+            Dim code =
+<Code>
+namespace N
+{
+    event System.EventHandler E
+    {
+        add { }
+        remove { }
+    }
+}
+</Code>
+
+            Dim changedCode =
+<Code>
+namespace N
+{
+    event System.EventHandler E
+    {
+        add { }
+        remove { }
+    }
+
+    event System.EventHandler E
+    {
+        add { }
+        remove { }
+    }
+}
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
         Protected Overrides ReadOnly Property LanguageName As String
             Get
                 Return LanguageNames.CSharp

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

@@ -579,6 +579,43 @@ End Class
         End Function
 #End Region
 
+#Region "OverrideKind tests"
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForEventOutsideClass1() As Task
+            Dim code =
+<Code>
+Namespace N
+    Event $$E()
+End Namespace
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForEventOutsideClass2() As Task
+            Dim code =
+<Code>
+Namespace N
+    Custom Event $$E As System.EventHandler
+        AddHandler(value As System.EventHandler)
+        End AddHandler
+        RemoveHandler(value As System.EventHandler)
+        End RemoveHandler
+        RaiseEvent(sender As Object, e As System.EventArgs)
+        End RaiseEvent
+    End Event
+End Namespace
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
+#End Region
+
 #Region "Type tests"
 
         <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>

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

@@ -1150,6 +1150,20 @@ End Class
             Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNew)
         End Function
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForMethodOutsideClass() As Task
+            Dim code =
+<Code>
+Namespace N
+    Sub $$M()
+    End Sub
+End Namespace
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
 #End Region
 
 #Region "Prototype tests"

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

@@ -470,6 +470,19 @@ End Class
             Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindAbstract)
         End Function
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModel)>
+        Public Async Function TestOverrideKind_DontCrashForPropertyOutsideClass() As Task
+            Dim code =
+<Code>
+Namespace N
+    ReadOnly Property $$P As Integer = 42
+End Namespace
+</Code>
+
+            Await TestOverrideKind(code, EnvDTE80.vsCMOverrideKind.vsCMOverrideKindNone)
+        End Function
+
 #End Region
 
 #Region "Prototype tests"

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

@@ -2260,6 +2260,120 @@ End Class
 
 #End Region
 
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedMethodsInNamespace() As Task
+            Dim code =
+<Code>
+Namespace N
+    Sub M()
+    End Sub
+End Namespace
+</Code>
+
+            Dim changedCode =
+<Code>
+Namespace N
+    Sub M()
+    End Sub
+
+    Sub M()
+    End Sub
+End Namespace
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedPropertiesInNamespace() As Task
+            Dim code =
+<Code>
+Namespace N
+    ReadOnly Property P As Integer = 42
+End Namespace
+</Code>
+
+            Dim changedCode =
+<Code>
+Namespace N
+    ReadOnly Property P As Integer = 42
+    ReadOnly Property P As Integer = 42
+End Namespace
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedEventsInNamespace1() As Task
+            Dim code =
+<Code>
+Namespace N
+    Event E()
+End Namespace
+</Code>
+
+            Dim changedCode =
+<Code>
+Namespace N
+    Event E()
+    Event E()
+End Namespace
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
+        <WorkItem(150349)>
+        <ConditionalWpfFact(GetType(x86)), Trait(Traits.Feature, Traits.Features.CodeModelEvents)>
+        Public Async Function DontCrashOnDuplicatedEventsInNamespace2() As Task
+            Dim code =
+<Code>
+Namespace N
+    Custom Event E As System.EventHandler
+        AddHandler(value As System.EventHandler)
+        End AddHandler
+        RemoveHandler(value As System.EventHandler)
+        End RemoveHandler
+        RaiseEvent(sender As Object, e As System.EventArgs)
+        End RaiseEvent
+    End Event
+End Namespace
+</Code>
+
+            Dim changedCode =
+<Code>
+Namespace N
+    Custom Event E As System.EventHandler
+        AddHandler(value As System.EventHandler)
+        End AddHandler
+        RemoveHandler(value As System.EventHandler)
+        End RemoveHandler
+        RaiseEvent(sender As Object, e As System.EventArgs)
+        End RaiseEvent
+    End Event
+
+    Custom Event E As System.EventHandler
+        AddHandler(value As System.EventHandler)
+        End AddHandler
+        RemoveHandler(value As System.EventHandler)
+        End RemoveHandler
+        RaiseEvent(sender As Object, e As System.EventArgs)
+        End RaiseEvent
+    End Event
+End Namespace
+</Code>
+
+            Await TestAsync(code, changedCode,
+                 Unknown("N"))
+        End Function
+
         Protected Overrides ReadOnly Property LanguageName As String
             Get
                 Return LanguageNames.VisualBasic

src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.CodeModelEventCollector.vb

@@ -307,7 +307,6 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.CodeModel
                         eventQueue)
                 End If
 
-                Debug.Fail(String.Format("Invalid node: {0}", oldNamespaceOrType.Kind))
                 Return False
             End Function