Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BuildCheck] Editor config support #9811

Merged
merged 67 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
20db91a
Initial codechanges of editorconfig support
Mar 1, 2024
e010c36
Build error fixes
Mar 4, 2024
44983c2
fix the configuration
Mar 4, 2024
b2a05ad
Share the visibility of the Microsoft.Build for unit test purposes. A…
Mar 4, 2024
af05c7f
remote static from configurationProvider
Mar 4, 2024
8062014
Copy the tests for config file parsing and adjust based on the curren…
Mar 6, 2024
cd2abde
Merge branch 'exp/build-analyzers' into dev/f-alizada/support-editorc…
Mar 6, 2024
20433af
style fixes
Mar 6, 2024
d94d090
Remove the comments
Mar 6, 2024
1e63860
Handle caching properly
Mar 6, 2024
8d8c1e9
Add custom configuration data implementation
Mar 6, 2024
68a563e
check for null ref in BuildAnalyzerConfiguration creation and cover l…
Mar 7, 2024
88160f1
remove interface, add tests on editorconfig discovery
Mar 11, 2024
c88ec21
Make get custom config public only.
Mar 11, 2024
08219fa
Add success configuration tests and error scope for config exceptions
Mar 11, 2024
40af50e
Add more documentation
Mar 11, 2024
e495070
Merge branch 'exp/build-analyzers' into dev/f-alizada/support-editorc…
Mar 18, 2024
709199f
Add exception on building the config instance
Mar 18, 2024
dce8039
Update exceptions
Mar 19, 2024
d7ba200
Merge branch 'exp/build-analyzers' into dev/f-alizada/support-editorc…
Mar 19, 2024
e27f899
Fix merge conflicts
Mar 19, 2024
d273404
Fix the naming
Mar 19, 2024
933aee1
rename to build_check
Mar 19, 2024
e0dfb8d
Leave todos for the next iteration or version
Mar 19, 2024
ca3d5b7
Add more info on usage of the introduced changes.
Mar 19, 2024
df1b64c
address PR comments + compare custom configuration data
Mar 25, 2024
3dd6ee4
Add equals unit test
Mar 25, 2024
6716250
Merge branch 'exp/build-analyzers' into dev/f-alizada/support-editorc…
Apr 2, 2024
d659cad
Address formatting pr review comments
Apr 2, 2024
ee9eaaf
Update the comparison logic
Apr 3, 2024
5cb1c30
Address style issue/ null warnings
Apr 3, 2024
7217b68
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 16, 2024
1bc3c68
fixing merge conflicts
Apr 18, 2024
3259876
Fix merged conflicts. Update tests
Apr 18, 2024
54a1a4d
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 18, 2024
19115fb
Fix differences
Apr 18, 2024
4780871
small fixes
Apr 18, 2024
c593e38
empty lines (not-needed changes removed)
Apr 18, 2024
68c8667
Empty line
Apr 18, 2024
12f5167
Address PR comments add more docs to the fields
Apr 18, 2024
4b80888
Add comments
Apr 23, 2024
5a77f96
Comparer specified
Apr 23, 2024
f2f796b
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 23, 2024
68f9eeb
Cover the case when the project config is not presented
Apr 23, 2024
76fa557
Address PR comments
Apr 23, 2024
5ccdf6a
Address more comments
Apr 23, 2024
92c5099
Add comment TODO
Apr 23, 2024
c0fd216
Add comment to the configuration parameter
Apr 23, 2024
4148545
Add Concurrent Dictionary for thread safety of EditorConfigParser
Apr 23, 2024
7b4bc43
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 23, 2024
b5fb7bc
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 24, 2024
d33c02a
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 28, 2024
a91c971
Fix merge conflicts
Apr 28, 2024
9c6eae8
Address PR review
Apr 28, 2024
6d465b9
Add editorconfig with filescoped namespace warning enabled
Apr 28, 2024
ac29844
Merge branch 'main' into dev/f-alizada/support-editorconfig
Apr 30, 2024
a15089b
remove implementation of the GetHashCode of CustomConfigdata
Apr 30, 2024
4d0f193
Merge branch 'main' into dev/f-alizada/support-editorconfig
May 7, 2024
bb94e19
Fix warning
May 7, 2024
3685179
Merge branch 'main' into dev/f-alizada/support-editorconfig
May 14, 2024
11fc46d
fix filescope namespace
May 14, 2024
11dc945
Move to experimental
May 14, 2024
02f5ca3
Update the documentation, update the configurationContext
May 17, 2024
41ff1c5
Merge branch 'main' into dev/f-alizada/support-editorconfig
May 17, 2024
e1753d4
Merge branch 'main' into dev/f-alizada/support-editorconfig
May 20, 2024
d6381ce
Address PR comments first iteration
May 20, 2024
ed6c50d
Address PR comments
May 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Build/BackEnd/Components/Caching/IConfigCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ BuildRequestConfiguration this[int configId]
BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata configMetadata);

