Skip to content

Commit f89c700

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Fix layering of C# formatting options (#79083)
2 parents 02db999 + f104982 commit f89c700

File tree

21 files changed

+168
-70
lines changed

21 files changed

+168
-70
lines changed

‎src/Features/ExternalAccess/OmniSharp.CSharp/Formatting/OmniSharpSyntaxFormattingOptionsFactory.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public static OmniSharpSyntaxFormattingOptionsWrapper Create(
108108
(spaceBeforeComma ? SpacePlacement.BeforeComma : 0) |
109109
(spaceAfterDot ? SpacePlacement.AfterDot : 0) |
110110
(spaceBeforeDot ? SpacePlacement.BeforeDot : 0),
111-
SpacingAroundBinaryOperator = (BinaryOperatorSpacingOptions)spacingAroundBinaryOperator,
111+
SpacingAroundBinaryOperator = (BinaryOperatorSpacingOptionsInternal)spacingAroundBinaryOperator,
112112
NewLines =
113113
(newLineForMembersInObjectInit ? NewLinePlacement.BeforeMembersInObjectInitializers : 0) |
114114
(newLineForMembersInAnonymousTypes ? NewLinePlacement.BeforeMembersInAnonymousTypes : 0) |
@@ -125,7 +125,7 @@ public static OmniSharpSyntaxFormattingOptionsWrapper Create(
125125
(newLinesForBracesInLambdaExpressionBody ? NewLinePlacement.BeforeOpenBraceInLambdaExpressionBody : 0) |
126126
(newLinesForBracesInControlBlocks ? NewLinePlacement.BeforeOpenBraceInControlBlocks : 0) |
127127
(newLineForClausesInQuery ? NewLinePlacement.BetweenQueryExpressionClauses : 0),
128-
LabelPositioning = (LabelPositionOptions)labelPositioning,
128+
LabelPositioning = (LabelPositionOptionsInternal)labelPositioning,
129129
Indentation =
130130
(indentBraces ? IndentationPlacement.Braces : 0) |
131131
(indentBlock ? IndentationPlacement.BlockContents : 0) |

‎src/Tools/ExternalAccess/Razor/Features/RazorCSharpSyntaxFormattingOptions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ public CSharpSyntaxFormattingOptions ToCSharpSyntaxFormattingOptions()
4949
=> new()
5050
{
5151
Spacing = (SpacePlacement)Spacing,
52-
SpacingAroundBinaryOperator = (BinaryOperatorSpacingOptions)SpacingAroundBinaryOperator,
52+
SpacingAroundBinaryOperator = (BinaryOperatorSpacingOptionsInternal)SpacingAroundBinaryOperator,
5353
NewLines = (NewLinePlacement)NewLines,
54-
LabelPositioning = (LabelPositionOptions)LabelPositioning,
54+
LabelPositioning = (LabelPositionOptionsInternal)LabelPositioning,
5555
Indentation = (IndentationPlacement)Indentation,
5656
WrappingKeepStatementsOnSingleLine = WrappingKeepStatementsOnSingleLine,
5757
WrappingPreserveSingleLine = WrappingPreserveSingleLine,

‎src/VisualStudio/CSharp/Impl/Options/AutomationObject/AutomationObject.Formatting.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ public int Indent_CaseLabels
4242
public int Indent_FlushLabelsLeft
4343
{
4444
get { return (int)GetOption(CSharpFormattingOptions2.LabelPositioning); }
45-
set { SetOption(CSharpFormattingOptions2.LabelPositioning, (LabelPositionOptions)value); }
45+
set { SetOption(CSharpFormattingOptions2.LabelPositioning, (LabelPositionOptionsInternal)value); }
4646
}
4747

4848
public int Indent_UnindentLabels
4949
{
5050
get { return (int)GetOption(CSharpFormattingOptions2.LabelPositioning); }
51-
set { SetOption(CSharpFormattingOptions2.LabelPositioning, (LabelPositionOptions)value); }
51+
set { SetOption(CSharpFormattingOptions2.LabelPositioning, (LabelPositionOptionsInternal)value); }
5252
}
5353

5454
public int NewLines_AnonymousTypeInitializer_EachMember
@@ -186,7 +186,7 @@ public int Space_AfterSemicolonsInForStatement
186186
public int Space_AroundBinaryOperator
187187
{
188188
get { return (int)GetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator); }
189-
set { SetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator, (BinaryOperatorSpacingOptions)value); }
189+
set { SetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator, (BinaryOperatorSpacingOptionsInternal)value); }
190190
}
191191

192192
public int Space_BeforeBasesColon
@@ -282,7 +282,7 @@ public int Space_WithinSquares
282282
public int Wrapping_IgnoreSpacesAroundBinaryOperators
283283
{
284284
get { return (int)GetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator); }
285-
set { SetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator, (BinaryOperatorSpacingOptions)value); }
285+
set { SetOption(CSharpFormattingOptions2.SpacingAroundBinaryOperator, (BinaryOperatorSpacingOptionsInternal)value); }
286286
}
287287

288288
public int Wrapping_IgnoreSpacesAroundVariableDeclaration

‎src/VisualStudio/CSharp/Impl/Options/Formatting/IndentationViewModel.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ public IndentationViewModel(OptionStore optionStore, IServiceProvider servicePro
8686

8787
Items.Add(new TextBlock() { Text = CSharpVSResources.Label_Indentation });
8888

89-
Items.Add(new RadioButtonViewModel<LabelPositionOptions>(CSharpVSResources.Place_goto_labels_in_leftmost_column, GotoLabelPreview, "goto", LabelPositionOptions.LeftMost, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
90-
Items.Add(new RadioButtonViewModel<LabelPositionOptions>(CSharpVSResources.Indent_labels_normally, GotoLabelPreview, "goto", LabelPositionOptions.NoIndent, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
91-
Items.Add(new RadioButtonViewModel<LabelPositionOptions>(CSharpVSResources.Place_goto_labels_one_indent_less_than_current, GotoLabelPreview, "goto", LabelPositionOptions.OneLess, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
89+
Items.Add(new RadioButtonViewModel<LabelPositionOptionsInternal>(CSharpVSResources.Place_goto_labels_in_leftmost_column, GotoLabelPreview, "goto", LabelPositionOptionsInternal.LeftMost, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
90+
Items.Add(new RadioButtonViewModel<LabelPositionOptionsInternal>(CSharpVSResources.Indent_labels_normally, GotoLabelPreview, "goto", LabelPositionOptionsInternal.NoIndent, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
91+
Items.Add(new RadioButtonViewModel<LabelPositionOptionsInternal>(CSharpVSResources.Place_goto_labels_one_indent_less_than_current, GotoLabelPreview, "goto", LabelPositionOptionsInternal.OneLess, CSharpFormattingOptions2.LabelPositioning, this, optionStore));
9292
}
9393
}

‎src/VisualStudio/CSharp/Impl/Options/Formatting/SpacingViewModel.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ public SpacingViewModel(OptionStore optionStore, IServiceProvider serviceProvide
150150

151151
Items.Add(new HeaderItemViewModel() { Header = CSharpVSResources.Set_spacing_for_operators });
152152

153-
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptions>(CSharpVSResources.Ignore_spaces_around_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptions.Ignore, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
154-
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptions>(CSharpVSResources.Remove_spaces_before_and_after_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptions.Remove, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
155-
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptions>(CSharpVSResources.Insert_space_before_and_after_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptions.Single, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
153+
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptionsInternal>(CSharpVSResources.Ignore_spaces_around_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptionsInternal.Ignore, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
154+
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptionsInternal>(CSharpVSResources.Remove_spaces_before_and_after_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptionsInternal.Remove, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
155+
Items.Add(new RadioButtonViewModel<BinaryOperatorSpacingOptionsInternal>(CSharpVSResources.Insert_space_before_and_after_binary_operators, s_expressionSpacingPreview, "binary", BinaryOperatorSpacingOptionsInternal.Single, CSharpFormattingOptions2.SpacingAroundBinaryOperator, this, OptionStore));
156156
}
157157
}

‎src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ public SpacePlacementInternalStorageMapping(IOption2 internalOption, SpacePlacem
133133
public static Option<bool> SpaceBeforeSemicolonsInForStatement { get; } = CSharpFormattingOptions2.SpaceBeforeSemicolonsInForStatement.ToPublicOption();
134134

135135
/// <inheritdoc cref="CSharpFormattingOptions2.SpacingAroundBinaryOperator"/>
136-
public static Option<BinaryOperatorSpacingOptions> SpacingAroundBinaryOperator { get; } = CSharpFormattingOptions2.SpacingAroundBinaryOperator.ToPublicOption();
136+
public static Option<BinaryOperatorSpacingOptions> SpacingAroundBinaryOperator { get; } =
137+
CSharpFormattingOptions2.SpacingAroundBinaryOperator.ToPublicOption().ConvertEnumOption<BinaryOperatorSpacingOptionsInternal, BinaryOperatorSpacingOptions>();
137138

138139
/// <inheritdoc cref="CSharpFormattingOptions2.IndentBraces"/>
139140
public static Option<bool> IndentBraces { get; } = CSharpFormattingOptions2.IndentBraces.ToPublicOption();
@@ -151,7 +152,8 @@ public SpacePlacementInternalStorageMapping(IOption2 internalOption, SpacePlacem
151152
public static Option<bool> IndentSwitchCaseSectionWhenBlock { get; } = CSharpFormattingOptions2.IndentSwitchCaseSectionWhenBlock.ToPublicOption();
152153

153154
/// <inheritdoc cref="CSharpFormattingOptions2.LabelPositioning"/>
154-
public static Option<LabelPositionOptions> LabelPositioning { get; } = CSharpFormattingOptions2.LabelPositioning.ToPublicOption();
155+
public static Option<LabelPositionOptions> LabelPositioning { get; } =
156+
CSharpFormattingOptions2.LabelPositioning.ToPublicOption().ConvertEnumOption<LabelPositionOptionsInternal, LabelPositionOptions>();
155157

156158
/// <inheritdoc cref="CSharpFormattingOptions2.WrappingPreserveSingleLine"/>
157159
public static Option<bool> WrappingPreserveSingleLine { get; } = CSharpFormattingOptions2.WrappingPreserveSingleLine.ToPublicOption();
@@ -187,3 +189,27 @@ public SpacePlacementInternalStorageMapping(IOption2 internalOption, SpacePlacem
187189
/// <inheritdoc cref="CSharpFormattingOptions2.NewLineForClausesInQuery"/>
188190
public static Option<bool> NewLineForClausesInQuery { get; } = CSharpFormattingOptions2.NewLineForClausesInQuery.ToPublicOption();
189191
}
192+
193+
public enum LabelPositionOptions
194+
{
195+
/// Placed in the Zeroth column of the text editor
196+
LeftMost = LabelPositionOptionsInternal.LeftMost,
197+
198+
/// Placed at one less indent to the current context
199+
OneLess = LabelPositionOptionsInternal.OneLess,
200+
201+
/// Placed at the same indent as the current context
202+
NoIndent = LabelPositionOptionsInternal.NoIndent,
203+
}
204+
205+
public enum BinaryOperatorSpacingOptions
206+
{
207+
/// Single Spacing
208+
Single = BinaryOperatorSpacingOptionsInternal.Single,
209+
210+
/// Ignore Formatting
211+
Ignore = BinaryOperatorSpacingOptionsInternal.Ignore,
212+
213+
/// Remove Spacing
214+
Remove = BinaryOperatorSpacingOptionsInternal.Remove,
215+
}

‎src/Workspaces/CSharpTest/CodeStyle/CSharpEditorConfigCodeStyleParserTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public sealed class CSharpEditorConfigCodeStyleParserTests
2525
[InlineData(" none ", BinaryOperatorSpacingOptions.Remove)]
2626
[InlineData(" before_and_after ", BinaryOperatorSpacingOptions.Single)]
2727
public void TestParseSpacingAroundBinaryOperator(string rawValue, BinaryOperatorSpacingOptions parsedValue)
28-
=> Assert.Equal(parsedValue, CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(rawValue));
28+
=> Assert.Equal(parsedValue, (BinaryOperatorSpacingOptions)CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(rawValue));
2929

3030
[Theory]
3131
[InlineData("flush_left", LabelPositionOptions.LeftMost)]
@@ -37,7 +37,7 @@ public void TestParseSpacingAroundBinaryOperator(string rawValue, BinaryOperator
3737
[InlineData(" no_change ", LabelPositionOptions.NoIndent)]
3838
[InlineData(" one_less_than_current ", LabelPositionOptions.OneLess)]
3939
public void TestParseLabelPositioning(string rawValue, LabelPositionOptions parsedValue)
40-
=> Assert.Equal(parsedValue, CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(rawValue));
40+
=> Assert.Equal(parsedValue, (LabelPositionOptions)CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(rawValue));
4141

4242
[Theory]
4343
[InlineData("false:none", (int)ExpressionBodyPreference.Never, ReportDiagnostic.Suppress)]

‎src/Workspaces/CSharpTest/Formatting/EditorConfigOptionParserTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal void TestParseSpacingWithinParenthesesList(string list, SpacePlacementW
3737
InlineData("before_and_after", BinaryOperatorSpacingOptions.Single)]
3838
public void TestParseEditorConfigSpacingAroundBinaryOperatorTrue(string value, BinaryOperatorSpacingOptions expectedResult)
3939
{
40-
Assert.True(CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(value) == expectedResult,
40+
Assert.True(CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(value) == (BinaryOperatorSpacingOptionsInternal)expectedResult,
4141
$"Expected option {value} to be parsed as set.");
4242
}
4343

@@ -47,7 +47,7 @@ public void TestParseEditorConfigSpacingAroundBinaryOperatorTrue(string value, B
4747
InlineData("before_and_after,ignore")]
4848
public void TestParseEditorConfigSpacingAroundBinaryOperatorFalse(string value)
4949
{
50-
Assert.True(CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(value) == BinaryOperatorSpacingOptions.Single,
50+
Assert.True(CSharpFormattingOptions2.ParseEditorConfigSpacingAroundBinaryOperator(value) == BinaryOperatorSpacingOptionsInternal.Single,
5151
$"Expected option {value} to be parsed as default option.");
5252
}
5353

@@ -57,7 +57,7 @@ public void TestParseEditorConfigSpacingAroundBinaryOperatorFalse(string value)
5757
InlineData("one_less_than_current", LabelPositionOptions.OneLess)]
5858
public void TestParseEditorConfigLabelPositioningTrue(string value, LabelPositionOptions expectedValue)
5959
{
60-
Assert.True(CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(value) == expectedValue,
60+
Assert.True(CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(value) == (LabelPositionOptionsInternal)expectedValue,
6161
$"Expected option {value} to be parsed as set.");
6262
}
6363

@@ -67,7 +67,7 @@ public void TestParseEditorConfigLabelPositioningTrue(string value, LabelPositio
6767
InlineData("one_less_thancurrent")]
6868
public void TestParseEditorConfigLabelPositioningFalse(string value)
6969
{
70-
Assert.True(CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(value) == LabelPositionOptions.NoIndent,
70+
Assert.True(CSharpFormattingOptions2.ParseEditorConfigLabelPositioning(value) == LabelPositionOptionsInternal.NoIndent,
7171
$"Expected option {value} to be parsed default");
7272
}
7373

‎src/Workspaces/Core/Portable/Options/Option.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,17 @@ namespace Microsoft.CodeAnalysis.Options;
1414
/// <inheritdoc cref="Option2{T}"/>
1515
public class Option<T> : IPublicOption
1616
{
17-
private readonly OptionDefinition<T> _optionDefinition;
17+
internal readonly OptionDefinition<T> OptionDefinition;
1818

1919
public string Feature { get; }
2020

21-
internal OptionGroup Group => _optionDefinition.Group;
21+
internal OptionGroup Group => OptionDefinition.Group;
2222

2323
public string Name { get; }
2424

25-
public T DefaultValue => (T)_optionDefinition.DefaultValue!;
25+
public T DefaultValue => (T)OptionDefinition.DefaultValue!;
2626

27-
public Type Type => _optionDefinition.Type;
27+
public Type Type => OptionDefinition.Type;
2828

2929
public ImmutableArray<OptionStorageLocation> StorageLocations { get; }
3030

@@ -75,7 +75,7 @@ internal Option(OptionDefinition<T> optionDefinition, string feature, string nam
7575
{
7676
Feature = feature;
7777
Name = name;
78-
_optionDefinition = optionDefinition;
78+
OptionDefinition = optionDefinition;
7979
StorageLocations = storageLocations;
8080
}
8181

@@ -85,13 +85,13 @@ internal Option(OptionDefinition<T> optionDefinition, string feature, string nam
8585

8686
IPublicOption? IOption2.PublicOption => null;
8787

88-
OptionDefinition IOption2.Definition => _optionDefinition;
88+
OptionDefinition IOption2.Definition => OptionDefinition;
8989

9090
bool IEquatable<IOption2?>.Equals(IOption2? other) => Equals(other);
9191

9292
public override string ToString() => this.PublicOptionDefinitionToString();
9393

94-
public override int GetHashCode() => _optionDefinition.GetHashCode();
94+
public override int GetHashCode() => OptionDefinition.GetHashCode();
9595

9696
public override bool Equals(object? obj) => Equals(obj as IOption2);
9797

‎src/Workspaces/Core/Portable/Options/OptionExtensions.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
#pragma warning disable RS0030 // Do not used banned APIs: Option<T>
6+
7+
using System;
8+
using Microsoft.CodeAnalysis.Shared.Utilities;
9+
10+
namespace Microsoft.CodeAnalysis.Options;
11+
12+
internal static class OptionExtensions
13+
{
14+
/// <summary>
15+
/// Allows an option of one enum type to be converted to another enum type, provided that both enums share the same underlying type.
16+
/// Useful for some cases in Roslyn where we have an existing shipped public option in the Workspace layer, and an internal option
17+
/// in the CodeStyle layer, and we want to map between them.
18+
/// </summary>
19+
public static Option<TToEnum> ConvertEnumOption<TFromEnum, TToEnum>(this Option<TFromEnum> option)
20+
where TFromEnum : struct, Enum
21+
where TToEnum : struct, Enum
22+
{
23+
var definition = option.OptionDefinition;
24+
var newDefaultValue = EnumValueUtilities.ConvertEnum<TFromEnum, TToEnum>(definition.DefaultValue);
25+
var newSerializer = EditorConfigValueSerializer.ConvertEnumSerializer<TFromEnum, TToEnum>(definition.Serializer);
26+
27+
var newDefinition = new OptionDefinition<TToEnum>(
28+
defaultValue: newDefaultValue, newSerializer, definition.Group, definition.ConfigName, definition.StorageMapping, definition.IsEditorConfigOption);
29+
30+
return new(newDefinition, option.Feature, option.Name, option.StorageLocations);
31+
}
32+
}

0 commit comments

Comments
 (0)