From 6a78772568401b2eaa89ad91f035ddd53d68c6b7 Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Fri, 10 Feb 2017 21:34:27 -0800 Subject: [PATCH 1/4] ensure that TryParse is used for all user input in editorconfig --- .../CodeStyle/CSharpCodeStyleOptions.cs | 26 +++++++++---------- .../Options/EditorConfigStorageLocation.cs | 16 ++++++++++-- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs b/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs index 4bc1635a6e49f..fd84d16e021ea 100644 --- a/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs +++ b/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs @@ -12,79 +12,79 @@ internal static class CSharpCodeStyleOptions public static readonly Option> UseImplicitTypeForIntrinsicTypes = new Option>( nameof(CodeStyleOptions), nameof(UseImplicitTypeForIntrinsicTypes), defaultValue: CodeStyleOption.Default, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_var_for_built_in_types", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_var_for_built_in_types"), new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.UseImplicitTypeForIntrinsicTypes")}); public static readonly Option> UseImplicitTypeWhereApparent = new Option>( nameof(CodeStyleOptions), nameof(UseImplicitTypeWhereApparent), defaultValue: CodeStyleOption.Default, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_var_when_type_is_apparent", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_var_when_type_is_apparent"), new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.UseImplicitTypeWhereApparent")}); public static readonly Option> UseImplicitTypeWherePossible = new Option>( nameof(CodeStyleOptions), nameof(UseImplicitTypeWherePossible), defaultValue: CodeStyleOption.Default, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_var_elsewhere", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_var_elsewhere"), new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.UseImplicitTypeWherePossible")}); public static readonly Option> PreferConditionalDelegateCall = new Option>( nameof(CodeStyleOptions), nameof(PreferConditionalDelegateCall), defaultValue: CodeStyleOptions.TrueWithSuggestionEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_conditional_delegate_call", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_conditional_delegate_call"), new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.PreferConditionalDelegateCall")}); public static readonly Option> PreferPatternMatchingOverAsWithNullCheck = new Option>( nameof(CodeStyleOptions), nameof(PreferPatternMatchingOverAsWithNullCheck), defaultValue: CodeStyleOptions.TrueWithSuggestionEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_pattern_matching_over_as_with_null_check", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_pattern_matching_over_as_with_null_check"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferPatternMatchingOverAsWithNullCheck)}")}); public static readonly Option> PreferPatternMatchingOverIsWithCastCheck = new Option>( nameof(CodeStyleOptions), nameof(PreferPatternMatchingOverIsWithCastCheck), defaultValue: CodeStyleOptions.TrueWithSuggestionEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_pattern_matching_over_is_with_cast_check", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_pattern_matching_over_is_with_cast_check"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferPatternMatchingOverIsWithCastCheck)}")}); public static readonly Option> PreferExpressionBodiedConstructors = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedConstructors), defaultValue: CodeStyleOptions.FalseWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_constructors", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_constructors"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedConstructors)}")}); public static readonly Option> PreferExpressionBodiedMethods = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedMethods), defaultValue: CodeStyleOptions.FalseWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_methods", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_methods"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedMethods)}")}); public static readonly Option> PreferExpressionBodiedOperators = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedOperators), defaultValue: CodeStyleOptions.FalseWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_operators", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_operators"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedOperators)}")}); public static readonly Option> PreferExpressionBodiedProperties = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedProperties), defaultValue: CodeStyleOptions.TrueWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_properties", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_properties"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedProperties)}")}); public static readonly Option> PreferExpressionBodiedIndexers = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedIndexers), defaultValue: CodeStyleOptions.TrueWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_indexers", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_indexers"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedIndexers)}")}); public static readonly Option> PreferExpressionBodiedAccessors = new Option>( nameof(CodeStyleOptions), nameof(PreferExpressionBodiedAccessors), defaultValue: CodeStyleOptions.TrueWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_style_expression_bodied_accessors", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_style_expression_bodied_accessors"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferExpressionBodiedAccessors)}")}); public static readonly Option> PreferBraces = new Option>( nameof(CodeStyleOptions), nameof(PreferBraces), defaultValue: CodeStyleOptions.TrueWithNoneEnforcement, storageLocations: new OptionStorageLocation[] { - new EditorConfigStorageLocation("csharp_prefer_braces", ParseEditorConfigCodeStyleOption), + new EditorConfigStorageLocation("csharp_prefer_braces"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferBraces)}")}); public static IEnumerable>> GetCodeStyleOptions() diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index 1e049161ddbef..2654170fe06d4 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -27,6 +27,10 @@ public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRa if (allRawConventions.TryGetValue(KeyName, out object value)) { result = _parseValue(value.ToString(), type); + if (result == null) + { + return false; + } return true; } } @@ -49,11 +53,19 @@ public EditorConfigStorageLocation(string keyName) { if (type == typeof(int)) { - return int.Parse(s); + if (int.TryParse(s, out var value)) + { + return value; + } + return null; } else if (type == typeof(bool)) { - return bool.Parse(s); + if (bool.TryParse(s, out var value)) + { + return value; + } + return null; } else if (type == typeof(CodeStyleOption)) { From f68b04ceb4d2789f112d268c842b269153e090f7 Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Thu, 23 Feb 2017 14:50:01 -0800 Subject: [PATCH 2/4] small tweaks based on Cyrus' feedback --- .../Options/EditorConfigStorageLocation.cs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index 2654170fe06d4..c58535a654a3d 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -27,11 +27,7 @@ public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRa if (allRawConventions.TryGetValue(KeyName, out object value)) { result = _parseValue(value.ToString(), type); - if (result == null) - { - return false; - } - return true; + return result != null; } } else if (_tryParseDictionary != null) @@ -53,19 +49,11 @@ public EditorConfigStorageLocation(string keyName) { if (type == typeof(int)) { - if (int.TryParse(s, out var value)) - { - return value; - } - return null; + return int.TryParse(s, out var value) ? (object)value : null; } else if (type == typeof(bool)) { - if (bool.TryParse(s, out var value)) - { - return value; - } - return null; + return bool.TryParse(s, out var value) ? (object)value : null; } else if (type == typeof(CodeStyleOption)) { From c3ab8f0ce70b22f9c9e27a268609252c727e63f4 Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Fri, 24 Feb 2017 13:07:45 -0800 Subject: [PATCH 3/4] cache delegates --- .../Options/EditorConfigStorageLocation.cs | 83 ++++++++++--------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index c58535a654a3d..c29132115f42a 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -18,8 +18,50 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation private Func _parseValue; + private static Func _cachedParseValue = (s, type) => + { + if (type == typeof(int)) + { + var value = 0; + return int.TryParse(s, out value) ? (object)value : null; + } + else if (type == typeof(bool)) + { + var value = false; + return bool.TryParse(s, out value) ? (object)value : null; + } + else if (type == typeof(CodeStyleOption)) + { + return ParseEditorConfigCodeStyleOption(s); + } + else + { + throw new NotSupportedException(WorkspacesResources.Option_0_has_an_unsupported_type_to_use_with_1_You_should_specify_a_parsing_function); + } + }; + private Func, Type, (object result, bool succeeded)> _tryParseDictionary; + private static Func, Type, (object result, bool succeeded)> _cachedTryParseDictionary = (dictionary, type) => + { + if (type == typeof(NamingStylePreferences)) + { + var result = EditorConfigNamingStyleParser.GetNamingStylesFromDictionary(dictionary); + if (!result.NamingRules.Any() && + !result.NamingStyles.Any() && + !result.SymbolSpecifications.Any()) + { + return (result: result, succeeded: false); + } + + return (result: result, succeeded: true); + } + else + { + throw new NotSupportedException(WorkspacesResources.Option_0_has_an_unsupported_type_to_use_with_1_You_should_specify_a_parsing_function); + } + }; + public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRawConventions, Type type, out object result) { if (_parseValue != null && KeyName != null) @@ -44,26 +86,7 @@ public bool TryParseReadonlyDictionary(IReadOnlyDictionary allRa public EditorConfigStorageLocation(string keyName) { KeyName = keyName; - - _parseValue = (s, type) => - { - if (type == typeof(int)) - { - return int.TryParse(s, out var value) ? (object)value : null; - } - else if (type == typeof(bool)) - { - return bool.TryParse(s, out var value) ? (object)value : null; - } - else if (type == typeof(CodeStyleOption)) - { - return ParseEditorConfigCodeStyleOption(s); - } - else - { - throw new NotSupportedException(WorkspacesResources.Option_0_has_an_unsupported_type_to_use_with_1_You_should_specify_a_parsing_function); - } - }; + _parseValue = _cachedParseValue; } public EditorConfigStorageLocation(string keyName, Func parseValue) @@ -77,25 +100,7 @@ public EditorConfigStorageLocation(string keyName, Func parseVal public EditorConfigStorageLocation() { // If the user didn't pass a keyName assume we need to parse the entire dictionary - _tryParseDictionary = (dictionary, type) => - { - if (type == typeof(NamingStylePreferences)) - { - var result = EditorConfigNamingStyleParser.GetNamingStylesFromDictionary(dictionary); - if (!result.NamingRules.Any() && - !result.NamingStyles.Any() && - !result.SymbolSpecifications.Any()) - { - return (result: result, succeeded: false); - } - - return (result: result, succeeded: true); - } - else - { - throw new NotSupportedException(WorkspacesResources.Option_0_has_an_unsupported_type_to_use_with_1_You_should_specify_a_parsing_function); - } - }; + _tryParseDictionary = _cachedTryParseDictionary; } public EditorConfigStorageLocation(Func, (object result, bool succeeded)> tryParseDictionary) From e1c4a0b12d24c7c59a28b9aed5ccd469a57f82c5 Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Fri, 24 Feb 2017 14:11:46 -0800 Subject: [PATCH 4/4] add try parse methods for CodeStyleOption --- .../Portable/CodeStyle/CodeStyleHelpers.cs | 30 ++++++++++++------- .../Options/EditorConfigStorageLocation.cs | 3 +- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeStyle/CodeStyleHelpers.cs b/src/Workspaces/Core/Portable/CodeStyle/CodeStyleHelpers.cs index 8e9586ecfab30..87507627fef52 100644 --- a/src/Workspaces/Core/Portable/CodeStyle/CodeStyleHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeStyle/CodeStyleHelpers.cs @@ -5,37 +5,47 @@ namespace Microsoft.CodeAnalysis.CodeStyle internal static class CodeStyleHelpers { public static CodeStyleOption ParseEditorConfigCodeStyleOption(string arg) + { + TryParseEditorConfigCodeStyleOption(arg, out var option); + return option; + } + + public static bool TryParseEditorConfigCodeStyleOption(string arg, out CodeStyleOption option) { var args = arg.Split(':'); - bool isEnabled = false; + var isEnabled = false; if (args.Length != 2) { if (args.Length == 1) { if (bool.TryParse(args[0].Trim(), out isEnabled) && !isEnabled) { - return new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); + option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); + return true; } else { - return CodeStyleOption.Default; + option = CodeStyleOption.Default; + return false; } } - return CodeStyleOption.Default; + option = CodeStyleOption.Default; + return false; } if (!bool.TryParse(args[0].Trim(), out isEnabled)) { - return CodeStyleOption.Default; + option = CodeStyleOption.Default; + return false; } switch (args[1].Trim()) { - case EditorConfigSeverityStrings.Silent: return new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); - case EditorConfigSeverityStrings.Suggestion: return new CodeStyleOption(value: isEnabled, notification: NotificationOption.Suggestion); - case EditorConfigSeverityStrings.Warning: return new CodeStyleOption(value: isEnabled, notification: NotificationOption.Warning); - case EditorConfigSeverityStrings.Error: return new CodeStyleOption(value: isEnabled, notification: NotificationOption.Error); - default: return new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); + case EditorConfigSeverityStrings.Silent: option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); return true; + case EditorConfigSeverityStrings.Suggestion: option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.Suggestion); return true; + case EditorConfigSeverityStrings.Warning: option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.Warning); return true; + case EditorConfigSeverityStrings.Error: option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.Error); return true; + default: option = new CodeStyleOption(value: isEnabled, notification: NotificationOption.None); return false; } } } diff --git a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs index c29132115f42a..f84ed604eb319 100644 --- a/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs +++ b/src/Workspaces/Core/Portable/Options/EditorConfigStorageLocation.cs @@ -32,7 +32,8 @@ internal sealed class EditorConfigStorageLocation : OptionStorageLocation } else if (type == typeof(CodeStyleOption)) { - return ParseEditorConfigCodeStyleOption(s); + var value = CodeStyleOption.Default; + return TryParseEditorConfigCodeStyleOption(s, out value) ? value : null; } else {