Order arguments properly (RCS1205)

ChangeLog.md

@@ -28,7 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add parentheses if necessary in a code fix for [RCS1197](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1197.md) ([#928](https://github.com/josefpihrt/roslynator/pull/928) by @karl-sjogren).
 - Do not simplify default expression if it would change semantics ([RCS1244](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1244.md)) ([#939](https://github.com/josefpihrt/roslynator/pull/939).
 - Fix NullReferenceException in [RCS1198](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1198.md) ([#940](https://github.com/josefpihrt/roslynator/pull/940).
-- Order named arguments even if optional arguments are not specified [RCS1205](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1205.md) ([#941](https://github.com/josefpihrt/roslynator/pull/941).
+- Order named arguments even if optional arguments are not specified [RCS1205](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1205.md) ([#941](https://github.com/josefpihrt/roslynator/pull/941), ([#965](https://github.com/josefpihrt/roslynator/pull/965).
 - Prefix identifier with `@` if necessary ([RCS1220](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1220.md)) ([#943](https://github.com/josefpihrt/roslynator/pull/943).
 - Do not suggest to make local variable a const when it is used in ref extension method ([RCS1118](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1118.md)) ([#948](https://github.com/josefpihrt/roslynator/pull/948).
 - Fix formatting of argument list ([#952](https://github.com/josefpihrt/roslynator/pull/952).

src/Analyzers.CodeFixes/CSharp/CodeFixes/BaseArgumentListCodeFixProvider.cs

@@ -1,8 +1,9 @@
 // Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
-using System.Diagnostics;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
@@ -30,21 +31,24 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
             if (!TryFindFirstAncestorOrSelf(root, context.Span, out BaseArgumentListSyntax baseArgumentList))
                 return;
 
-            foreach (Diagnostic diagnostic in context.Diagnostics)
+            if (baseArgumentList.ContainsDirectives(context.Span))
+                return;
+
+            Document document = context.Document;
+            Diagnostic diagnostic = context.Diagnostics[0];
+
+            switch (diagnostic.Id)
             {
-                switch (diagnostic.Id)
-                {
-                    case DiagnosticIdentifiers.OrderNamedArguments:
-                        {
-                            CodeAction codeAction = CodeAction.Create(
-                                "Order arguments",
-                                ct => OrderNamedArgumentsAsync(context.Document, baseArgumentList, ct),
-                                GetEquivalenceKey(diagnostic));
-
-                            context.RegisterCodeFix(codeAction, diagnostic);
-                            break;
-                        }
-                }
+                case DiagnosticIdentifiers.OrderNamedArguments:
+                    {
+                        CodeAction codeAction = CodeAction.Create(
+                            "Order arguments",
+                            ct => OrderNamedArgumentsAsync(document, baseArgumentList, ct),
+                            GetEquivalenceKey(diagnostic));
+
+                        context.RegisterCodeFix(codeAction, diagnostic);
+                        break;
+                    }
             }
         }
 
@@ -61,23 +65,26 @@ private static async Task<Document> OrderNamedArgumentsAsync(
 
             SeparatedSyntaxList<ArgumentSyntax> arguments = argumentList.Arguments;
 
-            int firstIndex = OrderNamedArgumentsAnalyzer.IndexOfFirstFixableParameter(argumentList, arguments, semanticModel, cancellationToken);
-
-            SeparatedSyntaxList<ArgumentSyntax> newArguments = arguments;
+            (int first, int last) = OrderNamedArgumentsAnalyzer.FindFixableSpan(arguments, semanticModel, cancellationToken);
 
-            for (int i = firstIndex; i < arguments.Count; i++)
-            {
-                IParameterSymbol parameter = parameters[i];
+            List<ArgumentSyntax> sortedArguments = arguments
+                .Skip(first)
+                .Take(last - first + 1)
+                .Select(a =>
+                {
+                    IParameterSymbol parameter = parameters.First(p => p.Name == a.NameColon.Name.Identifier.ValueText);
 
-                int index = arguments.IndexOf(f => f.NameColon?.Name.Identifier.ValueText == parameter.Name);
+                    return (argument: a, ordinal: parameters.IndexOf(parameter));
+                })
+                .OrderBy(a => a.ordinal)
+                .Select(a => a.argument)
+                .ToList();
 
-                Debug.Assert(index != -1, parameter.Name);
+            SeparatedSyntaxList<ArgumentSyntax> newArguments = arguments;
 
-                if (index != -1
-                    && index != i)
-                {
-                    newArguments = newArguments.ReplaceAt(i, arguments[index]);
-                }
+            for (int i = first; i <= last; i++)
+            {
+                newArguments = newArguments.ReplaceAt(i, sortedArguments[i - first]);
             }
 
             BaseArgumentListSyntax newNode = argumentList

src/Analyzers/CSharp/Analysis/OrderNamedArgumentsAnalyzer.cs

@@ -1,8 +1,10 @@
 // Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
@@ -41,91 +43,157 @@ private static void AnalyzeBaseArgumentList(SyntaxNodeAnalysisContext context)
             if (context.Node.ContainsDiagnostics)
                 return;
 
-            var argumentList = (BaseArgumentListSyntax)context.Node;
+            SeparatedSyntaxList<ArgumentSyntax> arguments = ((BaseArgumentListSyntax)context.Node).Arguments;
 
-            SeparatedSyntaxList<ArgumentSyntax> arguments = argumentList.Arguments;
-
-            int index = IndexOfFirstFixableParameter(argumentList, arguments, context.SemanticModel, context.CancellationToken);
-
-            if (index == -1)
-                return;
-
-            TextSpan span = TextSpan.FromBounds(arguments[index].SpanStart, arguments.Last().Span.End);
-
-            if (argumentList.ContainsDirectives(span))
-                return;
+            if (arguments.Count >= 2)
+            {
+                (int first, int last) = FindFixableSpan(arguments, context.SemanticModel, context.CancellationToken);
 
-            DiagnosticHelpers.ReportDiagnostic(
-                context,
-                DiagnosticRules.OrderNamedArguments,
-                Location.Create(argumentList.SyntaxTree, span));
+                if (first >= 0
+                    && last > first)
+                {
+                    DiagnosticHelpers.ReportDiagnostic(
+                        context,
+                        DiagnosticRules.OrderNamedArguments,
+                        Location.Create(
+                            context.Node.SyntaxTree,
+                            TextSpan.FromBounds(arguments[first].SpanStart, arguments[last].Span.End)));
+                }
+            }
         }
 
-        public static int IndexOfFirstFixableParameter(
-            BaseArgumentListSyntax argumentList,
+        internal static (int first, int last) FindFixableSpan(
             SeparatedSyntaxList<ArgumentSyntax> arguments,
             SemanticModel semanticModel,
             CancellationToken cancellationToken)
         {
             int firstIndex = -1;
 
-            for (int i = 0; i < arguments.Count; i++)
+            for (int i = arguments.Count - 1; i >= 0; i--)
             {
                 if (arguments[i].NameColon != null)
                 {
                     firstIndex = i;
+                }
+                else
+                {
                     break;
                 }
             }
 
-            if (firstIndex != -1
-                && firstIndex != arguments.Count - 1)
+            if (firstIndex >= 0
+                && firstIndex < arguments.Count - 1)
             {
-                ISymbol symbol = semanticModel.GetSymbol(argumentList.Parent, cancellationToken);
+                ISymbol symbol = semanticModel.GetSymbol(arguments.First().Parent.Parent, cancellationToken);
 
-                if (symbol != null)
+                if (symbol is not null)
                 {
                     ImmutableArray<IParameterSymbol> parameters = symbol.ParametersOrDefault();
 
                     Debug.Assert(!parameters.IsDefault, symbol.Kind.ToString());
 
                     if (!parameters.IsDefault
-                        && parameters.Length >= arguments.Count)
+                        && parameters.Length >= arguments.Count
+                        && IsFixable(firstIndex, arguments, parameters))
                     {
-                        for (int i = firstIndex; i < arguments.Count; i++)
-                        {
-                            ArgumentSyntax argument = arguments[i];
+                        return GetFixableSpan(firstIndex, arguments, parameters);
+                    }
+                }
+            }
+
+            return default;
+        }
+
+        private static bool IsFixable(
+            int firstIndex,
+            SeparatedSyntaxList<ArgumentSyntax> arguments,
+            ImmutableArray<IParameterSymbol> parameters)
+        {
+            int j = -1;
+            string firstName = arguments[firstIndex].NameColon.Name.Identifier.ValueText;
+
+            for (int i = 0; i < parameters.Length; i++)
+            {
+                if (parameters[i].Name == firstName)
+                {
+                    j = i;
+                    break;
+                }
+            }
 
-                            NameColonSyntax nameColon = argument.NameColon;
+            if (j >= 0)
+            {
+                for (int i = firstIndex + 1; i < arguments.Count; i++)
+                {
+                    string name = arguments[i].NameColon.Name.Identifier.ValueText;
 
-                            if (nameColon == null)
-                                break;
+                    while (!string.Equals(
+                        name,
+                        parameters[j].Name,
+                        StringComparison.Ordinal))
+                    {
+                        j++;
 
-                            if (!string.Equals(
-                                nameColon.Name.Identifier.ValueText,
-                                parameters[i].Name,
-                                StringComparison.Ordinal))
+                        if (j == parameters.Length)
+                        {
+                            foreach (IParameterSymbol parameter in parameters)
                             {
-                                int fixableIndex = i;
+                                if (parameter.Name == name)
+                                    return true;
+                            }
+                        }
+                    }
+                }
+            }
 
-                                i++;
+            return false;
+        }
 
-                                while (i < arguments.Count)
-                                {
-                                    if (arguments[i].NameColon == null)
-                                        break;
+        private static (int first, int last) GetFixableSpan(
+            int firstIndex,
+            SeparatedSyntaxList<ArgumentSyntax> arguments,
+            ImmutableArray<IParameterSymbol> parameters)
+        {
+            var sortedArgs = new List<(ArgumentSyntax argument, int ordinal)>();
 
-                                    i++;
-                                }
+            for (int i = firstIndex; i < arguments.Count; i++)
+            {
+                IParameterSymbol parameter = parameters.FirstOrDefault(p => p.Name == arguments[i].NameColon.Name.Identifier.ValueText);
 
-                                return fixableIndex;
-                            }
+                if (parameter is null)
+                    return default;
+
+                sortedArgs.Add((arguments[i], parameters.IndexOf(parameter)));
+            }
+
+            sortedArgs.Sort((x, y) => x.ordinal.CompareTo(y.ordinal));
+
+            int first = firstIndex;
+            int last = arguments.Count - 1;
+
+            while (first < arguments.Count)
+            {
+                if (sortedArgs[first - firstIndex].argument == arguments[first])
+                {
+                    first++;
+                }
+                else
+                {
+                    while (last > first)
+                    {
+                        if (sortedArgs[last - firstIndex].argument == arguments[last])
+                        {
+                            last--;
+                        }
+                        else
+                        {
+                            return (first, last);
                         }
                     }
                 }
             }
 
-            return -1;
+            return default;
         }
     }
 }