diff --git a/documentation/specs/BuildCheck/BuildCheck.md b/documentation/specs/BuildCheck/BuildCheck.md index 68e643030c7..a4db495c6ae 100644 --- a/documentation/specs/BuildCheck/BuildCheck.md +++ b/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,13 +196,23 @@ 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. @@ -208,6 +220,12 @@ Same rule can have `EvaluationAnalysisScope` configured to different values for 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 diff --git a/src/Build/BuildCheck/API/BuildAnalyzerConfiguration.cs b/src/Build/BuildCheck/API/BuildAnalyzerConfiguration.cs index 3bf35931156..25492910718 100644 --- a/src/Build/BuildCheck/API/BuildAnalyzerConfiguration.cs +++ b/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 { /// /// 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. /// A new instance of with the specified settings. - internal static BuildAnalyzerConfiguration Create(Dictionary? configDictionary) + internal static BuildAnalyzerConfiguration Create(Dictionary? configDictionary) => new() { - return new() + EvaluationAnalysisScope = TryExtractEvaluationAnalysisScope(configDictionary), + Severity = TryExtractSeverity(configDictionary), + }; + + + private static EvaluationAnalysisScope? TryExtractEvaluationAnalysisScope(Dictionary? 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(string key, Dictionary? config, out T value) where T : struct, Enum + private static BuildAnalyzerResultSeverity? TryExtractSeverity(Dictionary? 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? 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) diff --git a/src/Build/BuildCheck/Infrastructure/ConfigurationProvider.cs b/src/Build/BuildCheck/Infrastructure/ConfigurationProvider.cs index ed513636726..a927682ef76 100644 --- a/src/Build/BuildCheck/Infrastructure/ConfigurationProvider.cs +++ b/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 /// private readonly ConcurrentDictionary _customConfigurationData = new ConcurrentDictionary(StringComparer.InvariantCultureIgnoreCase); - private readonly string[] _infrastructureConfigurationKeys = new string[] { - nameof(BuildAnalyzerConfiguration.EvaluationAnalysisScope).ToLower(), - nameof(BuildAnalyzerConfiguration.Severity).ToLower() + private readonly string[] _infrastructureConfigurationKeys = { + BuildCheckConstants.scopeConfigurationKey, + BuildCheckConstants.severityConfigurationKey, }; /// diff --git a/src/Build/BuildCheck/Utilities/Constants.cs b/src/Build/BuildCheck/Utilities/Constants.cs index 50a3d1bc63c..a95f02452e5 100644 --- a/src/Build/BuildCheck/Utilities/Constants.cs +++ b/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"; } diff --git a/src/BuildCheck.UnitTests/BuildAnalyzerConfiguration_Test.cs b/src/BuildCheck.UnitTests/BuildAnalyzerConfiguration_Test.cs index f1aff479f9e..a017df0f9c3 100644 --- a/src/BuildCheck.UnitTests/BuildAnalyzerConfiguration_Test.cs +++ b/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() { - { "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(() => { + var exception = Should.Throw(() => + { BuildAnalyzerConfiguration.Create(config); }); exception.Message.ShouldContain($"Incorrect value provided in config for key {key}"); diff --git a/src/BuildCheck.UnitTests/ConfigurationProvider_Tests.cs b/src/BuildCheck.UnitTests/ConfigurationProvider_Tests.cs index d4fdb9d49df..32f8554bc75 100644 --- a/src/BuildCheck.UnitTests/ConfigurationProvider_Tests.cs +++ b/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,8 +107,8 @@ 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(); @@ -114,9 +116,9 @@ public void GetRuleIdConfiguration_ReturnsBuildRuleConfiguration() 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();