Use AnalyzerConfigFiles instead of AnalyzerConfigDocument

[Youssef1313]

This was recently added in the testing library in dotnet/roslyn-sdk#734.
The main goal of this is to (later) remove the logic of parsing editorconfig files and leave that up to the compiler (dotnet/roslyn-sdk#734 (comment)) This comment is about AdditionalFiles, not AnalyzerConfigDocument.

[Youssef1313]

@sharwell @mavasani With this test-only change, the product code no longer gets the correct value for GetMSBuildPropertyValue.

It looks like GetMSBuildPropertyValue tries to read from AdditionalFiles. Does that indicate a product bug? I'm uncertain how to proceed with working on this.

Edit: The above isn't accurate.
The product code uses a combination of both the manual parsing and the compiler parsing. (only under CODEANALYSIS_V3_OR_BETTER):

            // Local functions.
            ICategorizedAnalyzerConfigOptions ComputeCategorizedAnalyzerConfigOptions()
            {
                var categorizedOptionsFromAdditionalFiles = ComputeCategorizedAnalyzerConfigOptionsFromAdditionalFiles(); /* This is the parsing done manually and don't adhere to the compiler rules */

#if CODEANALYSIS_V3_OR_BETTER
                return AggregateCategorizedAnalyzerConfigOptions.Create(options.AnalyzerConfigOptionsProvider, compilation, categorizedOptionsFromAdditionalFiles); /* This combines both options.AnalyzerConfigOptionsProvider (from the compiler) with the parsing done from additional files */
#else
                return categorizedOptionsFromAdditionalFiles;
#endif
            }

If the tests are run with additional files (which I'm not doing in this PR), they won't require [*.cs] or [*], etc.
If the tests are run without additional files (which I'm doing), they will require both the file section and CODEANALYSIS_V3_OR_BETTER

Edit2: The above might still not be 100% accurate 😕

One thing that isn't clear to me is about MSBuild properties, they seem to be configurable by both csproj and editorconfig (<Property>value</Property> vs build_property.Property = value), this is very confusing. An MSBuild property should have the same value across all syntax trees in the compilation. But configuring it with editorconfig breaks this assumption.

[Youssef1313]

AnalyzerConfigDocument doesn't work with named sections despite it looks like it should parse from the compiler?
This is a point of confusion to me.

While I'm deleting AnalyzerConfigDocument altogether, the behavior of it not working when its value is like "[*]\r\nprop=value" but works when it's "prop=value" is very very strange to me.

I'm either missing something or this may indicate a bug from roslyn side.

[sharwell]

Global config files don't use [*]. They just have is_global=true as the first line followed by the properties themselves.

[codecov]

Codecov Report

Merging #5065 (ffe1ce5) into main (90660a6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##             main    #5065    +/-   ##
========================================
  Coverage   95.52%   95.52%            
========================================
  Files        1199     1200     +1     
  Lines      274508   274871   +363     
  Branches    16602    16616    +14     
========================================
+ Hits       262219   262574   +355     
- Misses      10110    10111     +1     
- Partials     2179     2186     +7     

[sharwell]

I need to look and see how this relates to #4921

[mavasani]

IMO, this deserves a static helper method to return AnalyzerConfigFiles given string editorConfigText as the input.

[Youssef1313]

@mavasani For this specific test file, I used it only twice (in the same test). Did you mean to introduce it in a new helper class and use it from all tests?

[sharwell]

The form used here is the form I generally expect to see.

[Youssef1313]

I need to look and see how this relates to #4921

@sharwell Have you got the time to look? Thanks :)

[sharwell]

😕 I thought I already implemented this as part of another change