Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompilationUnitSyntax.Members[i].ToFullString() skips #endif if it's at the end of the file #66019

Closed
eduherminio opened this issue Dec 14, 2022 · 11 comments
Labels

Comments

@eduherminio
Copy link
Member

eduherminio commented Dec 14, 2022

Version Used:

Microsoft.CodeAnalysis.CSharp.Workspaces v4.4.0

Steps to Reproduce:

git clone https://github.com/eduherminio/roslyn-66019-compilationunitsyntax-endif
cd roslyn-66019-compilationunitsyntax-endif
dotnet run
# Compare output with file contents

Expected Behavior:

This piece of code:

using Microsoft.CodeAnalysis.CSharp;

var root = CSharpSyntaxTree.ParseText(await File.ReadAllTextAsync("valid_file.cs")).GetCompilationUnitRoot();
foreach (var member in root.Members)
{
    Console.Write(member.ToFullString());
}

prints the whole content of valid_file.cs:

namespace Endif;

#if DEBUG
public class A1
{

}
#else

public class B1
{

}

#endif

Actual Behavior:

The mentioned piece of code prints:

namespace Endif;

#if DEBUG
public class A1
{

}
#else

public class B1
{

}

Skipping the #endif

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 14, 2022
@DoctorKrolic
Copy link
Contributor

The #endif is attached to the EndOfFileToken, that's why you don't see it: https://sharplab.io/#v2:C4LghgzsA0AmIGoA+A7MBbAphADmAxpgAQCiKsAlgGYDcAsAFCMDE1RASiQDIkCCAyiUYABAMxFhAJiK8AjIwDejRgF8WmADYRMyhmInSAQvIZKmDNeeaZy1IA==

@Youssef1313
Copy link
Member

Yes. This is by-design.

@eduherminio
Copy link
Member Author

I see, thanks both of you! I'm always impressed how awesome sharplab is, btw.

Do you folks happen to know/remember/can think of any other gotchas of the following approach to translate CompilationUnitSyntax back to the *.cs files they came from?

  • CompilationUnitSyntax.Usings
  • CompilationUnitSyntax.Members
  • CompilationUnitSyntax.EndOfFileToken

I'd be happy to be pointed to any documentation/code if you know a good source to learn about it 😃

@DoctorKrolic
Copy link
Contributor

If you have it as CompilationUnitSyntax you can just call ToFullString(). This preserves all necessary trivia

@CyrusNajmabadi CyrusNajmabadi added Question Resolution-Answered The question has been answered and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 14, 2022
@eduherminio
Copy link
Member Author

If you have it as CompilationUnitSyntax you can just call ToFullString(). This preserves all necessary trivia

I'm afraid that's not the case, since I'm mixing multiple files into one, but good to know, thanks!

@Youssef1313
Copy link
Member

Youssef1313 commented Dec 15, 2022

since I'm mixing multiple files into one

What exactly the scenario where you want to do this? Why it is needed to merge multiple trees?

Anyway, my suggestion for that is:

  • Walk every tree you have, take strip all usings from it, and strip the leading trivia of the first member only if it is #endif.
  • Now you got a bunch of usings and all the trees without the using.
  • In the new file, concatenate all the usings that were stripped, then concatenate the rest of all trees.
  • This will be trickier if trees have file-scoped namespaces.
  • If one of the trees has top-level statements, it should be the first once to add after all the usings. What about multiple trees having top-level statements? Is that something that can happen for your scenario? I can't think of a good behavior for that case.

@eduherminio
Copy link
Member Author

It's for dotnet-combine tool, which aims to do exactly that, merge multiple C# files into a single one and keep everything working.

I'm fully aware there are some scenarios that will very tricky or just impossible to support, such as the one with multiple top-level statements that you mention, but I'm fine with that.

I got it working more or less as I expected 2 years ago, following a process similar to what you described, but I'm revisiting now (i.e. added support for file-scoped namespaces) and realizing that the devil's in the detail, of course.

Walk every tree you have, take strip all usings from it, and strip the leading trivia of the first member only if it is #endif.

I need to have a look that last part of this, since I'm definitely not doing that right now.

@Youssef1313
Copy link
Member

I need to have a look that last part of this, since I'm definitely not doing that right now.

Basically what I mean here is for something like this

#if DEBUG
using System;
#endif

public class C { }

In this case, #endif is attached to the class (the first member of compilation unit). So in a tree such as the above, you need to strip all the usings in addition to #endif

@eduherminio
Copy link
Member Author

Thanks a lot @Youssef1313, good example of edge case that represents a bug in the tool as it is right now.

@svick
Copy link
Contributor

svick commented Dec 15, 2022

@eduherminio If you want to figure out what are all the parts of a CompilationUnitSyntax, then they're here:

<Node Name="CompilationUnitSyntax" Base="CSharpSyntaxNode">
<Kind Name="CompilationUnit"/>
<Field Name="Externs" Type="SyntaxList&lt;ExternAliasDirectiveSyntax&gt;"/>
<Field Name="Usings" Type="SyntaxList&lt;UsingDirectiveSyntax&gt;"/>
<Field Name="AttributeLists" Type="SyntaxList&lt;AttributeListSyntax&gt;">
<PropertyComment>
<summary>Gets the attribute declaration list.</summary>
</PropertyComment>
</Field>
<Field Name="Members" Type="SyntaxList&lt;MemberDeclarationSyntax&gt;"/>
<Field Name="EndOfFileToken" Type="SyntaxToken">
<Kind Name="EndOfFileToken"/>
</Field>
</Node>

@eduherminio
Copy link
Member Author

Awesome, thanks a lot @svick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants