Follow user style when generating fields from parameters.

src/EditorFeatures/CSharpTest/InitializeParameter/InitializeMemberFromParameterTests.cs

@@ -1560,5 +1560,63 @@ class C
     public string S { get; }
 }", options: this.Option(CSharpFormattingOptions.NewLinesForBracesInMethods, false));
         }
+
+        [WorkItem(23308, "https://github.com/dotnet/roslyn/issues/23308")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)]
+        public async Task TestGenerateFieldIfParameterFollowsExistingFieldAssignment()
+        {
+            await TestInRegularAndScript1Async(
+@"
+class C
+{
+    private readonly string s;
+
+    public C(string s, [||]int i)
+    {
+        this.s = s;
+    }
+}",
+@"
+class C
+{
+    private readonly string s;
+    private readonly int i;
+
+    public C(string s, int i)
+    {
+        this.s = s;
+        this.i = i;
+    }
+}");
+        }
+
+        [WorkItem(23308, "https://github.com/dotnet/roslyn/issues/23308")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)]
+        public async Task TestGenerateFieldIfParameterPrecedesExistingFieldAssignment()
+        {
+            await TestInRegularAndScript1Async(
+@"
+class C
+{
+    private readonly string s;
+
+    public C([||]int i, string s)
+    {
+        this.s = s;
+    }
+}",
+@"
+class C
+{
+    private readonly int i;
+    private readonly string s;
+
+    public C(int i, string s)
+    {
+        this.i = i;
+        this.s = s;
+    }
+}");
+        }
     }
 }

src/Features/Core/Portable/InitializeParameter/AbstractInitializeMemberFromParameterCodeRefactoringProviderMemberCreation.cs

@@ -2,7 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+
+using System;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -12,6 +16,7 @@
 using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.Naming;
 using Microsoft.CodeAnalysis.Shared.Utilities;
@@ -31,20 +36,22 @@ internal abstract partial class AbstractInitializeMemberFromParameterCodeRefacto
         where TStatementSyntax : SyntaxNode
         where TExpressionSyntax : SyntaxNode
     {
-        protected abstract SyntaxNode TryGetLastStatement(IBlockOperation blockStatementOpt);
+        protected abstract SyntaxNode TryGetLastStatement(IBlockOperation? blockStatementOpt);
 
         protected abstract Accessibility DetermineDefaultFieldAccessibility(INamedTypeSymbol containingType);
 
         protected abstract Accessibility DetermineDefaultPropertyAccessibility();
 
-        protected override Task<ImmutableArray<CodeAction>> GetRefactoringsForAllParametersAsync(Document document, SyntaxNode functionDeclaration, IMethodSymbol method, IBlockOperation blockStatementOpt, ImmutableArray<SyntaxNode> listOfParameterNodes, TextSpan parameterSpan, CancellationToken cancellationToken)
+        protected override Task<ImmutableArray<CodeAction>> GetRefactoringsForAllParametersAsync(
+            Document document, SyntaxNode functionDeclaration, IMethodSymbol method, IBlockOperation? blockStatementOpt,
+            ImmutableArray<SyntaxNode> listOfParameterNodes, TextSpan parameterSpan, CancellationToken cancellationToken)
         {
             return Task.FromResult(ImmutableArray<CodeAction>.Empty);
         }
 
         protected override async Task<ImmutableArray<CodeAction>> GetRefactoringsForSingleParameterAsync(
             Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method,
-            IBlockOperation blockStatementOpt, CancellationToken cancellationToken)
+            IBlockOperation? blockStatementOpt, CancellationToken cancellationToken)
         {
             // Only supported for constructor parameters.
             if (method.MethodKind != MethodKind.Constructor)
@@ -103,17 +110,40 @@ protected override Task<ImmutableArray<CodeAction>> GetRefactoringsForAllParamet
                 var field = CreateField(requireAccessibilityModifiers, parameter, rules, parameterNameParts.BaseNameParts);
                 var property = CreateProperty(requireAccessibilityModifiers, parameter, rules, parameterNameParts.BaseNameParts);
 
-                // Offer to generate either a property or a field.  Currently we place the property
-                // suggestion first (to help users with the immutable object+property pattern). But
-                // we could consider swapping this if people prefer creating private fields more.
-                return ImmutableArray.Create<CodeAction>(
-                    new MyCodeAction(string.Format(FeaturesResources.Create_and_initialize_property_0, property.Name),
-                        c => AddSymbolInitializationAsync(document, parameter, functionDeclaration, method, blockStatementOpt, property, c)),
-                    new MyCodeAction(string.Format(FeaturesResources.Create_and_initialize_field_0, field.Name),
-                        c => AddSymbolInitializationAsync(document, parameter, functionDeclaration, method, blockStatementOpt, field, c)));
+                var fieldAction = new MyCodeAction(string.Format(FeaturesResources.Create_and_initialize_field_0, field.Name),
+                    c => AddSymbolInitializationAsync(document, parameter, functionDeclaration, method, blockStatementOpt, field, c));
+                var propertyAction = new MyCodeAction(string.Format(FeaturesResources.Create_and_initialize_property_0, property.Name),
+                    c => AddSymbolInitializationAsync(document, parameter, functionDeclaration, method, blockStatementOpt, property, c));
+
+                // Check if the surrounding parameters are assigned to another field in this class.  If so, offer to
+                // make this parameter into a field as well.
+                var siblingFieldOrProperty = TryFindSiblingFieldOrProperty(parameter, blockStatementOpt);
+                if (siblingFieldOrProperty is IFieldSymbol)
+                {
+                    return ImmutableArray.Create<CodeAction>(fieldAction, propertyAction);
+                }
+                else
+                {
+                    // Otherwise, default to generating a property
+                    return ImmutableArray.Create<CodeAction>(propertyAction, fieldAction);
+                }
             }
         }
 
