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

Additional tab white space on XML comment leading trivia if Attribute is added prior #63227

Closed
elachlan opened this issue Aug 5, 2022 · 7 comments
Labels
Area-Compilers Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@elachlan
Copy link
Contributor

elachlan commented Aug 5, 2022

I am currently working on this PR for CsWin32: microsoft/CsWin32#621

We have this code that causes the xml comment to have an extra leading tab white space:

typeDeclaration = typeDeclaration
                    .WithLeadingTrivia()
                    .AddAttributeLists(AttributeList().AddAttributes(GeneratedCodeAttribute))
                    .WithLeadingTrivia(typeDeclaration.GetLeadingTrivia());

typeDeclaration is MemberDeclarationSyntax.

The generated code attribute is:

private static readonly AttributeSyntax GeneratedCodeAttribute = Attribute(IdentifierName("global::System.CodeDom.Compiler.GeneratedCode"))
        .WithArgumentList(FixTrivia(AttributeArgumentList().AddArguments(
            AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(ThisAssembly.AssemblyName))),
            AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(ThisAssembly.AssemblyInformationalVersion))))));

If we do not add the attribute, then the leading trivia xml comment is formatted correctly.

Am I missing something obvious?

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 5, 2022
@Youssef1313
Copy link
Member

typeDeclaration = typeDeclaration
                    .WithLeadingTrivia()
                    .AddAttributeLists(AttributeList().AddAttributes(GeneratedCodeAttribute))
                    .WithLeadingTrivia(typeDeclaration.GetLeadingTrivia());

I'm not seeing this line being hit with typeDeclaration corresponding to PInvoke in test TestSimpleMethod. Can you confirm?

It looks like trivia is messed up at WhitespaceRewriter

@elachlan
Copy link
Contributor Author

elachlan commented Aug 8, 2022

Thank you for pointing me in the right direction!

@elachlan
Copy link
Contributor Author

elachlan commented Aug 8, 2022

@Youssef1313, You can hit via HasGeneratedCodeAttribute test. I am fairly sure the issue is in VisitList. Though I am unsure how to solve it. It seems like it thinks that the XML comments are related to the attribute and not the class?

@jaredpar jaredpar added Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 9, 2022
@jaredpar jaredpar added this to the Backlog milestone Aug 9, 2022
@Youssef1313
Copy link
Member

@elachlan Yeah the doc comment trivia indeed belongs to the attribute.

@elachlan
Copy link
Contributor Author

elachlan commented Aug 9, 2022

Should it belong to the class and not the attribute? This issue really only applied to comment trivia on classes with attributes. In VisitList we can't really test for that because the parent is null.

@Youssef1313
Copy link
Member

Should it belong to the class and not the attribute?

Unfortunately, no. That's how trivia works. They're specifically attached to a token (more specifically, the first token of the method declaration). When there is an attribute, the first token of the method declaration is the [ (OpenBracketToken).

@Youssef1313
Copy link
Member

Hope elachlan/CsWin32#1 corrects the implementation :)

@elachlan elachlan closed this as completed Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

3 participants