Symbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) does not escape everything it should
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.
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.
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?
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.
@CyrusNajmabadi, did you have a response to my comment?
Fine with me always escape it.
Still need to handle escaping record itself.
Sorry, lost track of this one @333fred . Should I: a) create a new issue and a new PR b) create a new PR and reference this issue c) [something else]
Guessing b) seeing as this was just reopened
B; if you want to take care of it, we'd absolutely welcome the PR :slightly_smiling_face:.
B; if you want to take care of, we'd absolutely welcome the PR 🙂.
Leave it to me boss!
https://github.com/dotnet/roslyn/pull/74227
This was done in #74227.