SyntaxRewriter.Visit* does not visit all nodes of type *

Question

Solved: mike z was right, I wasn't calling the base properly to continue the recursion. Thanks, mike

I'm doing some code rewriting using Roslyn, via implementing a SyntaxRewriter.

The odd thing I'm running into is that when overriding SyntaxNode.VisitInvocationExpression(InvocationExpressionSyntax), it does NOT visit all InvocationExpressionSyntax nodes in the tree. (I assume it's the same for all SyntaxNode types)

For example, given this invocation expression:

  controller.Add(5, 6).ToString();

it only Visits the node for the entire expression, even though there are 2 invocations there.

While I can certainly write a recursive function or similar to parse child/nested InvocationExpression nodes, this seems inconsistent and inconvenient.
Why isn't it visiting all nodes of * type in the entire tree?

Here's my override:

    public override SyntaxNode VisitInvocationExpression(InvocationExpressionSyntax node)
    {
        IdentifierNameSyntax ident = node.ChildNodes().OfType<IdentifierNameSyntax>().FirstOrDefault();
        if (ident == null)
            return node;//In my test case, the example above returns here when it's node is encountered.  Shouldn't this then allow the walker to continue deeper into the node,
                        // finding the deeper nested Invocations?

        string name = ident.PlainName;
        if (!TempStore.ConstructedInvocations.ContainsKey(name))//not replacing this then
            return node;

        InvocationExpressionSyntax newInvocation = ((InvocationExpressionSyntax)TempStore.ConstructedInvocations[name]).WithArgumentList(node.ArgumentList);
        return newInvocation;
    }

Stepping through that code in debug confirms that the InvocationExpressionNode for controller.Add(5, 6).ToString(); does indeed have child InvocationExpressionNodes nested inside.

Answer 1

I was playing with the Roslyn API and had a similar problem. The Visit method is being called recursively in the base class implementation of the Visit* methods. Once you override one of those you are taking responsibility for Visiting all of the child nodes. You can do this by either calling the base.Visit* method on the node you are rewriting or calling Visit on each of the child nodes.

Here is some sample code where I rewrite the logical operators, && and ||, by swapping them. I construct a new node with a different operator and then call the base.VisitBinaryExpression to make sure all child nodes are visited. Otherwise, we would only rewrite part of an expression like var1 && (var2 || var3). Another possible implementation would be to call Visit on node.Left and node.Right and then construct the new node from those results.

public class LogicalOperatorRewriter : SyntaxRewriter
{
    public override SyntaxNode VisitBinaryExpression(BinaryExpressionSyntax node)
    {
        SyntaxKind newExpressionKind = GetNewKind(node.Kind);
        BinaryExpressionSyntax newNode = (BinaryExpressionSyntax)Syntax.BinaryExpression(newExpressionKind, left: node.Left, right: node.Right).Format().GetFormattedRoot();
        return base.VisitBinaryExpression(newNode);
    }

    private SyntaxKind GetNewKind(SyntaxKind kind)
    {
        switch (kind)
        {
            case SyntaxKind.LogicalAndExpression:
                return SyntaxKind.LogicalOrExpression;
            case SyntaxKind.LogicalOrExpression:
                return SyntaxKind.LogicalAndExpression;
            default: return kind;
        }
    }
}

Answer 2

I don't know the Roslyn API but with expression trees (ExpressionVisitor), you need to call the base method to continue visiting down the tree.

This is because your method might want to stop visiting down the tree. If you wanted to stop, you could just not call the base method.

This sample code seems to support this theory.