[release/6.0] Emit diagnostics & exceptions for sourcegen handling init-only properties & JsonInclude attributes

[github-actions]

Backport of #58993 to release/6.0

/cc @eiriktsarpalis

Customer Impact

Customer reported #58770

Source generation mode currently does not support deserializing classes with init-only properties. Furthermore, it cannot support private properties that have been marked with the JsonInclude attribute. Current behavior is to silently ignore these conditions and not deserialize the impacted properties at all. This PR

1. Makes the source generator emit a compile-time diagnostic warning and also
2. Changes the runtime behavior so an InvalidOperationException is thrown.

Moreover it is making a related improvement in which source generated exception messages have been moved to the project's resource strings. #58292

Testing

Testing has been added validating both the compile-time diagnostic messages and the runtime exceptions.

Risk

Low. Relatively straightforward changes to product code to account to account for init-only property and JsonInclude attribute detection.

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Backport of #58993 to release/6.0

/cc @eiriktsarpalis

Customer Impact

Testing

Risk

Author:	github-actions[bot]
Assignees:	-
Labels:	area-System.Text.Json
Milestone:	-

#59097 (comment) should be addressed.

[steveharter]

@eiriktsarpalis do you want to get into main first, then cherry-pick commit to this PR?

[eiriktsarpalis]

@steveharter I've already cherry picked the changes in #59104

[danmoseley]

@MattGal can you remind me of the telltale signature of the issue you added retries for? is this one?

❌[Log] .dotnet\sdk\6.0.100-rc.1.21411.28\NuGet.RestoreEx.targets(19,5): error : Failed to retrieve information about 'optimization.windows_nt-x86.PGO.CoreCLR' from remote source 'https://pkgs.dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/\_packaging/c9f8ac11-6bd8-4926-8306-f075241547f7/nuget/v3/flat2/optimization.windows\_nt-x86.pgo.coreclr/index.json'.
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.FindPackagesByIdAsync(String id, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.EnsurePackagesAsync(String id, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.DoesPackageExistAsync(String id, NuGetVersion version, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Commands.SourceRepositoryDependencyProvider.FindLibraryCoreAsync(LibraryRange libraryRange, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Commands.SourceRepositoryDependencyProvider.<>c__DisplayClass19_0.<<FindLibraryAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
...

[danmoseley]

@lewing the JSON "failure" in net6.0-Browser-Release-wasm-Mono_Release-normal-Ubuntu.1804.Amd64.Open is another one where there seems to be no relevant info. What should we do in case of failures like this?

[danmoseley]

Edited template to add customer reported (which helps us make decisions) and link.

[danmoseley]

Approved. Fixes customer reported blocker in new generator feature. Changes are entirely limited to generator.

[danmoseley]

Restarting to pick up fix for #59136

[eiriktsarpalis]

We seem to have merge conflicts, I'll see if I can rebase on top of the latest release/6.0

[MattGal]

@MattGal can you remind me of the telltale signature of the issue you added retries for? is this one?

❌[Log] .dotnet\sdk\6.0.100-rc.1.21411.28\NuGet.RestoreEx.targets(19,5): error : Failed to retrieve information about 'optimization.windows_nt-x86.PGO.CoreCLR' from remote source 'https://pkgs.dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/\_packaging/c9f8ac11-6bd8-4926-8306-f075241547f7/nuget/v3/flat2/optimization.windows\_nt-x86.pgo.coreclr/index.json'.
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.FindPackagesByIdAsync(String id, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.EnsurePackagesAsync(String id, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Protocol.HttpFileSystemBasedFindPackageByIdResource.DoesPackageExistAsync(String id, NuGetVersion version, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Commands.SourceRepositoryDependencyProvider.FindLibraryCoreAsync(LibraryRange libraryRange, SourceCacheContext cacheContext, ILogger logger, CancellationToken cancellationToken)
at NuGet.Commands.SourceRepositoryDependencyProvider.<>c__DisplayClass19_0.<<FindLibraryAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
...

Yes, this looks like it's cut off from the specific error message but otherwise it looks like that problem and an 8/11 SDK would not have the change (the first, IIRC, was 6.0.100-rc.1.21430.12), from 8/30. Eventually I'd assume the 6.0 branch will have newer 6.0 SDK, but it may need also need an arcade update or a cherry pick of where I initially set the variables directly in runtime

[danmoseley]

Yes, this looks like it's cut off from the specific error message but otherwise it looks like that problem and an 8/11 SDK would not have the change (the first, IIRC, was 6.0.100-rc.1.21430.12), from 8/30. Eventually I'd assume the 6.0 branch will have newer 6.0 SDK, but it may need also need an arcade update or a cherry pick of where I initially set the variables directly in runtime

@ViktorHofer thoughts about picking up Matt's fix in release/6.0? It seems proven now in main and we'll be servicing out of 6.0 for years.

[ViktorHofer]

@ViktorHofer thoughts about picking up Matt's fix in release/6.0? It seems proven now in main and we'll be servicing out of 6.0 for years.

Absolutely. e72aafc was merged earlier today which includes Matt's fix.

[MattGal]

@ViktorHofer thoughts about picking up Matt's fix in release/6.0? It seems proven now in main and we'll be servicing out of 6.0 for years.

Absolutely. e72aafc was merged earlier today which includes Matt's fix.

Was typing similar. I think no more actions needed though once you have arcade updates you could remove the variables from your common .yml place (that was for before Arcade was updated)

[danmoseley]

Oh great, I just missed it here then.