none
Formatting gets messed up for nodes that I do not modify

    質問

  • Hello fellow CTPers.

    I am modifying some code files using Roslyn, but the formatting of the file gets messed up for all Syntax elements except the one I modify. I'd like some guidance as to what I'm doing wrong.

    What I'm doing: I want to search all csharp files for classes that have the TestFixture attribute (I know, it's nunit, please don't crucify me). I want to make sure all those classes inherit a BaseTest class that exposes some common logic for our app.

    Here's what I have so far:

    	static void Main(string[] args)
            {
                var rootPath = ProcessArgs(args);
                if (string.IsNullOrWhiteSpace(rootPath))
                {
                    rootPath = Path.GetFullPath(DefaultRootPath);
                }
                Console.WriteLine(String.Format("Making all unit test classes found in '{0}' and below inherit BaseTest...", rootPath));
    
                var solutionFiles = Directory.GetFiles(rootPath, "*.sln", SearchOption.AllDirectories);
                foreach (var solutionFile in solutionFiles)
                {
                    var workspace = Workspace.LoadSolution(solutionFile);
                    var newSolution = workspace.CurrentSolution;
    
                    foreach (var project in newSolution.Projects)
                    {
                        //Test if the project can be loaded and has any documents at the same time.
                        try
                        {
                            if (project.HasDocuments)
                            {
                                foreach (var document in from document in project.Documents
                                                         where document.SourceCodeKind == SourceCodeKind.Regular && document.LanguageServices.Language == LanguageNames.CSharp
                                                         select document)
                                {
                                    var rootNode = (CompilationUnitSyntax)document.GetSyntaxTree().Root;
                                    var visitor = new MakeUnitTestInheritBaseTest();
                                    var newRoot = visitor.Visit(rootNode);
                                    newSolution = newSolution.UpdateDocument(document.Id, newRoot.GetFullTextAsIText());
                                }
                            }
                        }
                        catch (InvalidProjectFileException ex)
                        {
                            //Ignore projects that cannot be loaded.
                            Console.WriteLine(String.Format("A project in solution {0} was ignored. {1}", solutionFile, ex.Message));
                        }
                    }
    
                    if (!workspace.ApplyChanges(workspace.CurrentSolution, newSolution))
                    {
                        Console.WriteLine("Failed to update solution " + solutionFile);
                    }
                }
    
                Console.WriteLine("Done.");
            }

    and

        class MakeUnitTestInheritBaseTest : SyntaxRewriter
        {
            protected override SyntaxNode VisitClassDeclaration(ClassDeclarationSyntax node)
            {
                //TODO: Fully qualify TestFixture and BaseTest on the off chance that there are more than attribute/class named like this.
                //Look for the TestFixture attribute, and make sure the class doesn't already inherit BaseTest.
                var classHasTestFixtureAttr = (from attributeDeclarationSyntax in node.Attributes
                                               from attributeSyntax in attributeDeclarationSyntax.Attributes
                                               where attributeSyntax.Name.PlainName == "TestFixture"
                                               select attributeSyntax).Any();
                var classInheritsBaseTest = node.BaseListOpt != null &&
                                            node.BaseListOpt.Types.Where(typeSyntax => typeSyntax.PlainName == "BaseTest").
                                                Any();
    
                if (classHasTestFixtureAttr && !classInheritsBaseTest)
                {
                    var newBaseNode = Syntax.BaseList(Syntax.Token(SyntaxKind.ColonToken), Syntax.SeparatedList(Syntax.ParseTypeName("BaseTest")));
                    return node.Update(node.Attributes, node.Modifiers, node.Keyword, node.Identifier,
                                       node.TypeParameterListOpt, newBaseNode, node.ConstraintClauses, node.OpenBraceToken,
                                       node.Members, node.CloseBraceToken, node.SemicolonTokenOpt).Format();
                }
                //TODO: Optimization: return yield the next sibling. I can't see how a TestFixture could be nested in another one.
                return base.VisitClassDeclaration(node);
            }
        }

    In before "you're going to overwrite the previous base class with this logic", "BaseTest may not be in the right namespace". I know, this is the first draft of the code, let it go.

    Here's a sample code file before I run this code:

    namespace Test_Solution
    {
        [TestFixture]
        public class Class1
        {
        }
    }

    and here's what it looks like after

    namespace Test_Solution
    {
    [TestFixture]
    public class Class1 : BaseTest
    {
    }}

    My question is two-fold. First, why is it that the only node that has proper formatting is the one I modified? Second, am I doing it right? Is there an easier way to use Roslyn to just slug through a bunch of code and modify classes.

    Thanks for your time.

    2012年3月7日 18:58

回答

  • Success! You were on the right track svick. The only thing that was missing was to remove the trailing trivia of the Identifier node, and replace it with a space trivia.

    The final SyntraxRewriter code looks like this:

    var newBaseNode = Syntax.BaseList(Syntax.Token(SyntaxKind.ColonToken), Syntax.SeparatedList(Syntax.ParseTypeName("BaseTest"))).Format().WithTrailingTrivia(node.Identifier.TrailingTrivia);
    var newIdentifierNode = node.Identifier.WithLeadingTrivia(node.Identifier.LeadingTrivia).WithTrailingTrivia(Syntax.Space);
    return node.Update(node.Attributes, node.Modifiers, node.Keyword, newIdentifierNode,
                                       node.TypeParameterListOpt, newBaseNode, node.ConstraintClauses, node.OpenBraceToken,
                                       node.Members, node.CloseBraceToken, node.SemicolonTokenOpt);
    Thanks everyone!

    edit: I probably need to worry about the TypeParameterListOpt node, but that's for another day.
    2012年3月9日 20:08

すべての返信

  • What I think is happening is that when you call Format() at

                    return node.Update(node.Attributes, node.Modifiers, node.Keyword, node.Identifier,
                                       node.TypeParameterListOpt, newBaseNode, node.ConstraintClauses, node.OpenBraceToken,
                                       node.Members, node.CloseBraceToken, node.SemicolonTokenOpt).Format();
    

    it formats it for that specific node, and that node does not need any indentation or a newline at the end, because you format for that node only. It doesn't look at its parent when formatting, I think. A simple solution would be to call Format() after visiting your rootnode, so

    var newRoot = visitor.Visit(rootNode).Format();

    This formats all the nodes that are visited, though. Even if nothing else changes. Another way would be to manually format your newBaseNode, and then don't call Format anywhere. Doing it this way makes sure that nothing else gets formatted. Or you could update the parent of that node, format that parent, and then return the same child node of that parent.

    Hope this helps!

    2012年3月8日 9:05
  • Thanks for your reply Nico.

    Unfortunately, none of your suggested solutions work for me:

    - Formatting the root node: I don't want to change anything else than the refactoring I'm doing. The main issue here is that it would seriously freak out our diff tools and scare code reviewers away.

    - Manually format the new node: I tried to include the leading and trailing trivia of the old node in the new node. This is what the result looks like:

    namespace Test_Solution
    {
        [TestFixture]
    public class Class1 : BaseTest
    {
    }
    }
    

    We see that the leading and trailing are there, but the body of the class is not indented properly. Not good enough.

    - Update the parent node: This would be the same as solution #2, because I would then lose all trivia of the parent node in the grandparent node. Recursively, I would end up formatting the root node.

    2012年3月9日 19:17
  • I'm still quite unclear how exactly are you supposed to work with trivia, so maybe there is a better solution to this, but this is the best I could come up with:

    You should move the trailing trivia from the Identifier (“Class1”) to your base node and replace it with just a space. Also add a space after the colon but don't call Format() otherwise to keep the whitespace exactly the way it is. So the part of the rewriter would look like this:

    var newBaseNode = Syntax.BaseList(Syntax.Token(SyntaxKind.ColonToken), Syntax.SeparatedList(Syntax.ParseTypeName("BaseTest")))
        .Format() // to insert space after colon; easier than specifying the space explicitly
        .WithTrailingTrivia(node.Identifier.TrailingTrivia);
    return node.Update(
        node.Attributes, node.Modifiers, node.Keyword, node.Identifier.WithTrailingTrivia(Syntax.ParseTrailingTrivia(" ")),
        node.TypeParameterListOpt, newBaseNode, node.ConstraintClauses, node.OpenBraceToken,
        node.Members, node.CloseBraceToken, node.SemicolonTokenOpt);
    • 編集済み svick 2012年3月9日 19:56
    2012年3月9日 19:55
  • Success! You were on the right track svick. The only thing that was missing was to remove the trailing trivia of the Identifier node, and replace it with a space trivia.

    The final SyntraxRewriter code looks like this:

    var newBaseNode = Syntax.BaseList(Syntax.Token(SyntaxKind.ColonToken), Syntax.SeparatedList(Syntax.ParseTypeName("BaseTest"))).Format().WithTrailingTrivia(node.Identifier.TrailingTrivia);
    var newIdentifierNode = node.Identifier.WithLeadingTrivia(node.Identifier.LeadingTrivia).WithTrailingTrivia(Syntax.Space);
    return node.Update(node.Attributes, node.Modifiers, node.Keyword, newIdentifierNode,
                                       node.TypeParameterListOpt, newBaseNode, node.ConstraintClauses, node.OpenBraceToken,
                                       node.Members, node.CloseBraceToken, node.SemicolonTokenOpt);
    Thanks everyone!

    edit: I probably need to worry about the TypeParameterListOpt node, but that's for another day.
    2012年3月9日 20:08