[release/8.0] Fix STJ SG regression in handling of property names that are reserved keywords. #98082
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsBackport of #98058 to release/8.0-staging Customer ImpactFixes a customer reported issue that was a regression from .NET 7. A change during .NET 8 development made it so that running the STJ source generator against properties that use reserved C# keywords as identifiers results in uncompilable code. The same inputs compile and run as expected using the .NET 7 source generator. TestingAdded a regression test covering the scenario in question. RiskLow. Makes a small and targeted fix to product code.
|
eiriktsarpalis
added
Servicing-consider
|
Do you need package authoring? |
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsBackport of #98058 to release/8.0-staging Customer ImpactFixes a customer reported issue that was a regression from .NET 7. A change during .NET 8 development made it so that running the STJ source generator against properties that use reserved C# keywords as identifiers results in uncompilable code. The same inputs compile and run as expected using the .NET 7 source generator. TestingAdded a regression test covering the scenario in question. RiskLow. Makes a small and targeted fix to product code.
|
I saw this question after approving, @eiriktsarpalis. I think @tarekgh is right that we need package authoring updates. Example from a previous backport: https://github.com/dotnet/runtime/pull/94882/files#diff-4ea579c158e463560c7b8e18d2c256f1274dc62db33a4564fc8906dcca7b90bb |
|
Ah, never mind, @eiriktsarpalis. I see that the runtime/src/libraries/System.Text.Json/src/System.Text.Json.csproj Lines 11 to 12 in 7e7dbd4 |
|
Last month, we shipped a fix for S.T.J.: #95325 So the fact that GeneratePackageOnBuild is currently true is expected. I manually turn off all the OOB packages property on Code Complete day to prevent widespread build failures in PRs, unless we are shipping new bug fixes this month, in which case the GeneratePackageOnBuild property stays as true. If we are shipping more than one bug fix in the same OOB assembly in the same month, then we only bump the ServicingVersion once for both fixes. In other words, the first PR bumps the version and sets GeneratePackageOnBuild to true (if needed), and the second PR does nothing to these properties. Hope that clarifies it. |
|
The PR that last incremented the servicing version is this one #96669 which got merged on January 11. Am I right to understand that this PR will be batched with that earlier fix or does the servicing version need to be incremented again? |
There was a problem hiding this comment.
I see the ServicingVersion bump to 3, LGTM.
Thanks for sharing the source of confusion on a call, @eiriktsarpalis. I now see that the current process can beconfusing for folks modifying an OOB package, since the GeneratePackageOnBuild property does not get reset until the Code Complete date of the next month, which could make you think its ServicingVersion has already been bumped on that release month. We need better guidance for that.
eiriktsarpalis
added
Servicing-approved
|
Approved over email. |
eiriktsarpalis
merged commit c2415d7
into
dotnet:release/8.0-staging
Backport of #98058 to release/8.0-staging
Customer Impact
Fixes a customer reported issue that was a regression from .NET 7.
A change during .NET 8 development made it so that running the STJ source generator against properties that use reserved C# keywords as identifiers results in uncompilable code. The same inputs compile and run as expected using the .NET 7 source generator.
Testing
Added a regression test covering the scenario in question.
Risk
Low. Makes a small and targeted fix to product code.