I am in the process of developing a C# roslyn code analyzer and code fix and am struggling with the proper code to perform the code changes. The analyzer and code fix logic itself does (hopefully) not matter too much.

I may be struggling with a general understanding of how to apply code changes with GetSyntaxRootAsync(), but my understanding is that the overall approach is to retrieve a root, apply changes (each change returns a new root to be used to apply subsequent changes) and then return document.WithSyntaxRoot(root).

However, the following code fix does not seem to apply the correct changes and I fail to grasp why:

private async Task<Document> ApplyCodeFixAsync(Document document, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken)
{
    var root = await document.GetSyntaxRootAsync(cancellationToken);
    var classDeclaration = methodDeclaration.FirstAncestorOrSelf<ClassDeclarationSyntax>();
    var propertyName = LegacyPropertyAnalyzer.GetPropertyNameFromMethod(methodDeclaration.Identifier.Text);

    if (classDeclaration == null) return document;

    // Find the setter method
    var setterMethodDeclaration = classDeclaration.Members
        .OfType<MethodDeclarationSyntax>()
        .FirstOrDefault(m => m.Identifier.Text.Equals("set" + propertyName, StringComparison.InvariantCultureIgnoreCase));

    // Find the getter method (if needed)
    var getterMethodDeclaration = classDeclaration.Members
        .OfType<MethodDeclarationSyntax>()
        .FirstOrDefault(m => m.Identifier.Text.Equals("get" + propertyName, StringComparison.InvariantCultureIgnoreCase));

    var propertyDeclaration = classDeclaration.Members 
        .OfType<PropertyDeclarationSyntax>()
        .FirstOrDefault(p => p.Identifier.Text.Equals(propertyName, StringComparison.InvariantCultureIgnoreCase));

    var fieldDeclaration = LegacyPropertyAnalyzer.GetFieldFromPropertyMethod(methodDeclaration);

    if (fieldDeclaration != null)
    {
        // Remove the field
        root = root.RemoveNode(fieldDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }


    if (setterMethodDeclaration != null)
    {
        // Remove the setter method
        root = root.RemoveNode(setterMethodDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    if (getterMethodDeclaration != null)
    {
        // Remove the getter method
        root = root.RemoveNode(getterMethodDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    if (propertyDeclaration != null)
    {
        // Remove the property
        root = root.RemoveNode(propertyDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    // Return the document with the updated root
    return document.WithSyntaxRoot(root);
}

Debugging a unit test, I can see that the code fix is entered and root.RemoveNode(...) is called multiple times to remove multiple elements. Given the following test code:

class TestMethod2
{   
    private int prop1;

    public int getProp1()
    {
        return prop1;
    }

    public void setProp1(int prop1)
    {
        this.prop1 = prop1;
    }

    public int Prop1 { get => prop1; set => this.prop1 = value; }

    public void foo()
    {
        var x = getProp1();
        setProp1(42);
    }
}

I would expect everything but foo() to be removed.

However, the unit test fails, claiming not everything has been removed.

• the code as is removes just the field, but not the methods and the property
• if I disable the RemoveNode(fieldDeclaration,...) it removes the get and set methods, but not the property (and ofc not the field)
• if I also disable the RemoveNode on the methods, it removes the property

Can someone explain this behavior? I am really confused.

I know there is also a RemoveNodes() option and passing all nodes at once DOES work, but that is not my key issue, as the particular code fix is just an illustration. There should be a proper way to remove - or alter/add - multiple nodes in multiple steps.

I am in the process of developing a C# roslyn code analyzer and code fix and am struggling with the proper code to perform the code changes. The analyzer and code fix logic itself does (hopefully) not matter too much.

I may be struggling with a general understanding of how to apply code changes with GetSyntaxRootAsync(), but my understanding is that the overall approach is to retrieve a root, apply changes (each change returns a new root to be used to apply subsequent changes) and then return document.WithSyntaxRoot(root).

However, the following code fix does not seem to apply the correct changes and I fail to grasp why:

private async Task<Document> ApplyCodeFixAsync(Document document, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken)
{
    var root = await document.GetSyntaxRootAsync(cancellationToken);
    var classDeclaration = methodDeclaration.FirstAncestorOrSelf<ClassDeclarationSyntax>();
    var propertyName = LegacyPropertyAnalyzer.GetPropertyNameFromMethod(methodDeclaration.Identifier.Text);

    if (classDeclaration == null) return document;

    // Find the setter method
    var setterMethodDeclaration = classDeclaration.Members
        .OfType<MethodDeclarationSyntax>()
        .FirstOrDefault(m => m.Identifier.Text.Equals("set" + propertyName, StringComparison.InvariantCultureIgnoreCase));

    // Find the getter method (if needed)
    var getterMethodDeclaration = classDeclaration.Members
        .OfType<MethodDeclarationSyntax>()
        .FirstOrDefault(m => m.Identifier.Text.Equals("get" + propertyName, StringComparison.InvariantCultureIgnoreCase));

    var propertyDeclaration = classDeclaration.Members 
        .OfType<PropertyDeclarationSyntax>()
        .FirstOrDefault(p => p.Identifier.Text.Equals(propertyName, StringComparison.InvariantCultureIgnoreCase));

    var fieldDeclaration = LegacyPropertyAnalyzer.GetFieldFromPropertyMethod(methodDeclaration);

    if (fieldDeclaration != null)
    {
        // Remove the field
        root = root.RemoveNode(fieldDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }


    if (setterMethodDeclaration != null)
    {
        // Remove the setter method
        root = root.RemoveNode(setterMethodDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    if (getterMethodDeclaration != null)
    {
        // Remove the getter method
        root = root.RemoveNode(getterMethodDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    if (propertyDeclaration != null)
    {
        // Remove the property
        root = root.RemoveNode(propertyDeclaration, SyntaxRemoveOptions.KeepNoTrivia);
    }

    // Return the document with the updated root
    return document.WithSyntaxRoot(root);
}

Debugging a unit test, I can see that the code fix is entered and root.RemoveNode(...) is called multiple times to remove multiple elements. Given the following test code:

class TestMethod2
{   
    private int prop1;

    public int getProp1()
    {
        return prop1;
    }

    public void setProp1(int prop1)
    {
        this.prop1 = prop1;
    }

    public int Prop1 { get => prop1; set => this.prop1 = value; }

    public void foo()
    {
        var x = getProp1();
        setProp1(42);
    }
}

I would expect everything but foo() to be removed.

However, the unit test fails, claiming not everything has been removed.

• the code as is removes just the field, but not the methods and the property
• if I disable the RemoveNode(fieldDeclaration,...) it removes the get and set methods, but not the property (and ofc not the field)
• if I also disable the RemoveNode on the methods, it removes the property

Can someone explain this behavior? I am really confused.

I know there is also a RemoveNodes() option and passing all nodes at once DOES work, but that is not my key issue, as the particular code fix is just an illustration. There should be a proper way to remove - or alter/add - multiple nodes in multiple steps.

• roslyn

1 Answer 1

Syntax trees are immutable. Once you do root = root.RemoveNode(...), you get a new syntax tree. Then your subsequent RemoveNode calls are passing a node that no longer exists in the new root.

You can use SyntaxEditor for that, which does proper tracking of nodes across multiple changes.