From eb719b7b4d1b846a255d6bec6266072ff1eaada8 Mon Sep 17 00:00:00 2001 From: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Date: Fri, 3 Mar 2023 19:43:12 +0300 Subject: [PATCH] Fix crash of type snippets after accessibility modifier (#67055) --- ...harpClassSnippetCompletionProviderTests.cs | 57 +++++++++++++++++-- ...InterfaceSnippetCompletionProviderTests.cs | 57 +++++++++++++++++-- ...arpStructSnippetCompletionProviderTests.cs | 57 +++++++++++++++++-- ...s => AbstractCSharpTypeSnippetProvider.cs} | 38 +++++++++---- .../Snippets/CSharpClassSnippetProvider.cs | 2 +- .../CSharpInterfaceSnippetProvider.cs | 2 +- .../Snippets/CSharpStructSnippetProvider.cs | 2 +- .../AbstractTypeSnippetProvider.cs | 10 +--- 8 files changed, 191 insertions(+), 34 deletions(-) rename src/Features/CSharp/Portable/Snippets/{CSharpTypeSnippetProvider.cs => AbstractCSharpTypeSnippetProvider.cs} (80%) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpClassSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpClassSnippetCompletionProviderTests.cs index 68ed7924b86f6..5600fb19b1f99 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpClassSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpClassSnippetCompletionProviderTests.cs @@ -2,10 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; @@ -234,5 +230,58 @@ public Program() }"; await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier(string modifier) + { + var markupBeforeCommit = $"{modifier} $$"; + + var expectedCodeAfterCommit = $$""" + {{modifier}} class MyClass + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier_RequireAccessibilityModifiers(string modifier) + { + var markupBeforeCommit = $$""" + + + {{modifier}} $$ + + root = true + + [*] + # IDE0008: Use explicit type + dotnet_style_require_accessibility_modifiers = always + + + + """; + + var expectedCodeAfterCommit = $$""" + {{modifier}} class MyClass + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } } } diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpInterfaceSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpInterfaceSnippetCompletionProviderTests.cs index 28ca8b95098c2..cdeebcfc7c7b8 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpInterfaceSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpInterfaceSnippetCompletionProviderTests.cs @@ -2,10 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; @@ -234,5 +230,58 @@ public Program() }"; await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier(string modifier) + { + var markupBeforeCommit = $"{modifier} $$"; + + var expectedCodeAfterCommit = $$""" + {{modifier}} interface MyInterface + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier_RequireAccessibilityModifiers(string modifier) + { + var markupBeforeCommit = $$""" + + + {{modifier}} $$ + + root = true + + [*] + # IDE0008: Use explicit type + dotnet_style_require_accessibility_modifiers = always + + + + """; + + var expectedCodeAfterCommit = $$""" + {{modifier}} interface MyInterface + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } } } diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpStructSnippetCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpStructSnippetCompletionProviderTests.cs index ae21e8802f851..6fef3792a3a3d 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpStructSnippetCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpStructSnippetCompletionProviderTests.cs @@ -2,10 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; @@ -245,5 +241,58 @@ public Program() ; await VerifyItemIsAbsentAsync(markupBeforeCommit, ItemToCommit); } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier(string modifier) + { + var markupBeforeCommit = $"{modifier} $$"; + + var expectedCodeAfterCommit = $$""" + {{modifier}} struct MyStruct + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } + + [WpfTheory] + [InlineData("public")] + [InlineData("private")] + [InlineData("protected")] + [InlineData("private protected")] + [InlineData("protected internal")] + public async Task AfterAccessibilityModifier_RequireAccessibilityModifiers(string modifier) + { + var markupBeforeCommit = $$""" + + + {{modifier}} $$ + + root = true + + [*] + # IDE0008: Use explicit type + dotnet_style_require_accessibility_modifiers = always + + + + """; + + var expectedCodeAfterCommit = $$""" + {{modifier}} struct MyStruct + { + $$ + } + """; + + await VerifyCustomCommitProviderAsync(markupBeforeCommit, ItemToCommit, expectedCodeAfterCommit); + } } } diff --git a/src/Features/CSharp/Portable/Snippets/CSharpTypeSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/AbstractCSharpTypeSnippetProvider.cs similarity index 80% rename from src/Features/CSharp/Portable/Snippets/CSharpTypeSnippetProvider.cs rename to src/Features/CSharp/Portable/Snippets/AbstractCSharpTypeSnippetProvider.cs index 5959d7b177365..3030c630a2b99 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpTypeSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/AbstractCSharpTypeSnippetProvider.cs @@ -2,10 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Utilities; @@ -19,20 +21,26 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets { - internal abstract class CSharpTypeSnippetProvider : AbstractTypeSnippetProvider + internal abstract class AbstractCSharpTypeSnippetProvider : AbstractTypeSnippetProvider { private static readonly ISet s_validModifiers = new HashSet(SyntaxFacts.EqualityComparer) - { - SyntaxKind.NewKeyword, - SyntaxKind.PublicKeyword, - SyntaxKind.ProtectedKeyword, - SyntaxKind.InternalKeyword, - SyntaxKind.PrivateKeyword, - SyntaxKind.AbstractKeyword, - SyntaxKind.SealedKeyword, - SyntaxKind.StaticKeyword, - SyntaxKind.UnsafeKeyword - }; + { + SyntaxKind.NewKeyword, + SyntaxKind.PublicKeyword, + SyntaxKind.ProtectedKeyword, + SyntaxKind.InternalKeyword, + SyntaxKind.PrivateKeyword, + SyntaxKind.AbstractKeyword, + SyntaxKind.SealedKeyword, + SyntaxKind.StaticKeyword, + SyntaxKind.UnsafeKeyword + }; + + protected override async Task HasPrecedingAccessibilityModifiersAsync(Document document, int position, CancellationToken cancellationToken) + { + var tree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); + return tree.GetPrecedingModifiers(position, cancellationToken).Any(SyntaxFacts.IsAccessibilityModifier); + } protected override async Task IsValidSnippetLocationAsync(Document document, int position, CancellationToken cancellationToken) { @@ -73,6 +81,12 @@ private static string GetIndentation(Document document, TypeDeclarationSyntax ty return newIndentation.GetIndentationString(parsedDocument.Text, syntaxFormattingOptions.UseTabs, syntaxFormattingOptions.TabSize) + newLine; } + protected override SyntaxNode? FindAddedSnippetSyntaxNode(SyntaxNode root, int position, Func isCorrectContainer) + { + var node = root.FindNode(TextSpan.FromBounds(position, position)); + return node.GetAncestorOrThis(); + } + protected override async Task AddIndentationToDocumentAsync(Document document, int position, ISyntaxFacts syntaxFacts, CancellationToken cancellationToken) { var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/Features/CSharp/Portable/Snippets/CSharpClassSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpClassSnippetProvider.cs index f7af0daa7ebf9..e07100bcd418b 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpClassSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpClassSnippetProvider.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets { [ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared] - internal sealed class CSharpClassSnippetProvider : CSharpTypeSnippetProvider + internal sealed class CSharpClassSnippetProvider : AbstractCSharpTypeSnippetProvider { [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] diff --git a/src/Features/CSharp/Portable/Snippets/CSharpInterfaceSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpInterfaceSnippetProvider.cs index 0a5b62bc1c8cf..0c278f6fa5623 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpInterfaceSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpInterfaceSnippetProvider.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets { [ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared] - internal sealed class CSharpInterfaceSnippetProvider : CSharpTypeSnippetProvider + internal sealed class CSharpInterfaceSnippetProvider : AbstractCSharpTypeSnippetProvider { [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] diff --git a/src/Features/CSharp/Portable/Snippets/CSharpStructSnippetProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpStructSnippetProvider.cs index 141432e40323d..9b08dd1bf4fe4 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpStructSnippetProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpStructSnippetProvider.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets { [ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared] - internal sealed class CSharpStructSnippetProvider : CSharpTypeSnippetProvider + internal sealed class CSharpStructSnippetProvider : AbstractCSharpTypeSnippetProvider { [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] diff --git a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractTypeSnippetProvider.cs b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractTypeSnippetProvider.cs index cb63a597612b9..a3295b0e6f34e 100644 --- a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractTypeSnippetProvider.cs +++ b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractTypeSnippetProvider.cs @@ -2,30 +2,26 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; using System.Collections.Immutable; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeStyle; -using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Snippets.SnippetProviders { internal abstract class AbstractTypeSnippetProvider : AbstractSnippetProvider { + protected abstract Task HasPrecedingAccessibilityModifiersAsync(Document document, int position, CancellationToken cancellationToken); protected abstract void GetTypeDeclarationIdentifier(SyntaxNode node, out SyntaxToken identifier); protected abstract Task GenerateTypeDeclarationAsync(Document document, int position, bool useAccessibility, CancellationToken cancellationToken); protected override async Task> GenerateSnippetTextChangesAsync(Document document, int position, CancellationToken cancellationToken) { - var useAccessibility = await AreAccessibilityModifiersRequiredAsync(document, cancellationToken).ConfigureAwait(false); + var hasAccessibilityModifiers = await HasPrecedingAccessibilityModifiersAsync(document, position, cancellationToken).ConfigureAwait(false); + var useAccessibility = !hasAccessibilityModifiers && await AreAccessibilityModifiersRequiredAsync(document, cancellationToken).ConfigureAwait(false); var typeDeclaration = await GenerateTypeDeclarationAsync(document, position, useAccessibility, cancellationToken).ConfigureAwait(false);