From 379d9f3b05d5018f9ebdcb9c934dfe002876d650 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 30 Jan 2024 11:41:16 +0300 Subject: [PATCH 1/5] Allow `ctor` snippet after accessibility modifiers or `static` keyword --- ...nstructorSnippetCompletionProviderTests.cs | 71 ++++++++++++++++++- .../CSharpConstructorSnippetProvider.cs | 41 +++++++++++ .../AbstractConstructorSnippetProvider.cs | 23 +----- 3 files changed, 112 insertions(+), 23 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs index 11e7c74a1de5d..6af8b5c869cac 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs @@ -29,7 +29,7 @@ namespace Namespace } [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] - public async Task ConstructorSnippetMissingInFilescopedNamespace() + public async Task ConstructorSnippetMissingInFileScopedNamespace() { var markupBeforeCommit = """ @@ -254,5 +254,74 @@ public MyClass1() """; await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("internal")] + [InlineData("private protected")] + [InlineData("protected internal")] + [InlineData("static")] + public async Task InsertConstructorSnippetAfterValidModifiersTest(string modifiers) + { + var markupBeforeCommit = $$""" + class MyClass + { + {{modifiers}} $$ + } + """; + + var expectedCodeAfterCommit = $$""" + class MyClass + { + {{modifiers}} MyClass() + { + $$ + } + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfTheory] + [InlineData("abstract")] + [InlineData("sealed")] + [InlineData("virtual")] + [InlineData("override")] + [InlineData("readonly")] + [InlineData("new")] + [InlineData("file")] + public async Task ConstructorSnippetMissingAfterInvalidModifierTest(string modifier) + { + var markupBeforeCommit = $$""" + class MyClass + { + {{modifier}} $$ + } + """; + + await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); + } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("internal")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task ConstructorSnippetMissingAfterBothAccessibilityModifierAndStaticKeywordTest(string accessibilityModifier) + { + var markupBeforeCommit = $$""" + class MyClass + { + {{accessibilityModifier}} static $$ + } + """; + + await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); + } } } diff --git a/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs index 27449bc8b4b23..780c7f918c8e9 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs @@ -3,12 +3,15 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Composition; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Utilities; +using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -16,12 +19,22 @@ using Microsoft.CodeAnalysis.Snippets; using Microsoft.CodeAnalysis.Snippets.SnippetProviders; using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Snippets { [ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared] internal sealed class CSharpConstructorSnippetProvider : AbstractConstructorSnippetProvider { + private static readonly ISet s_validModifiers = new HashSet(SyntaxFacts.EqualityComparer) + { + SyntaxKind.PublicKeyword, + SyntaxKind.PrivateKeyword, + SyntaxKind.ProtectedKeyword, + SyntaxKind.InternalKeyword, + SyntaxKind.StaticKeyword, + }; + [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public CSharpConstructorSnippetProvider() @@ -33,13 +46,41 @@ protected override async Task IsValidSnippetLocationAsync(Document documen var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false); var syntaxContext = (CSharpSyntaxContext)document.GetRequiredLanguageService().CreateContext(document, semanticModel, position, cancellationToken); + var precedingModifiers = syntaxContext.PrecedingModifiers; + + if (!(precedingModifiers.All(SyntaxFacts.IsAccessibilityModifier) || + precedingModifiers.Count == 1 && precedingModifiers.Single() == SyntaxKind.StaticKeyword)) + { + return false; + } + return syntaxContext.IsMemberDeclarationContext( + validModifiers: s_validModifiers, validTypeDeclarations: SyntaxKindSet.ClassStructRecordTypeDeclarations, canBePartial: true, cancellationToken: cancellationToken); } + protected override async Task GenerateSnippetTextChangeAsync(Document document, int position, CancellationToken cancellationToken) + { + var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false); + var syntaxContext = (CSharpSyntaxContext)document.GetRequiredLanguageService().CreateContext(document, semanticModel, position, cancellationToken); + + var containingType = syntaxContext.ContainingTypeDeclaration; + Contract.ThrowIfNull(containingType); + + var containingTypeSymbol = semanticModel.GetDeclaredSymbol(containingType, cancellationToken); + Contract.ThrowIfNull(containingTypeSymbol); + + var generator = SyntaxGenerator.GetGenerator(document); + var constructorDeclaration = generator.ConstructorDeclaration( + containingTypeName: containingType.Identifier.ToString(), + accessibility: syntaxContext.PrecedingModifiers.Any() ? Accessibility.NotApplicable : (containingTypeSymbol.IsAbstract ? Accessibility.Protected : Accessibility.Public)); + + return new TextChange(TextSpan.FromBounds(position, position), constructorDeclaration.NormalizeWhitespace().ToFullString()); + } + protected override int GetTargetCaretPosition(ISyntaxFactsService syntaxFacts, SyntaxNode caretTarget, SourceText sourceText) { return CSharpSnippetHelpers.GetTargetCaretPositionInBlock( diff --git a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractConstructorSnippetProvider.cs b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractConstructorSnippetProvider.cs index 653975a012174..29b7a7067306d 100644 --- a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractConstructorSnippetProvider.cs +++ b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractConstructorSnippetProvider.cs @@ -5,12 +5,7 @@ using System; using System.Collections.Immutable; using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageService; -using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Snippets.SnippetProviders { @@ -19,28 +14,12 @@ internal abstract class AbstractConstructorSnippetProvider : AbstractSingleChang public override string Identifier => "ctor"; public override string Description => FeaturesResources.constructor; + public override ImmutableArray AdditionalFilterTexts { get; } = ["constructor"]; protected override Func GetSnippetContainerFunction(ISyntaxFacts syntaxFacts) => syntaxFacts.IsConstructorDeclaration; protected override ImmutableArray GetPlaceHolderLocationsList(SyntaxNode node, ISyntaxFacts syntaxFacts, CancellationToken cancellationToken) => []; - - protected override async Task GenerateSnippetTextChangeAsync(Document document, int position, CancellationToken cancellationToken) - { - var generator = SyntaxGenerator.GetGenerator(document); - var syntaxFacts = document.GetRequiredLanguageService(); - var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var nodeAtPosition = root.FindNode(TextSpan.FromBounds(position, position)); - var containingType = nodeAtPosition.FirstAncestorOrSelf(syntaxFacts.IsTypeDeclaration); - Contract.ThrowIfNull(containingType); - var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var containingTypeSymbol = semanticModel.GetDeclaredSymbol(containingType, cancellationToken); - Contract.ThrowIfNull(containingTypeSymbol); - var constructorDeclaration = generator.ConstructorDeclaration( - containingTypeName: syntaxFacts.GetIdentifierOfTypeDeclaration(containingType).ToString(), - accessibility: containingTypeSymbol.IsAbstract ? Accessibility.Protected : Accessibility.Public); - return new TextChange(TextSpan.FromBounds(position, position), constructorDeclaration.NormalizeWhitespace().ToFullString()); - } } } From 2b2e77d2194a311b2275082b5f4c4487e2dc6924 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 30 Jan 2024 11:47:48 +0300 Subject: [PATCH 2/5] Verify nested type cases work as expected --- ...nstructorSnippetCompletionProviderTests.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs index 6af8b5c869cac..f6b5a938056b5 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs @@ -323,5 +323,63 @@ class MyClass await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } + + [WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/68176")] + public async Task InsertCorrectConstructorSnippetInNestedTypeTest_CtorBeforeNestedType() + { + var markupBeforeCommit = """ + class Outer + { + $$ + class Inner + { + } + } + """; + + var expectedCodeAfterCommit = """ + class Outer + { + public Outer() + { + $$ + } + class Inner + { + } + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/68176")] + public async Task InsertCorrectConstructorSnippetInNestedTypeTest_CtorAfterNestedType() + { + var markupBeforeCommit = """ + class Outer + { + class Inner + { + } + $$ + } + """; + + var expectedCodeAfterCommit = """ + class Outer + { + class Inner + { + } + public Outer() + { + $$ + } + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } } } From 436c497c88c70522d7201129a5ed2210051c3103 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 30 Jan 2024 11:48:54 +0300 Subject: [PATCH 3/5] Remove unnecessary traits --- ...nstructorSnippetCompletionProviderTests.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs index f6b5a938056b5..c31e515c5be35 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs @@ -14,7 +14,7 @@ public class CSharpConstructorSnippetCompletionProviderTests : AbstractCSharpSni { protected override string ItemToCommit => "ctor"; - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task ConstructorSnippetMissingInNamespace() { var markupBeforeCommit = @@ -28,7 +28,7 @@ namespace Namespace await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task ConstructorSnippetMissingInFileScopedNamespace() { var markupBeforeCommit = @@ -41,7 +41,7 @@ namespace Namespace; await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task ConstructorSnippetMissingInTopLevelContext() { var markupBeforeCommit = @@ -53,7 +53,7 @@ public async Task ConstructorSnippetMissingInTopLevelContext() await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInClassTest() { var markupBeforeCommit = @@ -77,7 +77,7 @@ public MyClass() await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInAbstractClassTest() { var markupBeforeCommit = @@ -101,7 +101,7 @@ protected MyClass() await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInAbstractClassTest_AbstractModifierInOtherPartialDeclaration() { var markupBeforeCommit = @@ -133,7 +133,7 @@ abstract partial class MyClass await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInNestedAbstractClassTest() { var markupBeforeCommit = @@ -163,7 +163,7 @@ protected NestedClass() await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInStructTest() { var markupBeforeCommit = @@ -187,7 +187,7 @@ public MyStruct() await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInRecordTest() { var markupBeforeCommit = @@ -211,7 +211,7 @@ public MyRecord() await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task ConstructorSnippetMissingInInterface() { var markupBeforeCommit = @@ -225,7 +225,7 @@ interface MyInterface await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } - [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)] + [WpfFact] public async Task InsertConstructorSnippetInNestedClassTest() { var markupBeforeCommit = From 479e0af534e0bf92ae60945101f4c5244a39bdd0 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 13 Feb 2024 14:54:13 +0300 Subject: [PATCH 4/5] Fix --- .../Portable/Snippets/CSharpConstructorSnippetProvider.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs index a4d0bd1c178b2..30c8becb4d9c7 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpConstructorSnippetProvider.cs @@ -14,6 +14,8 @@ using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageService; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery; using Microsoft.CodeAnalysis.Snippets; using Microsoft.CodeAnalysis.Snippets.SnippetProviders; using Microsoft.CodeAnalysis.Text; From 94baa7fa646b1dd4f98d3625298b634bbca7d677 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 13 Feb 2024 18:44:48 +0300 Subject: [PATCH 5/5] Add tests --- ...nstructorSnippetCompletionProviderTests.cs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs index c31e515c5be35..b5d536c3ad635 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpConstructorSnippetCompletionProviderTests.cs @@ -324,6 +324,81 @@ class MyClass await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } + [WpfFact] + public async Task InsertConstructorSnippetAfterAccessibilityModifierBeforeOtherMemberTest() + { + var markupBeforeCommit = """ + class C + { + private $$ + readonly int Value = 3; + } + """; + + var expectedCodeAfterCommit = """ + class C + { + private C() + { + $$ + } + readonly int Value = 3; + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfFact] + public async Task InsertConstructorSnippetBetweenAccessibilityModifiersBeforeOtherMemberTest() + { + var markupBeforeCommit = """ + class C + { + protected $$ + internal int Value = 3; + } + """; + + var expectedCodeAfterCommit = """ + class C + { + protected C() + { + $$ + } + internal int Value = 3; + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfFact] + public async Task InsertConstructorSnippetAfterAccessibilityModifierBeforeOtherStaticMemberTest() + { + var markupBeforeCommit = """ + class C + { + internal $$ + static int Value = 3; + } + """; + + var expectedCodeAfterCommit = """ + class C + { + internal C() + { + $$ + } + static int Value = 3; + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + [WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/68176")] public async Task InsertCorrectConstructorSnippetInNestedTypeTest_CtorBeforeNestedType() {