[8.0.100-preview.3.23178.7] Incremental Roslyn Source Generator ToDisplayString() method returns different value in 7.0.20X and later

[Junjun-zhao]

Describe the bug

App is using source generator to create additional C# code, but code generated is different in 7.0.20X and later (Include 8.0 preview), so code doesn't compile, build fails.

Application Name: PixiEditorDotNetCore
OS: Windows 10 21H2
CPU: X64
.NET Build Number: dotnet-sdk-8.0.100-preview.3.23178.7

App Location checking at: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1784818/
Or Github link: https://github.com/flabbet/PixiEditor

Verify Scenarios:
1). Windows 10 21H2 AMD64 + dotnet-sdk-8.0.100-preview.3.23178.7: Fail
2). Windows 10 21H2 AMD64 + dotnet-sdk-8.0.100-preview.2.23153.6: Fail
3). Windows 10 21H2 AMD64 + dotnet-sdk-8.0.100-preview.1.23115.2: Fail
4). Windows 10 21H2 AMD64 + dotnet-sdk-7.0.104: Pass
5). Windows 10 21H2 AMD64 + dotnet-sdk-7.0.105: Pass
6). Windows 10 21H2 AMD64 + dotnet-sdk-7.0.202: Fail

Repro steps:
Build project with 7.0.20X SDK or later.

1. Build PixiEditor\PixiEditor.csproj net7.0-windows

Expected Result:

build successfully.

Actual Result:

build failed with errors.

Repro Steps to produce from sample project: Attached a sample project:
TestCodeGeneration.zip

1. In CMD, go to TestCodeGeneration\WinFormsApp1
2. dotnet build /t:rebuild

build with SDK 7.0.202 or later

 Exception was of type 'Exception' with message 'typeof(bool disposing)' [C:\Users\v-alt
unc\source\repos\TestCodeGeneration\WinFormsApp1\WinFormsApp1.csproj

build with SDK 7.0.104

Exception was of type 'Exception' with message 'typeof(bool)' [C:\Users\v-altunc\source
\repos\TestCodeGeneration\WinFormsApp1\WinFormsApp1.csproj]

For Sample APP, when we compare the source code generated from app, we noticed the difference between 7.0.10X and 7.0.20X

.NET 7.0.202 or later

 Commands[typeof(PixiEditor.ViewModels.SubViewModels.Document.DocumentManagerViewModel)].Add(("FlipImage", new Type[] { typeof(PixiEditor.ChangeableDocument.Enums.FlipType type) }));  //this line won't compile

.NET 7.0.104

 Commands[typeof(PixiEditor.ViewModels.SubViewModels.Document.DocumentManagerViewModel)].Add(("FlipImage", new Type[] { typeof(PixiEditor.ChangeableDocument.Enums.FlipType) }));

Findings

It looks like ToDisplayString() method, returns different values in the code below.

  public void Initialize(IncrementalGeneratorInitializationContext context)
        {
            var commandList = context.SyntaxProvider.CreateSyntaxProvider(
              (x, token) =>
              {
                  return x is MethodDeclarationSyntax method;
              }, static (context, cancelToken) =>
              {
                  var method = (MethodDeclarationSyntax)context.Node;
                  var symbol = context.SemanticModel.GetDeclaredSymbol(method, cancelToken);
                  if (symbol is IMethodSymbol methodSymbol)
                  {  
                      return (methodSymbol.ReceiverType.ToDisplayString(), methodSymbol.Name, methodSymbol.Parameters.Select(x => x.ToDisplayString()));
                  }
                  else
                  {
                      return (null, null, null);
                  }
              }).Where(x => x.Item1 != null);

            context.RegisterSourceOutput(commandList.Collect(), static (context, methodNames) =>
            {
                var param = string.Join(",", methodNames.FirstOrDefault().Item3.Select(x => $"typeof({x})"));
                context.AddSource("Test.Generator", TestSourceCode(param));
            });
        }

@dotnet-actwx-bot @dotnet/compat

[Junjun-zhao]

@jcouv Could you help check this issue and confirm whether it is a blocker for .NET 8.0 Preview3 validation? Please help move to the right area if it was assigned incorrect. Thanks.

[CyrusNajmabadi]

This is by design. Getting the display string for a parameter always returns its name now.

[PriyaPurkayastha]

@CyrusNajmabadi is this documented as a breaking change? .NET breaking changes are published on https://learn.microsoft.com/en-us/dotnet/core/compatibility/8.0 and template for breaking changes is here

[CyrusNajmabadi]

I don't believe so. This api is only for display purposes, so the expectation is that it might change over time if we feel the display is better.

[PriyaPurkayastha]

We are the .NET AppCompat team and we have a representative set of apps in our lab. Generally, an issue that we find in-house could potentially impact a bigger customer base in the real world. As a result, we ask teams if the change can be made in a less breaking way and secondly, we ask all teams to follow the breaking change process since it is important to communicate to customers about the breaking change. cc @marklio for additional inputs.

[CyrusNajmabadi]

As a result, we ask teams if the change can be made in a less breaking way
we ask all teams to follow the breaking change process since it is important to communicate to customers about the breaking change.

This is not a breaking change. This is a display string (similar to how .ToString would work). That's a core part of it's name and contract here in its doc comment

Convert a symbol to a string that can be displayed to the user.

You'd use it to put something in a display that a user might look at. The expectation is that it could potentially change and adapt as we decide what looks good for users.

[CyrusNajmabadi]

Note: the code in questino does this:

methodSymbol.Parameters.Select(x => x.ToDisplayString())

We have never stated in docs (and have stated plainly to customers elsewhere like in our communities) that ToDisplayString'ing random symbols is not a guarantee of parseable code. It is not a part of any contract (implied, or explicit) that this would ever work, and we def have and will continue to change display strings such that this may not work. Users are strongly advised to produce trees/code using supported mechanisms (SyntaxFactory/SyntaxGenerator) if they want to ensure parseable/compilable code.

[marklio]

This all makes sense to me. I do think there is an opportunity to be more explicit in the docs about not using the output of such "display" APIs to generate code, and that such output is subject to change to most appropriately reflect a useful display string.

I think there's an opportunity to do even more here, like analyzers that would warn about the use of such APIs in source generators.

The compat lab has a good track record of forecasting customer disruption. I would expect that other customers will encounter this issue.

[jcouv]

Assigned to @jjonescz to add a note in breaking changes doc as FYI. We may need to discuss with Jared and Cyrus to confirm.