Skip to content
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

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

Merged
24 changes: 14 additions & 10 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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)"
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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");
BenjaminBrienen marked this conversation as resolved.
Show resolved Hide resolved

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,
Expand Down
3 changes: 3 additions & 0 deletions src/Analyzers/CSharp/Analysis/InvocationExpressionAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/Analyzers/CSharp/Analysis/OptimizeLinqMethodCallAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
47 changes: 41 additions & 6 deletions src/Tests/Analyzers.Tests/RCS1077OptimizeLinqMethodCallTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ void M()
{
IEnumerable<object> x = null;

x = x.[|OrderBy(f => f).Reverse()|];
x = x.[|OrderBy(f => { return f; }).Reverse()|];
}
}
", @"
Expand All @@ -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();
}
}
");
Expand All @@ -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)|];
}
}
", @"
Expand All @@ -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; });
}
}
");
Expand Down Expand Up @@ -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)|];
}
}
", @"
Expand All @@ -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; });
}
}
");
Expand Down