Skip to content

Commit

Permalink
Improve editorconfig options caching on ProjectState (#61131)
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat authored May 10, 2022
1 parent a22346b commit 7be9fe7
Show file tree
Hide file tree
Showing 34 changed files with 439 additions and 320 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Xunit;

namespace Microsoft.CodeAnalysis.UnitTests
{
public class DictionaryAnalyzerConfigOptionsTests
{
[Fact]
public void BackwardCompatibility()
{
// Note: Older versions of analyzers access this field via reflection.
// https://github.com/dotnet/roslyn/blob/8e3d62a30b833631baaa4e84c5892298f16a8c9e/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Options/EditorConfig/EditorConfigStorageLocationExtensions.cs#L21
Assert.Equal(
typeof(ImmutableDictionary<string, string>),
typeof(DictionaryAnalyzerConfigOptions).GetField("Options", BindingFlags.Instance | BindingFlags.NonPublic)?.FieldType);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.CodeAnalysis.Diagnostics
Expand All @@ -19,5 +20,12 @@ public abstract class AnalyzerConfigOptions
/// Get an analyzer config value for the given key, using the <see cref="KeyComparer"/>.
/// </summary>
public abstract bool TryGetValue(string key, [NotNullWhen(true)] out string? value);

/// <summary>
/// Enumerates unique keys of all available options in no specific order.
/// </summary>
/// <exception cref="NotImplementedException">Not implemented by the derived type.</exception>
public virtual IEnumerable<string> Keys
=> throw new NotImplementedException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;

Expand All @@ -13,12 +14,17 @@ internal sealed class DictionaryAnalyzerConfigOptions : AnalyzerConfigOptions

public static DictionaryAnalyzerConfigOptions Empty { get; } = new DictionaryAnalyzerConfigOptions(EmptyDictionary);

// Note: Do not rename. Older versions of analyzers access this field via reflection.
// https://github.com/dotnet/roslyn/blob/8e3d62a30b833631baaa4e84c5892298f16a8c9e/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Options/EditorConfig/EditorConfigStorageLocationExtensions.cs#L21
internal readonly ImmutableDictionary<string, string> Options;

public DictionaryAnalyzerConfigOptions(ImmutableDictionary<string, string> options)
=> Options = options;

public override bool TryGetValue(string key, [NotNullWhen(true)] out string? value)
=> Options.TryGetValue(key, out value);

public override IEnumerable<string> Keys
=> Options.Keys;
}
}
1 change: 1 addition & 0 deletions src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const Microsoft.CodeAnalysis.WellKnownMemberNames.CheckedExplicitConversionName
Microsoft.CodeAnalysis.OperationKind.UTF8String = 124 -> Microsoft.CodeAnalysis.OperationKind
Microsoft.CodeAnalysis.Operations.IUTF8StringOperation
Microsoft.CodeAnalysis.Operations.IUTF8StringOperation.Value.get -> string!
virtual Microsoft.CodeAnalysis.Diagnostics.AnalyzerConfigOptions.Keys.get -> System.Collections.Generic.IEnumerable<string!>!
virtual Microsoft.CodeAnalysis.Operations.OperationVisitor.VisitUTF8String(Microsoft.CodeAnalysis.Operations.IUTF8StringOperation! operation) -> void
virtual Microsoft.CodeAnalysis.Operations.OperationVisitor<TArgument, TResult>.VisitUTF8String(Microsoft.CodeAnalysis.Operations.IUTF8StringOperation! operation, TArgument argument) -> TResult?
virtual Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.DefaultVisit(Microsoft.CodeAnalysis.ISymbol! symbol, TArgument argument) -> TResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles;
using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics.NamingStyles;
using Roslyn.Test.Utilities;
using Xunit;
using static Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles.EditorConfigNamingStyleParser;
using static Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles.SymbolSpecification;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics.NamingStyles
{
public class EditorConfigNamingStyleParserTests
{
private static NamingStylePreferences ParseDictionary(Dictionary<string, string> options)
=> EditorConfigNamingStyleParser.ParseDictionary(new DictionaryAnalyzerConfigOptions(options.ToImmutableDictionary()));

[Fact]
public static void TestPascalCaseRule()
{
Expand Down Expand Up @@ -469,5 +472,111 @@ public static void TestEditorConfigParseForApplicableSymbolKinds()
Assert.True(!string.IsNullOrEmpty(editorConfigString));
}
}

[Theory]
[InlineData("a", "b", "a", "public", "public, private")]
[InlineData("b", "a", "a", "public, private", "public")]
[InlineData("b", "a", "b", "public", "public, private")]
[InlineData("a", "b", "b", "public, private", "public")]
[InlineData("a", "b", "a", "*", "*")]
[InlineData("b", "a", "a", "*", "*")]
[InlineData("A", "b", "A", "*", "*")]
[InlineData("b", "A", "A", "*", "*")]
[InlineData("a", "B", "a", "*", "*")]
[InlineData("B", "a", "a", "*", "*")]
[InlineData("A", "B", "A", "*", "*")]
[InlineData("B", "A", "A", "*", "*")]
public static void TestOrderedByAccessibilityBeforeName(string firstName, string secondName, string firstNameAfterOrdering, string firstAccessibility, string secondAccessibility)
{
var namingStylePreferences = ParseDictionary(new Dictionary<string, string>()
{
[$"dotnet_naming_rule.{firstName}.severity"] = "error",
[$"dotnet_naming_rule.{firstName}.symbols"] = "first_symbols",
[$"dotnet_naming_rule.{firstName}.style"] = $"{firstName}_style",
["dotnet_naming_symbols.first_symbols.applicable_kinds"] = "method,property",
["dotnet_naming_symbols.first_symbols.applicable_accessibilities"] = firstAccessibility,
[$"dotnet_naming_style.{firstName}_style.capitalization"] = "pascal_case",
[$"dotnet_naming_style.{secondName}_style.capitalization"] = "camel_case",
[$"dotnet_naming_rule.{secondName}.severity"] = "error",
[$"dotnet_naming_rule.{secondName}.symbols"] = "second_symbols",
[$"dotnet_naming_rule.{secondName}.style"] = $"{secondName}_style",
["dotnet_naming_symbols.second_symbols.applicable_kinds"] = "method,property",
["dotnet_naming_symbols.second_symbols.applicable_accessibilities"] = secondAccessibility,
});

var secondNameAfterOrdering = firstNameAfterOrdering == firstName ? secondName : firstName;
Assert.Equal($"{firstNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[0].NamingStyle.Name);
Assert.Equal($"{secondNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[1].NamingStyle.Name);
}

[Theory]
[InlineData("a", "b", "a", "static, readonly", "static")]
[InlineData("b", "a", "a", "static", "static, readonly")]
[InlineData("b", "a", "b", "static, readonly", "static")]
[InlineData("a", "b", "b", "static", "static, readonly")]
[InlineData("a", "b", "a", "", "")]
[InlineData("b", "a", "a", "", "")]
[InlineData("A", "b", "A", "", "")]
[InlineData("b", "A", "A", "", "")]
[InlineData("a", "B", "a", "", "")]
[InlineData("B", "a", "a", "", "")]
[InlineData("A", "B", "A", "", "")]
[InlineData("B", "A", "A", "", "")]
public static void TestOrderedByModifiersBeforeName(string firstName, string secondName, string firstNameAfterOrdering, string firstModifiers, string secondModifiers)
{
var namingStylePreferences = ParseDictionary(new Dictionary<string, string>()
{
[$"dotnet_naming_rule.{firstName}.severity"] = "error",
[$"dotnet_naming_rule.{firstName}.symbols"] = "first_symbols",
[$"dotnet_naming_rule.{firstName}.style"] = $"{firstName}_style",
["dotnet_naming_symbols.first_symbols.applicable_kinds"] = "method,property",
["dotnet_naming_symbols.first_symbols.required_modifiers"] = firstModifiers,
[$"dotnet_naming_style.{firstName}_style.capitalization"] = "pascal_case",
[$"dotnet_naming_style.{secondName}_style.capitalization"] = "camel_case",
[$"dotnet_naming_rule.{secondName}.severity"] = "error",
[$"dotnet_naming_rule.{secondName}.symbols"] = "second_symbols",
[$"dotnet_naming_rule.{secondName}.style"] = $"{secondName}_style",
["dotnet_naming_symbols.second_symbols.applicable_kinds"] = "method,property",
["dotnet_naming_symbols.second_symbols.required_modifiers"] = secondModifiers,
});

var secondNameAfterOrdering = firstNameAfterOrdering == firstName ? secondName : firstName;
Assert.Equal($"{firstNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[0].NamingStyle.Name);
Assert.Equal($"{secondNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[1].NamingStyle.Name);
}

[Theory]
[InlineData("a", "b", "a", "method", "method, property")]
[InlineData("b", "a", "a", "method, property", "method")]
[InlineData("b", "a", "b", "method", "method, property")]
[InlineData("a", "b", "b", "method, property", "method")]
[InlineData("a", "b", "a", "*", "*")]
[InlineData("b", "a", "a", "*", "*")]
[InlineData("A", "b", "A", "*", "*")]
[InlineData("b", "A", "A", "*", "*")]
[InlineData("a", "B", "a", "*", "*")]
[InlineData("B", "a", "a", "*", "*")]
[InlineData("A", "B", "A", "*", "*")]
[InlineData("B", "A", "A", "*", "*")]
public static void TestOrderedBySymbolsBeforeName(string firstName, string secondName, string firstNameAfterOrdering, string firstSymbols, string secondSymbols)
{
var namingStylePreferences = ParseDictionary(new Dictionary<string, string>()
{
[$"dotnet_naming_rule.{firstName}.severity"] = "error",
[$"dotnet_naming_rule.{firstName}.symbols"] = "first_symbols",
[$"dotnet_naming_rule.{firstName}.style"] = $"{firstName}_style",
["dotnet_naming_symbols.first_symbols.applicable_kinds"] = firstSymbols,
[$"dotnet_naming_style.{firstName}_style.capitalization"] = "pascal_case",
[$"dotnet_naming_style.{secondName}_style.capitalization"] = "camel_case",
[$"dotnet_naming_rule.{secondName}.severity"] = "error",
[$"dotnet_naming_rule.{secondName}.symbols"] = "second_symbols",
[$"dotnet_naming_rule.{secondName}.style"] = $"{secondName}_style",
["dotnet_naming_symbols.second_symbols.applicable_kinds"] = secondSymbols,
});

var secondNameAfterOrdering = firstNameAfterOrdering == firstName ? secondName : firstName;
Assert.Equal($"{firstNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[0].NamingStyle.Name);
Assert.Equal($"{secondNameAfterOrdering}_style", namingStylePreferences.Rules.NamingRules[1].NamingStyle.Name);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Utilities;
using static Microsoft.CodeAnalysis.ProjectState;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.EditorConfigSettings.DataProvider
{
Expand Down Expand Up @@ -51,7 +51,7 @@ protected void Update()
return;
}

var configOptionsProvider = new ProjectAnalyzerConfigOptionsProvider(project.State);
var configOptionsProvider = new ProjectState.ProjectAnalyzerConfigOptionsProvider(project.State);
var workspaceOptions = configOptionsProvider.GetOptionsForSourcePath(givenFolder.FullName);
var result = project.GetAnalyzerConfigOptions();
var options = new CombinedAnalyzerConfigOptions(workspaceOptions, result);
Expand Down Expand Up @@ -93,9 +93,9 @@ public void RegisterViewModel(ISettingsEditorViewModel viewModel)
private sealed class CombinedAnalyzerConfigOptions : AnalyzerConfigOptions
{
private readonly AnalyzerConfigOptions _workspaceOptions;
private readonly AnalyzerConfigOptionsResult? _result;
private readonly AnalyzerConfigData? _result;

public CombinedAnalyzerConfigOptions(AnalyzerConfigOptions workspaceOptions, AnalyzerConfigOptionsResult? result)
public CombinedAnalyzerConfigOptions(AnalyzerConfigOptions workspaceOptions, AnalyzerConfigData? result)
{
_workspaceOptions = workspaceOptions;
_result = result;
Expand Down Expand Up @@ -131,6 +131,34 @@ public override bool TryGetValue(string key, [NotNullWhen(true)] out string? val
value = null;
return false;
}

public override IEnumerable<string> Keys
{
get
{
foreach (var key in _workspaceOptions.Keys)
yield return key;

if (!_result.HasValue)
yield break;

foreach (var key in _result.Value.AnalyzerOptions.Keys)
{
if (!_workspaceOptions.TryGetValue(key, out _))
yield return key;
}

foreach (var (key, severity) in _result.Value.TreeOptions)
{
var diagnosticKey = "dotnet_diagnostic." + key + ".severity";
if (!_workspaceOptions.TryGetValue(diagnosticKey, out _) &&
!_result.Value.AnalyzerOptions.TryGetKey(diagnosticKey, out _))
{
yield return diagnosticKey;
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis.PooledObjects;

namespace Microsoft.CodeAnalysis.Diagnostics
{
Expand All @@ -28,6 +31,9 @@ public override bool TryGetValue(string key, [NotNullWhen(true)] out string? val

return _fallbackOptions.TryGetValue(key, out value);
}

public override IEnumerable<string> Keys
=> _options.Keys.Concat(_fallbackOptions.Keys.Where(key => !_options.TryGetValue(key, out _)));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ private IReadOnlyList<StateSet> GetStateSetsForFullSolutionAnalysis(IEnumerable<
return stateSets.Where(s => IsCandidateForFullSolutionAnalysis(s.Analyzer, project, analyzerConfigOptions)).ToList();
}

private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, Project project, AnalyzerConfigOptionsResult? analyzerConfigOptions)
private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, Project project, AnalyzerConfigData? analyzerConfigOptions)
{
// PERF: Don't query descriptors for compiler analyzer or workspace load analyzer, always execute them.
if (analyzer == FileContentLoadAnalyzer.Instance ||
Expand Down Expand Up @@ -413,7 +413,7 @@ private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, Pro

// For most of analyzers, the number of diagnostic descriptors is small, so this should be cheap.
var descriptors = DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer);
return descriptors.Any(d => d.GetEffectiveSeverity(project.CompilationOptions!, analyzerConfigOptions) != ReportDiagnostic.Hidden);
return descriptors.Any(d => d.GetEffectiveSeverity(project.CompilationOptions!, analyzerConfigOptions?.Result) != ReportDiagnostic.Hidden);
}

private void RaiseProjectDiagnosticsIfNeeded(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ private void UpdateSeverityMenuItemsChecked()

foreach (var diagnosticItem in group)
{
var severity = diagnosticItem.Descriptor.GetEffectiveSeverity(project.CompilationOptions, analyzerConfigOptions);
var severity = diagnosticItem.Descriptor.GetEffectiveSeverity(project.CompilationOptions, analyzerConfigOptions?.Result);
selectedItemSeverities.Add(severity);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal abstract partial class BaseDiagnosticAndGeneratorItemSource : IAttached
private BulkObservableCollection<BaseItem>? _items;
private ReportDiagnostic _generalDiagnosticOption;
private ImmutableDictionary<string, ReportDiagnostic>? _specificDiagnosticOptions;
private AnalyzerConfigOptionsResult? _analyzerConfigOptions;
private AnalyzerConfigData? _analyzerConfigOptions;

public BaseDiagnosticAndGeneratorItemSource(Workspace workspace, ProjectId projectId, IAnalyzersCommandHandler commandHandler, IDiagnosticAnalyzerService diagnosticAnalyzerService)
{
Expand Down Expand Up @@ -95,7 +95,7 @@ public IEnumerable Items
}
}

private BulkObservableCollection<BaseItem> CreateDiagnosticAndGeneratorItems(ProjectId projectId, string language, CompilationOptions options, AnalyzerConfigOptionsResult? analyzerConfigOptions)
private BulkObservableCollection<BaseItem> CreateDiagnosticAndGeneratorItems(ProjectId projectId, string language, CompilationOptions options, AnalyzerConfigData? analyzerConfigOptions)
{
// Within an analyzer assembly, an individual analyzer may report multiple different diagnostics
// with the same ID. Or, multiple analyzers may report diagnostics with the same ID. Or a
Expand All @@ -117,7 +117,7 @@ private BulkObservableCollection<BaseItem> CreateDiagnosticAndGeneratorItems(Pro
.Select(g =>
{
var selectedDiagnostic = g.OrderBy(d => d, s_comparer).First();
var effectiveSeverity = selectedDiagnostic.GetEffectiveSeverity(options, analyzerConfigOptions);
var effectiveSeverity = selectedDiagnostic.GetEffectiveSeverity(options, analyzerConfigOptions?.Result);
return new DiagnosticItem(projectId, AnalyzerReference, selectedDiagnostic, effectiveSeverity, CommandHandler);
}));

Expand Down Expand Up @@ -183,7 +183,7 @@ void OnProjectConfigurationChanged()

foreach (var item in _items.OfType<DiagnosticItem>())
{
var effectiveSeverity = item.Descriptor.GetEffectiveSeverity(project.CompilationOptions, newAnalyzerConfigOptions);
var effectiveSeverity = item.Descriptor.GetEffectiveSeverity(project.CompilationOptions, newAnalyzerConfigOptions?.Result);
item.UpdateEffectiveSeverity(effectiveSeverity);
}
}
Expand Down
Loading

0 comments on commit 7be9fe7

Please sign in to comment.