From dc1c9b0162223d4388dbd62b08b85f1f6c8b9d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9rald=20Barr=C3=A9?= Date: Sat, 18 Mar 2023 00:03:37 -0400 Subject: [PATCH] MA0020 suggests Exists and TrueForAll (#462) --- .../Rules/OptimizeLinqUsageAnalyzer.cs | 66 +++++++ .../Rules/OptimizeLinqUsageData.cs | 4 + .../Rules/OptimizeLinqUsageFixer.cs | 30 ++- ...eLinqUsageAnalyzerUseDirectMethodsTests.cs | 176 ++++++++++++++++++ 4 files changed, 271 insertions(+), 5 deletions(-) diff --git a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs index 5370f241c..618fb3ed5 100644 --- a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs @@ -161,6 +161,8 @@ public void AnalyzeInvocation(OperationAnalysisContext context) return; UseFindInsteadOfFirstOrDefault(context, operation); + UseTrueForAllInsteadOfAll(context, operation); + UseExistsInsteadOfAny(context, operation); UseCountPropertyInsteadOfMethod(context, operation); UseIndexerInsteadOfElementAt(context, operation); CombineWhereWithNextMethod(context, operation); @@ -287,6 +289,70 @@ private void UseFindInsteadOfFirstOrDefault(OperationAnalysisContext context, II context.ReportDiagnostic(s_listMethodsRule, properties, operation, DiagnosticReportOptions.ReportOnMethodName, "Find()", operation.TargetMethod.Name); } } + + private void UseTrueForAllInsteadOfAll(OperationAnalysisContext context, IInvocationOperation operation) + { + if (operation.TargetMethod.Name != nameof(Enumerable.All)) + return; + + if (operation.Arguments.Length != 2) + return; + + var firstArgumentType = operation.Arguments[0].Value.GetActualType(); + if (firstArgumentType == null) + return; + + if (firstArgumentType.OriginalDefinition.IsEqualTo(ListOfTSymbol)) + { + ImmutableDictionary properties; + var predicateArgument = operation.Arguments[1].Value; + if (predicateArgument is IDelegateCreationOperation) + { + properties = CreateProperties(OptimizeLinqUsageData.UseTrueForAllMethod); + } + else + { + if (!context.Options.GetConfigurationValue(operation, s_listMethodsRule.Id + ".report_when_conversion_needed", defaultValue: false)) + return; + + properties = CreateProperties(OptimizeLinqUsageData.UseTrueForAllMethodWithConversion); + } + + context.ReportDiagnostic(s_listMethodsRule, properties, operation, DiagnosticReportOptions.ReportOnMethodName, "TrueForAll()", operation.TargetMethod.Name); + } + } + + private void UseExistsInsteadOfAny(OperationAnalysisContext context, IInvocationOperation operation) + { + if (operation.TargetMethod.Name != nameof(Enumerable.Any)) + return; + + if (operation.Arguments.Length != 2) + return; + + var firstArgumentType = operation.Arguments[0].Value.GetActualType(); + if (firstArgumentType == null) + return; + + if (firstArgumentType.OriginalDefinition.IsEqualTo(ListOfTSymbol)) + { + ImmutableDictionary properties; + var predicateArgument = operation.Arguments[1].Value; + if (predicateArgument is IDelegateCreationOperation) + { + properties = CreateProperties(OptimizeLinqUsageData.UseExistsMethod); + } + else + { + if (!context.Options.GetConfigurationValue(operation, s_listMethodsRule.Id + ".report_when_conversion_needed", defaultValue: false)) + return; + + properties = CreateProperties(OptimizeLinqUsageData.UseExistsMethodWithConversion); + } + + context.ReportDiagnostic(s_listMethodsRule, properties, operation, DiagnosticReportOptions.ReportOnMethodName, "Exists()", operation.TargetMethod.Name); + } + } private void UseIndexerInsteadOfElementAt(OperationAnalysisContext context, IInvocationOperation operation) { diff --git a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageData.cs b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageData.cs index 6fab6e562..ce1e126f6 100644 --- a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageData.cs +++ b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageData.cs @@ -21,4 +21,8 @@ internal enum OptimizeLinqUsageData UseSkipAndNotAny, UseSkipAndAny, UseCastInsteadOfSelect, + UseTrueForAllMethod, + UseTrueForAllMethodWithConversion, + UseExistsMethod, + UseExistsMethodWithConversion, } diff --git a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageFixer.cs b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageFixer.cs index 81f9a36ca..9c2955272 100644 --- a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageFixer.cs +++ b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageFixer.cs @@ -69,11 +69,27 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) break; case OptimizeLinqUsageData.UseFindMethod: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseFindMethod(context.Document, nodeToFix, convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics); + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Find", convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics); break; case OptimizeLinqUsageData.UseFindMethodWithConversion: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseFindMethod(context.Document, nodeToFix, convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics); + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Find", convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics); + break; + + case OptimizeLinqUsageData.UseTrueForAllMethod: + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "TrueForAll", convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics); + break; + + case OptimizeLinqUsageData.UseTrueForAllMethodWithConversion: + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "TrueForAll", convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics); + break; + + case OptimizeLinqUsageData.UseExistsMethod: + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Exists", convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics); + break; + + case OptimizeLinqUsageData.UseExistsMethodWithConversion: + context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Exists", convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics); break; case OptimizeLinqUsageData.UseIndexer: @@ -389,7 +405,7 @@ private async static Task UseCountProperty(Document document, SyntaxNo return editor.GetChangedDocument(); } - private async static Task UseFindMethod(Document document, SyntaxNode nodeToFix, bool convertPredicate, CancellationToken cancellationToken) + private async static Task UseListMethod(Document document, SyntaxNode nodeToFix, string methodName, bool convertPredicate, CancellationToken cancellationToken) { var expression = GetMemberAccessExpression(nodeToFix); if (expression == null) @@ -397,12 +413,16 @@ private async static Task UseFindMethod(Document document, SyntaxNode var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var newExpression = expression.WithName(IdentifierName("Find")); + var newExpression = expression.WithName(IdentifierName(methodName)); editor.ReplaceNode(expression, newExpression); if (convertPredicate) { var compilation = editor.SemanticModel.Compilation; - var type = editor.SemanticModel.GetTypeInfo(nodeToFix, cancellationToken).Type; + var symbol = editor.SemanticModel.GetSymbolInfo(nodeToFix, cancellationToken: cancellationToken).Symbol as IMethodSymbol; + if(symbol == null || symbol.TypeArguments.Length != 1) + return document; + + var type = symbol.TypeArguments[0]; if (type != null) { var predicateType = compilation.GetBestTypeByMetadataName("System.Predicate`1")?.Construct(type); diff --git a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerUseDirectMethodsTests.cs b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerUseDirectMethodsTests.cs index ba161aff3..b8dbe0361 100644 --- a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerUseDirectMethodsTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerUseDirectMethodsTests.cs @@ -105,6 +105,182 @@ await CreateProjectBuilder() .ShouldFixCodeWith(CodeFix) .ValidateAsync(); } + + [Fact] + public async Task TrueForAll() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var enumerable = System.Linq.Enumerable.Empty(); + var list = new System.Collections.Generic.List(); + list.[|All|](x => x == 0); + enumerable.All(x => x == 0); + } +} +"; + const string CodeFix = @"using System.Linq; +class Test +{ + public Test() + { + var enumerable = System.Linq.Enumerable.Empty(); + var list = new System.Collections.Generic.List(); + list.TrueForAll(x => x == 0); + enumerable.All(x => x == 0); + } +} +"; + + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Use 'TrueForAll()' instead of 'All()'") + .ShouldFixCodeWith(CodeFix) + .ValidateAsync(); + } + + [Fact] + public async Task TrueForAll_Cast() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.All(predicate); + } +} +"; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task TrueForAll_Cast_ConfigureEnabled() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.[|All|](predicate); + } +} +"; + const string CodeFix = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.TrueForAll(new System.Predicate(predicate)); + } +} +"; + + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .AddAnalyzerConfiguration("MA0020.report_when_conversion_needed", "true") + .ShouldReportDiagnosticWithMessage("Use 'TrueForAll()' instead of 'All()'") + .ShouldFixCodeWith(CodeFix) + .ValidateAsync(); + } + + [Fact] + public async Task Exists() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var enumerable = System.Linq.Enumerable.Empty(); + var list = new System.Collections.Generic.List(); + list.[|Any|](x => x == 0); + enumerable.Any(x => x == 0); + } +} +"; + const string CodeFix = @"using System.Linq; +class Test +{ + public Test() + { + var enumerable = System.Linq.Enumerable.Empty(); + var list = new System.Collections.Generic.List(); + list.Exists(x => x == 0); + enumerable.Any(x => x == 0); + } +} +"; + + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Use 'Exists()' instead of 'Any()'") + .ShouldFixCodeWith(CodeFix) + .ValidateAsync(); + } + + [Fact] + public async Task Exists_Cast() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.Any(predicate); + } +} +"; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Exists_Cast_ConfigureEnabled() + { + const string SourceCode = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.[|Any|](predicate); + } +} +"; + const string CodeFix = @"using System.Linq; +class Test +{ + public Test() + { + var list = new System.Collections.Generic.List(); + System.Func predicate = _ => true; + list.Exists(new System.Predicate(predicate)); + } +} +"; + + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .AddAnalyzerConfiguration("MA0020.report_when_conversion_needed", "true") + .ShouldReportDiagnosticWithMessage("Use 'Exists()' instead of 'Any()'") + .ShouldFixCodeWith(CodeFix) + .ValidateAsync(); + } [Fact] public async Task Count_IEnumerableAsync()