Merge remote-tracking branch 'upstream/features/field-keyword' into main

src/Analyzers/CSharp/Analyzers/UseAutoProperty/CSharpUseAutoPropertyAnalyzer.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
@@ -29,6 +30,12 @@ internal sealed class CSharpUseAutoPropertyAnalyzer : AbstractUseAutoPropertyAna
     protected override SyntaxKind PropertyDeclarationKind
         => SyntaxKind.PropertyDeclaration;
 
+    protected override bool CanExplicitInterfaceImplementationsBeFixed
+        => false;
+
+    protected override bool SupportsFieldAttributesOnProperties
+        => true;
+
     protected override ISemanticFacts SemanticFacts
         => CSharpSemanticFacts.Instance;
 
@@ -38,31 +45,49 @@ protected override bool SupportsReadOnlyProperties(Compilation compilation)
     protected override bool SupportsPropertyInitializer(Compilation compilation)
         => compilation.LanguageVersion() >= LanguageVersion.CSharp6;
 
-    protected override bool CanExplicitInterfaceImplementationsBeFixed()
-        => false;
+    protected override bool SupportsFieldExpression(Compilation compilation)
+        => compilation.LanguageVersion() >= LanguageVersion.Preview;
 
     protected override ExpressionSyntax? GetFieldInitializer(VariableDeclaratorSyntax variable, CancellationToken cancellationToken)
         => variable.Initializer?.Value;
 
