Suppress union ambiguity warning when a global classified is specified at compile time - #131797
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63fffa05-9788-4529-b41f-983227e77428
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/area-system-text-json |
There was a problem hiding this comment.
Pull request overview
Updates the System.Text.Json source generator’s union ambiguity warning logic so that global (options-level) type classifiers are treated as configured classifier support, and adjusts diagnostics tests accordingly.
Changes:
- Treat non-empty
JsonSourceGenerationOptionsAttribute.TypeClassifiersas satisfying union classifier configuration (suppressingSYSLIB1227). - Update the union ambiguity diagnostic unit test to validate no
SYSLIB1227is produced when options-level classifiers are present.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs | Initializes the “classifier configured” flag from options-level TypeClassifiers so union ambiguity diagnostics aren’t emitted when global classifiers are registered. |
| src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs | Updates/renames the relevant diagnostic test to assert SYSLIB1227 is not produced for options-level classifier registration. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63fffa05-9788-4529-b41f-983227e77428
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs:819
- Now that this test expects the generator to produce no diagnostics, leaving
disableDiagnosticValidation: trueweakens the test by skipping compilation+generator severity validation inCompilationHelper.RunJsonSourceGenerator. Consider removing the flag so the helper asserts there are no warnings/errors anywhere, matching other "CompilesWithout..." tests in this file.
Compilation compilation = CompilationHelper.CreateCompilation(source);
JsonSourceGeneratorResult result = CompilationHelper.RunJsonSourceGenerator(compilation, disableDiagnosticValidation: true);
Assert.Empty(result.Diagnostics);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs:991
hasUnionTypeClassifierSpecifiedis now initialized from options-levelTypeClassifiers, but the name can be misread as only reflectingJsonUnionAttribute.TypeClassifier. Adding an inline comment here would make it clear that a non-empty context/options classifier list also opts a union into runtime classification and therefore suppresses ambiguity diagnostics.
bool hasUnionTypeClassifierSpecified = options?.TypeClassifiers is { Count: > 0 };
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs:819
Assert.Empty(result.Diagnostics)makes this test brittle and inconsistent with the neighboring union-ambiguity tests, which only assert that SYSLIB1227 is (not) produced. If any unrelated source-generator diagnostic is added in the future, this test will start failing even though the behavior under test (suppressing SYSLIB1227 when options-level TypeClassifiers are present) still works.
Assert.Empty(result.Diagnostics);
|
/ba-g test failures are unrelated |
Options-level type classifiers are emitted into
JsonSerializerOptions.TypeClassifiers, but the source generator's union ambiguity diagnostic only checkedJsonUnionAttribute.TypeClassifier. Treat a non-empty validatedJsonSourceGenerationOptionsAttribute.TypeClassifierslist as configured classifier support soSYSLIB1227isn't emitted in that case.The existing source-generator diagnostic tests now cover both per-union and options-level classifier registration.
Validated with
build.cmd clr+libs -rc releaseand all System.Text.Json test projects (130,518 executions, 0 failures; 17 existing skips).Note
This pull request description was generated by GitHub Copilot.