+        private ISymbol? TryFindSiblingFieldOrProperty(
+            IParameterSymbol parameter, IBlockOperation? blockStatementOpt)
+        {
+            foreach (var (siblingParam, _) in GetSiblingParameters(parameter))
+            {
+                TryFindFieldOrPropertyAssignmentStatement(
+                    siblingParam, blockStatementOpt, out var sibling);
+                if (sibling != null)
+                    return sibling;
+            }
+
+            return null;
+        }
+
         private IFieldSymbol CreateField(
             CodeStyleOption<AccessibilityModifiersRequired> requireAccessibilityModifiers,
             IParameterSymbol parameter,
@@ -210,7 +240,7 @@ private static string GenerateUniqueName(IParameterSymbol parameter, ImmutableAr
 
         private async Task<Document> AddSymbolInitializationAsync(
             Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method,
-            IBlockOperation blockStatementOpt, ISymbol fieldOrProperty, CancellationToken cancellationToken)
+            IBlockOperation? blockStatementOpt, ISymbol fieldOrProperty, CancellationToken cancellationToken)
         {
             var workspace = document.Project.Solution.Workspace;
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
@@ -274,51 +304,45 @@ private static string GenerateUniqueName(IParameterSymbol parameter, ImmutableAr
             // parameters for the constructor.
             var statementToAddAfterOpt = TryGetStatementToAddInitializationAfter(parameter, blockStatementOpt);
 
-            InsertStatement(editor, functionDeclaration, method, statementToAddAfterOpt, initializationStatement);
+            InsertStatement(
+                editor, functionDeclaration, method,
+                statementToAddAfterOpt, initializationStatement);
 
             return document.WithSyntaxRoot(editor.GetChangedRoot());
         }
 
-        private CodeGenerationOptions GetAddOptions<TSymbol>(
-            IParameterSymbol parameter, IBlockOperation blockStatementOpt,
+        private CodeGenerationOptions? GetAddOptions<TSymbol>(
+            IParameterSymbol parameter, IBlockOperation? blockStatementOpt,
             SyntaxNode typeDeclaration, CancellationToken cancellationToken)
             where TSymbol : ISymbol
         {
-            var methodSymbol = (IMethodSymbol)parameter.ContainingSymbol;
-            var parameterIndex = methodSymbol.Parameters.IndexOf(parameter);
-
-            for (var i = parameterIndex - 1; i >= 0; i--)
+            foreach (var (sibling, before) in GetSiblingParameters(parameter))
             {
                 var statement = TryFindFieldOrPropertyAssignmentStatement(
-                    methodSymbol.Parameters[i], blockStatementOpt, out var fieldOrProperty);
+                    sibling, blockStatementOpt, out var fieldOrProperty);
 
                 if (statement != null &&
                     fieldOrProperty is TSymbol symbol)
                 {
-                    var symbolSyntax = symbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken);
-                    if (symbolSyntax.Ancestors().Contains(typeDeclaration))
+                    if (before)
                     {
-                        // Found an existing field/property that corresponds to a preceding parameter.
-                        // Place ourselves directly after it.
-                        return new CodeGenerationOptions(afterThisLocation: symbolSyntax.GetLocation());
+                        var symbolSyntax = symbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken);
+                        if (symbolSyntax.Ancestors().Contains(typeDeclaration))
+                        {
+                            // Found an existing field/property that corresponds to a preceding parameter.
+                            // Place ourselves directly after it.
+                            return new CodeGenerationOptions(afterThisLocation: symbolSyntax.GetLocation());
+                        }
                     }
-                }
-            }
-
-            for (var i = parameterIndex + 1; i < methodSymbol.Parameters.Length; i++)
-            {
-                var statement = TryFindFieldOrPropertyAssignmentStatement(
-                    methodSymbol.Parameters[i], blockStatementOpt, out var fieldOrProperty);
-
-                if (statement != null &&
-                    fieldOrProperty is TSymbol symbol)
-                {
-                    // Found an existing field/property that corresponds to a following parameter.
-                    // Place ourselves directly before it.
-                    var symbolSyntax = symbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken);
-                    if (symbolSyntax.Ancestors().Contains(typeDeclaration))
+                    else
                     {
-                        return new CodeGenerationOptions(beforeThisLocation: symbolSyntax.GetLocation());
+                        // Found an existing field/property that corresponds to a following parameter.
+                        // Place ourselves directly before it.
+                        var symbolSyntax = symbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken);
+                        if (symbolSyntax.Ancestors().Contains(typeDeclaration))
+                        {
+                            return new CodeGenerationOptions(beforeThisLocation: symbolSyntax.GetLocation());
+                        }
                     }
                 }
             }
