Implement code fixer to change the code from calling Configure methods to using builder.Configuration

[anhthidao]

{Fixer: Replacing builder.Host.ConfigureAppConfiguration/builder.Host.ConfigureHostConfiguration/builder.WebHost.ConfigureAppConfiguration with builder.Configuration directly}

Implement

Description

Add code fixer that calls builder.Configuration instead of using ConfigureAppConfiguration/ConfigureHostConfiguration with builder.Host/builder.WebHost.

Fixes #35815

[captainsafia]

What project is being added as a dependency here? Ideally, there should be no changes to the solution file...

[captainsafia]

Suggested change

	invocationExpression = invocationExpression.WithExpression(expression);
	invocationExpression = invocationExpression.WithExpression(expression).WithArgumentList(argument);

This modifications can all be chained together.

[captainsafia]

Suggested change

	var expression = invocationExpression.Expression as MemberAccessExpressionSyntax;
	var invokedMethodExpr = invocationExpression.Expression as MemberAccessExpressionSyntax;

To make it a little clearer what this is.

[captainsafia]

Let's add a guard here in case there are no arguments provided to the invocation.

[captainsafia]

It might help to move this to a inline static method called ExtractArgumentBody to make it clearer what this is doing.

[captainsafia]

Suggested change

	editor.ReplaceNode(invocation, invocationExpression);
	editor.ReplaceNode(invocation, newInvocationExpression);

So it is easier to distinguish between the unmodified and modified invocation.

[captainsafia]

Can you add some comments here that show what each of these expressions reflects?

// logging.AddJson();
var body = lambdaExpression.ExpressionBody;

Or what have you.

[captainsafia]

Let's add a test scenario where the same ConfigureAppConfiguration contains multiple calls inside it.

builder.Host.ConfigureAppConfiguration(builder =>
{
    builder.AddJsonFile("foo.json", optional: true);
    builder.AddEnvironmentVariables();
});

should become

builder.Configuration.
  .AddJsonFile("foo.json", optional: true)
  .AddEnvironmentVariables();

[captainsafia]

This doesn't seem like the correct behavior here. I think it's unlikely for users to make the same invocations in different ConfigureX calls, but if they do, it should only produce a single invocation.

[captainsafia]

Suggested change

	var invocation = root.FindNode(diagnostic.Location.SourceSpan); //whole invocation starting from "builder"
	var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); //whole invocation starting from "builder"

Since at this point, we haven't actually verified at runtime that it's an invocation node. That doesn't happen until the if-statement below. This also helps us disambiguate between the different invocations being modified.

[captainsafia]

Suggested change

	if (invocation is InvocationExpressionSyntax { } newInvocationExpr)
	if (diagnosticTarget is InvocationExpressionSyntax { } originalInvocation)

[captainsafia]

We can probably combine the if-statement on L56 and this one into a single on that clearly communicates that we are looking "up" the accessors.

if (newInvocationExpr.Expression is not MemberAccessExpressionSyntax hostBasedInvocationMethodExpr
 && hostBasedInvocationMethodExpr.Expression is not MemberAccessExpressionSyntax hostExpression)
{
}

The values should be assigned correctly here.

[captainsafia]

Is Name actually nullable here? Also, it might help to invert the if statement and return early here? I'm having trouble visualizing what happens if we don't hit this case.

[captainsafia]

We can combine this if-statement and the one below into one statement with an &&.

[captainsafia]

Suggested change

	var bodyEnum = ExtractParenLambdaBody(parenLambdaExpr).GetEnumerator();
	var lambdaStatements = ExtractParenLambdaBody(parenLambdaExpr);

[captainsafia]

You can use a foreach here.

foreach (var statement in lambdaStatements)
{
}

[captainsafia]

Do we need to do any validation on Block here? For example, check if it is null...

[anhthidao]

 var method = bodyExpression.Name; // line 119

 hostBasedInvocationMethodExpr = hostBasedInvocationMethodExpr.WithName(method); // line 121
 originalInvocation = originalInvocation.Update(hostBasedInvocationMethodExpr, argument); // line 122
 hostBasedInvocationMethodExpr = hostBasedInvocationMethodExpr.WithExpression(originalInvocation); // line 123

This part, when placed inside a loop, would continuously add the methods called to the final invocation. originalInvocation would be returned as the modified invocation, while hostBasedInvocationMethodExpr would be discarded.

Example:

Before:

 builder.Host.ConfigureHostConfiguration(builder =>
{
     builder.AddJsonFile("foo.json", optional: true);
     builder.AddEnvironmentVariables();
});

After:

 builder.Configuration.AddJsonFile("foo.json", optional: true).AddEnvironmentVariables();

Walkthrough:

First iteration:

method: null -> AddJsonFile

hostBasedInvocationMethodExpr: builder.Configuration.ConfigureHostConfiguration -> builder.Configuration.AddJsonFile

originalInvocation: Before
-> builder.Configuration.AddJsonFile("foo.json", optional: true)

hostBasedInvocationMethodExpr: builder.Configuration.AddJsonFile -> builder.Configuration.AddJsonFile("foo.json", optional: true).AddJsonFile

Second iteration:

method: AddJsonFile -> AddEnvironmentVariables

hostBasedInvocationMethodExpr: builder.Configuration.AddJsonFile("foo.json", optional: true).AddJsonFile -> builder.Configuration.AddJsonFile("foo.json", optional: true).AddEnvironmentVariables

originalInvocation: builder.Configuration.AddJsonFile("foo.json", optional: true) -> builder.Configuration.AddJsonFile("foo.json", optional: true).AddEnvironmentVariables()

hostBasedInvocationMethodExpr: builder.Configuration.AddJsonFile("foo.json", optional: true).AddEnvironmentVariables
-> builder.Configuration.AddJsonFile("foo.json", optional: true).AddEnvironmentVariables().AddEnvironmentVariables