-    protected override void RegisterIneligibleFieldsAction(
+    protected override bool ContainsFieldExpression(PropertyDeclarationSyntax propertyDeclaration, CancellationToken cancellationToken)
+    {
+        foreach (var node in propertyDeclaration.DescendantNodes())
+        {
+            if (node.IsKind(SyntaxKind.FieldExpression))
+                return true;
+        }
+
+        return false;
+    }
+
+    protected override void RecordIneligibleFieldLocations(
         HashSet<string> fieldNames,
-        ConcurrentSet<IFieldSymbol> ineligibleFields,
+        ConcurrentDictionary<IFieldSymbol, ConcurrentSet<SyntaxNode>> ineligibleFieldUsageIfOutsideProperty,
         SemanticModel semanticModel,
         SyntaxNode codeBlock,
         CancellationToken cancellationToken)
     {
         foreach (var argument in codeBlock.DescendantNodesAndSelf().OfType<ArgumentSyntax>())
         {
             // An argument will disqualify a field if that field is used in a ref/out position.  
-            // We can't change such field references to be property references in C#.
+            // We can't change such field references to be property references in C#, unless we
+            // are converting to the `field` keyword.
             if (argument.RefKindKeyword.Kind() != SyntaxKind.None)
                 AddIneligibleFieldsForExpression(argument.Expression);
+
+            // Use of a field in a nameof(...) expression can't *ever* be converted to use `field`.
+            // So hard block in this case.
+            if (argument.Expression.IsNameOfArgumentExpression())
+                AddIneligibleFieldsForExpression(argument.Expression, alwaysRestricted: true);
         }
 
         foreach (var refExpression in codeBlock.DescendantNodesAndSelf().OfType<RefExpressionSyntax>())
             AddIneligibleFieldsForExpression(refExpression.Expression);
 
-        // Can't take the address of an auto-prop.  So disallow for fields that we do `&x` on.
+        // Can't take the address of an auto-prop.  So disallow for fields that we do `&x` on.  Unless we are converting
+        // to the `field` keyword.
         foreach (var addressOfExpression in codeBlock.DescendantNodesAndSelf().OfType<PrefixUnaryExpressionSyntax>())
         {
             if (addressOfExpression.Kind() == SyntaxKind.AddressOfExpression)
@@ -72,60 +97,74 @@ protected override void RegisterIneligibleFieldsAction(
         foreach (var memberAccess in codeBlock.DescendantNodesAndSelf().OfType<MemberAccessExpressionSyntax>())
         {
             if (CouldReferenceField(memberAccess))
-                AddIneligibleFieldsIfAccessedOffNotDefinitelyAssignedValue(semanticModel, memberAccess, ineligibleFields, cancellationToken);
+                AddIneligibleFieldsIfAccessedOffNotDefinitelyAssignedValue(memberAccess);
         }
 
+        return;
+
         bool CouldReferenceField(ExpressionSyntax expression)
         {
             // Don't bother binding if the expression isn't even referencing the name of a field we know about.
             var rightmostName = expression.GetRightmostName()?.Identifier.ValueText;
             return rightmostName != null && fieldNames.Contains(rightmostName);
         }
 
-        void AddIneligibleFieldsForExpression(ExpressionSyntax expression)
+        void AddIneligibleFieldsForExpression(ExpressionSyntax expression, bool alwaysRestricted = false)
         {
             if (!CouldReferenceField(expression))
                 return;
 
             var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken);
-            AddIneligibleFields(ineligibleFields, symbolInfo);
+            AddIneligibleFields(symbolInfo, expression, alwaysRestricted);
         }
-    }
 
-    private static void AddIneligibleFieldsIfAccessedOffNotDefinitelyAssignedValue(
-        SemanticModel semanticModel, MemberAccessExpressionSyntax memberAccess, ConcurrentSet<IFieldSymbol> ineligibleFields, CancellationToken cancellationToken)
-    {
-        // `c.x = ...` can't be converted to `c.X = ...` if `c` is a struct and isn't definitely assigned as that point.
+        void AddIneligibleFieldsIfAccessedOffNotDefinitelyAssignedValue(
+            MemberAccessExpressionSyntax memberAccess)
+        {
+            // `c.x = ...` can't be converted to `c.X = ...` if `c` is a struct and isn't definitely assigned as that point.
 
-        // only care about writes.  if this was a read, then it must be def assigned and thus is safe to convert to a prop.
-        if (!memberAccess.IsOnlyWrittenTo())
-            return;
+            // only care about writes.  if this was a read, then it must be def assigned and thus is safe to convert to a prop.
+            if (!memberAccess.IsOnlyWrittenTo())
+                return;
 
-        // this only matters for a field access off of a struct.  They can be declared unassigned and have their
-        // fields directly written into.
-        var symbolInfo = semanticModel.GetSymbolInfo(memberAccess, cancellationToken);
-        if (symbolInfo.GetAnySymbol() is not IFieldSymbol { ContainingType.TypeKind: TypeKind.Struct })
-            return;
+            // this only matters for a field access off of a struct.  They can be declared unassigned and have their
+            // fields directly written into.
+            var symbolInfo = semanticModel.GetSymbolInfo(memberAccess, cancellationToken);
+            if (symbolInfo.GetAnySymbol() is not IFieldSymbol { ContainingType.TypeKind: TypeKind.Struct })
+                return;
 
-        var exprSymbol = semanticModel.GetSymbolInfo(memberAccess.Expression, cancellationToken).GetAnySymbol();
-        if (exprSymbol is not IParameterSymbol and not ILocalSymbol)
-            return;
+            var exprSymbol = semanticModel.GetSymbolInfo(memberAccess.Expression, cancellationToken).GetAnySymbol();
+            if (exprSymbol is not IParameterSymbol and not ILocalSymbol)
+                return;
 
-        var dataFlow = semanticModel.AnalyzeDataFlow(memberAccess.Expression);
-        if (dataFlow != null && !dataFlow.DefinitelyAssignedOnEntry.Contains(exprSymbol))
-            AddIneligibleFields(ineligibleFields, symbolInfo);
-    }
+            var dataFlow = semanticModel.AnalyzeDataFlow(memberAccess.Expression);
+            if (dataFlow != null && !dataFlow.DefinitelyAssignedOnEntry.Contains(exprSymbol))
+                AddIneligibleFields(symbolInfo, memberAccess);
+        }
 
-    private static void AddIneligibleFields(ConcurrentSet<IFieldSymbol> ineligibleFields, SymbolInfo symbolInfo)
-    {
-        AddIneligibleField(symbolInfo.Symbol);
-        foreach (var symbol in symbolInfo.CandidateSymbols)
-            AddIneligibleField(symbol);
+        void AddIneligibleFields(
+            SymbolInfo symbolInfo,
+            SyntaxNode location,
+            bool alwaysRestricted = false)
+        {
+            AddIneligibleField(symbolInfo.Symbol, location, alwaysRestricted);
+            foreach (var symbol in symbolInfo.CandidateSymbols)
+                AddIneligibleField(symbol, location, alwaysRestricted);
+        }
 
-        void AddIneligibleField(ISymbol? symbol)
+        void AddIneligibleField(
+            ISymbol? symbol,
+            SyntaxNode location,
+            bool alwaysRestricted)
         {
+            // If the field is always restricted, then add the compilation unit itself to the ineligibility locations.
+            // that way we never think we can convert this field. 
             if (symbol is IFieldSymbol field)
-                ineligibleFields.Add(field);
+            {
+                AddFieldUsage(ineligibleFieldUsageIfOutsideProperty, field, alwaysRestricted
+                    ? location.SyntaxTree.GetRoot(cancellationToken)
+                    : location);
+            }
         }
     }
 
@@ -181,7 +220,7 @@ private static bool CheckExpressionSyntactically(ExpressionSyntax expression)
         => accessorDeclaration is { Body.Statements: [T statement] } ? statement : null;
 
     protected override ExpressionSyntax? GetSetterExpression(
-        IMethodSymbol setMethod, SemanticModel semanticModel, CancellationToken cancellationToken)
+        SemanticModel semanticModel, IMethodSymbol setMethod, CancellationToken cancellationToken)
     {
         // Setter has to be of the form:
         //
@@ -213,4 +252,24 @@ protected override SyntaxNode GetFieldNode(
             ? fieldDeclaration
             : variableDeclarator;
     }
+
+    protected override void AddAccessedFields(
+        SemanticModel semanticModel,
+        IMethodSymbol accessor,
+        HashSet<string> fieldNames,
+        HashSet<IFieldSymbol> result,
+        CancellationToken cancellationToken)
+    {
+        var syntax = accessor.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken);
+        foreach (var descendant in syntax.DescendantNodesAndSelf())
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+
+            if (descendant is IdentifierNameSyntax identifierName)
+            {
+                result.AddIfNotNull(TryGetDirectlyAccessedFieldSymbol(
+                    semanticModel, identifierName, fieldNames, cancellationToken));
+            }
+        }
+    }
 }

