From b0b477b379e07b35c9dc049535e8f1667d180979 Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Sat, 14 Jan 2017 01:10:57 -0800 Subject: [PATCH 1/2] append editorconfig namingstyle options to the top of the workspace options --- ...DocumentOptionsProvider.DocumentOptions.cs | 5 +- .../NamingStylePreferencesExtensions.cs | 15 +++++ .../Options/EditorConfigStorageLocation.cs | 27 +++++--- .../Core/Portable/Options/IDocumentOptions.cs | 2 +- .../Portable/Options/OptionServiceFactory.cs | 2 +- .../Core/Portable/Workspaces.csproj | 1 + .../EditorConfigStorageLocationTests.cs | 64 ++++++++++++++++++- src/Workspaces/CoreTest/ServicesTest.csproj | 2 +- 8 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs diff --git a/src/EditorFeatures/Next/Options/EditorConfigDocumentOptionsProvider.DocumentOptions.cs b/src/EditorFeatures/Next/Options/EditorConfigDocumentOptionsProvider.DocumentOptions.cs index fc77ada4ceb59..73968188aa8c7 100644 --- a/src/EditorFeatures/Next/Options/EditorConfigDocumentOptionsProvider.DocumentOptions.cs +++ b/src/EditorFeatures/Next/Options/EditorConfigDocumentOptionsProvider.DocumentOptions.cs @@ -20,7 +20,7 @@ public DocumentOptions(ICodingConventionsSnapshot codingConventionSnapshot, IErr _errorLogger = errorLogger; } - public bool TryGetDocumentOption(Document document, OptionKey option, out object value) + public bool TryGetDocumentOption(Document document, OptionKey option, OptionSet underlyingOptions, out object value) { var editorConfigPersistence = option.Option.StorageLocations.OfType().SingleOrDefault(); if (editorConfigPersistence == null) @@ -32,7 +32,8 @@ public bool TryGetDocumentOption(Document document, OptionKey option, out object var allRawConventions = _codingConventionSnapshot.AllRawConventions; try { - return editorConfigPersistence.TryParseReadonlyDictionary(allRawConventions, option.Option.Type, out value); + var underlyingOption = underlyingOptions.GetOption(option); + return editorConfigPersistence.TryGetOption(underlyingOption, allRawConventions, option.Option.Type, out value); } catch (Exception ex) { diff --git a/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs b/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs new file mode 100644 index 0000000000000..39500dabe3069 --- /dev/null +++ b/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles +{ + internal static class NamingStylePreferencesExtensions + { + public static NamingStylePreferences AppendNamingStylePreferencesToFront(this NamingStylePreferences original, NamingStylePreferences newPreferences) + { + var symbolSpecifications = original.SymbolSpecifications.InsertRange(0, newPreferences.SymbolSpecifications); + var namingStyles = original.NamingStyles.InsertRange(0, newPreferences.NamingStyles); + var namingRules = original.NamingRules.InsertRange(0, newPreferences.NamingRules); + return new NamingStylePreferences(symbolSpecifications, namingStyles, namingRules); + } + } +} diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index f84ed604eb319..5240e1e1a0d01 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -47,15 +47,24 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation { if (type == typeof(NamingStylePreferences)) { - var result = EditorConfigNamingStyleParser.GetNamingStylesFromDictionary(dictionary); - if (!result.NamingRules.Any() && - !result.NamingStyles.Any() && - !result.SymbolSpecifications.Any()) + var editorconfigNamingStylePreferences = EditorConfigNamingStyleParser.GetNamingStylesFromDictionary(dictionary); + + if (!editorconfigNamingStylePreferences.NamingRules.Any() && + !editorconfigNamingStylePreferences.NamingStyles.Any() && + !editorconfigNamingStylePreferences.SymbolSpecifications.Any()) + { + // We were not able to parse any rules from editorconfig, tell the caller that the parse failed + return (result: editorconfigNamingStylePreferences, succeeded: false); + } + else if (underlyingOption is NamingStylePreferences workspaceNamingStylePreferences) { - return (result: result, succeeded: false); + // We parsed naming styles from editorconfig, append them to our existing styles + var combinedNamingStylePreferences = workspaceNamingStylePreferences.AppendNamingStylePreferencesToFront(editorconfigNamingStylePreferences); + return (result: combinedNamingStylePreferences, succeeded: true); } - return (result: result, succeeded: true); + // no existing naming styles were passed so just return the set of styles that were parsed from editorconfig + return (result: editorconfigNamingStylePreferences, succeeded: true); } else { @@ -63,7 +72,7 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation } }; - public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRawConventions, Type type, out object result) + public bool TryGetOption(IReadOnlyDictionary allRawConventions, Type type, out object result) { if (_parseValue != null && KeyName != null) { @@ -75,7 +84,7 @@ public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRa } else if (_tryParseDictionary != null) { - var tuple = _tryParseDictionary(allRawConventions, type); + var tuple = _tryParseDictionary(allRawConventions, type, underlyingOption); result = tuple.result; return tuple.succeeded; } @@ -107,7 +116,7 @@ public EditorConfigStorageLocation() public EditorConfigStorageLocation(Func, (object result, bool succeeded)> tryParseDictionary) { // If we're explicitly given a parsing function we can throw away the type when parsing - _tryParseDictionary = (dictionary, type) => tryParseDictionary(dictionary); + _tryParseDictionary = (dictionary, type, underlyingOption) => tryParseDictionary(dictionary); } } } diff --git a/src/Workspaces/Core/Portable/Options/IDocumentOptions.cs b/src/Workspaces/Core/Portable/Options/IDocumentOptions.cs index 9119bd28a6183..519c765f7eba4 100644 --- a/src/Workspaces/Core/Portable/Options/IDocumentOptions.cs +++ b/src/Workspaces/Core/Portable/Options/IDocumentOptions.cs @@ -11,6 +11,6 @@ namespace Microsoft.CodeAnalysis.Options /// interface IDocumentOptions { - bool TryGetDocumentOption(Document document, OptionKey option, out object value); + bool TryGetDocumentOption(Document document, OptionKey option, OptionSet underlyingOptions, out object value); } } diff --git a/src/Workspaces/Core/Portable/Options/OptionServiceFactory.cs b/src/Workspaces/Core/Portable/Options/OptionServiceFactory.cs index ab59d900866c1..0ebf494df90fa 100644 --- a/src/Workspaces/Core/Portable/Options/OptionServiceFactory.cs +++ b/src/Workspaces/Core/Portable/Options/OptionServiceFactory.cs @@ -188,7 +188,7 @@ public override object GetOption(OptionKey optionKey) foreach (var documentOptionSource in _documentOptions) { - if (documentOptionSource.TryGetDocumentOption(_document, optionKey, out value)) + if (documentOptionSource.TryGetDocumentOption(_document, optionKey, _underlyingOptions, out value)) { // Cache and return lock (_gate) diff --git a/src/Workspaces/Core/Portable/Workspaces.csproj b/src/Workspaces/Core/Portable/Workspaces.csproj index 41c481b063f5d..1e3fe422d3a61 100644 --- a/src/Workspaces/Core/Portable/Workspaces.csproj +++ b/src/Workspaces/Core/Portable/Workspaces.csproj @@ -383,6 +383,7 @@ + diff --git a/src/Workspaces/CoreTest/EditorConfigStorageLocation/EditorConfigStorageLocationTests.cs b/src/Workspaces/CoreTest/EditorConfigStorageLocation/EditorConfigStorageLocationTests.cs index 19dbd1c311bdf..8d81e9a5b191e 100644 --- a/src/Workspaces/CoreTest/EditorConfigStorageLocation/EditorConfigStorageLocationTests.cs +++ b/src/Workspaces/CoreTest/EditorConfigStorageLocation/EditorConfigStorageLocationTests.cs @@ -2,29 +2,87 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles; +using Microsoft.CodeAnalysis.NamingStyles; using Microsoft.CodeAnalysis.Options; using Xunit; +using static Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles.EditorConfigNamingStyleParser; namespace Microsoft.CodeAnalysis.UnitTests.EditorConfig.StorageLocation { public class EditorConfigStorageLocationTests { [Fact] - public static void TestEmptyDictionaryReturnFalse() + public static void TestEmptyDictionaryReturnNoNamingStylePreferencesObjectReturnsFalse() { var editorConfigStorageLocation = new EditorConfigStorageLocation(); - var result = editorConfigStorageLocation.TryParseReadonlyDictionary(new Dictionary(), typeof(NamingStylePreferences), out var @object); + var result = editorConfigStorageLocation.TryGetOption(new object(), new Dictionary(), typeof(NamingStylePreferences), out var @object); Assert.False(result, "Expected TryParseReadonlyDictionary to return 'false' for empty dictionary"); } + [Fact] + public static void TestEmptyDictionaryDefaultNamingStylePreferencesObjectReturnsFalse() + { + var editorConfigStorageLocation = new EditorConfigStorageLocation(); + var existingNamingStylePreferences = new NamingStylePreferences( + ImmutableArray.Create(), + ImmutableArray.Create(), + ImmutableArray.Create()); + + var result = editorConfigStorageLocation.TryGetOption( + existingNamingStylePreferences, + new Dictionary(), + typeof(NamingStylePreferences), + out var @object); + + Assert.False(result, "Expected TryParseReadonlyDictionary to return 'false' for empty dictionary"); + } + + [Fact] + public static void TestNonEmptyDictionaryReturnsTrue() + { + var initialDictionary = new Dictionary() + { + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.severity"] = "warning", + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.symbols"] = "method_and_property_symbols", + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.style"] = "pascal_case_style", + ["dotnet_naming_symbols.method_and_property_symbols.applicable_kinds"] = "method,property", + ["dotnet_naming_symbols.method_and_property_symbols.applicable_accessibilities"] = "*", + ["dotnet_naming_style.pascal_case_style.capitalization"] = "pascal_case" + }; + var existingNamingStylePreferences = ParseDictionary(initialDictionary); + + var editorConfigStorageLocation = new EditorConfigStorageLocation(); + var newDictionary = new Dictionary() + { + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.severity"] = "error", + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.symbols"] = "method_and_property_symbols", + ["dotnet_naming_rule.methods_and_properties_must_be_pascal_case.style"] = "pascal_case_style", + ["dotnet_naming_symbols.method_and_property_symbols.applicable_kinds"] = "method,property", + ["dotnet_naming_symbols.method_and_property_symbols.applicable_accessibilities"] = "*", + ["dotnet_naming_style.pascal_case_style.capitalization"] = "pascal_case" + }; + + var result = editorConfigStorageLocation.TryGetOption( + existingNamingStylePreferences, + newDictionary, + typeof(NamingStylePreferences), + out var combinedNamingStyles); + + Assert.True(result, "Expected non-empty dictionary to return true"); + var isNamingStylePreferencesObject = combinedNamingStyles is NamingStylePreferences; + Assert.True(isNamingStylePreferencesObject, $"Expected returned object to be of type '{nameof(NamingStylePreferences)}'"); + Assert.Equal(DiagnosticSeverity.Error, ((NamingStylePreferences)combinedNamingStyles).Rules.NamingRules[0].EnforcementLevel); + } + [Fact] public static void TestObjectTypeThrowsNotSupportedException() { var editorConfigStorageLocation = new EditorConfigStorageLocation(); Assert.Throws(() => { - editorConfigStorageLocation.TryParseReadonlyDictionary(new Dictionary(), typeof(object), out var @object); + editorConfigStorageLocation.TryGetOption(new object(), new Dictionary(), typeof(object), out var @object); }); } } diff --git a/src/Workspaces/CoreTest/ServicesTest.csproj b/src/Workspaces/CoreTest/ServicesTest.csproj index 2cce678826625..7e3220c33dbf2 100644 --- a/src/Workspaces/CoreTest/ServicesTest.csproj +++ b/src/Workspaces/CoreTest/ServicesTest.csproj @@ -68,10 +68,10 @@ - + From 2f07e1d9543b1da73872eb04b0eb10b15f04ee2c Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Fri, 24 Feb 2017 14:24:57 -0800 Subject: [PATCH 2/2] renaming method --- .../NamingStylePreferencesExtensions.cs | 2 +- .../Options/EditorConfigStorageLocation.cs | 20 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs b/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs index 39500dabe3069..ded67addb1e08 100644 --- a/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs +++ b/src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferencesExtensions.cs @@ -4,7 +4,7 @@ namespace Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles { internal static class NamingStylePreferencesExtensions { - public static NamingStylePreferences AppendNamingStylePreferencesToFront(this NamingStylePreferences original, NamingStylePreferences newPreferences) + public static NamingStylePreferences PrependNamingStylePreferences(this NamingStylePreferences original, NamingStylePreferences newPreferences) { var symbolSpecifications = original.SymbolSpecifications.InsertRange(0, newPreferences.SymbolSpecifications); var namingStyles = original.NamingStyles.InsertRange(0, newPreferences.NamingStyles); diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index 5240e1e1a0d01..5a7749563e17e 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -41,9 +41,9 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation } }; - private Func, Type, (object result, bool succeeded)> _tryParseDictionary; + private Func, Type, (object result, bool succeeded)> _tryParseDictionary; - private static Func, Type, (object result, bool succeeded)> _cachedTryParseDictionary = (dictionary, type) => + private static Func, Type, (object result, bool succeeded)> _cachedTryParseDictionary = (underlyingOption, dictionary, type) => { if (type == typeof(NamingStylePreferences)) { @@ -56,11 +56,13 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation // We were not able to parse any rules from editorconfig, tell the caller that the parse failed return (result: editorconfigNamingStylePreferences, succeeded: false); } - else if (underlyingOption is NamingStylePreferences workspaceNamingStylePreferences) + + var workspaceNamingStylePreferences = underlyingOption as NamingStylePreferences; + if (workspaceNamingStylePreferences != null) { // We parsed naming styles from editorconfig, append them to our existing styles - var combinedNamingStylePreferences = workspaceNamingStylePreferences.AppendNamingStylePreferencesToFront(editorconfigNamingStylePreferences); - return (result: combinedNamingStylePreferences, succeeded: true); + var combinedNamingStylePreferences = workspaceNamingStylePreferences.PrependNamingStylePreferences(editorconfigNamingStylePreferences); + return (result: (object)combinedNamingStylePreferences, succeeded: true); } // no existing naming styles were passed so just return the set of styles that were parsed from editorconfig @@ -72,7 +74,7 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation } }; - public bool TryGetOption(IReadOnlyDictionary allRawConventions, Type type, out object result) + public bool TryGetOption(object underlyingOption, IReadOnlyDictionary allRawConventions, Type type, out object result) { if (_parseValue != null && KeyName != null) { @@ -84,7 +86,7 @@ public bool TryGetOption(IReadOnlyDictionary allRawConventions, } else if (_tryParseDictionary != null) { - var tuple = _tryParseDictionary(allRawConventions, type, underlyingOption); + var tuple = _tryParseDictionary(underlyingOption, allRawConventions, type); result = tuple.result; return tuple.succeeded; } @@ -113,10 +115,10 @@ public EditorConfigStorageLocation() _tryParseDictionary = _cachedTryParseDictionary; } - public EditorConfigStorageLocation(Func, (object result, bool succeeded)> tryParseDictionary) + public EditorConfigStorageLocation(Func, (object result, bool succeeded)> tryParseDictionary) { // If we're explicitly given a parsing function we can throw away the type when parsing - _tryParseDictionary = (dictionary, type, underlyingOption) => tryParseDictionary(dictionary); + _tryParseDictionary = (underlyingOption, dictionary, type) => tryParseDictionary(underlyingOption, dictionary); } } }