Add fine-grained disabling of SourceGenerators #71049
Conversation
|
Hmm I suspect the |
alexdlm
force-pushed
the
dotnet-generator-selective-disable
branch
from
4adeead to
560ba84
Compare
This allows manual testing and/or alternate source generators to provide functionality without conflict.
alexdlm
force-pushed
the
dotnet-generator-selective-disable
branch
from
560ba84 to
8ab3295
Compare
|
8ab3295 rebased to current master, fixed merged conflict in |
| public static bool IsGodotSourceGeneratorDisabled(this GeneratorExecutionContext context, string generatorName) => | ||
| AreGodotSourceGeneratorsDisabled(context) || | ||
| (context.TryGetGlobalAnalyzerProperty("GodotDisabledSourceGenerators", out string? disabledGenerators) && | ||
| disabledGenerators != null && |
There was a problem hiding this comment.
Can the string be empty, or would it return null in that case? If it can be empty, we should use IsNullOrEmpty.
There was a problem hiding this comment.
I think it could be empty, but the split below would return an empty array in that case so it's not strictly necessary.
I tried it, however !string.IsNullOrEmpty(disabledGenerators) strangely doesn't seem to pass the non-nullablility through to the following line so I'm inclined to leave it as is. It seems like there are other nullability warnings in this project already though so I'm happy to push it if you prefer @neikeq ?
There was a problem hiding this comment.
The string.IsNullOrEmpty method is not annotated for nullability in netstandard2.0 which is the TFM that source generators must target.
There was a problem hiding this comment.
I believe checking if it's empty early should be faster than going through the Split and Contains calls, at the very least because those are not inlined.
I don't remember how we deal with this nullability issue elsewhere. We probably check for null and Length manually.
There was a problem hiding this comment.
What's the status here? I can't tell from this discussion if the PR should be merged or should be worked on further.
There was a problem hiding this comment.
This discussion is about saving one allocation and maybe 100 instructions executed in a code path that is executed exactly 7 times for each build of the csharp user project (saving like a microsecond on an action usually taking a few seconds) so it doesn't really matter all that much.
There was a problem hiding this comment.
I approved the PR, this requested change is optional.
(...) a code path that is executed exactly 7 times for each build of the csharp user project (...) so it doesn't really matter all that much.
Source generators are also run very frequently by IDEs while editing code, so performance is very important. Still, just nitpicking in this case.
|
Thanks! |
This allows alternate source generators to provide similar functionality without conflict. Primarily I would like to disable the
ScriptMethodsgenerator to replace it with my own, but figured it made sense to extend to all generators.Example: Add a
<GodotDisabledSourceGenerators>ScriptMethods;ScriptSerialization</GodotDisableSourceGenerators>property to your csproj to disable those two generators, but leave the rest functional.