Recognize C# record as a type in CreateCSharpManifestResourceName#13870
Recognize C# record as a type in CreateCSharpManifestResourceName#13870Copilot wants to merge 3 commits into
record as a type in CreateCSharpManifestResourceName#13870Conversation
|
Hello @copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/5094be19-c00d-4a83-84cb-a703a31a7db8 Co-authored-by: OvesN <150850103+OvesN@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/5094be19-c00d-4a83-84cb-a703a31a7db8 Co-authored-by: OvesN <150850103+OvesN@users.noreply.github.com>
record as a type in CreateCSharpManifestResourceName
| } | ||
| } | ||
| else if (t.InnerText == "class") | ||
| else if (t.InnerText == "class" || t.InnerText == "record") |
There was a problem hiding this comment.
hm I am a bit concerned what happens when people use record struct and record class, how is this tokenized in roslyn? 🤔 . But this PR improves on the current state where we don't recognize records at all...
There was a problem hiding this comment.
At the end, also "enum", "structs" and interfaces are not recognized when it comes to resource name generation. Not sure if this method is the right place to find the type name regarding all possible types or if it will break other code.
Context
CreateCSharpManifestResourceNamederives the manifest resource name from the first type declared in the adjacent.csfile. Its parser only triggered on theclasskeyword, andrecord(a C# 9 contextual keyword) was tokenized as a plain identifier — so files whose first top-level type is arecordproduced wrong or missing resource names, breaking embedded resource lookup and satellite-assembly localization.Changes Made
src/Shared/LanguageParser/CSharptokenEnumerator.cs— add"record"tos_keywordListso it tokenizes as aKeywordToken.src/Tasks/CSharpParserUtilities.cs— inExtract, treatrecordthe same asclassto start resolving the type name.record class Fooalso works because the subsequentclasskeyword resets and re-triggers resolution.src/Tasks.UnitTests/CSharpParserUtilitites_Tests.cs— newRecordTypeResolutiontheory covering positional records, file-scoped namespaces,record class,sealed record, and record-before-class ordering.Testing
Existing
CreateCSharpManifestResourceName_Tests(33) still pass. New theory cases plus a reflection-based driver confirm the parser now returns the expected qualified name (e.g.MyNs.Person) for the scenarios above while leaving plainclassbehavior untouched.Notes
recordis contextual in C#, so promoting it to a reserved keyword in this lexer could mis-tokenize it when used as an identifier (e.g.var record = ...;). In practice this is benign here:Extractreturns on the first detected type, so anything whererecordappears as an identifier is either inside a body the parser never reaches, or an edge case (top-level statements) that already has no class name to report.