Add docs for DisableFrameworkReferenceAnalyzers

[ericstj]

Summary

Add docs for property added in dotnet/sdk#18501.

Design doc: https://github.com/dotnet/designs/blob/main/accepted/2021/InboxSourceGenerators.md

[dsplaisted]

This makes me question, should we really have two different properties for this? If someone set EnableNETAnalyzers to false, wouldn't they expect not to get the FrameworkReference-based analyzers.

[ericstj]

I was thinking we should have a separate property since these are different in the source component and the reasons for disabling them might be different (eg: source build on old frameworks).

[dsplaisted]

To me it's pretty much an implementation detail that we decided to put analyzers in FrameworkReferences. Most developers don't even need to know what FrameworkReferences are, or the fact that we have implicit ones for the base Framework.

[ericstj]

@jmarolf what is your thinking about this? Does it make sense to have different properties?

[jmarolf]

1. I think we want to keep the Enable/Disable language consistent here. If we do have both I do not think it makes sense to have EnableNETAnalyzers and DisableFrameworkReferenceAnalyzers. It should either be DisableNETAnalyzers and DisableFrameworkReferenceAnalyzers or EnableNETAnalyzers and EnableFrameworkReferenceAnalyzers
2. I need to think about these cases. For this case what is the user trying to do? They are trying to disable framework analyzers but keep other ones? Why?

<DisableFrameworkReferenceAnalyzers>true</DisableFrameworkReferenceAnalyzers>
<EnableNETAnalyzers>true</EnableNETAnalyzers>

alternatively, what is happening here?

<DisableFrameworkReferenceAnalyzers>false</DisableFrameworkReferenceAnalyzers>
<EnableNETAnalyzers>false</EnableNETAnalyzers>

I agree with @dsplaisted that we should not add configuration complexity just to add it. It must enable a specific scenario and that scenario must happen "in the wild" for at least one customer. As it stands I cannot think of a situation this applies to and am learning towards just having one property. Alternatively, we could have both, but I do not think we need to document both.

[jmarolf]

How is the user going to be able to determine that it is a source generator coming from a framework reference? If the build is broken by turning off framework reference generators then its not less broken by turning off all analyzers.

[fitdev]

@jmarolf Just wanted to clarify that our case with dotnet/runtime#56702 had to do with a specific framework SG, which we were not depending on or using at all in our code. Just because there is a problematic SG, especially the one that's not being relied upon by user code, it does not make sense to turn of all SGs, let alone all analyzers. Ideally a user should have granular control over which SGs are enabled on a per-project basis, without resorting to targets trick, perhaps with the super option (in IDE maybe) to disable all SGs.

[jmarolf]

Right, that makes sense. But that is not what this property does. It disables all source generators coming as framework references. Still do not understand the user case of disabling all framework source generators but not all source generators. If you are trying to disable source generators for your build you are doing build surgery and frankly, really need to know what you are doing. My position is that adding this property does not make this daunting task simpler or easier.

[ericstj]

All right, I think I'm in agreement. This property provides limited value, and might even be worse than that. Potentially there is an opportunity for another feature here, which is to provide a hook to disable certain / all source generators. Let's discuss that as a new SDK issue. I will close this PR and remove the property from the SDK.

[ericstj]

Just to follow up on a few questions.

It looks like a problem with the specific SG (is it trying to do too much?).

It's hot path is too costly, so large SLNs pay that hot path on a lot of source and it becomes too expensive. I think we might have a similar problem in multiple source generators added this release.

How is the user going to be able to determine that it is a source generator coming from a framework reference

I think we need some source generator diagnostics like https://github.com/dotnet/roslyn/blob/315c2e149ba7889b0937d872274c33fcbfe9af5f/docs/analyzers/Report%20Analyzer%20Format.md to help folks realize what's going on. We're going to talk to roslyn folks about this.