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

JsonSourceGenerator can fail with global using aliases #110242

Closed
ltrzesniewski opened this issue Nov 28, 2024 · 3 comments
Closed

JsonSourceGenerator can fail with global using aliases #110242

ltrzesniewski opened this issue Nov 28, 2024 · 3 comments
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Comments

@ltrzesniewski
Copy link
Contributor

Description

Under some particular conditions, JsonSourceGenerator can fail with the following warning:

CSC : warning CS8785: Generator 'JsonSourceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'InvalidOperationException' with message 'Unreachable'.

Reproduction Steps

Build the following project:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <LangVersion>13.0</LangVersion>
  </PropertyGroup>
</Project>
global using Foo = object;

Expected behavior

I do realize that net6.0 + C# 13 is not officially supported, but I didn't expect a source generator to fail. I expected this to compile successfully without warnings.

Actual behavior

This compiles successfully but JsonSourceGenerator fails.

The stack trace in the binlog is:

System.InvalidOperationException: Unreachable
at Microsoft.CodeAnalysis.DotnetRuntime.Extensions.RoslynExtensions.GetUnqualifiedName(NameSyntax name)
at Microsoft.CodeAnalysis.DotnetRuntime.Extensions.CSharpSyntaxHelper.AddAliases(SyntaxList`1 usings, ValueListBuilder`1& aliases, Boolean global)
at Microsoft.CodeAnalysis.DotnetRuntime.Extensions.CSharpSyntaxHelper.AddAliases(SyntaxNode node, ValueListBuilder`1& aliases, Boolean global)
at Microsoft.CodeAnalysis.DotnetRuntime.Extensions.SyntaxValueProviderExtensions.<ForAttributeWithSimpleName>g__getGlobalAliasesInCompilationUnit|5_7(SyntaxNode compilationUnit)
at Microsoft.CodeAnalysis.TransformNode`2.UpdateStateTable(Builder builder, NodeStateTable`1 previousTable, CancellationToken cancellationToken)

Regression?

I don't know. I was introducing this global using alias.

Known Workarounds

  • using System.Object instead of object in the alias
  • targeting a different framework version than net6.0

I suspect the generator doesn't handle keywords in global using aliases.

Configuration

Tested on Windows 11 x64 and Linux x64 (Ubuntu WSL) with the .NET 9 SDK:

.NET SDK:
Version: 9.0.100
Commit: 59db016f11
Workload version: 9.0.100-manifests.c6f19616
MSBuild version: 17.12.7+5b8665660

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22631
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\9.0.100\

Other information

I stumbled upon this by writing the following code in a multi-targeted project:

#if NET9_0_OR_GREATER
global using Lock = System.Threading.Lock;
#else
global using Lock = object;
#endif

Using System.Object fixed the issue.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 28, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 28, 2024
@ltrzesniewski
Copy link
Contributor Author

ltrzesniewski commented Nov 28, 2024

Looks like this thing is missing a PredefinedTypeSyntax branch:

public static SimpleNameSyntax GetUnqualifiedName(this NameSyntax name)
=> name switch
{
AliasQualifiedNameSyntax alias => alias.Name,
QualifiedNameSyntax qualified => qualified.Right,
SimpleNameSyntax simple => simple,
_ => throw new InvalidOperationException("Unreachable"),
};

and this probably also applies there:

public static SimpleNameSyntax GetUnqualifiedName (this NameSyntax name)
=> name switch {
AliasQualifiedNameSyntax alias => alias.Name,
QualifiedNameSyntax qualified => qualified.Right,
SimpleNameSyntax simple => simple,
_ => throw new InvalidOperationException ("Unreachable"),
};

(I guess NameSyntax should be changed to TypeSyntax, but you get the idea)

@MichalPetryka
Copy link
Contributor

MichalPetryka commented Nov 28, 2024

I do realize that net6.0 + C# 13 is not officially supported, but I didn't expect a source generator to fail. I expected this to compile successfully without warnings.

.NET 6 is out of support now so unless the issue also reproduces on .NET 8 or .NET Framework 4.8, it won't be actioned on.

@ltrzesniewski
Copy link
Contributor Author

Hmm actually I didn't realize it's the source generator from .NET 6 which gets executed during the build:

Image

So I guess you're right: I missed the deadline by a few days... 😬

Anyway I compared v6.0.36 to v9.0.0, the GetUnqualifiedName method is the same, but AddAlias has the following change:

diff --git a/src/libraries/Common/src/Roslyn/CSharpSyntaxHelper.cs b/src/libraries/Common/src/Roslyn/CSharpSyntaxHelper.cs
index f7ab0d494a5..4638d138e24 100644
--- a/src/libraries/Common/src/Roslyn/CSharpSyntaxHelper.cs
+++ b/src/libraries/Common/src/Roslyn/CSharpSyntaxHelper.cs
@@ -93,6 +93,11 @@ private static void AddAliases(SyntaxList<UsingDirectiveSyntax> usings, ref Valu
                 if (global != usingDirective.GlobalKeyword.Kind() is SyntaxKind.GlobalKeyword)
                     continue;

+                // We only care about aliases from one name to another name.  e.g. `using X = A.B.C;`  That's because
+                // the caller is only interested in finding a fully-qualified-metadata-name to an attribute.
+                if (usingDirective.Name is null)
+                    continue;
+
                 var aliasName = usingDirective.Alias.Name.Identifier.ValueText;
                 var symbolName = usingDirective.Name.GetUnqualifiedName().Identifier.ValueText;
                 aliases.Append((aliasName, symbolName));

So I haven't really debugged this, but I suppose GetUnqualifiedName actually received a null name parameter, and we reached the unreachable switch case because of that. 😅

So this is basically fixed by #96048, thanks @CyrusNajmabadi 🙂

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

No branches or pull requests

2 participants