src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems

@@ -108,6 +108,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)SimplifyInterpolation\SimplifyInterpolationTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UpdateProjectToAllowUnsafe\UpdateProjectToAllowUnsafeTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UpgradeProject\UpgradeProjectTests.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)UseAutoProperty\UseAutoPropertyTests_Field.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseAutoProperty\UseAutoPropertyTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCoalesceExpression\UseCoalesceExpressionForIfNullStatementCheckTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCoalesceExpression\UseCoalesceExpressionForNullableTernaryConditionalCheckTests.cs" />

src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs

@@ -18,9 +18,11 @@
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.UseAutoProperty;
 
 [Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)]
-public sealed class UseAutoPropertyTests(ITestOutputHelper logger)
+public sealed partial class UseAutoPropertyTests(ITestOutputHelper logger)
     : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor(logger)
 {
+    private readonly ParseOptions CSharp12 = CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp12);
+
     internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
         => (new CSharpUseAutoPropertyAnalyzer(), GetCSharpUseAutoPropertyCodeFixProvider());
 
@@ -667,7 +669,7 @@ class Class
     }
 
     [Fact]
-    public async Task TestGetterWithMutipleStatements()
+    public async Task TestGetterWithMultipleStatements_CSharp12()
     {
         await TestMissingInRegularAndScriptAsync(
             """
@@ -684,11 +686,11 @@ int P
                     }
                 }
             }
-            """);
+            """, new TestParameters(parseOptions: CSharp12));
     }
 
     [Fact]
-    public async Task TestSetterWithMutipleStatements()
+    public async Task TestSetterWithMultipleStatements_CSharp12()
     {
         await TestMissingInRegularAndScriptAsync(
             """
@@ -731,7 +733,7 @@ int P
                     }
                 }
             }
-            """);
+            """, new TestParameters(parseOptions: CSharp12));
     }
 
     [Fact]
@@ -1153,9 +1155,9 @@ partial class Class
     }
 
     [Fact]
-    public async Task TestNotWithFieldWithAttribute()
+    public async Task TestWithFieldWithAttribute()
     {
-        await TestMissingInRegularAndScriptAsync(
+        await TestInRegularAndScriptAsync(
             """
             class Class
             {
@@ -1170,6 +1172,13 @@ int P
                     }
                 }
             }
+            """,
+            """
+            class Class
+            {
+                [field: A]
+                int P { get; }
+            }
             """);
     }