Log warnings for skipped STR resource keys instead of failing the build#13291
Log warnings for skipped STR resource keys instead of failing the build#13291OvesN wants to merge 11 commits intodotnet:mainfrom
Conversation
…stead of failing the build, ensuring STR files are still generated when possible
|
@dotnet-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Pull request overview
This PR changes the behavior of the GenerateResource task when processing resource keys that cannot be converted to valid C# identifiers in strongly typed resource (STR) files. Instead of failing the entire build with errors, the task now logs warnings and skips problematic keys while still generating the STR file with valid properties. This aligns MSBuild's MSBuild:Compile generator behavior with Visual Studio's ResXFileCodeGenerator, which silently skips invalid keys.
Changes:
- Modified
StronglyTypedResourceBuilderto return structured warning information (key, reason, extraArg) instead of error strings - Added four new warning codes: MSB3827 (reserved name), MSB3828 (invalid identifier), MSB3829 (name collision), MSB3830 (void type)
- Updated
GenerateResource.csto log warnings instead of errors and mark STR generation as successful even when some properties are skipped - Added comprehensive unit tests covering reserved names, invalid identifiers, name collisions, and the no-warnings case
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Tasks/system.design/stronglytypedresourcebuilder.cs | Changed error tracking from string array to tuple array with structured warning data (key, reason, extraArg); updated all error-adding sites to include reason codes |
| src/Tasks/GenerateResource.cs | Changed from error logging to warning logging for STR property generation issues; task now succeeds even with warnings |
| src/Tasks/Resources/Strings.resx | Added four new warning message strings: MSB3827, MSB3828, MSB3829, MSB3830 |
| src/Tasks/Resources/xlf/*.xlf | Added localization placeholders (marked as "new") for the four new warning messages across all language files |
| src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs | Added four comprehensive tests covering reserved names, invalid identifiers, name collisions, and the no-warnings case |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The input dictionary uses StringComparer.InvariantCultureIgnoreCase, so pure case-insensitive collisions (e.g. "MyKey" vs "mykey") cannot exist in the input.
JanProvaznik
left a comment
JanProvaznik
left a comment
There was a problem hiding this comment.
I would suggest partitioning the logic so that the builder tries to create the file and notes down what it had to skip to do that and for what reason (with enum for reasons), and then the Task decides how to log that information.
Architecture fixes - StronglyTypedResourceBuilder now produces list of records that represents warning, logic to choose resource key for message loggind moved to GenFixes after code review. StronglyTypedResourceBuilder now produces a list of records representing warnings; the logic to choose the resource key for message logging has been moved to GenerateResource. Added a HashSet to optimize searching for already-warned keys.erateResource. Added hashset to optimize sraching of already warned keys
JanProvaznik
left a comment
JanProvaznik
left a comment
There was a problem hiding this comment.
looking better, still some code quality comments and a logical oversight, your extra else branch was actually correct
Changed naming from "warning" to "skipResource". Converted reservedNames to const. Changed extraArg from nullable string to list of strings.
…rce to the warning
…//github.com/OvesN/msbuild into dev/veronikao/dev/str-skip-property-warnings
Fixes #13061
Context
The
GenerateResourcetask withMSBuild:Compilefailed the entire build when a resource key (e.g. containing = or `) could not be converted to a valid C# identifier. In contrast,ResXFileCodeGeneratorsilently skipped such entries. This aligns the behavior by skipping invalid keys with warnings instead of failing.Changes Made
Testing
Added unit tests verifying that invalid resource keys emit the expected warnings and valid keys still generate STR properties.
Notes
ResXFileCodeGeneratorin VS silently skips invalid keys with no diagnostic. A separate VS issue has been created to add warnings there as well.