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

Improve editorconfig options caching on ProjectState #61131

Merged
merged 4 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
tmat marked this conversation as resolved.
Show resolved Hide resolved
=> 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;
tmat marked this conversation as resolved.
Show resolved Hide resolved

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]
tmat marked this conversation as resolved.
Show resolved Hide resolved
[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)
tmat marked this conversation as resolved.
Show resolved Hide resolved
{
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