So, I have a few updates to the code fix that we built. (And I'm still waiting to hear from my friends who have been working with Roslyn for a while; I'm sure they have some suggestions as well.)
The Original Method
Here's the method that we build in the video:
Here's a reminder of what this does:
- We take the "Statement" property of our "ifStatement" (which comes in as a parameter) and cast this to "ExpressionStatementSyntax".
We can safely make this cast (and we don't need to check for nulls here) because we did that filtering in our diagnostic analyzer. So we already know that the "Statement" is an "ExpressionStatement" that needs to be wrapped in braces. - Next. we create a new "Block" statement with the "SyntaxFactory". The parameter is the statement we want to wrap.
In addition, we add "WithAdditionalAnnotations" so that the block will be formatted nicely in our code. - Then we use the "ReplaceNode" method to swap out the old (non-braces) statement for the new block that we just created.
As a reminder, nodes are immutable, so this returns us a new object. - We get the "root" node from our document. This contains the entire syntax tree.
- Then we use the "ReplaceNode" method to swap out the old if statement for the updated if statement.
Again, this creates a new root rather than modifying the existing one. - After we have our new root, we call "document.WithSyntaxRoot" to create a new document.
This new document will have all of the same properties as our old document, but we are replacing the contents with our updated root. - Finally, we return the new document.
A Few Updates
There are a few minor updates that we can do to this code. The first has to do with our call to "ReplaceNode" on the "ifStatement". Here is the original:
We need to cast our parameters to "SyntaxNode" because the types are different: the first is "ExpressionStatementSyntax" and the second is "BlockSyntax". Now it's a bit subtle, but notice that "SyntaxNode" is greyed-out. Technically, we only need one cast -- it doesn't matter which one -- and the compiler will take care of the other one. But that may make things unclear when we're reading the code.
To clean things up a little, we can remove the cast and provide explicit generic types for the method. Here's what that looks like:
The first generic type is the type that we're working with ("IfStatementSyntax"), and the second generic type is the type for our 2 parameters ("SyntaxNode"). By specifying the generic type, we no longer need to cast the parameters; the compiler will handle everything for us. I think this is a bit easier to read.
The next fix has to do with how we retrieve the syntax root of our document. Here's the original:
This is an asynchronous method call. It turns out our entire method is also async, and it includes a cancellation token as a parameter. So we really should pass the cancellation token along to any async methods that we call.
Here's our updated call:
Eliminating Steps
The biggest change we'll make is to eliminate some steps. It turns out we're doing some extra work that we don't need. Let's look at our "ReplaceNode" method calls.
First, we use "ReplaceNode" to update the if statement -- this is where we swap out our old expression for the one wrapped in a block.
Later on, we use "ReplaceNode" to update the root -- this is where we swap out the old if statement for the updated if statement.We can skip the intermediate step and simply replace the old expression for the new block in the root itself. Here's the updated method:
And here are the steps:
- We take the "Statement" property of our "ifStatement" (which comes in as a parameter) and cast this to "ExpressionStatementSyntax".
- Next. we create a new "Block" statement with the "SyntaxFactory". The parameter is the statement we want to wrap.
Then we use the "ReplaceNode" method to swap out the old (non-braces) statement for the new block that we just created. - We get the "root" node from our document. This contains the entire syntax tree.
Note that we pass the cancellation token to this asynchronous method. - Then we use the "ReplaceNode" method to swap out the old (non-block) statement for the new block statement.
Since we specify "SyntaxNode" for the generic type, we do not need to cast the parameters. - After we have our new root, we call "document.WithSyntaxRoot" to create a new document.
- Finally, we return the new document.
As a reminder, we want our diagnostic analyzer code to run as quickly as possible. This code is constantly running against our code in Visual Studio. If we aren't careful, this can really bog things down. The code fix only runs when we choose to show the fixes, so this requires some user interaction. But the code fix does need to run to show the preview, and it also may run multiple times if we choose "fix all occurrences", so we don't want it to be slow, either.
Wrap Up
A big part of being a developer is going back and taking another look at the code we've written. Often we'll find that we can optimize the code because we've learned more about the API. Sometimes, we question readability because it doesn't look quite as clear as when we wrote it.
This doesn't bother me at all. I see this as a sign that I am constantly improving. And I look forward to hearing what some other folks say about this code so that I can improve further.
Happy Coding!
