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

Symbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) does not escape everything it should #74117

Open
SteveDunn opened this issue Jun 24, 2024 · 5 comments · May be fixed by #74125
Assignees
Milestone

Comments

@SteveDunn
Copy link

SteveDunn commented Jun 24, 2024

I think I've found a bug in Symbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat).

Previously, in my source generator, I used the following to get the fully qualified name:
value.FullName() ?? value.Name ?? throw new InvalidOperationException()

But it was recommended I use .ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) instead.

But it doesn't escape everything it should.

Version Used:
Happens in all versions tested.

Steps to Reproduce:

In Vogen, my source generator, I look for ValueObject attributes and get a symbol from the underlying type argument. If I give the following horrendous (but legal) C#@

using Vogen;
namespace @class
{
    namespace record.@struct.@float
    {
        public readonly record struct @decimal();
        public readonly record struct @event2();
        public readonly record struct @event();
    }

  [ValueObject(underlyingType: typeof(record.@struct.@float.@decimal))]
  public partial class MyVo 
  { 
  }

... then I get different results. Here's the difference between the two:

value.FullName()
"@class.@record.@struct.@float.@decimal"

value.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)
"global::@class.record.@[email protected]"

Expected Behavior:
value.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) should escape decimal. It might also want to escape record

Actual Behavior:
It doesn't escape decimal and leads to illegal C# code being emitted from my source generator.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 24, 2024
@CyrusNajmabadi
Copy link
Member

Sorry. I missed this was for creating a type syntax. The decimal part would be a bug. The record part looks correct to me. It's not a keyword.

@SteveDunn
Copy link
Author

I'm a bit confused now. In my source generator, I want to emit the fully qualified name of a symbol. Should I use FullName() or ToDisplayString(), or perhaps something else entirely?

@333fred
Copy link
Member

333fred commented Jun 24, 2024

The record part looks correct to me. It's not a keyword.

I could go either way here. It will generate a warning if it's a class name, so it feels like the safer thing to do is just escape it always.

@333fred
Copy link
Member

333fred commented Jun 26, 2024

@CyrusNajmabadi, did you have a response to my comment?

@CyrusNajmabadi
Copy link
Member

Fine with me always escape it.

@jaredpar jaredpar added this to the 17.12 milestone Jun 27, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants