Skip to content

[WIP] Update Format Document to account for preferred modifier order #27943

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
Expand Up @@ -4,7 +4,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing
Expand All @@ -15,12 +15,14 @@ private class Rewriter : CSharpSyntaxRewriter
{
private readonly Func<SyntaxNode, IEnumerable<ISyntaxOrganizer>> _nodeToOrganizersGetter;
private readonly SemanticModel _semanticModel;
private readonly OptionSet _optionSet;
private readonly CancellationToken _cancellationToken;

public Rewriter(CSharpOrganizingService treeOrganizer, IEnumerable<ISyntaxOrganizer> organizers, SemanticModel semanticModel, CancellationToken cancellationToken)
public Rewriter(CSharpOrganizingService treeOrganizer, IEnumerable<ISyntaxOrganizer> organizers, SemanticModel semanticModel, OptionSet optionSet, CancellationToken cancellationToken)
{
_nodeToOrganizersGetter = treeOrganizer.GetNodeToOrganizers(organizers.ToList());
_semanticModel = semanticModel;
_optionSet = optionSet;
_cancellationToken = cancellationToken;
}

Expand All @@ -46,7 +48,7 @@ public override SyntaxNode Visit(SyntaxNode node)
var organizers = _nodeToOrganizersGetter(node);
foreach (var organizer in organizers)
{
node = organizer.OrganizeNode(_semanticModel, node, _cancellationToken);
node = organizer.OrganizeNode(_semanticModel, _optionSet, node, _cancellationToken);
}

return node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public CSharpOrganizingService(
protected override async Task<Document> ProcessAsync(Document document, IEnumerable<ISyntaxOrganizer> organizers, CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var rewriter = new Rewriter(this, organizers, await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false), cancellationToken);
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
var rewriter = new Rewriter(this, organizers, semanticModel, options, cancellationToken);
return document.WithSyntaxRoot(rewriter.Visit(root));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,11 +13,12 @@ internal class ClassDeclarationOrganizer : AbstractSyntaxNodeOrganizer<ClassDecl
{
protected override ClassDeclarationSyntax Organize(
ClassDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(
syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.Keyword,
syntax.Identifier,
syntax.TypeParameterList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,10 +13,11 @@ internal class ConstructorDeclarationOrganizer : AbstractSyntaxNodeOrganizer<Con
{
protected override ConstructorDeclarationSyntax Organize(
ConstructorDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.Identifier,
syntax.ParameterList,
syntax.Initializer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,10 +13,11 @@ internal class DestructorDeclarationOrganizer : AbstractSyntaxNodeOrganizer<Dest
{
protected override DestructorDeclarationSyntax Organize(
DestructorDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.TildeToken,
syntax.Identifier,
syntax.ParameterList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,11 +13,12 @@ internal class EnumDeclarationOrganizer : AbstractSyntaxNodeOrganizer<EnumDeclar
{
protected override EnumDeclarationSyntax Organize(
EnumDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(
syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.EnumKeyword,
syntax.Identifier,
syntax.BaseList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,10 +13,11 @@ internal class EventDeclarationOrganizer : AbstractSyntaxNodeOrganizer<EventDecl
{
protected override EventDeclarationSyntax Organize(
EventDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.EventKeyword,
syntax.Type,
syntax.ExplicitInterfaceSpecifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,10 +13,11 @@ internal class EventFieldDeclarationOrganizer : AbstractSyntaxNodeOrganizer<Even
{
protected override EventFieldDeclarationSyntax Organize(
EventFieldDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.EventKeyword,
syntax.Declaration,
syntax.SemicolonToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,10 +13,11 @@ internal class FieldDeclarationOrganizer : AbstractSyntaxNodeOrganizer<FieldDecl
{
protected override FieldDeclarationSyntax Organize(
FieldDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.Declaration,
syntax.SemicolonToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,11 +13,12 @@ internal class IndexerDeclarationOrganizer : AbstractSyntaxNodeOrganizer<Indexer
{
protected override IndexerDeclarationSyntax Organize(
IndexerDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(
attributeLists: syntax.AttributeLists,
modifiers: ModifiersOrganizer.Organize(syntax.Modifiers),
modifiers: ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
type: syntax.Type,
explicitInterfaceSpecifier: syntax.ExplicitInterfaceSpecifier,
thisKeyword: syntax.ThisKeyword,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,11 +13,12 @@ internal class InterfaceDeclarationOrganizer : AbstractSyntaxNodeOrganizer<Inter
{
protected override InterfaceDeclarationSyntax Organize(
InterfaceDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(
syntax.AttributeLists,
ModifiersOrganizer.Organize(syntax.Modifiers),
ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
syntax.Keyword,
syntax.Identifier,
syntax.TypeParameterList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Organizing.Organizers;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
Expand All @@ -12,11 +13,12 @@ internal class MethodDeclarationOrganizer : AbstractSyntaxNodeOrganizer<MethodDe
{
protected override MethodDeclarationSyntax Organize(
MethodDeclarationSyntax syntax,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return syntax.Update(
attributeLists: syntax.AttributeLists,
modifiers: ModifiersOrganizer.Organize(syntax.Modifiers),
modifiers: ModifiersOrganizer.ForCodeStyle(optionSet).Organize(syntax.Modifiers),
returnType: syntax.ReturnType,
explicitInterfaceSpecifier: syntax.ExplicitInterfaceSpecifier,
identifier: syntax.Identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,28 @@

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
{
internal partial class ModifiersOrganizer
internal partial class ModifiersOrganizer : IComparer<SyntaxToken>
{
private class Comparer : IComparer<SyntaxToken>
int IComparer<SyntaxToken>.Compare(SyntaxToken x, SyntaxToken y)
{
// TODO(cyrusn): Allow users to specify the ordering they want
private enum Ordering
return GetOrdering(x) - GetOrdering(y);
}

private int GetOrdering(SyntaxToken token)
{
if (_preferredOrder.TryGetValue(token.RawKind, out var order))
{
Accessibility,
StaticInstance,
Remainder
return order;
}

public int Compare(SyntaxToken x, SyntaxToken y)
else if (token.IsKind(SyntaxKind.PartialKeyword))
{
if (x.Kind() == y.Kind())
{
return 0;
}

// partial always goes last.
if (x.Kind() == SyntaxKind.PartialKeyword)
{
return 1;
}

if (y.Kind() == SyntaxKind.PartialKeyword)
{
return -1;
}

var xOrdering = GetOrdering(x);
var yOrdering = GetOrdering(y);

return xOrdering - yOrdering;
// 'partial' comes at the end unless explicitly specified in the code style option
return int.MaxValue;
}

private Ordering GetOrdering(SyntaxToken token)
else
{
switch (token.Kind())
{
case SyntaxKind.StaticKeyword:
return Ordering.StaticInstance;
case SyntaxKind.PrivateKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.InternalKeyword:
case SyntaxKind.PublicKeyword:
return Ordering.Accessibility;
default:
return Ordering.Remainder;
}
// other unspecified options come at the end except for the special case of 'partial'
return int.MaxValue - 1;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,44 @@

using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.OrderModifiers;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Organizing.Organizers
{
internal static partial class ModifiersOrganizer
internal partial class ModifiersOrganizer
{
public static SyntaxTokenList Organize(SyntaxTokenList modifiers)
private readonly Dictionary<int, int> _preferredOrder;

public ModifiersOrganizer(Dictionary<int, int> preferredOrder)
{
_preferredOrder = preferredOrder;
}

public static ModifiersOrganizer ForCodeStyle(OptionSet optionSet)
{
var option = optionSet.GetOption(CSharpCodeStyleOptions.PreferredModifierOrder);
if (!CSharpOrderModifiersHelper.Instance.TryGetOrComputePreferredOrder(option.Value, out var preferredOrder))
{
CSharpOrderModifiersHelper.Instance.TryGetOrComputePreferredOrder(CSharpCodeStyleOptions.PreferredModifierOrder.DefaultValue.Value, out preferredOrder);
}

return new ModifiersOrganizer(preferredOrder);
}

public SyntaxTokenList Organize(SyntaxTokenList modifiers)
{
if (modifiers.Count > 1 && !modifiers.SpansPreprocessorDirective())
{
var initialList = new List<SyntaxToken>(modifiers);
var leadingTrivia = initialList.First().LeadingTrivia;
initialList[0] = initialList[0].WithLeadingTrivia(SpecializedCollections.EmptyEnumerable<SyntaxTrivia>());

var finalList = initialList.OrderBy(new Comparer()).ToList();
var finalList = initialList.OrderBy(this).ToList();
if (!initialList.SequenceEqual(finalList))
{
finalList[0] = finalList[0].WithLeadingTrivia(leadingTrivia);
Expand Down
Loading