Options validation source generator

[tarekgh]

No description provided.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	tarekgh
Assignees:	tarekgh
Labels:	new-api-needs-documentation, area-Extensions-Options
Milestone:	-

[jeffhandley]

I was just skimming to get an overview, and I saw a couple cosmetics I left comments on.

[eiriktsarpalis]

I would strongly recommend not taking a dependency on this pattern, since it makes it impractical for the source generator to take advantage of incremental caching. The general pattern is to extract data as early as possible from the compilation before applying it to a SourceProductionContext. You can find more details in the Roslyn guide as well as see real-world examples in runtime here:

1. JsonSourceGenerator
2. RegexGenerator
3. LibraryImportGenerator

cc @chsienki

[tarekgh]

@eiriktsarpalis could you please have a look at the last commit? Finally, I got it work using ForAttributeWithMetadataName.

[tarekgh]

CC @geeknoid

[eiriktsarpalis]

Instead of List, consider using a collection type with sequence equality so that your model has the desired equality semantics. Here's one from the repo that you can use:

https://github.com/dotnet/runtime/blob/be6a1389c852fdc96a919769867fb3f3ddb0fbde/src/libraries/System.Text.Json/gen/Helpers/ImmutableEquatableArray.cs

You could try moving the class to the common folder so that it's shared between the two projects.

[eiriktsarpalis]

Likewise here https://github.com/dotnet/runtime/pull/87587/files#r1231933555

[eiriktsarpalis]

As a next step, you might want to move the parser logic inside an IncrementalValuesProvider.Select declaration so that you end up with an IncrementalValuesProvider<ValidatorType>.

To achieve this, the Parser class needs to be updated so that it just stores Diagnostic values, to be reported by the SourceProductionContext later:

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Lines 47 to 57 in be6a138

	public List<DiagnosticInfo> Diagnostics { get; } = new();
	public void ReportDiagnostic(DiagnosticDescriptor descriptor, Location? location, params object?[]? messageArgs)
	{
	Diagnostics.Add(new DiagnosticInfo
	{
	Descriptor = descriptor,
	Location = location.GetTrimmedLocation(),
	MessageArgs = messageArgs ?? Array.Empty<object?>(),
	});
	}

[eiriktsarpalis]

Consider adding a separate unit test project on top of the existing source generator tests project which lets you test the source generator in isolation like so:

runtime/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs

Lines 631 to 662 in 36439c5

	[Fact]
	public void EquivalentTupleDeclarations_DoNotConflict()
	{
	string source = """
	using System.Text.Json.Serialization;
	#nullable enable
	namespace HelloWorld
	{
	[JsonSerializable(typeof((string? Label1, string Label2, int Integer)))]
	[JsonSerializable(typeof((string, string, int)))]
	internal partial class JsonContext : JsonSerializerContext
	{
	}
	}
	""";
	Compilation compilation = CompilationHelper.CreateCompilation(source);
	JsonSourceGeneratorResult result = CompilationHelper.RunJsonSourceGenerator(compilation);
	// Make sure compilation was successful.
	Assert.Empty(result.Diagnostics);
	Assert.Empty(result.NewCompilation.GetDiagnostics());
	// Should find the generated type.
	Assert.Equal(3, result.AllGeneratedTypes.Count());
	result.AssertContainsType("(string, string, int)");
	result.AssertContainsType("string");
	result.AssertContainsType("int");
	}

Among other things this lets you test for diagnostic warnings and incremental compilation properties.

[tarekgh]

@eiriktsarpalis

Consider adding a separate unit test project on top of the existing source generator tests project which lets you test the source generator in isolation

I added that in the commit 54a2f97 (#87587). We can add more stuff there as needed.

[eiriktsarpalis]

Thanks