/// <summary>
/// Gets a matching configuration. If no such configration exists, one is created and optionally loaded.
/// Gets a matching configuration. If no such configuration exists, one is created and optionally loaded.
/// </summary>
/// <param name="configMetadata">The configuration metadata to match.</param>
/// <param name="callback">Callback to be invoked if the configuration does not exist.</param>
Expand Down
2 changes: 2 additions & 0 deletions src/Build/BuildCheck/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[*.cs]
csharp_style_namespace_declarations = file_scoped:warning
73 changes: 73 additions & 0 deletions src/Build/BuildCheck/API/BuildAnalyzerConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;

namespace Microsoft.Build.Experimental.BuildCheck;

/// <summary>
Expand Down Expand Up @@ -42,4 +46,73 @@ public class BuildAnalyzerConfiguration
/// If some rules are enabled and some are not, the analyzer will be run and reports will be post-filtered.
/// </summary>
public bool? IsEnabled { get; internal init; }

/// <summary>
/// Creates a <see cref="BuildAnalyzerConfiguration"/> object based on the provided configuration dictionary.
/// If the BuildAnalyzerConfiguration's property name presented in the dictionary, the value of this key-value pair is parsed and assigned to the instance's field.
/// If parsing failed the value will be equal to null.
/// </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)
{
return new()
{
EvaluationAnalysisScope = TryExtractValue(nameof(EvaluationAnalysisScope), configDictionary, out EvaluationAnalysisScope evaluationAnalysisScope) ? evaluationAnalysisScope : null,
Severity = TryExtractValue(nameof(Severity), configDictionary, out BuildAnalyzerResultSeverity severity) ? severity : null,
IsEnabled = TryExtractValue(nameof(IsEnabled), configDictionary, out bool isEnabled) ? isEnabled : null,
};
}

private static bool TryExtractValue<T>(string key, Dictionary<string, string>? config, out T value) where T : struct, Enum
{
value = default;

if (config == null || !config.TryGetValue(key.ToLower(), out var stringValue) || stringValue is null)
{
return false;
}

var isParsed = Enum.TryParse(stringValue, true, out value);

if (!isParsed)
{
ThrowIncorrectValueException(key, stringValue);
}

return isParsed;
}

private static bool TryExtractValue(string key, Dictionary<string, string>? config, out bool value)
{
value = default;

if (config == null || !config.TryGetValue(key.ToLower(), out var stringValue) || stringValue is null)
{
return false;
}

bool isParsed = false;

if (bool.TryParse(stringValue, out bool boolValue))
{
value = boolValue;
isParsed = true;
}

if (!isParsed)
{
ThrowIncorrectValueException(key, stringValue);
}

return isParsed;
}

private static void ThrowIncorrectValueException(string key, string value)
{
// TODO: It will be nice to have the filename where the incorrect configuration was placed.
throw new BuildCheckConfigurationException(
$"Incorrect value provided in config for key {key}: '{value}'",
f-alizada marked this conversation as resolved.
Show resolved Hide resolved
buildCheckConfigurationErrorScope: BuildCheckConfigurationErrorScope.EditorConfigParser);
}
}
12 changes: 1 addition & 11 deletions src/Build/BuildCheck/API/ConfigurationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,9 @@ private ConfigurationContext(CustomConfigurationData[] customConfigurationData)

internal static ConfigurationContext FromDataEnumeration(CustomConfigurationData[] customConfigurationData)
{
if (!customConfigurationData.Any(BuildCheck.CustomConfigurationData.NotNull))
f-alizada marked this conversation as resolved.
Show resolved Hide resolved
{
return Null;
}

return new ConfigurationContext(
customConfigurationData
.Where(BuildCheck.CustomConfigurationData.NotNull)
.ToArray());
return new ConfigurationContext(customConfigurationData);
}

internal static ConfigurationContext Null { get; } = new(Array.Empty<CustomConfigurationData>());

