Skip to content

Commit

Permalink
Code style options cleanup (#73744)
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat authored Jun 5, 2024
1 parent 24d3887 commit d988414
Show file tree
Hide file tree
Showing 19 changed files with 91 additions and 179 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ protected AbstractBuiltInCodeStyleDiagnosticAnalyzer(ImmutableArray<(DiagnosticD
{
foreach (var (descriptor, option) in supportedDiagnosticsWithOptions)
{
Debug.Assert(option is { Definition.DefaultValue: ICodeStyleOption } == descriptor.CustomTags.Contains(WellKnownDiagnosticTags.CustomSeverityConfigurable));
Debug.Assert(option is { Definition.DefaultValue: ICodeStyleOption2 } == descriptor.CustomTags.Contains(WellKnownDiagnosticTags.CustomSeverityConfigurable));
AddDiagnosticIdToOptionMapping(descriptor.Id, option);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ void SymbolAction(SymbolAnalysisContext symbolContext)
symbolContext.Symbol,
sourceTree,
symbolContext.Options,
idToCachedResult,
symbolContext.CancellationToken);
idToCachedResult);

if (diagnostic != null)
{
Expand All @@ -97,8 +96,7 @@ void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
symbol,
sourceTree,
syntaxContext.Options,
idToCachedResult,
syntaxContext.CancellationToken);
idToCachedResult);

if (diagnostic != null)
{
Expand All @@ -115,8 +113,7 @@ void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
ISymbol symbol,
SyntaxTree sourceTree,
AnalyzerOptions options,
ConcurrentDictionary<Guid, ConcurrentDictionary<string, string?>> idToCachedResult,
CancellationToken cancellationToken)
ConcurrentDictionary<Guid, ConcurrentDictionary<string, string?>> idToCachedResult)
{
if (string.IsNullOrEmpty(symbol.Name))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal abstract class CodeStyleSetting(OptionKey2 optionKey, string descriptio
{
private static readonly bool[] s_boolValues = [true, false];

public abstract ICodeStyleOption GetCodeStyle();
public abstract ICodeStyleOption2 GetCodeStyle();

public ReportDiagnostic GetSeverity()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public override string[] GetValueDescriptions()
public override string GetCurrentValueDescription()
=> _valueDescriptions[_possibleValues.IndexOf(_value.Value)];

public override ICodeStyleOption GetCodeStyle()
public override ICodeStyleOption2 GetCodeStyle()
=> _value;

protected override object GetPossibleValue(int valueIndex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal static partial class SettingsUpdateHelper
var optionName = option.Definition.ConfigName;
var optionValue = option.Definition.Serializer.Serialize(value);

if (value is ICodeStyleOption codeStyleOption && !optionValue.Contains(':'))
if (value is ICodeStyleOption2 codeStyleOption && !optionValue.Contains(':'))
{
var severity = codeStyleOption.Notification.Severity switch
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,39 @@

Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles
Imports Microsoft.CodeAnalysis.NamingStyles

Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
Partial Public Class NamingStyleTests
Private Shared Function CreateNamingStyle(
Optional prefix As String = "",
Optional suffix As String = "",
Optional wordSeparator As String = "",
Optional capitalizationScheme As Capitalization = Capitalization.PascalCase) As MutableNamingStyle
Optional capitalizationScheme As Capitalization = Capitalization.PascalCase) As NamingStyle

Return New MutableNamingStyle With
Return New NamingStyle With
{
.Prefix = prefix,
.Suffix = suffix,
.WordSeparator = wordSeparator,
.CapitalizationScheme = capitalizationScheme
.prefix = prefix,
.suffix = suffix,
.wordSeparator = wordSeparator,
.capitalizationScheme = capitalizationScheme
}
End Function

Private Shared Sub TestNameCreation(namingStyle As MutableNamingStyle, expectedName As String, ParamArray words As String())
Assert.Equal(expectedName, namingStyle.NamingStyle.CreateName(words.ToImmutableArray()))
Private Shared Sub TestNameCreation(namingStyle As NamingStyle, expectedName As String, ParamArray words As String())
Assert.Equal(expectedName, namingStyle.CreateName(words.ToImmutableArray()))
End Sub

Private Shared Sub TestNameCompliance(namingStyle As MutableNamingStyle, candidateName As String)
Private Shared Sub TestNameCompliance(namingStyle As NamingStyle, candidateName As String)
Dim reason As String = Nothing
Assert.True(namingStyle.NamingStyle.IsNameCompliant(candidateName, reason))
Assert.True(namingStyle.IsNameCompliant(candidateName, reason))
End Sub

Private Shared Sub TestNameNoncomplianceAndFixedNames(namingStyle As MutableNamingStyle, candidateName As String, ParamArray expectedFixedNames As String())
Private Shared Sub TestNameNoncomplianceAndFixedNames(namingStyle As NamingStyle, candidateName As String, ParamArray expectedFixedNames As String())
Dim reason As String = Nothing
Assert.False(namingStyle.NamingStyle.IsNameCompliant(candidateName, reason))
Assert.False(namingStyle.IsNameCompliant(candidateName, reason))

Dim actualFixedNames = namingStyle.NamingStyle.MakeCompliant(candidateName).ToList()
Dim actualFixedNames = namingStyle.MakeCompliant(candidateName).ToList()

Assert.Equal(expectedFixedNames.Length, actualFixedNames.Count)
For i = 0 To expectedFixedNames.Length - 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ internal static ImmutableArray<IOption2> GetCodeStyleOptionsForDiagnostic(Diagno
if (IDEDiagnosticIdToOptionMappingHelper.TryGetMappedOptions(diagnostic.Id, project.Language, out var options))
{
return [.. from option in options
where option.DefaultValue is ICodeStyleOption
where option.DefaultValue is ICodeStyleOption2
orderby option.Definition.ConfigName
select option];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private static ImmutableArray<CodeFix> GetConfigurations(Project project, IEnume

// Try to get the parsed editorconfig string representation of the new code style option value
var optionName = option.Definition.ConfigName;
var defaultValue = (ICodeStyleOption?)option.DefaultValue;
var defaultValue = (ICodeStyleOption2?)option.DefaultValue;
Contract.ThrowIfNull(defaultValue);

if (defaultValue.Value is bool)
Expand Down Expand Up @@ -139,7 +139,7 @@ private static ImmutableArray<CodeFix> GetConfigurations(Project project, IEnume
return null;

// Local functions
void AddCodeActionWithOptionValue(ICodeStyleOption codeStyleOption, object newValue)
void AddCodeActionWithOptionValue(ICodeStyleOption2 codeStyleOption, object newValue)
{
// Create a new code style option value with the newValue
var configuredCodeStyleOption = codeStyleOption.WithValue(newValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public Task PersistAsync(string storageKey, object? value)
{
Contract.ThrowIfNull(_settingManager);

if (value is ICodeStyleOption codeStyleOption)
if (value is ICodeStyleOption2 codeStyleOption)
{
// We store these as strings, so serialize
value = codeStyleOption.ToXElement().ToString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ protected AbstractOptionPreviewViewModel(OptionStore optionStore, IServiceProvid
public void SetOptionAndUpdatePreview<T>(T value, IOption2 option, string preview)
{
object actualValue;
if (option.DefaultValue is ICodeStyleOption codeStyleOption)
if (option.DefaultValue is ICodeStyleOption2 codeStyleOption)
{
// The value provided is either an ICodeStyleOption OR the underlying ICodeStyleOption.Value
if (value is ICodeStyleOption newCodeStyleOption)
// The value provided is either an ICodeStyleOption2 OR the underlying ICodeStyleOption2.Value
if (value is ICodeStyleOption2 newCodeStyleOption)
{
actualValue = codeStyleOption.WithValue(newCodeStyleOption.Value).WithNotification(newCodeStyleOption.Notification);
}
Expand Down
21 changes: 13 additions & 8 deletions src/Workspaces/Core/Portable/CodeStyle/CodeStyleOption.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,27 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

#pragma warning disable RS0030 // Do not used banned APIs: CodeStyleOption<T>

using System;
using System.Xml.Linq;

namespace Microsoft.CodeAnalysis.CodeStyle;

internal interface ICodeStyleOption
{
XElement ToXElement();
object? Value { get; }
NotificationOption2 Notification { get; }
ICodeStyleOption WithValue(object value);
ICodeStyleOption WithNotification(NotificationOption2 notification);
}

/// <inheritdoc cref="CodeStyleOption2{T}"/>
public sealed class CodeStyleOption<T> : ICodeStyleOption, IEquatable<CodeStyleOption<T>>
{
private readonly CodeStyleOption2<T> _codeStyleOptionImpl;
public static CodeStyleOption<T> Default => new(default, NotificationOption.Silent);
public static CodeStyleOption<T> Default => new(default!, NotificationOption.Silent);

internal CodeStyleOption(CodeStyleOption2<T> codeStyleOptionImpl)
=> _codeStyleOptionImpl = codeStyleOptionImpl;
Expand All @@ -33,12 +40,10 @@ public T Value
set => throw new InvalidOperationException();
}

object ICodeStyleOption.Value => this.Value;
object? ICodeStyleOption.Value => this.Value;
NotificationOption2 ICodeStyleOption.Notification => _codeStyleOptionImpl.Notification;
ICodeStyleOption ICodeStyleOption.WithValue(object value) => new CodeStyleOption<T>((T)value, Notification);
ICodeStyleOption ICodeStyleOption.WithNotification(NotificationOption2 notification) => new CodeStyleOption<T>(Value, (NotificationOption)notification);
ICodeStyleOption ICodeStyleOption.AsInternalCodeStyleOption() => _codeStyleOptionImpl;
ICodeStyleOption ICodeStyleOption.AsPublicCodeStyleOption() => this;

public NotificationOption Notification
{
Expand All @@ -55,10 +60,10 @@ public NotificationOption Notification
public static CodeStyleOption<T> FromXElement(XElement element)
=> new(CodeStyleOption2<T>.FromXElement(element));

public bool Equals(CodeStyleOption<T> other)
public bool Equals(CodeStyleOption<T>? other)
=> _codeStyleOptionImpl.Equals(other?._codeStyleOptionImpl);

public override bool Equals(object obj)
public override bool Equals(object? obj)
=> obj is CodeStyleOption<T> option &&
Equals(option);

Expand Down
4 changes: 2 additions & 2 deletions src/Workspaces/Core/Portable/Options/OptionSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ bool IOptionsReader.TryGetOption<T>(OptionKey2 optionKey, out T value)
/// Checks if the value is an internal representation -- does not cover all cases, just code style options.
/// </summary>
internal static bool IsInternalOptionValue(object? value)
=> value is not ICodeStyleOption codeStyle || ReferenceEquals(codeStyle, codeStyle.AsInternalCodeStyleOption());
=> value is not ICodeStyleOption;

/// <summary>
/// Checks if the value is an public representation -- does not cover all cases, just code style options.
/// </summary>
internal static bool IsPublicOptionValue(object? value)
=> value is not ICodeStyleOption codeStyle || ReferenceEquals(codeStyle, codeStyle.AsPublicCodeStyleOption());
=> value is not ICodeStyleOption2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ _ when Nullable.GetUnderlyingType(type) is { IsEnum: true } underlying => value
ICodeStyleOption codeStyle => codeStyle
.WithValue(GetDifferentValue(codeStyle.GetType().GetGenericArguments()[0], codeStyle.Value!)!)
.WithNotification((codeStyle.Notification == NotificationOption2.Error) ? NotificationOption2.Warning : NotificationOption2.Error),
ICodeStyleOption2 codeStyle => codeStyle
.WithValue(GetDifferentValue(codeStyle.GetType().GetGenericArguments()[0], codeStyle.Value!)!)
.WithNotification((codeStyle.Notification == NotificationOption2.Error) ? NotificationOption2.Warning : NotificationOption2.Error),
NamingStylePreferences namingPreference => namingPreference.IsEmpty ? NamingStylePreferences.Default : NamingStylePreferences.Empty,
_ when type == typeof(bool?) => value is null ? true : null,
_ when type == typeof(int?) => value is null ? 1 : null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,19 @@

namespace Microsoft.CodeAnalysis.CodeStyle;

internal interface ICodeStyleOption
internal interface ICodeStyleOption2
{
XElement ToXElement();
object? Value { get; }
NotificationOption2 Notification { get; }
ICodeStyleOption WithValue(object value);
ICodeStyleOption WithNotification(NotificationOption2 notification);
#if !CODE_STYLE
ICodeStyleOption AsInternalCodeStyleOption();
ICodeStyleOption AsPublicCodeStyleOption();
#endif
}
ICodeStyleOption2 WithValue(object value);
ICodeStyleOption2 WithNotification(NotificationOption2 notification);

internal interface ICodeStyleOption2 : ICodeStyleOption
{
/// <summary>
/// Creates a new <see cref="ICodeStyleOption2"/> from a specified <paramref name="element"/>.
/// </summary>
/// <exception cref="Exception">
/// The type of the serialized data does not match the type of <see cref="ICodeStyleOption.Value"/> or the format of the serialized data is invalid.
/// The type of the serialized data does not match the type of <see cref="ICodeStyleOption2.Value"/> or the format of the serialized data is invalid.
/// </exception>
ICodeStyleOption2 FromXElement(XElement element);
}
Expand Down Expand Up @@ -93,16 +86,9 @@ internal sealed partial class CodeStyleOption2<T>(T value, NotificationOption2 n
[DataMember(Order = 1)]
public NotificationOption2 Notification { get; } = notification;

object? ICodeStyleOption.Value => this.Value;
ICodeStyleOption ICodeStyleOption.WithValue(object value) => WithValue((T)value);
ICodeStyleOption ICodeStyleOption.WithNotification(NotificationOption2 notification) => new CodeStyleOption2<T>(Value, notification);

#pragma warning disable RS0030 // Do not used banned APIs: CodeStyleOption<T>
#if !CODE_STYLE
ICodeStyleOption ICodeStyleOption.AsPublicCodeStyleOption() => new CodeStyleOption<T>(this);
ICodeStyleOption ICodeStyleOption.AsInternalCodeStyleOption() => this;
#endif
#pragma warning restore
object? ICodeStyleOption2.Value => this.Value;
ICodeStyleOption2 ICodeStyleOption2.WithValue(object value) => WithValue((T)value);
ICodeStyleOption2 ICodeStyleOption2.WithNotification(NotificationOption2 notification) => new CodeStyleOption2<T>(Value, notification);

public CodeStyleOption2<T> WithValue(T value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,6 @@
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\NamingStyle.WordSpanEnumerator.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\NamingStyleOptions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\NamingStyleRules.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\Serialization\AccessibilityExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\Serialization\MutableNamingStyle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\Serialization\NamingStylePreferences.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\Serialization\SerializableNamingRule.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NamingStyles\Serialization\SymbolSpecification.cs" />
Expand Down

This file was deleted.

Loading

0 comments on commit d988414

Please sign in to comment.