Update configuration parsing logic (#10361)

documentation/specs/BuildCheck/BuildCheck.md

@@ -163,11 +163,11 @@ For the `.editorconfig` file configuration, following will apply:
 
 ```ini
 [*.csproj]
-build_check.BC0101.Severity=warning
+build_check.BC0101.severity=warning
 
-build_check.COND0543.Severity=none
-build_check.COND0543.EvaluationAnalysisScope=AnalyzedProjectOnly
-build_check.COND0543.CustomSwitch=QWERTY
+build_check.COND0543.severity=none
+build_check.COND0543.scope=project
+build_check.COND0543.custom_switch=QWERTY
 ```
 
 ### User Configurable Options
@@ -176,15 +176,17 @@ Initial version of BuildCheck plans a limited set of options configurable by use
 
 **NOTE:** The actual naming of the configuration options is yet to be determined.
 
-#### Severity
+#### Severity levels
 
 Option `Severity` with following values will be available:
 
-* `Default`
-* `None`
-* `Suggestion`
-* `Warning`
-* `Error`
+| Severity      | EditorConfig option      |
+| ------------- | ------------- |
+| Default | `default` |
+| None | `none` |
+| Suggestion | `suggestion` |
+| Warning | `warning` |
+| Error | `error` |
 
 Severity levels are in line with [roslyn analyzers severity levels](https://learn.microsoft.com/en-us/visualstudio/code-quality/use-roslyn-analyzers). `Default` severity in `.editorconfig` will lead to using build-in severity from the analyzer (so this can be used for clearing custom severity setting from higher level `.editorconfig` file). `Default` severity in the build-in code has same effect as if the code doesn't specify severity at all - an infrastruture default of `None` is considered.
 
@@ -194,20 +196,36 @@ Each rule has a severity, even if multiple rules are defined in a single analyze
 
 If all the rules from a single analyzer have severity `None` - analyzer won't be given any data for such configured part of the build (specific project or a whole build). If analyzer have some rules enabled and some disabled - it will be still fed with data, but the reports will be post-filtered.
 
+#### Configuring severity level
+
+```ini
+[*.csproj]
+build_check.BC0101.severity=warning
+```
+
 #### Scope of Analysis
 
 Option `EvaluationAnalysisScope` with following possible options will be available:
-* `ProjectOnly` - Only the data from currently analyzed project will be sent to the analyzer. Imports will be discarded.
-* `ProjectWithImportsFromCurrentWorkTree` - Only the data from currently analyzed project and imports from files under the entry project or solution will be sent to the analyzer. Other imports will be discarded.
-* `ProjectWithImportsWithoutSdks` - Imports from SDKs will not be sent to the analyzer. Other imports will be sent.
-* `ProjectWithAllImports` - All data will be sent to the analyzer.
+
+| EvaluationAnalysisScope (Solution Explorer)   | EditorConfig option      |  Behavior  | 
+| ------------- | ------------- |   ------------- |
+| ProjectOnly | `project` | Only the data from currently analyzed project will be sent to the analyzer. Imports will be discarded. | 
+| ProjectWithImportsFromCurrentWorkTree | `current_imports` |  Only the data from currently analyzed project and imports from files under the entry project or solution will be sent to the analyzer. Other imports will be discarded. | 
+| ProjectWithImportsWithoutSdks | `without_sdks` | Imports from SDKs will not be sent to the analyzer. Other imports will be sent. | 
+| ProjectWithAllImports | `all` | All data will be sent to the analyzer. | 
 
 All rules of a single analyzer must have the `EvaluationAnalysisScope` configured to a same value. If any rule from the analyzer have the value configured differently - a warning will be issued during the build and analyzer will be deregistered.
 
 Same rule can have `EvaluationAnalysisScope` configured to different values for different projects.
 
 BuildCheck might not be able to guarantee to properly filter the data with this distinction for all [registration types](#RegisterActions) - in case an explicit value is attempted to be configured (either [from the analyzer code](#BuildAnalyzerConfiguration) or from `.editorconfig` file) for an analyzer that has a subscription to unfilterable data - a warning will be issued during the build and analyzer will be deregistered.
 
+#### Configuring evalution scope
+
+```ini
+[*.csproj]
+build_check.BC0101.scope=all
+```
 
 ## Analyzers and Rules Identification
 

src/Build/BuildCheck/API/BuildAnalyzerConfiguration.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
+using Microsoft.Build.Experimental.BuildCheck.Utilities;
 
 namespace Microsoft.Build.Experimental.BuildCheck;
 
@@ -66,32 +67,78 @@ public bool? IsEnabled {
     /// </summary>
     /// <param name="configDictionary">The configuration dictionary containing the settings for the build analyzer. The configuration's keys are expected to be in lower case or the EqualityComparer to ignore case.</param>
     /// <returns>A new instance of <see cref="BuildAnalyzerConfiguration"/> with the specified settings.</returns>
-    internal static BuildAnalyzerConfiguration Create(Dictionary<string, string>? configDictionary)
+    internal static BuildAnalyzerConfiguration Create(Dictionary<string, string>? configDictionary) => new()
     {
-        return new()
+        EvaluationAnalysisScope = TryExtractEvaluationAnalysisScope(configDictionary),
+        Severity = TryExtractSeverity(configDictionary),
+    };
+
+
+    private static EvaluationAnalysisScope? TryExtractEvaluationAnalysisScope(Dictionary<string, string>? config)
+    {
+
+        if (!TryExtractValue(BuildCheckConstants.scopeConfigurationKey, config, out string? stringValue) || stringValue is null)
         {
-            EvaluationAnalysisScope = TryExtractValue(nameof(EvaluationAnalysisScope), configDictionary, out EvaluationAnalysisScope evaluationAnalysisScope) ? evaluationAnalysisScope : null,
-            Severity = TryExtractValue(nameof(Severity), configDictionary, out BuildAnalyzerResultSeverity severity) ? severity : null
-        };
+            return null;
+        }
+
+        switch (stringValue)
+        {
+            case "project":
+                return BuildCheck.EvaluationAnalysisScope.ProjectOnly;
+            case "current_imports":
+                return BuildCheck.EvaluationAnalysisScope.ProjectWithImportsFromCurrentWorkTree;
+            case "without_sdks":
+                return BuildCheck.EvaluationAnalysisScope.ProjectWithImportsWithoutSdks;
+            case "all":
+                return BuildCheck.EvaluationAnalysisScope.ProjectWithAllImports;
+            default:
+                ThrowIncorrectValueException(BuildCheckConstants.scopeConfigurationKey, stringValue);
+                break;
+        }
+
+        return null;
     }
 
-    private static bool TryExtractValue<T>(string key, Dictionary<string, string>? config, out T value) where T : struct, Enum
+    private static BuildAnalyzerResultSeverity? TryExtractSeverity(Dictionary<string, string>? config)
     {
-        value = default;
+        if (!TryExtractValue(BuildCheckConstants.severityConfigurationKey, config, out string? stringValue) || stringValue is null)
+        {
+            return null;
+        }
 
-        if (config == null || !config.TryGetValue(key.ToLower(), out var stringValue) || stringValue is null)
+        switch (stringValue)
         {
-            return false;
+            case "none":
+                return BuildAnalyzerResultSeverity.None;
+            case "default":
+                return BuildAnalyzerResultSeverity.Default;
+            case "suggestion":
+                return BuildAnalyzerResultSeverity.Suggestion;
+            case "warning":
+                return BuildAnalyzerResultSeverity.Warning;
+            case "error":
+                return BuildAnalyzerResultSeverity.Error;
+            default:
+                ThrowIncorrectValueException(BuildCheckConstants.severityConfigurationKey, stringValue);
+                break;
         }
 
-        var isParsed = Enum.TryParse(stringValue, true, out value);
+        return null;
+    }
+
+    private static bool TryExtractValue(string key, Dictionary<string, string>? config, out string? stringValue)
+    {
+        stringValue = null;
 
-        if (!isParsed)
+        if (config == null || !config.TryGetValue(key.ToLower(), out stringValue) || stringValue is null)
         {
-            ThrowIncorrectValueException(key, stringValue);
+            return false;
         }
 
-        return isParsed;
+        stringValue = stringValue.ToLower();
+
+        return true;
     }
 
     private static void ThrowIncorrectValueException(string key, string value)

src/Build/BuildCheck/Infrastructure/ConfigurationProvider.cs

@@ -7,6 +7,7 @@
 using Microsoft.Build.Experimental.BuildCheck.Infrastructure.EditorConfig;
 using Microsoft.Build.Experimental.BuildCheck;
 using System.Collections.Concurrent;
+using Microsoft.Build.Experimental.BuildCheck.Utilities;
 
 namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;
 
@@ -31,9 +32,9 @@ internal sealed class ConfigurationProvider
     /// </summary>
     private readonly ConcurrentDictionary<string, CustomConfigurationData> _customConfigurationData = new ConcurrentDictionary<string, CustomConfigurationData>(StringComparer.InvariantCultureIgnoreCase);
 
-    private readonly string[] _infrastructureConfigurationKeys = new string[] {
-        nameof(BuildAnalyzerConfiguration.EvaluationAnalysisScope).ToLower(),
-        nameof(BuildAnalyzerConfiguration.Severity).ToLower()
+    private readonly string[] _infrastructureConfigurationKeys = {
+        BuildCheckConstants.scopeConfigurationKey,
+        BuildCheckConstants.severityConfigurationKey,
     };
 
     /// <summary>

src/Build/BuildCheck/Utilities/Constants.cs

@@ -15,4 +15,7 @@ namespace Microsoft.Build.Experimental.BuildCheck.Utilities;
 internal static class BuildCheckConstants
 {
     internal const string infraStatPrefix = "infrastructureStat_";
+
+    internal const string severityConfigurationKey = "severity";
+    internal const string scopeConfigurationKey = "scope";
 }

src/BuildCheck.UnitTests/BuildAnalyzerConfiguration_Test.cs

@@ -75,16 +75,19 @@ public void CreateBuildAnalyzerConfiguration_SeverityAndEnabledOrder(string para
     }
 
     [Theory]
-    [InlineData("ProjectOnly", EvaluationAnalysisScope.ProjectOnly)]
-    [InlineData("ProjectWithImportsFromCurrentWorkTree", EvaluationAnalysisScope.ProjectWithImportsFromCurrentWorkTree)]
-    [InlineData("ProjectWithImportsWithoutSdks", EvaluationAnalysisScope.ProjectWithImportsWithoutSdks)]
-    [InlineData("ProjectWithAllImports", EvaluationAnalysisScope.ProjectWithAllImports)]
-    [InlineData("projectwithallimports", EvaluationAnalysisScope.ProjectWithAllImports)]
+    [InlineData("project", EvaluationAnalysisScope.ProjectOnly)]
+    [InlineData("PROJECT", EvaluationAnalysisScope.ProjectOnly)]
+    [InlineData("current_imports", EvaluationAnalysisScope.ProjectWithImportsFromCurrentWorkTree)]
+    [InlineData("CURRENT_IMPORTS", EvaluationAnalysisScope.ProjectWithImportsFromCurrentWorkTree)]
+    [InlineData("without_sdks", EvaluationAnalysisScope.ProjectWithImportsWithoutSdks)]
+    [InlineData("WITHOUT_SDKS", EvaluationAnalysisScope.ProjectWithImportsWithoutSdks)]
+    [InlineData("all", EvaluationAnalysisScope.ProjectWithAllImports)]
+    [InlineData("ALL", EvaluationAnalysisScope.ProjectWithAllImports)]
     public void CreateBuildAnalyzerConfiguration_EvaluationAnalysisScope(string parameter, EvaluationAnalysisScope? expected)
     {
         var config = new Dictionary<string, string>()
         {
-            { "evaluationanalysisscope" , parameter },
+            { "scope" , parameter },
         };
 
         var buildConfig = BuildAnalyzerConfiguration.Create(config);
@@ -97,7 +100,7 @@ public void CreateBuildAnalyzerConfiguration_EvaluationAnalysisScope(string para
     }
 
     [Theory]
-    [InlineData("evaluationanalysisscope", "incorrec-value")]
+    [InlineData("scope", "incorrec-value")]
     [InlineData("severity", "incorrec-value")]
     public void CreateBuildAnalyzerConfiguration_ExceptionOnInvalidInputValue(string key, string value)
     {
@@ -106,7 +109,8 @@ public void CreateBuildAnalyzerConfiguration_ExceptionOnInvalidInputValue(string
             { key , value },
         };
 
-        var exception = Should.Throw<BuildCheckConfigurationException>(() => {
+        var exception = Should.Throw<BuildCheckConfigurationException>(() =>
+        {
             BuildAnalyzerConfiguration.Create(config);
         });
         exception.Message.ShouldContain($"Incorrect value provided in config for key {key}");

src/BuildCheck.UnitTests/ConfigurationProvider_Tests.cs

@@ -76,7 +76,9 @@ public void GetRuleIdConfiguration_CustomConfigurationData()
         [*.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
+        build_check.rule_id.is_enabled_2=true
+        build_check.rule_id.scope=project
+        build_check.rule_id.severity=default
         any_other_key1=any_other_value1
         any_other_key2=any_other_value2
         any_other_key3=any_other_value3
@@ -91,7 +93,7 @@ public void GetRuleIdConfiguration_CustomConfigurationData()
 
         configs.ContainsKey("property1").ShouldBeTrue();
         configs.ContainsKey("property2").ShouldBeTrue();
-        configs.ContainsKey("isenabled2").ShouldBeTrue();
+        configs.ContainsKey("is_enabled_2").ShouldBeTrue();
     }
 
     [Fact]
@@ -105,18 +107,18 @@ public void GetRuleIdConfiguration_ReturnsBuildRuleConfiguration()
         root=true
 
         [*.csproj]
-        build_check.rule_id.Severity=Error
-        build_check.rule_id.EvaluationAnalysisScope=ProjectOnly
+        build_check.rule_id.severity=error
+        build_check.rule_id.scope=project
         """);
 
         var configurationProvider = new ConfigurationProvider();
         var buildConfig = configurationProvider.GetUserConfiguration(Path.Combine(workFolder1.Path, "test.csproj"), "rule_id");
 
         buildConfig.ShouldNotBeNull();
 
-        buildConfig.IsEnabled?.ShouldBeTrue();
-        buildConfig.Severity?.ShouldBe(BuildAnalyzerResultSeverity.Error);
-        buildConfig.EvaluationAnalysisScope?.ShouldBe(EvaluationAnalysisScope.ProjectOnly);
+        buildConfig.IsEnabled.ShouldBe(true);
+        buildConfig.Severity.ShouldBe(BuildAnalyzerResultSeverity.Error);
+        buildConfig.EvaluationAnalysisScope.ShouldBe(EvaluationAnalysisScope.ProjectOnly);
     }
 
     [Fact]
@@ -132,12 +134,12 @@ public void GetRuleIdConfiguration_CustomConfigurationValidity_NotValid_Differen
         [*.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
+        build_check.rule_id.is_enabled_2=true
 
         [test123.csproj]
         build_check.rule_id.property1=value2
         build_check.rule_id.property2=value3
-        build_check.rule_id.isEnabled2=tru1
+        build_check.rule_id.is_enabled_2=tru1
         """);
 
         var configurationProvider = new ConfigurationProvider();
@@ -163,13 +165,13 @@ public void GetRuleIdConfiguration_CustomConfigurationValidity_NotValid_Differen
         [*.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
+        build_check.rule_id.is_enabled_2=true
 
         [test123.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
-        build_check.rule_id.isEnabled3=true
+        build_check.rule_id.is_enabled_2=true
+        build_check.rule_id.is_enabled_3=true
         """);
 
         var configurationProvider = new ConfigurationProvider();
@@ -195,12 +197,12 @@ public void GetRuleIdConfiguration_CustomConfigurationValidity_Valid()
         [*.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
+        build_check.rule_id.is_enabled_2=true
 
         [test123.csproj]
         build_check.rule_id.property1=value1
         build_check.rule_id.property2=value2
-        build_check.rule_id.isEnabled2=true
+        build_check.rule_id.is_enabled_2=true
         """);
 
         var configurationProvider = new ConfigurationProvider();