Initialize fields when possible when adding parameters to constructors. #76352
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -45,7 +43,7 @@ protected override bool TryInitializeConstructorInitializerGeneration( | |||
| SyntaxNode node, | |||
| CancellationToken cancellationToken, | |||
| out SyntaxToken token, | |||
| out ImmutableArray<Argument> arguments, | |||
| out ImmutableArray<Argument<ExpressionSyntax>> arguments, | |||
There was a problem hiding this comment.
the 'Argument' type was moved out of a specific feature and placed in the 'CodeGeneration' namespace. It was previously using the TExpressionSyntax type from its container. But as a new top level type, it had to become generic, and this is the fallout.
| invocationDocument, | ||
| method, | ||
| argumentType, | ||
| refKind, | ||
| argumentNameSuggestion, | ||
| new ParameterName(argumentNameSuggestion, isNamedArgument, tryMakeCamelCase: !method.ContainingType.IsRecord), | ||
| GetArgument(argument), |
There was a problem hiding this comment.
passing more information along to the 'Add Parameter' service so it can attempt to add an appropriate assignment if desired.
| insertionIndex++; | ||
|
|
||
| var memberToAssignTo = await GetMemberToAssignToAsync(semanticDocument).ConfigureAwait(false); |
There was a problem hiding this comment.
this ends up calling into one service to see if it can find a field/prop to assign to.
| if (semanticDocument.SemanticModel.GetOperation(body, cancellationToken) is IBlockOperation blockOperation) | ||
| { | ||
| initializeParameterService.AddAssignment( | ||
| methodNode, blockOperation, parameterSymbol, memberToAssignTo, editor); |
There was a problem hiding this comment.
and, if so, calls into the InitializeParameterService to actually add the assignment.
By reusing these services, we share as much code logic as possible, so the features don't have to dupe them.
| if (argument is null) | ||
| return null; | ||
|
|
||
| var (_, parameterToExistingMember, _, _) = await GenerateConstructorHelpers.GetParametersAsync( |
There was a problem hiding this comment.
we don't need the other values returned here. just the computation about which existing fields we could map to.
| @@ -405,161 +409,6 @@ private async Task<bool> TryDetermineTypeToGenerateInAsync( | |||
| return TypeToGenerateIn?.TypeKind is (TypeKind?)TypeKind.Class or (TypeKind?)TypeKind.Struct; | |||
| } | |||
|
|
|||
| private void GetParameters( | |||
There was a problem hiding this comment.
this helper moved to a shared static class location. We want to be able to compute things like which existing fields/props can be initialized with the parameters we're adding in a common way across several features.
| @@ -117,4 +123,222 @@ private static bool IsCompatible<TExpressionSyntax>( | |||
|
|
|||
| return true; | |||
| } | |||
|
|
|||
| public static async Task< | |||
There was a problem hiding this comment.
this is just a move of the shared logic. It stayed the same except that it returns the values as it cannot directly as#to the destination fields of a type as it used to.
There was a problem hiding this comment.
all the logic to the end of the file is just a move and is unchange.d
| protected override SyntaxNode? TryGetLastStatement(IBlockOperation? blockStatement) | ||
| => InitializeParameterHelpers.TryGetLastStatement(blockStatement); | ||
|
|
||
| protected override void InsertStatement(SyntaxEditor editor, SyntaxNode functionDeclaration, bool returnsVoid, SyntaxNode? statementToAddAfter, StatementSyntax statement) |
There was a problem hiding this comment.
all these methods moved out of the refactoring provider into the lang service instead. that way other refactorings providers can access them.
| { | ||
| this.s = s; | ||
| this.t = t; | ||
| this.i = i; |
There was a problem hiding this comment.
here we get the logic for free that the assignment is placed correctly between the two existing assignments for the parameters that come before/after the added parameter.
| { | ||
| this.s = s; | ||
| this.t = t; | ||
| } |
There was a problem hiding this comment.
here we get the logic for free that the expression body is rewritten into a block body, with the assignment correctly placed.
|
|
||
| public C(string s, string t, string i) | ||
| { | ||
| (this.s, this.t, this.i) = (s, t, i); |
There was a problem hiding this comment.
here we get the logic for free that we recognize and updated an existing tuple assignment with the new field assignment.
|
Moving to draft whiel #76359 is being reviewed. |
|
@JoeRobich @ToddGrun ptal. this is now out of draft. |
The bulk of this PR is oriented around extracting needed functionality from several existing fixers, so it could be used by 'add parameter'.
Specifically, 'generate constructor' already had the logic to determine which fields/properties a constructor parameter could be assigned to. And 'initialize parameter' already had the logic to add such an assignment statement in a way that matched the existing code that was already in a constructor.
When extracting this logic, i used the following rules that we already follow for several similar scenarios.
IFeatureNameSerivice : ILanguageServiceand expose in that fashion.