@@ -326,34 +350,47 @@ private static string GenerateUniqueName(IParameterSymbol parameter, ImmutableAr
             return null;
         }
 
-        private SyntaxNode TryGetStatementToAddInitializationAfter(
-            IParameterSymbol parameter, IBlockOperation blockStatementOpt)
+        private ImmutableArray<(IParameterSymbol parameter, bool before)> GetSiblingParameters(IParameterSymbol parameter)
         {
-            var methodSymbol = (IMethodSymbol)parameter.ContainingSymbol;
-            var parameterIndex = methodSymbol.Parameters.IndexOf(parameter);
+            using var _ = ArrayBuilder<(IParameterSymbol, bool)>.GetInstance(out var siblings);
 
-            // look for an existing assignment for a parameter that comes before us.
-            // If we find one, we'll add ourselves after that parameter check.
-            for (var i = parameterIndex - 1; i >= 0; i--)
+            if (parameter.ContainingSymbol is IMethodSymbol method)
             {
-                var statement = TryFindFieldOrPropertyAssignmentStatement(
-                    methodSymbol.Parameters[i], blockStatementOpt);
-                if (statement != null)
-                {
-                    return statement.Syntax;
-                }
+                var parameterIndex = method.Parameters.IndexOf(parameter);
+
+                // look for an existing assignment for a parameter that comes before us.
+                // If we find one, we'll add ourselves after that parameter check.
+                for (var i = parameterIndex - 1; i >= 0; i--)
+                    siblings.Add((method.Parameters[i], true));
+
+                // look for an existing check for a parameter that comes before us.
+                // If we find one, we'll add ourselves after that parameter check.
+                for (var i = parameterIndex + 1; i < method.Parameters.Length; i++)
+                    siblings.Add((method.Parameters[i], false));
             }
 
-            // look for an existing check for a parameter that comes before us.
-            // If we find one, we'll add ourselves after that parameter check.
-            for (var i = parameterIndex + 1; i < methodSymbol.Parameters.Length; i++)
+            return siblings.ToImmutable();
+        }
+
+        private SyntaxNode? TryGetStatementToAddInitializationAfter(
+            IParameterSymbol parameter, IBlockOperation? blockStatementOpt)
+        {
+            // look for an existing assignment for a parameter that comes before/after us.
+            // If we find one, we'll add ourselves before/after that parameter check.
+            foreach (var (sibling, before) in GetSiblingParameters(parameter))
             {
-                var statement = TryFindFieldOrPropertyAssignmentStatement(
-                    methodSymbol.Parameters[i], blockStatementOpt);
+                var statement = TryFindFieldOrPropertyAssignmentStatement(sibling, blockStatementOpt);
                 if (statement != null)
                 {
-                    var statementIndex = blockStatementOpt.Operations.IndexOf(statement);
-                    return statementIndex > 0 ? blockStatementOpt.Operations[statementIndex - 1].Syntax : null;
+                    if (before)
+                    {
+                        return statement.Syntax;
+                    }
+                    else
+                    {
+                        var statementIndex = blockStatementOpt!.Operations.IndexOf(statement);
+                        return statementIndex > 0 ? blockStatementOpt.Operations[statementIndex - 1].Syntax : null;
+                    }
                 }
             }
 
@@ -362,11 +399,11 @@ private static string GenerateUniqueName(IParameterSymbol parameter, ImmutableAr
             return TryGetLastStatement(blockStatementOpt);
         }
 
-        private IOperation TryFindFieldOrPropertyAssignmentStatement(IParameterSymbol parameter, IBlockOperation blockStatementOpt)
-            => TryFindFieldOrPropertyAssignmentStatement(parameter, blockStatementOpt, out var fieldOrProperty);
+        private IOperation? TryFindFieldOrPropertyAssignmentStatement(IParameterSymbol parameter, IBlockOperation? blockStatementOpt)
+            => TryFindFieldOrPropertyAssignmentStatement(parameter, blockStatementOpt, out _);
 
-        private IOperation TryFindFieldOrPropertyAssignmentStatement(
-            IParameterSymbol parameter, IBlockOperation blockStatementOpt, out ISymbol fieldOrProperty)
+        private IOperation? TryFindFieldOrPropertyAssignmentStatement(
+            IParameterSymbol parameter, IBlockOperation? blockStatementOpt, out ISymbol? fieldOrProperty)
         {
             if (blockStatementOpt != null)
             {
@@ -407,14 +444,14 @@ private IOperation TryFindFieldOrPropertyAssignmentStatement(IParameterSymbol pa
             return false;
         }
 
-        private async Task<ISymbol> TryFindMatchingUninitializedFieldOrPropertySymbolAsync(
-            Document document, IParameterSymbol parameter, IBlockOperation blockStatementOpt, ImmutableArray<NamingRule> rules, ImmutableArray<string> parameterWords, CancellationToken cancellationToken)
+        private async Task<ISymbol?> TryFindMatchingUninitializedFieldOrPropertySymbolAsync(
+            Document document, IParameterSymbol parameter, IBlockOperation? blockStatementOpt, ImmutableArray<NamingRule> rules, ImmutableArray<string> parameterWords, CancellationToken cancellationToken)
         {
             // Look for a field/property that really looks like it corresponds to this parameter.
             // Use a variety of heuristics around the name/type to see if this is a match.
 
             var containingType = parameter.ContainingType;
-            var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
+            var compilation = await document.Project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
 
             // Walk through the naming rules against this parameter's name to see what
             // name the user would like for it as a member in this type.  Note that we
@@ -439,7 +476,6 @@ private IOperation TryFindFieldOrPropertyAssignmentStatement(IParameterSymbol pa
                         return field;
                     }
 
-
                     // If it's a writable property that we could assign this parameter to, and it's
                     // not already been assigned to, then this property is a good candidate for us to
                     // hook up to.
@@ -459,7 +495,7 @@ private IOperation TryFindFieldOrPropertyAssignmentStatement(IParameterSymbol pa
         }
 
         private bool ContainsMemberAssignment(
-            IBlockOperation blockStatementOpt, ISymbol member)
+            IBlockOperation? blockStatementOpt, ISymbol member)
         {
             if (blockStatementOpt != null)
             {

src/Features/Core/Portable/InitializeParameter/AbstractInitializeParameterCodeRefactoringProvider.cs

@@ -2,8 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+
 using System;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -36,7 +39,7 @@ internal abstract partial class AbstractInitializeParameterCodeRefactoringProvid
             Document document,
             SyntaxNode functionDeclaration,
             IMethodSymbol method,
-            IBlockOperation blockStatementOpt,
+            IBlockOperation? blockStatementOpt,
             ImmutableArray<SyntaxNode> listOfParameterNodes,
             TextSpan parameterSpan,
             CancellationToken cancellationToken);
@@ -46,12 +49,12 @@ internal abstract partial class AbstractInitializeParameterCodeRefactoringProvid
             IParameterSymbol parameter,
             SyntaxNode functionDeclaration,
             IMethodSymbol methodSymbol,
-            IBlockOperation blockStatementOpt,
+            IBlockOperation? blockStatementOpt,
             CancellationToken cancellationToken);
 
         protected abstract void InsertStatement(
             SyntaxEditor editor, SyntaxNode functionDeclaration, IMethodSymbol method,
-            SyntaxNode statementToAddAfterOpt, TStatementSyntax statement);
+            SyntaxNode? statementToAddAfterOpt, TStatementSyntax statement);
 
         public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
         {
@@ -74,8 +77,8 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
 
             var generator = SyntaxGenerator.GetGenerator(document);
             var parameterNodes = generator.GetParameters(functionDeclaration);
-            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
-            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
+            var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
 
             var parameterDefault = syntaxFacts.GetDefaultOfParameter(selectedParameter);
 
@@ -135,7 +138,9 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
             }
         }
 
-        protected bool CanOfferRefactoring(SyntaxNode functionDeclaration, SemanticModel semanticModel, ISyntaxFactsService syntaxFacts, CancellationToken cancellationToken, out IBlockOperation blockStatementOpt)
+        protected bool CanOfferRefactoring(
+            SyntaxNode functionDeclaration, SemanticModel semanticModel, ISyntaxFactsService syntaxFacts,
+            CancellationToken cancellationToken, [MaybeNullWhen(false)] out IBlockOperation? blockStatementOpt)
         {
             blockStatementOpt = null;
 
@@ -193,26 +198,26 @@ protected static IOperation UnwrapImplicitConversion(IOperation operation)
             foreach (var child in condition.Syntax.DescendantNodes().OfType<TExpressionSyntax>())
             {
                 var childOperation = semanticModel.GetOperation(child, cancellationToken);
-                if (IsParameterReference(childOperation, parameter))
-                {
+                if (childOperation != null && IsParameterReference(childOperation, parameter))
                     return true;
-                }
             }
 
             return false;
         }
 
-        protected static bool IsFieldOrPropertyAssignment(IOperation statement, INamedTypeSymbol containingType, out IAssignmentOperation assignmentExpression)
-            => IsFieldOrPropertyAssignment(statement, containingType, out assignmentExpression, out var fieldOrProperty);
+        protected static bool IsFieldOrPropertyAssignment(IOperation statement, INamedTypeSymbol containingType, [NotNullWhen(true)] out IAssignmentOperation? assignmentExpression)
+            => IsFieldOrPropertyAssignment(statement, containingType, out assignmentExpression, out _);
 
         protected static bool IsFieldOrPropertyAssignment(
             IOperation statement, INamedTypeSymbol containingType,
-            out IAssignmentOperation assignmentExpression, out ISymbol fieldOrProperty)
+            [NotNullWhen(true)] out IAssignmentOperation? assignmentExpression,
+            [NotNullWhen(true)] out ISymbol? fieldOrProperty)
         {
-            if (statement is IExpressionStatementOperation expressionStatement)
+            if (statement is IExpressionStatementOperation expressionStatement &&
+                expressionStatement.Operation is IAssignmentOperation assignment)
             {
-                assignmentExpression = expressionStatement.Operation as IAssignmentOperation;
-                return IsFieldOrPropertyReference(assignmentExpression?.Target, containingType, out fieldOrProperty);
+                assignmentExpression = assignment;
+                return IsFieldOrPropertyReference(assignmentExpression.Target, containingType, out fieldOrProperty);
             }
 
             fieldOrProperty = null;
@@ -224,7 +229,8 @@ protected static bool IsFieldOrPropertyReference(IOperation operation, INamedTyp
             => IsFieldOrPropertyAssignment(operation, containingType, out var fieldOrProperty);
 
         protected static bool IsFieldOrPropertyReference(
-            IOperation operation, INamedTypeSymbol containingType, out ISymbol fieldOrProperty)
+            IOperation? operation, INamedTypeSymbol containingType,
+            [NotNullWhen(true)] out ISymbol? fieldOrProperty)
         {
             if (operation is IMemberReferenceOperation memberReference &&
                 memberReference.Member.ContainingType.Equals(containingType))