diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs index 0bc9df7d74f2345c51e5f33647dee61575406cf8..dd9f7ca3f08194539b4756224500cd413ec7d35c 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs +++ b/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() await VerifyCustomCommitProviderAsync(markupBeforeCommit, "goo()", 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() { diff --git a/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.ItemGetter.cs b/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.ItemGetter.cs index fd387a1d9de3451a070cf6936ad4dcf203108192..da6e747b6f7b0676ace165193f7cca7152e0aaf7 100644 --- a/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.ItemGetter.cs +++ b/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; diff --git a/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.cs index d550d228d54cd9fc420ea2d5fbc1cc57bf59b211..74193feae6bf4736b1e16e2175eeda759478d437 100644 --- a/src/Features/Core/Portable/Completion/Providers/AbstractOverrideCompletionProvider.cs +++ b/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 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(); diff --git a/src/Workspaces/Core/Portable/CodeGeneration/CodeGenerationSymbolFactory.cs b/src/Workspaces/Core/Portable/CodeGeneration/CodeGenerationSymbolFactory.cs index 4e116f2c4347bc733f1ebfa4dd20f3fc517a518e..4fbda085ce038aa41387196142c037d812178dd5 100644 --- a/src/Workspaces/Core/Portable/CodeGeneration/CodeGenerationSymbolFactory.cs +++ b/src/Workspaces/Core/Portable/CodeGeneration/CodeGenerationSymbolFactory.cs @@ -427,14 +427,15 @@ public static INamespaceSymbol CreateNamespaceSymbol(string name, IList string name = null, ImmutableArray? parameters = default, ImmutableArray 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,