New generator apis

[chsienki]

Adds APIs to convert ISourceGenerators to IIncrementalGenerators and allows the generator driver to only update a sub-set of generators via a new filter parameter.

Fixes #74039
Fixes #74086

[chsienki]

I couldn't figure out a way to make generatorFilter non-optional (as discussed in the API review) without breaking our back compat guidelines.

Another option would be to just make this a differently named method, like RunGeneratorsWithFilter

[jjonescz]

I don't see the problem - what guidelines would be broken?

[chsienki]

Our analyzer requires that we don't have multiple overloads with optional parameters: https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md

That means that the signatures when we make generatorFilter required are:

RunGenerators(Compilation, CancellationToken)
RunGenerators(Compilation, Func<> filter, CancellationToken = default)

Anyone who was calling RunGenerators(Compilation) will now be source broken, because we no longer have an overload with only a single required parameter.

Note: It also says that we shouldn't add new optional parameters in the middle of a signature for source-compat. Imagine if you had RunGenerator(compilation, default) that might bind to a different overload now. So we're in a bit of tricky situation.

We can either: make the filter optional, and bend the 'don't put optional in the middle rule', or we can make it non-optional, and introduce a third overload of RunGenerators that only takes a Compilation parameter.

Tagging @333fred for any API shape thoughts on this one?

[jjonescz]

we can make it non-optional, and introduce a third overload of RunGenerators that only takes a Compilation parameter.

This sounds like the best option to me.

[jaredpar]

What is the problem with the signature in the current form? Yes it's optional but it just gets filled in as essentially "run everything". That behavior is compatible with the existing overload so that all seems to track for me. What am I missing?

[chsienki]

shrug I'm just following the guidelines here https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md where it says do not put optional params in the middle of signatures.

I don't see in this case why it would be an issue, as even in source RunGenerators(compilation, default) should still bind to the cancellation token version, but I assume the guidelines exist for a reason and don't want to break them :)

[chsienki]

@dotnet/roslyn-compiler for review please.

[jaredpar]

@jjonescz PTAL

[jjonescz]

I don't see the problem - what guidelines would be broken?

[jaredpar]

Is this a value that is controllable by customers? If so have reservations about using a magic string. Had trouble tracking this through all the APIs to see where it originates.

[chsienki]

No, it's only created internally. We track the file extension because we actively parse the tree, so we need to pass it in from the outer driver when we wrap an ISourceGenerator.

The generators on this path are never actually used directly, so we can just set it to anything. I chose a magic string so we can assert later on that it isn't used and dings us if we ever do allow it, so we know to fix it.

At some point we should probably refactor these so the string isn't needed at all, but this just seemed simpler for now.

[jaredpar]

What is the problem with the signature in the current form? Yes it's optional but it just gets filled in as essentially "run everything". That behavior is compatible with the existing overload so that all seems to track for me. What am I missing?

[chsienki]

Helps if I actually push my changes 🤦