Merge remote-tracking branch 'upstream/main' into asyncTupleExtractMethod

Author: CyrusNajmabadi

src/Analyzers/CSharp/Tests/AddParameter/AddParameterTests.cs

@@ -3340,4 +3340,110 @@ void M(string s)
             }
             """);
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71428")]
+    public async Task TestAddConstructorParameterWithExistingField_PrimaryConstructor()
+    {
+        await TestInRegularAndScript1Async(
+            """
+            class C()
+            {
+                private readonly string _name;
+            }
+
+            class D
+            {
+                void M(string name)
+                {
+                    new [|C|](name);
+                }
+            }
+            """,
+            """
+            class C(string name)
+            {
+                private readonly string _name = name;
+            }
+            
+            class D
+            {
+                void M(string name)
+                {
+                    new C(name);
+                }
+            }
+            """);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71428")]
+    public async Task TestAddConstructorParameterWithExistingProperty_PrimaryConstructor()
+    {
+        await TestInRegularAndScript1Async(
+            """
+            class C()
+            {
+                private string Name { get; }
+            }
+
+            class D
+            {
+                void M(string name)
+                {
+                    new [|C|](name);
+                }
+            }
+            """,
+            """
+            class C(string name)
+            {
+                private string Name { get; } = name;
+            }
+            
+            class D
+            {
+                void M(string name)
+                {
+                    new C(name);
+                }
+            }
+            """);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71428")]
+    public async Task TestAddConstructorParameterWithExistingThrowingProperty_PrimaryConstructor()
+    {
+        await TestInRegularAndScript1Async(
+            """
+            using System;
+
+            class C()
+            {
+                private string Name => throw new NotImplementedException();
+            }
+
+            class D
+            {
+                void M(string name)
+                {
+                    new [|C|](name);
+                }
+            }
+            """,
+            """
+            using System;
+
+            class C(string name)
+            {
+                private string Name { get; } = name;
+            }
+            
+            class D
+            {
+                void M(string name)
+                {
+                    new C(name);
+                }
+            }
+            """);
+    }
 }

src/Analyzers/Core/CodeFixes/AddParameter/AbstractAddParameterCodeFixProvider.cs

@@ -34,6 +34,12 @@ internal abstract class AbstractAddParameterCodeFixProvider<
     where TInvocationExpressionSyntax : TExpressionSyntax
     where TObjectCreationExpressionSyntax : TExpressionSyntax
 {
+    private static readonly SymbolDisplayFormat SimpleFormat = new(
+        typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameOnly,
+        genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
+        parameterOptions: SymbolDisplayParameterOptions.IncludeParamsRefOut | SymbolDisplayParameterOptions.IncludeType,
+        miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);
+
     protected abstract ImmutableArray<string> TooManyArgumentsDiagnosticIds { get; }
     protected abstract ImmutableArray<string> CannotConvertDiagnosticIds { get; }
 
@@ -422,13 +428,6 @@ private async Task<Solution> FixAsync(
         }
     }
 
-    private static readonly SymbolDisplayFormat SimpleFormat =
-                new(
-                    typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameOnly,
-                    genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
-                    parameterOptions: SymbolDisplayParameterOptions.IncludeParamsRefOut | SymbolDisplayParameterOptions.IncludeType,
-                    miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);
-
     private static TArgumentSyntax? DetermineFirstArgumentToAdd(
         SemanticModel semanticModel,
         ISyntaxFactsService syntaxFacts,

src/Analyzers/Core/CodeFixes/AddParameter/AddParameterService.cs

@@ -14,7 +14,6 @@
 using Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor;
 using Microsoft.CodeAnalysis.InitializeParameter;
 using Microsoft.CodeAnalysis.LanguageService;
-using Microsoft.CodeAnalysis.Operations;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.Utilities;
 using Roslyn.Utilities;
@@ -180,10 +179,6 @@ async Task<Solution> AddConstructorAssignmentsAsync(Solution rewrittenSolution)
 
             var documentId = documentsUpdated.Single().Id;
 
-            var memberToAssignTo = await GetMemberToAssignToAsync(documentId).ConfigureAwait(false);
-            if (memberToAssignTo is null)
-                return null;
-
             // Now go find the constructor after the parameter was added to it.
             var rewrittenDocument = rewrittenSolution.GetRequiredDocument(documentId);
             var rewrittenSyntaxRoot = await rewrittenDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
@@ -192,41 +187,28 @@ async Task<Solution> AddConstructorAssignmentsAsync(Solution rewrittenSolution)
             if (parameterDeclaration is null)
                 return null;
 
-            var initializeParameterService = rewrittenDocument.GetRequiredLanguageService<IInitializeParameterService>();
-            var semanticModel = await rewrittenDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
-            if (semanticModel.GetDeclaredSymbol(parameterDeclaration, cancellationToken) is not IParameterSymbol parameter)
+            var semanticDocument = await SemanticDocument.CreateAsync(rewrittenDocument, cancellationToken).ConfigureAwait(false);
+            if (semanticDocument.SemanticModel.GetDeclaredSymbol(parameterDeclaration, cancellationToken) is not IParameterSymbol parameter)
                 return null;
 
-            if (parameter.ContainingSymbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, DeclaringSyntaxReferences: [var reference] })
+            if (parameter.ContainingSymbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, DeclaringSyntaxReferences: [var reference] } constructor)
                 return null;
 
-            var methodNode = reference.GetSyntax(cancellationToken);
-            var body = initializeParameterService.GetBody(methodNode);
-            if (semanticModel.GetOperation(body, cancellationToken) is not IBlockOperation blockOperation)
-                return rewrittenSolution;
-
-            var editor = new SyntaxEditor(rewrittenSyntaxRoot, rewrittenSolution.Services);
-            initializeParameterService.AddAssignment(
-                methodNode, blockOperation, parameter, memberToAssignTo, editor);
-
-            var finalDocument = rewrittenDocument.WithSyntaxRoot(editor.GetChangedRoot());
-            return finalDocument.Project.Solution;
-        }
-
-        async Task<ISymbol?> GetMemberToAssignToAsync(DocumentId documentId)
-        {
-            var constructorDocument = invocationDocument.Project.Solution.GetRequiredDocument(documentId);
-            var constructorSemanticDocument = await SemanticDocument.CreateAsync(constructorDocument, cancellationToken).ConfigureAwait(false);
-
             var (_, parameterToExistingMember, _, _) = await GenerateConstructorHelpers.GetParametersAsync(
-                constructorSemanticDocument,
-                method.ContainingType,
+                semanticDocument,
+                constructor.ContainingType,
                 [argument.Value],
-                [newParameterType],
-                [parameterName],
+                [parameter.Type],
+                [new ParameterName(parameter.Name, isFixed: true)],
                 cancellationToken).ConfigureAwait(false);
 
-            return parameterToExistingMember.FirstOrDefault().Value;
+            var memberToAssignTo = parameterToExistingMember.FirstOrDefault().Value;
+            if (memberToAssignTo is null)
+                return null;
+
+            var initializeParameterService = rewrittenDocument.GetRequiredLanguageService<IInitializeParameterService>();
+            return await initializeParameterService.AddAssignmentAsync(
+                rewrittenDocument, parameter, memberToAssignTo, cancellationToken).ConfigureAwait(false);
         }
     }
 

src/Analyzers/Core/CodeFixes/GenerateConstructor/GenerateConstructorHelpers.cs

@@ -11,6 +11,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeGeneration;
 using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles;
+using Microsoft.CodeAnalysis.InitializeParameter;
 using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -185,7 +186,7 @@ ParameterName FindExistingOrCreateNewMember(
             var symbol = TryFindMatchingMember(parameterName);
             if (symbol != null)
             {
-                if (IsViableFieldOrProperty(document, parameterType, symbol))
+                if (IsViableFieldOrProperty(document, parameterType, symbol, cancellationToken))
                 {
                     // Ok!  We can just the existing field.  
                     parameterToExistingMemberMap[parameterName.BestNameForParameter] = symbol;
@@ -345,25 +346,28 @@ private static bool IsSymbolAccessible(ISymbol? symbol, SemanticDocument documen
     private static bool IsViableFieldOrProperty(
         SemanticDocument document,
         ITypeSymbol parameterType,
-        ISymbol symbol)
+        ISymbol symbol,
+        CancellationToken cancellationToken)
     {
         if (parameterType.Language != symbol.Language)
             return false;
 
         if (symbol != null && !symbol.IsStatic)
         {
-            if (symbol is IFieldSymbol field)
+            if (symbol is IFieldSymbol { IsConst: false } field)
             {
-                return
-                    !field.IsConst &&
-                    IsConversionImplicit(document.SemanticModel.Compilation, parameterType, field.Type);
+                return IsConversionImplicit(document.SemanticModel.Compilation, parameterType, field.Type);
             }
-            else if (symbol is IPropertySymbol property)
+            else if (symbol is IPropertySymbol { Parameters.Length: 0 } property)
             {
-                return
-                    property.Parameters.Length == 0 &&
-                    property.IsWritableInConstructor() &&
-                    IsConversionImplicit(document.SemanticModel.Compilation, parameterType, property.Type);
+                if (!IsConversionImplicit(document.SemanticModel.Compilation, parameterType, property.Type))
+                    return false;
+
+                if (property.IsWritableInConstructor())
+                    return true;
+
+                var service = document.Document.GetRequiredLanguageService<IInitializeParameterService>();
+                return service.IsThrowNotImplementedProperty(document.SemanticModel.Compilation, property, cancellationToken);
             }
         }
 

src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs

@@ -2630,9 +2630,8 @@ protected virtual AssemblySymbol GetForwardedToAssemblyInUsingNamespaces(string
             return Next?.GetForwardedToAssemblyInUsingNamespaces(metadataName, ref qualifierOpt, diagnostics, location);
         }
 
-        protected AssemblySymbol GetForwardedToAssembly(string fullName, BindingDiagnosticBag diagnostics, Location location)
+        protected AssemblySymbol GetForwardedToAssembly(MetadataTypeName metadataName, BindingDiagnosticBag diagnostics, Location location)
         {
-            var metadataName = MetadataTypeName.FromFullName(fullName);
             foreach (var referencedAssembly in
                 Compilation.Assembly.Modules[0].GetReferencedAssemblySymbols())
             {
@@ -2647,7 +2646,7 @@ protected AssemblySymbol GetForwardedToAssembly(string fullName, BindingDiagnost
                         if (diagInfo.Code == (int)ErrorCode.ERR_CycleInTypeForwarder)
                         {
                             Debug.Assert((object)forwardedType.ContainingAssembly != null, "How did we find a cycle if there was no forwarding?");
-                            diagnostics.Add(ErrorCode.ERR_CycleInTypeForwarder, location, fullName, forwardedType.ContainingAssembly.Name);
+                            diagnostics.Add(ErrorCode.ERR_CycleInTypeForwarder, location, metadataName.FullName, forwardedType.ContainingAssembly.Name);
                         }
                         else if (diagInfo.Code == (int)ErrorCode.ERR_TypeForwardedToMultipleAssemblies)
                         {
@@ -2717,7 +2716,10 @@ protected AssemblySymbol GetForwardedToAssembly(string name, int arity, ref Name
             // File types can't be forwarded, so we won't attempt to determine a file identifier to attach to the metadata name.
             var metadataName = MetadataHelpers.ComposeAritySuffixedMetadataName(name, arity, associatedFileIdentifier: null);
             var fullMetadataName = MetadataHelpers.BuildQualifiedName(qualifierOpt?.ToDisplayString(SymbolDisplayFormat.QualifiedNameOnlyFormat), metadataName);
-            var result = GetForwardedToAssembly(fullMetadataName, diagnostics, location);
+            var result = GetForwardedToAssembly(
+                MetadataTypeName.FromFullName(fullMetadataName),
+                diagnostics,
+                location);
             if ((object)result != null)
             {
                 return result;

src/Compilers/CSharp/Portable/Binder/WithUsingNamespacesAndTypesBinder.cs

@@ -52,8 +52,10 @@ protected WithUsingNamespacesAndTypesBinder(Binder next, bool withImportChainEnt
         {
             foreach (var typeOrNamespace in GetUsings(basesBeingResolved: null))
             {
-                var fullName = typeOrNamespace.NamespaceOrType + "." + name;
-                var result = GetForwardedToAssembly(fullName, diagnostics, location);
+                var result = GetForwardedToAssembly(
+                    MetadataTypeName.FromNamespaceAndTypeName(typeOrNamespace.NamespaceOrType.ToString(), name),
+                    diagnostics,
+                    location);
                 if (result != null)
                 {
                     qualifierOpt = typeOrNamespace.NamespaceOrType;

src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/InitializeMemberFromParameterTests.cs

@@ -21,7 +21,7 @@ public partial class InitializeMemberFromParameterTests : AbstractCSharpCodeActi
     protected override CodeRefactoringProvider CreateCodeRefactoringProvider(EditorTestWorkspace workspace, TestParameters parameters)
         => new CSharpInitializeMemberFromParameterCodeRefactoringProvider();
 
-    private readonly NamingStylesTestOptionSets options = new NamingStylesTestOptionSets(LanguageNames.CSharp);
+    private readonly NamingStylesTestOptionSets options = new(LanguageNames.CSharp);
 
     [Fact]
     public async Task TestInitializeFieldWithSameName()

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.Analyzer.cs

@@ -25,6 +25,9 @@ public static AnalyzerResult Analyze(CSharpSelectionResult selectionResult, bool
         protected override bool TreatOutAsRef
             => false;
 
+        protected override bool IsInPrimaryConstructorBaseType()
+            => this.SelectionResult.GetContainingScopeOf<PrimaryConstructorBaseTypeSyntax>() != null;
+
         protected override VariableInfo CreateFromSymbol(
             Compilation compilation,
             ISymbol symbol,

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.CallSiteContainerRewriter.cs

@@ -426,6 +426,15 @@ public override SyntaxNode VisitCompilationUnit(CompilationUnitSyntax node)
                 return node.WithMembers(newMembers);
             }
 
+            public override SyntaxNode VisitBaseList(BaseListSyntax node)
+            {
+                if (node != ContainerOfStatementsOrFieldToReplace)
+                    return base.VisitBaseList(node);
+
+                var primaryConstructorBase = (PrimaryConstructorBaseTypeSyntax)_statementsOrMemberOrAccessorToInsert.Single();
+                return node.WithTypes(node.Types.Replace(node.Types[0], primaryConstructorBase));
+            }
+
             private SyntaxNode GetUpdatedTypeDeclaration(TypeDeclarationSyntax node)
             {
                 var newMembers = VisitList(ReplaceMembers(node.Members, global: false));

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.ExpressionCodeGenerator.cs

@@ -136,7 +136,9 @@ protected override SyntaxNode GetFirstStatementOrInitializerSelectedAtCallSite()
 
                 // This is similar to FieldDeclaration case but we only want to do this 
                 // if the member has an expression body.
-                scope ??= this.SelectionResult.GetContainingScopeOf<ArrowExpressionClauseSyntax>().Parent;
+                scope ??= this.SelectionResult.GetContainingScopeOf<ArrowExpressionClauseSyntax>()?.Parent;
+
+                scope ??= this.SelectionResult.GetContainingScopeOf<PrimaryConstructorBaseTypeSyntax>();
 
                 return scope;
             }