Merge pull request #4117 from DustinCampbell/fix-4115

Add special case to object creation completion to not commit 'object' on '{'

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs

@@ -247,7 +247,7 @@ class C { }
 
             var expected = @"
 using System.Collections.Generic;
-/// <see cref=""List{T}.Enumerator""/>
+/// <see cref=""List{T}.Enumerator ""/>
 class C { }
 ";
             VerifyProviderCommit(text, "Enumerator", expected, ' ', "Enum");
@@ -314,7 +314,7 @@ class @void { }
 ";
 
             var expected = @"using System;
-/// <see cref=""@void""/>
+/// <see cref=""@void ""/>
 class @void { }
 ";
             VerifyProviderCommit(text, "@void", expected, ' ', "@vo");
@@ -380,7 +380,7 @@ class C { }
 
             var expected = @"
 using System.Collections.Generic;
-/// <see cref=""List""/>
+/// <see cref=""List{""/>
 class C { }
 ";
             VerifyProviderCommit(text, "List{T}", expected, '{', "List");
@@ -401,7 +401,7 @@ class C
 
             var expected = @"
 using System.Collections.Generic;
-/// <see cref=""foo""/>
+/// <see cref=""foo(""/>
 class C 
 { 
     public void foo(int x) { }

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ExplicitInterfaceCompletionProviderTests.cs

@@ -87,7 +87,7 @@ interface IFoo
 
 class Bar : IFoo
 {
-     void IFoo.Foo
+     void IFoo.Foo(
 }";
 
             VerifyProviderCommit(markup, "Foo()", expected, '(', "");

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectCreationCompletionProviderTests.cs

@@ -198,7 +198,7 @@ class Program
 {
     static void Main(string[] args)
     {
-        D d=  new D
+        D d=  new D(
     }
 }";
             VerifyProviderCommit(markup, "D", expected, '(', "");
@@ -262,5 +262,113 @@ static void Main(string[] args)
 }";
             VerifyItemExists(markup, "Program");
         }
+
+        [WorkItem(4115, "https://github.com/dotnet/roslyn/issues/4115")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void CommitObjectWithParenthesis1()
+        {
+            var markup = @"
+class C
+{
+    void M1()
+    {
+        object o = new $$
+    }
+}";
+
+            var expected = @"
+class C
+{
+    void M1()
+    {
+        object o = new object(
+    }
+}";
+
+            VerifyProviderCommit(markup, "object", expected, '(', "");
+        }
+
+        [WorkItem(4115, "https://github.com/dotnet/roslyn/issues/4115")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void CommitObjectWithParenthesis2()
+        {
+            var markup = @"
+class C
+{
+    void M1()
+    {
+        M2(new $$
+    }
+
+    void M2(object o) { }
+}";
+
+            var expected = @"
+class C
+{
+    void M1()
+    {
+        M2(new object(
+    }
+
+    void M2(object o) { }
+}";
+
+            VerifyProviderCommit(markup, "object", expected, '(', "");
+        }
+
+        [WorkItem(4115, "https://github.com/dotnet/roslyn/issues/4115")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void DontCommitObjectWithOpenBrace1()
+        {
+            var markup = @"
+class C
+{
+    void M1()
+    {
+        object o = new $$
+    }
+}";
+
+            var expected = @"
+class C
+{
+    void M1()
+    {
+        object o = new {
+    }
+}";
+
+            VerifyProviderCommit(markup, "object", expected, '{', "");
+        }
+
+        [WorkItem(4115, "https://github.com/dotnet/roslyn/issues/4115")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void DontCommitObjectWithOpenBrace2()
+        {
+            var markup = @"
+class C
+{
+    void M1()
+    {
+        M2(new $$
+    }
+
+    void M2(object o) { }
+}";
+
+            var expected = @"
+class C
+{
+    void M1()
+    {
+        M2(new {
+    }
+
+    void M2(object o) { }
+}";
+
+            VerifyProviderCommit(markup, "object", expected, '{', "");
+        }
     }
 }

src/EditorFeatures/Test/Completion/AbstractCompletionProviderTests.cs

@@ -354,7 +354,7 @@ private void VerifyCustomCommitProviderCheckResults(Document document, string co
             }
         }
 
-        private void VerifyProviderCommitCheckResults(Document document, int position, string itemToCommit, string expectedCodeAfterCommit, char? commitChar, string textTypedSoFar)
+        private void VerifyProviderCommitCheckResults(Document document, int position, string itemToCommit, string expectedCodeAfterCommit, char? commitCharOpt, string textTypedSoFar)
         {
             var textBuffer = WorkspaceFixture.Workspace.Documents.Single().TextBuffer;
             var textSnapshot = textBuffer.CurrentSnapshot.AsText();
@@ -363,13 +363,30 @@ private void VerifyProviderCommitCheckResults(Document document, int position, s
             var firstItem = items.First(i => CompareItems(i.DisplayText, itemToCommit));
 
             var completionRules = GetCompletionRules(document);
-            var textChange = completionRules.IsCommitCharacter(firstItem, commitChar.HasValue ? commitChar.Value : ' ', textTypedSoFar)
-                ? completionRules.GetTextChange(firstItem, commitChar, textTypedSoFar)
-                : new TextChange();
+            var commitChar = commitCharOpt ?? '\t';
 
-            var oldText = document.GetTextAsync().Result;
-            var newText = oldText.WithChanges(textChange);
-            Assert.Equal(expectedCodeAfterCommit, newText.ToString());
+            var text = document.GetTextAsync().Result;
+
+            if (commitChar == '\t' || completionRules.IsCommitCharacter(firstItem, commitChar, textTypedSoFar))
+            {
+                var textChange = completionRules.GetTextChange(firstItem, commitChar, textTypedSoFar);
+
+                // Adjust TextChange to include commit character, so long as it isn't TAB.
+                if (commitChar != '\t')
+                {
+                    textChange = new TextChange(textChange.Span, textChange.NewText.TrimEnd(commitChar) + commitChar);
+                }
+
+                text = text.WithChanges(textChange);
+            }
+            else
+            {
+                // nothing was commited, but we should insert the commit character.
+                var textChange = new TextChange(new TextSpan(firstItem.FilterSpan.End, 0), commitChar.ToString());
+                text = text.WithChanges(textChange);
+            }
+
+            Assert.Equal(expectedCodeAfterCommit, text.ToString());
         }
 
         protected void VerifyItemInEditorBrowsableContexts(string markup, string referencedCode, string item, int expectedSymbolsSameSolution, int expectedSymbolsMetadataReference,

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb

@@ -339,7 +339,7 @@ End Class
 
             Dim expected = <File><![CDATA[
 ''' <summary>
-''' <see cref="C."/>
+''' <see cref="C.("/>
 ''' </summary>
 Class C
 Sub bar(x As Integer, y As Integer)
@@ -366,7 +366,7 @@ End Class
             Dim expected = <File><![CDATA[
 Imports System.Collections.Generic
 ''' <summary>
-''' <see cref=""/>
+''' <see cref=" "/>
 ''' </summary>
 Class C
 Sub bar(x As Integer, y As Integer)

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/EnumCompletionProviderTests.vb

@@ -446,7 +446,7 @@ End Enum
  
 Class C
     Sub M()
-        Const e As E = E.A
+        Const e As E = E.A,
     End Sub
 End Class
 ]]></Text>.Value

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ImplementsClauseCompletionProviderTests.vb

@@ -500,7 +500,7 @@ End Interface
 
 Class C
     Implements [Interface]
-    Public Sub test Implements [Interface]
+    Public Sub test Implements [Interface].
 End Class</text>.Value
 
             VerifyProviderCommit(text, "Interface", expected, "."c, "")

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/NamedParameterCompletionProviderTests.vb

@@ -300,7 +300,7 @@ End Module
             Dim expected = <Text>
 Module Program
     Sub Main(args As String())
-        Main(args:
+        Main(args:=
     End Sub
 End Module
 
@@ -323,7 +323,7 @@ End Module
             Dim expected = <Text>
 Module Program
     Sub Main(args As String())
-        Main(args
+        Main(args:
     End Sub
 End Module
 
@@ -346,7 +346,7 @@ End Module
             Dim expected = <Text>
 Module Program
     Sub Main(args As String())
-        Main(args:=
+        Main(args:= 
     End Sub
 End Module
 

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/PartialTypeCompletionProviderTests.vb

@@ -47,6 +47,9 @@ Partial Class $$</text>
 
         <Fact, Trait(Traits.Feature, Traits.Features.Completion)>
         Public Sub PartialGenericClassCommitOnParen()
+            ' TODO(DustinCa): This is testing the wrong behavior and will need to be updated to the commented expected
+            ' result when https://github.com/dotnet/roslyn/issues/4137 is fixed.
+
             Dim text = <text>Class Bar
 End Class
                            
@@ -61,9 +64,17 @@ End Class
 Partial Class C(Of Bar)
 End Class
 
-Partial Class C(Of Bar)</text>
+Partial Class C(Of Bar)(</text>
+
+            '            Dim expected = <text>Class Bar
+            'End Class
+
+            'Partial Class C(Of Bar)
+            'End Class
+
+            'Partial Class C(</text>
 
-            VerifyProviderCommit(text.Value, "C(Of Bar)", expected.Value, "("c, "", Microsoft.CodeAnalysis.SourceCodeKind.Regular)
+            VerifyProviderCommit(text.Value, "C(Of Bar)", expected.Value, "("c, "", SourceCodeKind.Regular)
         End Sub
 
         <Fact, Trait(Traits.Feature, Traits.Features.Completion)>
@@ -84,7 +95,7 @@ End Class
 
 Partial Class C(Of Bar)</text>
 
-            VerifyProviderCommit(text.Value, "C(Of Bar)", expected.Value, Nothing, "", Microsoft.CodeAnalysis.SourceCodeKind.Regular)
+            VerifyProviderCommit(text.Value, "C(Of Bar)", expected.Value, Nothing, "", SourceCodeKind.Regular)
         End Sub
 
         <Fact, Trait(Traits.Feature, Traits.Features.Completion)>

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/SymbolCompletionProviderTests.vb

@@ -2529,7 +2529,7 @@ End Class
 Imports System
 Class classattribute : Inherits Attribute
 End Class
-<[class]
+<[class](
 Class C
 End Class
 </Text>.Value
@@ -5141,7 +5141,7 @@ Class C
 Public Class [Inherits]
 End Class
 Class C
-    Inherits [Inherits]
+    Inherits [Inherits].
 "
 
             VerifyProviderCommit(markup, "Inherits", expected, "."c, "")
@@ -5345,7 +5345,7 @@ Class G
 End Class
 
 Class DG
-    Inherits G
+    Inherits G(
 End Class
 "
             VerifyProviderCommit(markup, "G", expected, "("c, "")
@@ -5547,10 +5547,10 @@ Class G(Of T)
 End Class
 
 Class DG
-    Function Bar() as G(Of
+    Function Bar() as G(
 End Class</code>.Value
 
-            VerifyProviderCommit(text, "G(Of )", expected, Nothing, "")
+            VerifyProviderCommit(text, "G(Of )", expected, "("c, "")
         End Sub
 
         <WorkItem(668159)>
@@ -5670,7 +5670,7 @@ End Class]]></code>.Value
 <code><![CDATA[
 Class Await
     Sub Foo()
-        Dim x = new [Await
+        Dim x = new [Await]
     End Sub
 End Class]]></code>.Value
 

src/Features/CSharp/Completion/CompletionProviders/ObjectCreationCompletionProvider.ItemRules.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.Linq;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.Completion.Providers;
 using Microsoft.CodeAnalysis.LanguageServices;
@@ -15,10 +16,22 @@ internal class ItemRules : AbstractSymbolCompletionItemRules
 
             public override bool? IsCommitCharacter(CompletionItem completionItem, char ch, string textTypedSoFar)
             {
-                // TODO(cyrusn): We could just allow the standard list of completion characters.
-                // However, i'd like to see what the experience is like really filtering down to the set
-                // of things that is allowable.
-                return ch == ' ' || ch == '(' || ch == '{' || ch == '[';
+                if (ch == '{')
+                {
+                    // SPECIAL: If the preselected symbol is System.Object, don't commit on '{'.
+                    // Otherwise, it is cumbersome to type an anonymous object when the target type is object.
+                    // The user would get 'new object {' rather than 'new {'. Since object doesn't have any
+                    // properties, the user never really wants to commit 'new object {' anyway.
+                    var namedTypeSymbol = (completionItem as SymbolCompletionItem)?.Symbols.FirstOrDefault() as INamedTypeSymbol;
+                    if (namedTypeSymbol?.SpecialType == SpecialType.System_Object)
+                    {
+                        return false;
+                    }
+
+                    return true;
+                }
+                
+                return ch == ' ' || ch == '(' || ch == '[';
             }
 
             protected override string GetInsertionText(ISymbol symbol, AbstractSyntaxContext context, char ch)