Update override completion for nullable reference support

- Ensure we show the ? in all the appropriate places
- Ensure we don't spit ? if we shouldn't
- Ensure we drop ? for the special case of the ToString return

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

@@ -693,6 +693,23 @@ public class SomeClass : Base
             await VerifyItemExistsAsync(markup, "Goo(Exception e)");
         }
 
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task NullableAnnotationsIncluded()
+        {
+            var markup = @"#nullable enable
+
+public abstract class Base
+{
+    public abstract void Goo(string? s);
+}
+
+public class SomeClass : Base
+{
+    override $$
+}";
+            await VerifyItemExistsAsync(markup, "Goo(string? s)");
+        }
+
         [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task EscapedMethodNameInIntelliSenseList()
         {
@@ -1631,6 +1648,121 @@ public override void goo<T>()
             await VerifyCustomCommitProviderAsync(markupBeforeCommit, "goo<T>()", expectedCodeAfterCommit);
         }
 
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task CommitMethodWithNullableAttributes()
+        {
+            var markupBeforeCommit = @"
+#nullable enable
+
+class C
+{
+    public virtual string? Goo(string? s) { }
+}
+
+class D : C
+{
+    override $$
+}";
+
+            var expectedCodeAfterCommit = @"
+#nullable enable
+
+class C
+{
+    public virtual string? Goo(string? s) { }
+}
+
+class D : C
+{
+    public override string? Goo(string? s)
+    {
+        return base.Goo(s);$$
+    }
+}";
+
+            await VerifyCustomCommitProviderAsync(markupBeforeCommit, "Goo(string? s)", expectedCodeAfterCommit);
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task CommitMethodInNullableDisableContext()
+        {
+            var markupBeforeCommit = @"
+#nullable enable
+
+class C
+{
+    public virtual string? Goo(string? s) { }
+}
+
+#nullable disable
+
+class D : C
+{
+    override $$
+}";
+
+            var expectedCodeAfterCommit = @"
+#nullable enable
+
+class C
+{
+    public virtual string? Goo(string? s) { }
+}
+
+#nullable disable
+
+class D : C
+{
+    public override string Goo(string s)
+    {
+        return base.Goo(s);$$
+    }
+}";
+
+            await VerifyCustomCommitProviderAsync(markupBeforeCommit, "Goo(string? s)", expectedCodeAfterCommit);
+        }
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task CommitToStringIsExplicitlyNonNullReturning()
+        {
+            var markupBeforeCommit = @"
+#nullable enable
+
+namespace System
+{
+    public class Object
+    {
+        public virtual string? ToString() { }
+    }
+}
+
+class D : System.Object
+{
+    override $$
+}";
+
+            var expectedCodeAfterCommit = @"
+#nullable enable
+
+namespace System
+{
+    public class Object
+    {
+        public virtual string? ToString() { }
+    }
+}
+
+class D : System.Object
+{
+    public override string ToString()
+    {
+        return base.ToString();$$
+    }
+}";
+
+            await VerifyCustomCommitProviderAsync(markupBeforeCommit, "ToString()", expectedCodeAfterCommit);
+        }
+
         [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task CommitInsertIndexer()
         {

src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.ItemGetter.cs

@@ -5,6 +5,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeGeneration;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -24,7 +25,8 @@ private partial class ItemGetter
                 SymbolDisplayParameterOptions.IncludeExtensionThis |
                 SymbolDisplayParameterOptions.IncludeType |
                 SymbolDisplayParameterOptions.IncludeName |
-                SymbolDisplayParameterOptions.IncludeParamsRefOut);
+                SymbolDisplayParameterOptions.IncludeParamsRefOut)
+                .AddMiscellaneousOptions(SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier);
 
             private readonly Document _document;
             private readonly SourceText _text;

src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeGeneration;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
@@ -37,6 +38,20 @@ public override async Task ProvideCompletionsAsync(CompletionContext context)
 
         protected override Task<ISymbol> GenerateMemberAsync(ISymbol newOverriddenMember, INamedTypeSymbol newContainingType, Document newDocument, CompletionItem completionItem, CancellationToken cancellationToken)
         {
+            // Special case: if you are overriding object.ToString(), we will make the return value as non-nullable. The return was made nullable because
+            // are implementations out there that will return null, but that's not something we really want new implementations doing. We may need to consider
+            // expanding this behavior to other methods in the future; if that is the case then we would want there to be an attribute on the return type
+            // rather than updating this list, but for now there is no such attribute until we find more cases for it. See
+            // https://github.com/dotnet/roslyn/issues/30317 for some additional conversation about this design decision.
+            //
+            // We don't check if methodSymbol.ContainingType is object, in case you're overriding something that is itself an override
+            if (newOverriddenMember is IMethodSymbol methodSymbol &&
+                methodSymbol.Name == "ToString" &&
+                methodSymbol.Parameters.Length == 0)
+            {
+                newOverriddenMember = CodeGenerationSymbolFactory.CreateMethodSymbol(methodSymbol, returnType: methodSymbol.ReturnType.WithNullability(NullableAnnotation.NotAnnotated));
+            }
+
             // Figure out what to insert, and do it. Throw if we've somehow managed to get this far and can't.
             var syntaxFactory = newDocument.GetLanguageService<SyntaxGenerator>();
 

src/Workspaces/Core/Portable/CodeGeneration/CodeGenerationSymbolFactory.cs

@@ -427,14 +427,15 @@ public static INamespaceSymbol CreateNamespaceSymbol(string name, IList<ISymbol>
             string name = null,
             ImmutableArray<IParameterSymbol>? parameters = default,
             ImmutableArray<SyntaxNode> statements = default,
-            INamedTypeSymbol containingType = null)
+            INamedTypeSymbol containingType = null,
+            ITypeSymbol returnType = null)
         {
             return CreateMethodSymbol(
                 containingType,
                 attributes,
                 accessibility ?? method.DeclaredAccessibility,
                 modifiers ?? method.GetSymbolModifiers(),
-                method.GetReturnTypeWithAnnotatedNullability(),
+                returnType ?? method.GetReturnTypeWithAnnotatedNullability(),
                 method.RefKind,
                 explicitInterfaceImplementations,
                 name ?? method.Name,