Migrate to IIncrementalSourceGenerator

[ProphetLamb]

Migrate to IIncrementalSourceGenerator

Synopsis

.NET source generation using the ISourceGenerator interface is officially deprecated ¹².

This PR aims to migrate dotvariant to the IIncrementalSourceGenerator ecosystem.

Subject

Migration of the implementation and test environment.

• Migrate SourceGenerator implementation
  • Modify source generation process
  • Source generator compiles
  • Do not pass Compilation into cache, ever
  • Add more Diagnostics to CreateSyntaxProvider.transform instead of return default.
• Migrate Test implementation
  • Modify test case fixtures
  • Test cases compile
  • Test cases generate sources
  • Successful: dotVariant.Generator.Test
  • Successful: dotVariant.Test-net45
  • Successful: dotVariant.Test-net5.0
  • Resolve hack used to obtain RenderInfos from stateless IIncrementalGenerator when debugging.

Postscriptum

I've been using this lovely package for a while now. So I took some time this beautiful afternoon to do a good chunk of the dirty work required to keep this project up to date.
@mknejp Any and all help, in the form of reviews, or with the remaining open points is much appreciated.

Footnotes

1. https://github.com/dotnet/roslyn-analyzers/pull/6478 ↩

2. https://github.com/dotnet/roslyn/blob/main/docs/features/source-generators.cookbook.md ↩

[mknejp]

Thank you very much for tackling this. It might take a few days before I am able to do a review.

[ProphetLamb]

Alighty, thanks:)

[ProphetLamb]

@mknejp Is there any chance you could review the PR?
If your personal life doesn't allow you the time, that's fine. In that case we can either check, if you can add me as a maintainer, or I can publish my fork separately.

Best regards
Prophet

[mknejp]

Sorry it took me so long to take a look at this. I appreciate your patience.

Regarding your open points:

Add more Diagnostics to CreateSyntaxProvider.transform instead of return default.

What kind of diagnostics are you referring to? Do you mean the ones in CreateSyntaxProvider? Would these errors not indicate that either the predicate is wrong, or that there's a bug in Roslyn?

Resolve hack used to obtain RenderInfos from stateless IIncrementalGenerator when debugging.

I think the only "clean" way to resolve this (technically the previous solution wasn't "clean" either) would be have a static function that builds the transformation graph and then trigger it manually in the tests. However I have no idea how feasible this is so I'm willing to accept the current "hack". However I would sleep better if it was a thread-safe list as I have no idea how the generator transformations are threaded.

I also think it would be nicer to move some of the larger lambda expressions in Initialize to dedicated named functions to make the structure of the transformers stand out more. Do you agree?

Lastly, we need to find a solution for the copyright headers. You used my name in the new files, which I assume is a consequence of the .editorconfig file, but that is technically wrong. I also don't want to erase your effort. This is definitely a priority.

[ProphetLamb]

Add more Diagnostics to CreateSyntaxProvider.transform instead of return default.

What kind of diagnostics are you referring to? Do you mean the ones in CreateSyntaxProvider? Would these errors not indicate that either the predicate is wrong, or that there's a bug in Roslyn?

Generally I think a fatal error should be produced, instead of no output. For example, the user imports the package into F#.

Resolve hack used to obtain RenderInfos from stateless IIncrementalGenerator when debugging.

I think the only "clean" way to resolve this (technically the previous solution wasn't "clean" either) would be have a static function that builds the transformation graph and then trigger it manually in the tests. However I have no idea how feasible this is so I'm willing to accept the current "hack". However I would sleep better if it was a thread-safe list as I have no idea how the generator transformations are threaded.

I think this is best solved this using a static class annotated with debug only using a Monitor on a list. This way the SourceGenerator code is not contaminated. See #15c3c76

I also think it would be nicer to move some of the larger lambda expressions in Initialize to dedicated named functions to make the structure of the transformers stand out more. Do you agree?

Agreed. Done.

Lastly, we need to find a solution for the copyright headers. You used my name in the new files, which I assume is a consequence of the .editorconfig file, but that is technically wrong. I also don't want to erase your effort. This is definitely a priority.

Since .editorconfig does not support per file headers, I would go for the 1970s way of adding contributes to the bottom of the copyright headers used. Do you have any better ideas?

[mknejp]

Since .editorconfig does not support per file headers, I would go for the 1970s way of adding contributes to the bottom of the copyright headers used. Do you have any better ideas?

I was thinking of removing all the names from source files (including mine) and just keeping it in the git history. I would create a PR to change the existing files over and you can then rebase yours on top, if that is an acceptabl solution for you.

[mknejp]

I personally hate these giant license comments in files that just keep growing and growing.

[mknejp]

I had to move the project to net6.0 to make the CI work again and took the liberty of taking along your package updates as it otherwise wouldn't build anymore.

[ProphetLamb]

If this is such a critical property of the struct then please add a test to enshrine it for future contributors. This is something I can easily see someone trying to "cleanup".

I added a separate test project for isolated tests of internal functionality.

[ProphetLamb]

Since .editorconfig does not support per file headers, I would go for the 1970s way of adding contributes to the bottom of the copyright headers used. Do you have any better ideas?

I was thinking of removing all the names from source files (including mine) and just keeping it in the git history. I would create a PR to change the existing files over and you can then rebase yours on top, if that is an acceptabl solution for you.

Agreed. I will manually update files added by this PR once the rebase is incoming.

[mknejp]

I added a separate test project for isolated tests of internal functionality.

I don't think we need a separate test project. dotVariant.Generator.Test is intended to be the test project for the internals of dotVariant.Generator (as it doesn't reference the generator as Anazlyer).

The other test projects ensure that the generator can be used externally as Analyzer, by either <ProjectReference> or <PackageReference>.

Agreed. I will manually update files added by this PR once the rebase is incoming.

This work is now done. The file headers are adjusted and you can add yourself to the /AUTHORS.txt file.