Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added "propr" snippet for required properties #75339

Merged
merged 13 commits into from
Oct 8, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ protected override async Task<PropertyDeclarationSyntax> GenerateSnippetSyntaxAs
modifiers = SyntaxTokenList.Create(PublicKeyword);
}

modifiers = modifiers.AddRange(GetAdditionalPropertyModifiers(syntaxContext));

return SyntaxFactory.PropertyDeclaration(
attributeLists: default,
modifiers: modifiers,
Expand Down Expand Up @@ -87,4 +89,9 @@ protected override ImmutableArray<SnippetPlaceholder> GetPlaceHolderLocationsLis
var node = root.FindNode(TextSpan.FromBounds(position, position));
return node.GetAncestorOrThis<PropertyDeclarationSyntax>();
}

protected virtual SyntaxToken[] GetAdditionalPropertyModifiers(CSharpSyntaxContext? syntaxContext)
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
return [];
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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.Composition;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Snippets;
using Microsoft.CodeAnalysis.Snippets.SnippetProviders;
using Roslyn.Utilities;
using static Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTokens;

namespace Microsoft.CodeAnalysis.CSharp.Snippets;

[ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared]
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class CSharpProprSnippetProvider() : AbstractCSharpAutoPropertySnippetProvider
{
public override string Identifier => CSharpSnippetIdentifiers.RequiredProperty;

public override string Description => FeaturesResources.required_property;

protected override bool IsValidSnippetLocationCore(SnippetContext context, CancellationToken cancellationToken)
{
if (!base.IsValidSnippetLocationCore(context, cancellationToken))
return false;

var syntaxContext = (CSharpSyntaxContext)context.SyntaxContext;
var precedingModifiers = syntaxContext.PrecedingModifiers;

// The required modifier can't be applied to members of an interface
if (syntaxContext.ContainingTypeDeclaration is InterfaceDeclarationSyntax)
return false;

if (syntaxContext.PrecedingModifiers.IsEmpty())
return true;
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved

// "private" and "private protected" modifiers are NOT valid for required property
if (precedingModifiers.Any(syntaxKind => syntaxKind == SyntaxKind.PrivateKeyword))
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
return false;

// "protected internal" modifiers are valid for required property
if (precedingModifiers.IsSupersetOf([SyntaxKind.ProtectedKeyword, SyntaxKind.InternalKeyword]))
return true;

// "protected" and "private protected" modifiers are NOT valid for required property
if (precedingModifiers.Any(syntaxKind => syntaxKind == SyntaxKind.ProtectedKeyword))
return false;

return true;
}

protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator, CancellationToken cancellationToken)
{
// Having a property with `set` accessor in a readonly struct leads to a compiler error.
// So if user executes snippet inside a readonly struct the right thing to do is to not generate `set` accessor at all
if (syntaxContext.ContainingTypeDeclaration is StructDeclarationSyntax structDeclaration &&
syntaxContext.SemanticModel.GetDeclaredSymbol(structDeclaration, cancellationToken) is { IsReadOnly: true })
{
return null;
}

return base.GenerateSetAccessorDeclaration(syntaxContext, generator, cancellationToken);
}

protected override SyntaxToken[] GetAdditionalPropertyModifiers(CSharpSyntaxContext? syntaxContext)
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
return [RequiredKeyword];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ internal static class CSharpSnippetIdentifiers
public const string ReversedFor = "forr";
public const string ForEach = "foreach";
public const string InitOnlyProperty = "propi";
public const string RequiredProperty = "propr";
public const string If = "if";
public const string Interface = "interface";
public const string Lock = "lock";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ record MyRecord
""");
}

[Fact]
public async Task InsertSnippetInRecordStructTest()
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
await VerifyDefaultPropertyAsync("""
record struct MyRecordStruct
{
$$
}
""");
}

[Fact]
public async Task InsertSnippetInStructTest()
{
Expand Down Expand Up @@ -143,9 +154,7 @@ public Program()
""");
}

[Theory]
[MemberData(nameof(CommonSnippetTestData.AllAccessibilityModifiers), MemberType = typeof(CommonSnippetTestData))]
public async Task InsertSnippetAfterAccessibilityModifierTest(string modifier)
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
public virtual async Task InsertSnippetAfterAccessibilityModifierTest(string modifier)
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
await VerifyPropertyAsync($$"""
class Program
Expand All @@ -162,6 +171,6 @@ protected async Task VerifyPropertyAsync([StringSyntax(PredefinedEmbeddedLanguag
await VerifySnippetAsync(markup, expectedCode);
}

protected Task VerifyDefaultPropertyAsync([StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string markup, string propertyName = "MyProperty")
protected virtual Task VerifyDefaultPropertyAsync([StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string markup, string propertyName = "MyProperty")
=> VerifyPropertyAsync(markup, $$"""public {|0:int|} {|1:{{propertyName}}|} {{DefaultPropertyBlockText}}""");
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Snippets;

Expand Down Expand Up @@ -65,4 +66,8 @@ interface MyInterface
}
""");
}

