-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use .NET source generators to generate strongly-typed overloads #2015
Use .NET source generators to generate strongly-typed overloads #2015
Conversation
Hey, @thomaslevesque. For your consideration. Of course there are more classes to convert, but I thought I'd give you a peek before I invested more hours. I am content with the changes relative using T4, but am also open to leaving T4 in the mix and/or exploring a proper templating solution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks @blairconrad!
Two remarks:
- I see you used the original source generator model, rather than the newer "incremental" generators. In this case, I think it doesn't matter, because you don't have any syntax receiver that would trigger a regeneration on every keystroke.
- I don't see the generated code in your PR. By default it's generated in the
obj
folder, which is excluded from source control. If we want to include the generated code in source control, we can specify the path where it's emitted withCompilerGeneratedFilesOutputPath
.
tools/FakeItEasy.SourceGenerators/StronglyTypedExtensionsGenerator.cs
Outdated
Show resolved
Hide resolved
tools/FakeItEasy.SourceGenerators/FakeItEasy.SourceGenerators.csproj
Outdated
Show resolved
Hide resolved
tools/FakeItEasy.SourceGenerators/StronglyTypedExtensionsGenerator.cs
Outdated
Show resolved
Hide resolved
tools/FakeItEasy.SourceGenerators/StronglyTypedExtensionsGenerator.cs
Outdated
Show resolved
Hide resolved
Ah! I can look at the incremental one. I just used the first guidance I found.
I will explore this! |
e620b14
to
7e097ee
Compare
Sorry @thomaslevesque. I rebased. I even put some commits in before the originals. |
src/FakeItEasy.Extensions.ValueTask/FakeItEasy.Extensions.ValueTask.csproj
Outdated
Show resolved
Hide resolved
...nConfigurationExtensionsGenerator/ArgumentValidationConfigurationExtensions.StronglyTyped.cs
Outdated
Show resolved
Hide resolved
...lueConfigurationExtensionsGenerator/AsyncReturnValueConfigurationExtensions.StronglyTyped.cs
Outdated
Show resolved
Hide resolved
...en.CallbackConfigurationExtensionsGenerator/CallbackConfigurationExtensions.StronglyTyped.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @blairconrad! Looks good in general. I do have a few comments.
To be honest, after the first 2 "Use source generator" commits, I only gave the next ones a very cursory review, but I like the approach you took.
src/FakeItEasy/ArgumentValidationConfigurationExtensions.StronglyTyped.cs
Outdated
Show resolved
Hide resolved
src/FakeItEasy.Extensions.ValueTask/FakeItEasy.Extensions.ValueTask.csproj
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @blairconrad, looks good to me! Just squash the fixups, and I'll merge
Generated source files don't pay attention to the .editorconfig file, as described in [.editorconfig diagnostic suppressions don't apply to source-generated files](dotnet/roslyn#47384), but they do respect a .globalconfig file.
If we add a new project in a surprise subdirectory, as we recently did for recipes, and will soon add a source generator to tools, then we won't have to manually suppress the warning.
…ons.StronglyTyped.cs
…s.StronglyTyped.cs
ValueTask-related overloads (coming soon) will not always apply
…Extensions.StronglyTyped.cs
…sions.StronglyTyped.cs
f4fb043
to
f1ecb3f
Compare
@thomaslevesque, I squashed. While I was there I dropped c0e6a1c "Define constant LACKS_VALUETASK to exclude ValueTask-related functionality", as we discussed. With that removal, 445e88c "Use pre-.NET Standard 2.1 framework detection for FakeItEasy" didn't do anything for us, so I dropped that commit as well. While I was at it, I distributed 472c236 "Use source generator to create strongly-typed overload for FakeItEasy.Extensions.ValueTask" amongst 3 other commits:
I think this clears stuff up. Thanks for the great suggestions. I had a ball working on this PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks!
Thank you, @thomaslevesque! |
Closes #1804.