dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

[Bug]: Resource generation with DependentUpon doesn't support record types #8936

Open Sebazzz opened 1 year ago

Sebazzz commented 1 year ago

Issue Description

We use CQRS in our applications and decided to place the handlers together with their requests and validators in the same file. Our resources are "grouped" per request using <EmbeddedResource Include="Request.resx" DependentUpon="Request.cs"/>.

However just moving one class from one source file to another, this caused an unexpected bug in our application by which certain resources could no longer be found. It turned out the resources got a different name in some cases.

Steps to Reproduce

Have the following resource files (contents don't matter):

Project file MyProject.csproj:

<Project Sdk="Microsoft.NET.Sdk">
   <EmbeddedResource Update="RequestClass.resx"><DependentUpon>RequestClass.cs</DependentUpon></EmbeddedResource>
   <EmbeddedResource Update="RequestClass.nl.resx"><DependentUpon>RequestClass.cs</DependentUpon></EmbeddedResource>
   <EmbeddedResource Update="RequestRecord.resx"><DependentUpon>RequestRecord.cs</DependentUpon></EmbeddedResource>
   <EmbeddedResource Update="RequestRecord.nl.resx"><DependentUpon>RequestRecord.cs</DependentUpon></EmbeddedResource>
</Project>

RequestClass.cs:

public sealed class RequestClass { }

public sealed class RequestClassValidator { }

RequestRecord.cs:

public sealed record RequestRecord();

public sealed class RequestRecordValidator { }

Build the project (dotnet build) and open de binary in a decompiler like JetBrains dotPeek and open the listing of the resource files.

Expected Behavior

Listed resource files:

Actual Behavior

Listed resource files:

Analysis

The task CreateCSharpManifestResourceName parses a part of the source code in the source file to extract the class name. When it fails to do so, it falls back to the file name of the source class.

https://github.com/dotnet/msbuild/blob/227092b45a18b5892638d87fe4bc1b07ac4f8d96/src/Tasks/CreateCSharpManifestResourceName.cs#L112-L134

Previously it worked fine by accident: It didn't recognize any class (because it didn't parse records) and fell back to the file name. However, now we've moved the related classes together in the same file it found the first class in the file (RequestRecordValidator) and decided that should be the manifest resource name.

Workarounds

Versions & Configurations

No response

AR-May commented 1 year ago

Team triage: Thank you for the filing the bug and for the great analysis! This issue currently doesn't meet bar for fixing, but we are leaving this open for upvotes.