[Theory]
[MemberData(nameof(CommonSnippetTestData.AllAccessibilityModifiers), MemberType = typeof(CommonSnippetTestData))]
public override Task InsertSnippetAfterAccessibilityModifierTest(string modifier) => base.InsertSnippetAfterAccessibilityModifierTest(modifier);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Snippets;

Expand Down Expand Up @@ -66,4 +67,8 @@ interface MyInterface
}
""", "public {|0:int|} {|1:MyProperty|} { get; }");
}

[Theory]
[MemberData(nameof(CommonSnippetTestData.AllAccessibilityModifiers), MemberType = typeof(CommonSnippetTestData))]
public override Task InsertSnippetAfterAccessibilityModifierTest(string modifier) => base.InsertSnippetAfterAccessibilityModifierTest(modifier);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Snippets;

Expand Down Expand Up @@ -59,4 +60,8 @@ interface MyInterface
}
""");
}

[Theory]
[MemberData(nameof(CommonSnippetTestData.AllAccessibilityModifiers), MemberType = typeof(CommonSnippetTestData))]
public override Task InsertSnippetAfterAccessibilityModifierTest(string modifier) => base.InsertSnippetAfterAccessibilityModifierTest(modifier);
}
106 changes: 106 additions & 0 deletions src/Features/CSharpTest/Snippets/CSharpProprSnippetProviderTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Snippets;

public sealed class CSharpProprSnippetProviderTests : AbstractCSharpAutoPropertySnippetProviderTests
{
protected override string SnippetIdentifier => "propr";

protected override string DefaultPropertyBlockText => "{ get; set; }";

[WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
public override async Task InsertSnippetInReadonlyStructTest()
{
// Ensure we don't generate redundant `set` accessor when executed in readonly struct
await VerifyPropertyAsync("""
readonly struct MyStruct
{
$$
}
""", "public required {|0:int|} {|1:MyProperty|} { get; }");
}

[WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
public override async Task InsertSnippetInReadonlyStructTest_ReadonlyModifierInOtherPartialDeclaration()
{
// Ensure we don't generate redundant `set` accessor when executed in readonly struct
await VerifyPropertyAsync("""
partial struct MyStruct
{
$$
}

readonly partial struct MyStruct
{
}
""", "public required {|0:int|} {|1:MyProperty|} { get; }");
}

[WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
public override async Task InsertSnippetInReadonlyStructTest_ReadonlyModifierInOtherPartialDeclaration_MissingPartialModifier()
{
// Even though there is no `partial` modifier on the first declaration
// compiler still treats the whole type as partial since it is more likely that
// the user's intent was to have a partial type and they just forgot the modifier.
// Thus we still recognize that as `readonly` context and don't generate a setter
await VerifyPropertyAsync("""
struct MyStruct
{
$$
}

readonly partial struct MyStruct
{
}
""", "public required {|0:int|} {|1:MyProperty|} { get; }");
}

[WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
public override async Task InsertSnippetInInterfaceTest()
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
await VerifySnippetIsAbsentAsync("""
interface MyInterface
{
$$
}
""");
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
[InlineData("public")]
[InlineData("internal")]
[InlineData("protected internal")]
public override async Task InsertSnippetAfterAccessibilityModifierTest(string modifier)
{
await VerifyPropertyAsync($$"""
class Program
{
{{modifier}} $$
}
""", $$"""required {|0:int|} {|1:MyProperty|} {{DefaultPropertyBlockText}}""");
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/75128")]
[InlineData("private")]
[InlineData("protected")]
[InlineData("private protected")]
public async Task DoNotInsertSnippetAfterAccessibilityModifierTest(string modifier)
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
{
await VerifySnippetIsAbsentAsync($$"""
class Program
{
{{modifier}} $$
}
""");
}

protected override Task VerifyDefaultPropertyAsync([StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string markup, string propertyName = "MyProperty")
=> VerifyPropertyAsync(markup, $$"""public required {|0:int|} {|1:{{propertyName}}|} {{DefaultPropertyBlockText}}""");
}
3 changes: 3 additions & 0 deletions src/Features/Core/Portable/FeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -3171,4 +3171,7 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
<data name="Symbol_search" xml:space="preserve">
<value>Symbol search</value>
</data>
<data name="required_property" xml:space="preserve">
<value>required property</value>
victor-pogor marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading