diff --git a/ChangeLog.md b/ChangeLog.md index bb697c7e16..8ae12bc703 100644 --- a/ChangeLog.md +++ b/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). diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/BaseArgumentListCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/BaseArgumentListCodeFixProvider.cs index 7ab53ecf29..7375315080 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/BaseArgumentListCodeFixProvider.cs +++ b/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 OrderNamedArgumentsAsync( SeparatedSyntaxList arguments = argumentList.Arguments; - int firstIndex = OrderNamedArgumentsAnalyzer.IndexOfFirstFixableParameter(argumentList, arguments, semanticModel, cancellationToken); - - SeparatedSyntaxList newArguments = arguments; + (int first, int last) = OrderNamedArgumentsAnalyzer.FindFixableSpan(arguments, semanticModel, cancellationToken); - for (int i = firstIndex; i < arguments.Count; i++) - { - IParameterSymbol parameter = parameters[i]; + List 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 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 diff --git a/src/Analyzers/CSharp/Analysis/OrderNamedArgumentsAnalyzer.cs b/src/Analyzers/CSharp/Analysis/OrderNamedArgumentsAnalyzer.cs index 85e2d20e5f..315a537673 100644 --- a/src/Analyzers/CSharp/Analysis/OrderNamedArgumentsAnalyzer.cs +++ b/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 arguments = ((BaseArgumentListSyntax)context.Node).Arguments; - SeparatedSyntaxList 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 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 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 arguments, + ImmutableArray 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 arguments, + ImmutableArray 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; } } } diff --git a/src/Tests/Analyzers.Tests/RCS1205OrderNamedArgumentsTests.cs b/src/Tests/Analyzers.Tests/RCS1205OrderNamedArgumentsTests.cs index 48116826ed..205b616e52 100644 --- a/src/Tests/Analyzers.Tests/RCS1205OrderNamedArgumentsTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1205OrderNamedArgumentsTests.cs @@ -53,6 +53,90 @@ void M(string a, string b, string c, string d = null) M(a: ""a"", b: ""b"", c: ""c""); } } +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OrderNamedArguments)] + public async Task Test_OptionalArguments2() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(string a, string b = """", string c = """", string d = """", string e = """") + { + M( + """", + b: """", + """", + [|e: """", + d: """"|]); + } +} +", @" +class C +{ + void M(string a, string b = """", string c = """", string d = """", string e = """") + { + M( + """", + b: """", + """", + d: """", + e: """"); + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OrderNamedArguments)] + public async Task Test_OptionalArguments3() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(string a = """", string b = """", string c = """", string d = """") + { + M( + [|c: """", + b: """"|], + d: """"); + } +} +", @" +class C +{ + void M(string a = """", string b = """", string c = """", string d = """") + { + M( + b: """", + c: """", + d: """"); + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OrderNamedArguments)] + public async Task TestNoDiagnostic_OptionalArguments() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + void M(string a, string b = """", string c = """", string d = """") + { + M( + """", + b: """", + d: """"); + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OrderNamedArguments)] + public async Task TestNoDiagnostic_OptionalArguments2() + { + await VerifyNoDiagnosticAsync(@" "); } }