dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.97k stars 4.03k forks source link

Inconsistent display string for names which require escaping #75434

Open vladd opened 8 hours ago

vladd commented 8 hours ago

Version Used:

Microsoft.CodeAnalysis.CSharp version 4.11.0

Steps to Reproduce:

  1. Get an INamedTypeSymbol for a record struct named void
  2. Get a display string with ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)
  3. Observe no @-symbol in display string
  4. Repeat the same procedure with record class, class, record, observe escape symbol in display string.

Repro:

using System;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;

var source =
    """
    namespace RS { record struct @void { } }
    namespace RC { record  class @void { } }    
    namespace S  {        struct @void { } }    
    namespace C  {         class @void { } }    
    """;

var tree = CSharpSyntaxTree.ParseText(source);

var mscorlib = MetadataReference.CreateFromFile(typeof(object).Assembly.Location);
var compilation = CSharpCompilation.Create("TestCompilation", syntaxTrees: [tree], references: [mscorlib]);

foreach (var prefix in new[] { "RS", "RC", "S", "C" })
{
    var typeSymbol = compilation.GetTypeByMetadataName(prefix + ".void")!;
    Console.WriteLine(prefix + " -> " + typeSymbol.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat));
}

Project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0" />
  </ItemGroup>
</Project>

Expected Behavior:

Display string should be consistent for all kinds of symbols, especially for structs and record structs.

Test code above outputs

RS -> @void
RC -> @void
S -> @void
C -> @void

Actual Behavior:

Record struct's display string doesn't escape identifiers.

Test code outputs

RS -> void
RC -> @void
S -> @void
C -> @void
vladd commented 8 hours ago

Offtopic: I'm using ToDisplayString for determining type's escaped name in code generation. Is there a better way?

CyrusNajmabadi commented 7 hours ago

Use SyntaxFacts to ask if it's a keyword

vladd commented 6 hours ago

@CyrusNajmabadi Thank you, this fixes my problem. Nevertheless, the inconsistency of ToDisplayName is still there and should be corrected.

vladd commented 6 hours ago

@CyrusNajmabadi Well, my problem is not fixed completely, as ToDisplayName handles generic arguments as well.

vladd commented 5 minutes ago

The problem is here: https://github.com/dotnet/roslyn/blob/bbd787e1eb55b176f3274b84bd48ada0b210c846/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.cs#L114

private static bool IsEscapable(SymbolDisplayPartKind kind)
{
    switch (kind)
    {
        case SymbolDisplayPartKind.AliasName:
        case SymbolDisplayPartKind.ClassName:
        case SymbolDisplayPartKind.RecordClassName:
        case SymbolDisplayPartKind.StructName:
        case SymbolDisplayPartKind.RecordStructName:
        case SymbolDisplayPartKind.InterfaceName:
        case SymbolDisplayPartKind.EnumName:
        case SymbolDisplayPartKind.DelegateName:
        case SymbolDisplayPartKind.TypeParameterName:
        case SymbolDisplayPartKind.MethodName:
        case SymbolDisplayPartKind.PropertyName:
        case SymbolDisplayPartKind.FieldName:
        case SymbolDisplayPartKind.LocalName:
        case SymbolDisplayPartKind.NamespaceName:
        case SymbolDisplayPartKind.ParameterName:
            return true;
        default:
            return false;
    }
}

It must have included SymbolDisplayPartKind.RecordStructName too.