0

I have a situation where I need to modify a situation where a user writes this kind of code :

bool SomeMethod(object obj)
{
    if(obj == null)
        return false; 
    return true;
}

To the following code :

bool SomeMethod(object obj)
{
   return obj == null; 
} 

Currently, I have built an analyzer that works. I'll put the code below. Basically, the analyzer looks for if statements and verifies if the only statement of the if is a return statement.Not only that, it verifies that also, the next statement inside the method's declaration is a return statement.

The code fix provider looks for the ifStatement's condition and creates a new return statement using that condition. What I'm trying to do after replacing the if statement by the return statement, is to remove the second return statement. At this, I'm failing without knowing how.

At first, when the node's been replaced, I create a new root, because they can't be modify like a string, it's about the same thing. I try to delete the node, but this instruction is being ignored for some reason. And I've debugged it, when I access the next node(ReturnStatement), it's not null.

I guess my question is basically, how can I build a code fix provider which can modify a node without being "linked" on it.

Here's the code for the analyzer

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using System.Collections.Immutable;
using System.Linq;

namespace RefactoringEssentials.CSharp.Diagnostics
{
    [DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class RewriteIfReturnToReturnAnalyzer : DiagnosticAnalyzer
    {
        private static readonly DiagnosticDescriptor descriptor = new DiagnosticDescriptor(
            CSharpDiagnosticIDs.RewriteIfReturnToReturnAnalyzerID,
            GettextCatalog.GetString("Convert 'if...return' to 'return'"),
            GettextCatalog.GetString("Convert to 'return' statement"),
            DiagnosticAnalyzerCategories.Opportunities,
            DiagnosticSeverity.Info,
            isEnabledByDefault: true,
            helpLinkUri: HelpLink.CreateFor(CSharpDiagnosticIDs.RewriteIfReturnToReturnAnalyzerID)
            );

        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(descriptor);

        public override void Initialize(AnalysisContext context)
        {
            context.RegisterSyntaxNodeAction(
                (nodeContext) =>
                {
                    Diagnostic diagnostic;
                    if (TryGetDiagnostic(nodeContext, out diagnostic))
                    {
                        nodeContext.ReportDiagnostic(diagnostic);
                    }
                }, SyntaxKind.IfStatement);
        }

        private static bool TryGetDiagnostic(SyntaxNodeAnalysisContext nodeContext, out Diagnostic diagnostic)
        {
            diagnostic = default(Diagnostic);
            if (nodeContext.IsFromGeneratedCode())
                return false;

            var node = nodeContext.Node as IfStatementSyntax;
            var methodBody = node?.Parent as BlockSyntax;
            var ifStatementIndex = methodBody?.Statements.IndexOf(node);

            if (node?.Statement is ReturnStatementSyntax &&
                methodBody?.Statements.ElementAt(ifStatementIndex.Value + 1) is ReturnStatementSyntax)
            {
                diagnostic = Diagnostic.Create(descriptor, node.GetLocation());
                return true;
            }
            return false;
        }
    }
}

Here's the code for the Code fix provider

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;

namespace RefactoringEssentials.CSharp.Diagnostics
{
    [ExportCodeFixProvider(LanguageNames.CSharp), System.Composition.Shared]
    public class RewriteIfReturnToReturnCodeFixProvider : CodeFixProvider
    {
        public override ImmutableArray<string> FixableDiagnosticIds
        {
            get
            {
                return ImmutableArray.Create(CSharpDiagnosticIDs.RewriteIfReturnToReturnAnalyzerID);
            }
        }

        public override FixAllProvider GetFixAllProvider()
        {
            return WellKnownFixAllProviders.BatchFixer;
        }

        public async override Task RegisterCodeFixesAsync(CodeFixContext context)
        {
            var document = context.Document;
            var cancellationToken = context.CancellationToken;
            var span = context.Span;
            var diagnostics = context.Diagnostics;
            var root = await document.GetSyntaxRootAsync(cancellationToken);
            var diagnostic = diagnostics.First();
            var node = root.FindNode(context.Span);
            if (node == null)
                return;

            context.RegisterCodeFix(
                CodeActionFactory.Create(node.Span, diagnostic.Severity, "Convert to 'return' statement", token =>
                {
                    var statementCondition = (node as IfStatementSyntax)?.Condition;
                    var newReturn = SyntaxFactory.ReturnStatement(SyntaxFactory.Token(SyntaxKind.ReturnKeyword),
                        statementCondition, SyntaxFactory.Token(SyntaxKind.SemicolonToken));
                    var newRoot = root.ReplaceNode(node as IfStatementSyntax, newReturn
                        .WithLeadingTrivia(node.GetLeadingTrivia())
                        .WithAdditionalAnnotations(Formatter.Annotation));
                    var block = node.Parent as BlockSyntax;
                    if (block == null)
                        return null;

                    //This code (starting from here) does not do what I'd like to do ...
                    var returnStatementAfterIfStatementIndex = block.Statements.IndexOf(node as IfStatementSyntax) + 1;
                    var returnStatementToBeEliminated = block.Statements.ElementAt(returnStatementAfterIfStatementIndex) as ReturnStatementSyntax;
                    var secondNewRoot = newRoot.RemoveNode(returnStatementToBeEliminated, SyntaxRemoveOptions.KeepNoTrivia);
                    return Task.FromResult(document.WithSyntaxRoot(secondNewRoot));
                }), diagnostic);
        }
    }
}

And finally, this is my NUnit test :

    [Test]
        public void When_Retrurn_Statement_Corrected()
        {
            var input = @"
class TestClass
{
    bool TestMethod (object obj)
    {
        $if (obj != null)
            return true;$
        return false;
    }
}";

            var output = @"
class TestClass
{
    bool TestMethod (object obj)
    {
        return obj!= null;
    }
}";

            Analyze<RewriteIfReturnToReturnAnalyzer>(input, output);
        }
Kevin Avignon
  • 2,853
  • 3
  • 19
  • 40
  • @JohnKoerner, what's your take on this one ? – Kevin Avignon Aug 17 '15 at 16:38
  • 1
    Use the `DocumentEditor` to make multiple changes to a document. After you change one node, your tree is outdated and it won't find the other nodes anymore. More info: http://stackoverflow.com/questions/30562759/how-do-i-create-a-new-root-by-adding-and-removing-nodes-retrieved-from-the-old-r – Jeroen Vannevel Aug 18 '15 at 11:16
  • @JeroenVannevel , for the fun of it, if you wrote a sample code and little description, I could easily mark up your answer as the answer for my post ! – Kevin Avignon Aug 18 '15 at 12:18
  • @KevinAvignon FYI. The @ mentions only work if someone is already involved in the post (previously commented or answered). Otherwise it doesn't do anything. – John Koerner Aug 19 '15 at 13:16
  • @KevinAvignon: have you implemented it yet? It's really just a few `editor.RemoveNode()` and `editor.ReplaceNode()` calls that you have to make. Otherwise wait a few days until I'm finished with exams and I can go through my backlog of questions I wanted to answer still. – Jeroen Vannevel Aug 19 '15 at 13:37
  • @JeroenVannevel Should have updated this thread. I'll post the code I used as the answer and I'll refer the post you gave me ! – Kevin Avignon Aug 19 '15 at 13:40

2 Answers2

1

I believe the problem is probably this line: var block = node.Parent as BlockSyntax;

You're using the node from the original tree, with a .Parent also from the original tree (not the one with the updated newReturn).

Then, it eventually calculates returnStatementToBeEliminated using this old tree that's no longer up-to-date, so when you call var secondNewRoot = newRoot.RemoveNode(returnStatementToBeEliminated, SyntaxRemoveOptions.KeepNoTrivia);, nothing happens because newRoot does not contain returnStatementToBeEliminated.

So, you basically want to use the equivalent of node.Parent, but the version that lives under newRoot. The low-level tool we use for this is called a SyntaxAnnotation, and these have the property that they track forward between tree edits. You can add a specific annotation to the node.Parent before making any edits, then make your edits, and then ask the newRoot to find the node with your annotation.

You can track nodes manually like this, or you can use the SyntaxEditor class, which abstracts the Annotations part away into simpler methods like TrackNode (there's a few other nice features in SyntaxEditor that you may want to check out).

David Poeschl
  • 864
  • 4
  • 11
0

For this issue, I was refer to the following post : How do I create a new root by adding and removing nodes retrieved from the old root?

This post showed this class called DocumentEditor, which lets a user modify a document like he wants even though it's suppose to be immutable. The previous issue was that after deleting a node, if I was referring something that had a connection to that node, the relation would have disappear and I would be able to stuff. Basically, the documentation comment says that this class is "an editor for making changes to a document's syntax tree." After you're done modifying that document, you need to create a new document and return it as a Task in the code fix provider. To solve this issue I had with my code fix provider, I used the following code :

            context.RegisterCodeFix(CodeAction.Create("Convert to 'return' statement", async token =>
        {
            var editor = await DocumentEditor.CreateAsync(document, cancellationToken);
            var statementCondition = (node as IfStatementSyntax)?.Condition;
            var newReturn = SyntaxFactory.ReturnStatement(SyntaxFactory.Token(SyntaxKind.ReturnKeyword),
                statementCondition, SyntaxFactory.Token(SyntaxKind.SemicolonToken));
            editor.ReplaceNode(node as IfStatementSyntax, newReturn
                .WithLeadingTrivia(node.GetLeadingTrivia())
                .WithAdditionalAnnotations(Formatter.Annotation));


            var block = node.Parent as BlockSyntax;
            if (block == null)
                return null;

            var returnStatementAfterIfStatementIndex = block.Statements.IndexOf(node as IfStatementSyntax) + 1;
            var returnStatementToBeEliminated = block.Statements.ElementAt(returnStatementAfterIfStatementIndex) as ReturnStatementSyntax;
            editor.RemoveNode(returnStatementToBeEliminated);
            var newDocument = editor.GetChangedDocument();

            return newDocument;
        }, string.Empty), diagnostic);

It was really simple to solve my issue thanks to that class.

Kevin Avignon
  • 2,853
  • 3
  • 19
  • 40