Skip to content

Commit

Permalink
Improve editoroptions caching on ProjectState
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed May 5, 2022
1 parent e1f2e7d commit 7719856
Show file tree
Hide file tree
Showing 29 changed files with 410 additions and 308 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
// See the LICENSE file in the project root for more information.

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

namespace Microsoft.CodeAnalysis.Diagnostics
{
internal sealed class DictionaryAnalyzerConfigOptions : AnalyzerConfigOptions
internal sealed partial class DictionaryAnalyzerConfigOptions : AnalyzerConfigOptions
{
internal static readonly ImmutableDictionary<string, string> EmptyDictionary = ImmutableDictionary.Create<string, string>(KeyComparer);

public static readonly ImmutableDictionary<string, string> EmptyDictionary = ImmutableDictionary.Create<string, string>(KeyComparer);
public static DictionaryAnalyzerConfigOptions Empty { get; } = new DictionaryAnalyzerConfigOptions(EmptyDictionary);

internal readonly ImmutableDictionary<string, string> Options;
private readonly ImmutableDictionary<string, string> _options;

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

public override bool TryGetValue(string key, [NotNullWhen(true)] out string? value)
=> Options.TryGetValue(key, out value);
=> _options.TryGetValue(key, out value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -469,5 +469,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 @@ -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
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using Roslyn.Utilities;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.CodeAnalysis.Options.EditorConfig
{
Expand All @@ -21,19 +22,25 @@ private sealed class EditorConfigDocumentOptionsProvider : IDocumentOptionsProvi
{
public async Task<IDocumentOptions?> GetOptionsForDocumentAsync(Document document, CancellationToken cancellationToken)
{
var options = await document.GetAnalyzerOptionsAsync(cancellationToken).ConfigureAwait(false);

return new DocumentOptions(options);
var data = await document.GetAnalyzerOptionsAsync(cancellationToken).ConfigureAwait(false);
return new DocumentOptions(data?.AnalyzerConfigOptions);
}

private sealed class DocumentOptions : IDocumentOptions
{
private readonly ImmutableDictionary<string, string> _options;
public DocumentOptions(ImmutableDictionary<string, string> options)
private readonly StructuredAnalyzerConfigOptions? _options;

public DocumentOptions(StructuredAnalyzerConfigOptions? options)
=> _options = options;

public bool TryGetDocumentOption(OptionKey option, out object? value)
{
if (_options == null)
{
value = null;
return false;
}

var editorConfigPersistence = (IEditorConfigStorageLocation?)option.Option.StorageLocations.SingleOrDefault(static location => location is IEditorConfigStorageLocation);
if (editorConfigPersistence == null)
{
Expand All @@ -43,7 +50,7 @@ public bool TryGetDocumentOption(OptionKey option, out object? value)

try
{
return editorConfigPersistence.TryGetOption(_options.AsNullable(), option.Option.Type, out value);
return editorConfigPersistence.TryGetOption(_options, option.Option.Type, out value);
}
catch (Exception e) when (FatalError.ReportAndCatch(e))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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.Collections.Immutable;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.CodeAnalysis;

/// <summary>
/// Aggregate analyzer config options for a specific path.
/// </summary>
internal readonly struct AnalyzerConfigData
{
private readonly AnalyzerConfigOptionsResult _result;
private readonly StructuredAnalyzerConfigOptions _configOptions;

public AnalyzerConfigData(AnalyzerConfigOptionsResult result)
{
_result = result;
_configOptions = new StructuredAnalyzerConfigOptions(result.AnalyzerOptions);
}

public AnalyzerConfigOptionsResult Result => _result;
public StructuredAnalyzerConfigOptions AnalyzerConfigOptions => _configOptions;
public ImmutableDictionary<string, string> AnalyzerOptions => _result.AnalyzerOptions;
public ImmutableDictionary<string, ReportDiagnostic> TreeOptions => _result.TreeOptions;
}
13 changes: 5 additions & 8 deletions src/Workspaces/Core/Portable/Workspace/Solution/Document.cs
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ private void InitializeCachedOptions(OptionSet solutionOptions)
Interlocked.CompareExchange(ref _cachedOptions, newAsyncLazy, comparand: null);
}

internal Task<ImmutableDictionary<string, string>> GetAnalyzerOptionsAsync(CancellationToken cancellationToken)
internal async Task<AnalyzerConfigData?> GetAnalyzerOptionsAsync(CancellationToken cancellationToken)
{
var projectFilePath = Project.FilePath;
// We need to work out path to this document. Documents may not have a "real" file path if they're something created
Expand All @@ -535,15 +535,12 @@ internal Task<ImmutableDictionary<string, string>> GetAnalyzerOptionsAsync(Cance
}
}

if (effectiveFilePath != null)
if (effectiveFilePath == null)
{
return Project.State.GetAnalyzerOptionsForPathAsync(effectiveFilePath, cancellationToken);
}
else
{
// Really no idea where this is going, so bail
return Task.FromResult(DictionaryAnalyzerConfigOptions.EmptyDictionary);
return null;
}

return await Project.State.GetAnalyzerOptionsForPathAsync(effectiveFilePath, cancellationToken).ConfigureAwait(false);
}
}
}
2 changes: 1 addition & 1 deletion src/Workspaces/Core/Portable/Workspace/Solution/Project.cs
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ private void CheckIdsContainedInProject(ImmutableArray<DocumentId> documentIds)
}
}

internal AnalyzerConfigOptionsResult? GetAnalyzerConfigOptions()
internal AnalyzerConfigData? GetAnalyzerConfigOptions()
=> _projectState.GetAnalyzerConfigOptions();

private string GetDebuggerDisplay()
Expand Down
Loading

0 comments on commit 7719856

Please sign in to comment.