Implement analysis and fix for OrderBy(x => x) to Order() (#1522)

dotnet · Sep 15, 2024 · 9335c71 · 9335c71
1 parent e5cfbdb
commit 9335c71
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 16 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Analyzer [RCS1077](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1077) now suggests to use `Order` instead of `OrderBy` ([PR](https://github.com/dotnet/roslynator/pull/1522))
+
 ### Fixed
 
 - Fix analyzer [RCS0053](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0053) ([PR](https://github.com/dotnet/roslynator/pull/1518))
@@ -112,12 +116,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - These packages are recommended to be used in an environment where Roslynator IDE extension cannot be used, e.g. VS Code + C# Dev Kit (see related [issue](https://github.com/dotnet/vscode-csharp/issues/6790))
 - Add analyzer "Remove redundant catch block" [RCS1265](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1265) ([PR](https://github.com/dotnet/roslynator/pull/1364) by @jakubreznak)
 - [CLI] Spellcheck file names ([PR](https://github.com/dotnet/roslynator/pull/1368))
-  - `roslynator spellcheck --scope file-name` 
+  - `roslynator spellcheck --scope file-name`
 
 ### Changed
 
 - Update analyzer [RCS1197](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1197) ([PR](https://github.com/dotnet/roslynator/pull/1370))
-  - Do not report interpolated string and string concatenation 
+  - Do not report interpolated string and string concatenation
 
 ### Fixed
 
@@ -181,7 +185,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add analyzer "Unnecessary raw string literal" ([RCS1262](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1262)) ([PR](https://github.com/dotnet/roslynator/pull/1293))
 - Add analyzer "Invalid reference in a documentation comment" ([RCS1263](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1263)) ([PR](https://github.com/dotnet/roslynator/pull/1295))
 - Add analyzer "Add/remove blank line between switch sections" ([RCS0061](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0061)) ([PR](https://github.com/dotnet/roslynator/pull/1302))
-  - Option (required): `roslynator_blank_line_between_switch_sections = include|omit|omit_after_block` 
+  - Option (required): `roslynator_blank_line_between_switch_sections = include|omit|omit_after_block`
   - Make analyzer [RCS0014](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0014) obsolete
 
 ### Changed
@@ -271,7 +275,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Update logo ([PR](https://github.com/dotnet/roslynator/pull/1208), [PR](https://github.com/dotnet/roslynator/pull/1210)).
 - Migrate to .NET Foundation ([PR](https://github.com/dotnet/roslynator/pull/1206), [PR](https://github.com/dotnet/roslynator/pull/1207), [PR](https://github.com/dotnet/roslynator/pull/1219)).
 - Bump Roslyn to 4.7.0 ([PR](https://github.com/dotnet/roslynator/pull/1218)).
-  - Applies to CLI and testing library. 
+  - Applies to CLI and testing library.
 - Bump Microsoft.Build.Locator to 1.6.1 ([PR](https://github.com/dotnet/roslynator/pull/1194))
 - Improve testing framework ([PR](https://github.com/dotnet/roslynator/pull/1214))
   - Add methods to `DiagnosticVerifier`, `RefactoringVerifier` and `CompilerDiagnosticFixVerifier`.
@@ -611,7 +615,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### 3.0.0 (2020-06-16)
 
 * Update references to Roslyn API to 3.5.0
-* Release .NET Core Global Tool [Roslynator.DotNet.Cli](https://www.nuget.org/packages/roslynator.dotnet.cli) 
+* Release .NET Core Global Tool [Roslynator.DotNet.Cli](https://www.nuget.org/packages/roslynator.dotnet.cli)
 * Introduce concept of "[Analyzer Options](https://github.com/JosefPihrt/Roslynator/blob/main/docs/AnalyzerOptions)"
 * Reassign ID for some analyzers.
   * See "[How to: Migrate Analyzers to Version 3.0](https://github.com/JosefPihrt/Roslynator/blob/main/docs/HowToMigrateAnalyzersToVersion3)"
@@ -2170,13 +2174,13 @@ Code fixes has been added for the following compiler diagnostics:
 * Bug fixed in **"Uncomment"** refactoring
 
 ### 0.9.11 (2016-04-30)
- 
+
 * Bug fixes and minor improvements
- 
+
 ### 0.9.1 (2016-04-27)
- 
+
 * Bug fixes
- 
+
 ### 0.9.0 (2016-04-26)
- 
+
 * Initial release
diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeLinqMethodCallCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeLinqMethodCallCodeFixProvider.cs
@@ -267,6 +267,16 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
                             ct => UseElementAccessInsteadOfEnumerableMethodRefactoring.UseElementAccessInsteadOfLastAsync(document, invocation, ct),
                             GetEquivalenceKey(diagnostic, "UseElementAccessInsteadOfLast"));
 
+                        context.RegisterCodeFix(codeAction, diagnostic);
+                        return;
+                    }
+                case "OrderBy":
+                    {
+                        CodeAction codeAction = CodeAction.Create(
+                            "Call 'Order' instead of 'OrderBy'",
+                            ct => CallOrderInsteadOfOrderByIdentityAsync(document, invocationInfo, ct),
+                            GetEquivalenceKey(diagnostic, "CallOrderInsteadOfOrderByIdentity"));
+
                         context.RegisterCodeFix(codeAction, diagnostic);
                         return;
                     }
@@ -565,6 +575,18 @@ private static Task<Document> CallOrderByDescendingInsteadOfOrderByAndReverseAsy
         return document.ReplaceNodeAsync(invocationInfo.InvocationExpression, newInvocationExpression, cancellationToken);
     }
 
+    private static Task<Document> CallOrderInsteadOfOrderByIdentityAsync(
+        Document document,
+        in SimpleMemberInvocationExpressionInfo invocationInfo,
+        CancellationToken cancellationToken)
+    {
+        InvocationExpressionSyntax newInvocationExpression = ChangeInvokedMethodName(invocationInfo.InvocationExpression, "Order");
+
+        newInvocationExpression = newInvocationExpression.WithArgumentList(newInvocationExpression.ArgumentList.WithArguments(SeparatedList<ArgumentSyntax>()));
+
+        return document.ReplaceNodeAsync(invocationInfo.InvocationExpression, newInvocationExpression, cancellationToken);
+    }
+
     private static Task<Document> CallOrderByAndWhereInReverseOrderAsync(
         Document document,
         in SimpleMemberInvocationExpressionInfo invocationInfo,

diff --git a/src/Analyzers/CSharp/Analysis/InvocationExpressionAnalyzer.cs b/src/Analyzers/CSharp/Analysis/InvocationExpressionAnalyzer.cs
@@ -331,6 +331,9 @@ private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext contex
                                 if (DiagnosticRules.CallThenByInsteadOfOrderBy.IsEffective(context))
                                     CallThenByInsteadOfOrderByAnalysis.Analyze(context, invocationInfo);
 
+                                if (DiagnosticRules.OptimizeLinqMethodCall.IsEffective(context))
+                                    OptimizeLinqMethodCallAnalysis.AnalyzeOrderByIdentity(context, invocationInfo);
+
                                 break;
                             }
                     }

diff --git a/src/Analyzers/CSharp/Analysis/OptimizeLinqMethodCallAnalysis.cs b/src/Analyzers/CSharp/Analysis/OptimizeLinqMethodCallAnalysis.cs
@@ -690,6 +690,38 @@ public static void AnalyzeOrderByAndReverse(SyntaxNodeAnalysisContext context, i
         Report(context, invocationExpression, span, checkDirectives: true);
     }
 
+    // x.OrderBy(f => f) >>> x.Order()
+    public static void AnalyzeOrderByIdentity(SyntaxNodeAnalysisContext context, in SimpleMemberInvocationExpressionInfo invocationInfo)
+    {
+        InvocationExpressionSyntax invocationExpression = invocationInfo.InvocationExpression;
+
+        IMethodSymbol orderMethod = context.SemanticModel
+            .GetSymbolInfo(invocationExpression)
+            .Symbol
+            .ContainingType
+            .FindMember<IMethodSymbol>(method => method.Name == "Order" && method.Parameters.Length is 1);
+
+        if (orderMethod is null)
+            return;
+
+        ArgumentSyntax argument = invocationInfo.Arguments.SingleOrDefault(shouldThrow: false);
+
+        if (argument is null)
+            return;
+
+        if (!string.Equals(invocationInfo.NameText, "OrderBy", StringComparison.Ordinal))
+            return;
+
+        if (argument.Expression is not SimpleLambdaExpressionSyntax lambdaExpression)
+            return;
+
+        if (lambdaExpression.Body is not IdentifierNameSyntax identifier || identifier.Identifier.Text != lambdaExpression.Parameter.Identifier.Text)
+            return;
+
+        TextSpan span = TextSpan.FromBounds(invocationInfo.Name.SpanStart, invocationExpression.Span.End);
+        Report(context, invocationExpression, span, checkDirectives: true);
+    }
+
     // x.SelectMany(f => f).Count() >>> x.Sum(f = f.Count)
     public static bool AnalyzeSelectManyAndCount(SyntaxNodeAnalysisContext context, in SimpleMemberInvocationExpressionInfo invocationInfo)
     {

diff --git a/src/Tests/Analyzers.Tests/RCS1077OptimizeLinqMethodCallTests.cs b/src/Tests/Analyzers.Tests/RCS1077OptimizeLinqMethodCallTests.cs
@@ -940,7 +940,7 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.[|OrderBy(f => f).Reverse()|];
+        x = x.[|OrderBy(f => { return f; }).Reverse()|];
     }
 }
 ", @"
@@ -953,7 +953,42 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.OrderByDescending(f => f);
+        x = x.OrderByDescending(f => { return f; });
+    }
+}
+");
+    }
+
+    [Theory, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeLinqMethodCall)]
+    [InlineData("OrderBy(f => f)")]
+    [InlineData("OrderBy(_ => _)")]
+    [InlineData("OrderBy(@int => @int)")]
+    public async Task Test_CallOrderInsteadOfOrderByIdentity(string test)
+    {
+        await VerifyDiagnosticAndFixAsync($@"
+using System.Collections.Generic;
+using System.Linq;
+
+class C
+{{
+    void M()
+    {{
+        IEnumerable<object> x = null;
+
+        x = x.[|{test}|];
+    }}
+}}
+", @"
+using System.Collections.Generic;
+using System.Linq;
+
+class C
+{
+    void M()
+    {
+        IEnumerable<object> x = null;
+
+        x = x.Order();
     }
 }
 ");
@@ -972,7 +1007,7 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.[|OrderBy(f => f).Where(_ => true)|];
+        x = x.[|OrderBy(f => { return f; }).Where(_ => true)|];
     }
 }
 ", @"
@@ -985,7 +1020,7 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.Where(_ => true).OrderBy(f => f);
+        x = x.Where(_ => true).OrderBy(f => { return f; });
     }
 }
 ");
@@ -1036,7 +1071,7 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.[|OrderByDescending(f => f).Where(_ => true)|];
+        x = x.[|OrderByDescending(f => { return f; }).Where(_ => true)|];
     }
 }
 ", @"
@@ -1049,7 +1084,7 @@ void M()
     {
         IEnumerable<object> x = null;
 
-        x = x.Where(_ => true).OrderByDescending(f => f);
+        x = x.Where(_ => true).OrderByDescending(f => { return f; });
     }
 }
 ");