/// <summary>
/// Custom configuration data - per each rule that has some specified.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;
/// </summary>
internal sealed class BuildCheckCentralContext
{
private readonly ConfigurationProvider _configurationProvider;
f-alizada marked this conversation as resolved.
Show resolved Hide resolved

internal BuildCheckCentralContext(ConfigurationProvider configurationProvider)
{
_configurationProvider = configurationProvider;
}

private record CallbackRegistry(
List<(BuildAnalyzerWrapper, Action<BuildCheckDataContext<EvaluatedPropertiesAnalysisData>>)> EvaluatedPropertiesActions,
List<(BuildAnalyzerWrapper, Action<BuildCheckDataContext<ParsedItemsAnalysisData>>)> ParsedItemsActions,
Expand Down Expand Up @@ -126,7 +133,7 @@ private void RunRegisteredActions<T>(
else
{
configPerRule =
ConfigurationProvider.GetMergedConfigurations(projectFullPath,
_configurationProvider.GetMergedConfigurations(projectFullPath,
analyzerCallback.Item1.BuildAnalyzer);
if (configPerRule.All(c => !c.IsEnabled))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;

internal enum BuildCheckConfigurationErrorScope
{
/// <summary>
/// Error related to the single rule.
/// </summary>
SingleRule,

/// <summary>
/// Error related to the parsing of .editorconfig file.
/// </summary>
EditorConfigParser
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ internal sealed class BuildCheckConfigurationException : Exception
/// Exception to communicate issues with user specified configuration - unsupported scenarios, malformations, etc.
/// This exception usually leads to defuncting the particular analyzer for the rest of the build (even if issue occured with a single project).
/// </summary>
public BuildCheckConfigurationException(string message) : base(message)
internal BuildCheckConfigurationErrorScope buildCheckConfigurationErrorScope;

public BuildCheckConfigurationException(string message, Exception innerException, BuildCheckConfigurationErrorScope buildCheckConfigurationErrorScope = BuildCheckConfigurationErrorScope.SingleRule) : base(message, innerException)
{
this.buildCheckConfigurationErrorScope = buildCheckConfigurationErrorScope;
}

public BuildCheckConfigurationException(string message, BuildCheckConfigurationErrorScope buildCheckConfigurationErrorScope = BuildCheckConfigurationErrorScope.SingleRule) : base(message)
{
this.buildCheckConfigurationErrorScope = buildCheckConfigurationErrorScope;
}
}
14 changes: 8 additions & 6 deletions src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public void InitializeComponent(IBuildComponentHost host)
internal sealed class BuildCheckManager : IBuildCheckManager
{
private readonly TracingReporter _tracingReporter = new TracingReporter();
private readonly BuildCheckCentralContext _buildCheckCentralContext = new();
private readonly ConfigurationProvider _configurationProvider = new ConfigurationProvider();
private readonly BuildCheckCentralContext _buildCheckCentralContext;
private readonly ILoggingService _loggingService;
private readonly List<BuildAnalyzerFactoryContext> _analyzersRegistry;
private readonly bool[] _enabledDataSources = new bool[(int)BuildCheckDataSource.ValuesCount];
Expand All @@ -76,6 +77,7 @@ internal BuildCheckManager(ILoggingService loggingService)
_analyzersRegistry = new List<BuildAnalyzerFactoryContext>();
_acquisitionModule = new BuildCheckAcquisitionModule(loggingService);
_loggingService = loggingService;
_buildCheckCentralContext = new(_configurationProvider);
_buildEventsProcessor = new(_buildCheckCentralContext);
}

Expand Down Expand Up @@ -213,7 +215,7 @@ private void SetupSingleAnalyzer(BuildAnalyzerFactoryContext analyzerFactoryCont
if (analyzerFactoryContext.MaterializedAnalyzer == null)
{
BuildAnalyzerConfiguration[] userConfigs =
ConfigurationProvider.GetUserConfigurations(projectFullPath, analyzerFactoryContext.RuleIds);
_configurationProvider.GetUserConfigurations(projectFullPath, analyzerFactoryContext.RuleIds);

if (userConfigs.All(c => !(c.IsEnabled ?? analyzerFactoryContext.IsEnabledByDefault)))
{
Expand All @@ -222,7 +224,7 @@ private void SetupSingleAnalyzer(BuildAnalyzerFactoryContext analyzerFactoryCont
}

CustomConfigurationData[] customConfigData =
ConfigurationProvider.GetCustomConfigurations(projectFullPath, analyzerFactoryContext.RuleIds);
_configurationProvider.GetCustomConfigurations(projectFullPath, analyzerFactoryContext.RuleIds);

ConfigurationContext configurationContext = ConfigurationContext.FromDataEnumeration(customConfigData);

Expand All @@ -246,7 +248,7 @@ private void SetupSingleAnalyzer(BuildAnalyzerFactoryContext analyzerFactoryCont
$"The analyzer '{analyzer.FriendlyName}' exposes rules '{analyzer.SupportedRules.Select(r => r.Id).ToCsvString()}', but different rules were declared during registration: '{analyzerFactoryContext.RuleIds.ToCsvString()}'");
}

configurations = ConfigurationProvider.GetMergedConfigurations(userConfigs, analyzer);
configurations = _configurationProvider.GetMergedConfigurations(userConfigs, analyzer);

// technically all analyzers rules could be disabled, but that would mean
// that the provided 'IsEnabledByDefault' value wasn't correct - the only
Expand All @@ -261,9 +263,9 @@ private void SetupSingleAnalyzer(BuildAnalyzerFactoryContext analyzerFactoryCont
{
wrapper = analyzerFactoryContext.MaterializedAnalyzer;

configurations = ConfigurationProvider.GetMergedConfigurations(projectFullPath, wrapper.BuildAnalyzer);
configurations = _configurationProvider.GetMergedConfigurations(projectFullPath, wrapper.BuildAnalyzer);

ConfigurationProvider.CheckCustomConfigurationDataValidity(projectFullPath,
_configurationProvider.CheckCustomConfigurationDataValidity(projectFullPath,
analyzerFactoryContext.RuleIds[0]);

// Update the wrapper
Expand Down
Loading