Merge pull request #72893 from DoctorKrolic/use-semantic-check

Use semantics to check for readonly struct

Author: sharwell

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/AbstractCSharpAutoPropertyCompletionProviderTests.cs

@@ -80,6 +80,12 @@ struct MyStruct
         [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
         public abstract Task InsertSnippetInReadonlyStruct();
 
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public abstract Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration();
+
+        [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public abstract Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration_MissingPartialModifier();
+
         // This case might produce non-default results for different snippets (e.g. no `set` accessor in 'propg' snippet),
         // so it is tested separately for all of them
         [WpfFact, Trait(Traits.Feature, Traits.Features.Completion)]

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpPropSnippetCompletionProviderTests.cs

@@ -24,6 +24,39 @@ readonly struct MyStruct
                 """, "public int MyProperty { get; }");
         }
 
+        public override async Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration()
+        {
+            // Ensure we don't generate redundant `set` accessor when executed in readonly struct
+            await VerifyPropertyAsync("""
+                partial struct MyStruct
+                {
+                    $$
+                }
+
+                readonly partial struct MyStruct
+                {
+                }
+                """, "public int MyProperty { get; }");
+        }
+
+        public override async Task InsertSnippetInReadonlyStruct_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 int MyProperty { get; }");
+        }
+
         public override async Task InsertSnippetInInterface()
         {
             await VerifyDefaultPropertyAsync("""

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpPropgSnippetCompletionProviderTests.cs

@@ -24,6 +24,39 @@ readonly struct MyStruct
                 """, "public int MyProperty { get; }");
         }
 
+        public override async Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration()
+        {
+            // Ensure we don't generate redundant `set` accessor when executed in readonly struct
+            await VerifyPropertyAsync("""
+                partial struct MyStruct
+                {
+                    $$
+                }
+
+                readonly partial struct MyStruct
+                {
+                }
+                """, "public int MyProperty { get; }");
+        }
+
+        public override async Task InsertSnippetInReadonlyStruct_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 int MyProperty { get; }");
+        }
+
         public override async Task InsertSnippetInInterface()
         {
             // Ensure we don't generate redundant `set` accessor when executed in interface

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/Snippets/CSharpPropiSnippetCompletionProviderTests.cs

@@ -23,6 +23,34 @@ readonly struct MyStruct
                 """);
         }
 
+        public override async Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration()
+        {
+            await VerifyDefaultPropertyAsync("""
+                partial struct MyStruct
+                {
+                    $$
+                }
+
+                readonly partial struct MyStruct
+                {
+                }
+                """);
+        }
+
+        public override async Task InsertSnippetInReadonlyStruct_ReadonlyModifierInOtherPartialDeclaration_MissingPartialModifier()
+        {
+            await VerifyDefaultPropertyAsync("""
+                struct MyStruct
+                {
+                    $$
+                }
+
+                readonly partial struct MyStruct
+                {
+                }
+                """);
+        }
+
         public override async Task InsertSnippetInInterface()
         {
             await VerifyDefaultPropertyAsync("""

src/Features/CSharp/Portable/Snippets/AbstractCSharpAutoPropertySnippetProvider.cs

@@ -24,10 +24,10 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets;
 
 internal abstract class AbstractCSharpAutoPropertySnippetProvider : AbstractPropertySnippetProvider<PropertyDeclarationSyntax>
 {
-    protected virtual AccessorDeclarationSyntax? GenerateGetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator)
+    protected virtual AccessorDeclarationSyntax? GenerateGetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator, CancellationToken cancellationToken)
         => (AccessorDeclarationSyntax)generator.GetAccessorDeclaration();
 
-    protected virtual AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator)
+    protected virtual AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator, CancellationToken cancellationToken)
         => (AccessorDeclarationSyntax)generator.SetAccessorDeclaration();
 
     protected override bool IsValidSnippetLocation(in SnippetContext context, CancellationToken cancellationToken)
@@ -46,8 +46,8 @@ protected override async Task<PropertyDeclarationSyntax> GenerateSnippetSyntaxAs
         var syntaxContext = CSharpSyntaxContext.CreateContext(document, semanticModel, position, cancellationToken);
         var accessors = new AccessorDeclarationSyntax?[]
         {
-            GenerateGetAccessorDeclaration(syntaxContext, generator),
-            GenerateSetAccessorDeclaration(syntaxContext, generator),
+            GenerateGetAccessorDeclaration(syntaxContext, generator, cancellationToken),
+            GenerateSetAccessorDeclaration(syntaxContext, generator, cancellationToken),
         };
 
         SyntaxTokenList modifiers = default;

src/Features/CSharp/Portable/Snippets/CSharpPropSnippetProvider.cs

@@ -4,6 +4,7 @@
 
 using System;
 using System.Composition;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Editing;
@@ -22,16 +23,16 @@ internal sealed class CSharpPropSnippetProvider() : AbstractCSharpAutoPropertySn
 
     public override string Description => FeaturesResources.property_;
 
-    protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator)
+    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 &&
-            structDeclaration.Modifiers.Any(SyntaxKind.ReadOnlyKeyword))
+            syntaxContext.SemanticModel.GetDeclaredSymbol(structDeclaration, cancellationToken) is { IsReadOnly: true })
         {
             return null;
         }
 
-        return base.GenerateSetAccessorDeclaration(syntaxContext, generator);
+        return base.GenerateSetAccessorDeclaration(syntaxContext, generator, cancellationToken);
     }
 }

src/Features/CSharp/Portable/Snippets/CSharpPropgSnippetProvider.cs

@@ -4,6 +4,7 @@
 
 using System;
 using System.Composition;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Editing;
@@ -22,7 +23,7 @@ internal sealed class CSharpPropgSnippetProvider() : AbstractCSharpAutoPropertyS
 
     public override string Description => FeaturesResources.get_only_property;
 
-    protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator)
+    protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator, CancellationToken cancellationToken)
     {
         // Interface cannot have properties with `private set` accessor.
         // So if we are inside an interface, we just return null here.
@@ -35,7 +36,7 @@ internal sealed class CSharpPropgSnippetProvider() : AbstractCSharpAutoPropertyS
         // 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 &&
-            structDeclaration.Modifiers.Any(SyntaxKind.ReadOnlyKeyword))
+            syntaxContext.SemanticModel.GetDeclaredSymbol(structDeclaration, cancellationToken) is { IsReadOnly: true })
         {
             return null;
         }

src/Features/CSharp/Portable/Snippets/CSharpPropiSnippetProvider.cs

@@ -4,6 +4,7 @@
 
 using System;
 using System.Composition;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Editing;
@@ -24,6 +25,6 @@ internal sealed class CSharpPropiSnippetProvider() : AbstractCSharpAutoPropertyS
 
     public override string Description => CSharpFeaturesResources.init_only_property;
 
-    protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator)
+    protected override AccessorDeclarationSyntax? GenerateSetAccessorDeclaration(CSharpSyntaxContext syntaxContext, SyntaxGenerator generator, CancellationToken cancellationToken)
         => SyntaxFactory.AccessorDeclaration(SyntaxKind.InitAccessorDeclaration).WithSemicolonToken(SemicolonToken);
 }