From ed6845a2f6acc5316c5e9b501e0ba9a988a49081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9rald=20Barr=C3=A9?= Date: Sat, 18 May 2024 09:51:21 -0400 Subject: [PATCH] MA0029 should not report when for IQueryable when the predicate is a reference --- .../Rules/OptimizeLinqUsageFixer.cs | 39 ++++++++++----- .../Rules/OptimizeLinqUsageAnalyzer.cs | 18 +++++++ ...inqUsageAnalyzerCombineLinqMethodsTests.cs | 50 +++++++++++++++++++ 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs index 26520e59c..8b2126cb4 100644 --- a/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs @@ -594,22 +594,37 @@ private static async Task CombineWhereWithNextMethod(Document document if (argument1 is null) return argument2?.Syntax; - if (argument1.Value is not IDelegateCreationOperation value1 || argument2.Value is not IDelegateCreationOperation value2) - return null; + if (argument1.Value is IDelegateCreationOperation value1 && argument2.Value is IDelegateCreationOperation value2) + { + var anonymousMethod1 = value1.Target as IAnonymousFunctionOperation; + var anonymousMethod2 = value2.Target as IAnonymousFunctionOperation; + + var newParameterName = + anonymousMethod1?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? + anonymousMethod2?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? + "x"; - var anonymousMethod1 = value1.Target as IAnonymousFunctionOperation; - var anonymousMethod2 = value2.Target as IAnonymousFunctionOperation; + var left = PrepareSyntaxNode(generator, value1, newParameterName); + var right = PrepareSyntaxNode(generator, value2, newParameterName); - var newParameterName = - anonymousMethod1?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - anonymousMethod2?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - "x"; + return generator.ValueReturningLambdaExpression(newParameterName, + generator.LogicalAndExpression(left, right)); + } + else if (argument1.Value.UnwrapConversionOperations() is IAnonymousFunctionOperation anonymousMethod1 && argument2.Value.UnwrapImplicitConversionOperations() is IAnonymousFunctionOperation anonymousMethod2) + { + var newParameterName = + anonymousMethod1.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? + anonymousMethod2.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? + "x"; - var left = PrepareSyntaxNode(generator, value1, newParameterName); - var right = PrepareSyntaxNode(generator, value2, newParameterName); + var left = ReplaceParameter(anonymousMethod1, newParameterName); + var right = ReplaceParameter(anonymousMethod2, newParameterName); - return generator.ValueReturningLambdaExpression(newParameterName, - generator.LogicalAndExpression(left, right)); + return generator.ValueReturningLambdaExpression(newParameterName, + generator.LogicalAndExpression(left, right)); + } + + return null; } static SyntaxNode PrepareSyntaxNode(SyntaxGenerator generator, IDelegateCreationOperation delegateCreationOperation, string parameterName) diff --git a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs index 888b6e4d5..9de368dd1 100755 --- a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs @@ -130,6 +130,7 @@ public AnalyzerContext(Compilation compilation) ExtensionMethodOwnerTypes.AddIfNotNull(EnumerableSymbol); ExtensionMethodOwnerTypes.AddIfNotNull(QueryableSymbol); + ExpressionOfTSymbol = compilation.GetBestTypeByMetadataName("System.Linq.Expressions.Expression`1"); ICollectionOfTSymbol = compilation.GetBestTypeByMetadataName("System.Collections.Generic.ICollection`1"); IReadOnlyCollectionOfTSymbol = compilation.GetBestTypeByMetadataName("System.Collections.Generic.IReadOnlyCollection`1"); ListOfTSymbol = compilation.GetBestTypeByMetadataName("System.Collections.Generic.List`1"); @@ -144,6 +145,7 @@ public AnalyzerContext(Compilation compilation) private INamedTypeSymbol? EnumerableSymbol { get; set; } private INamedTypeSymbol? QueryableSymbol { get; set; } + private INamedTypeSymbol? ExpressionOfTSymbol { get; set; } private INamedTypeSymbol? ICollectionOfTSymbol { get; set; } private INamedTypeSymbol? IReadOnlyCollectionOfTSymbol { get; set; } private INamedTypeSymbol? ListOfTSymbol { get; set; } @@ -432,12 +434,28 @@ private void CombineWhereWithNextMethod(OperationAnalysisContext context, IInvoc .Add("LastOperationLength", parent.Syntax.Span.Length.ToString(CultureInfo.InvariantCulture)) .Add("MethodName", parent.TargetMethod.Name); + if (parent.Arguments.Length > 1 && IsExpressionPredicateReference(parent.Arguments[1].Value)) + return; + + if (operation.Arguments.Length > 1 && IsExpressionPredicateReference(operation.Arguments[1].Value)) + return; + context.ReportDiagnostic(CombineLinqMethodsRule, properties, parent, operation.TargetMethod.Name, parent.TargetMethod.Name); } } } } + private bool IsExpressionPredicateReference(IOperation operation) + { + if (operation.Type is null || !operation.Type.OriginalDefinition.IsEqualTo(ExpressionOfTSymbol)) + return false; + + operation = operation.UnwrapImplicitConversionOperations(); + + return operation.Kind is not OperationKind.AnonymousFunction; + } + private void RemoveTwoConsecutiveOrderBy(OperationAnalysisContext context, IInvocationOperation operation) { if (operation.TargetMethod.Name == nameof(Enumerable.OrderBy) || diff --git a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerCombineLinqMethodsTests.cs b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerCombineLinqMethodsTests.cs index 93c6f970f..cbcea8940 100644 --- a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerCombineLinqMethodsTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerCombineLinqMethodsTests.cs @@ -79,6 +79,56 @@ public Test() .ValidateAsync(); } + [Fact] + public async Task CombineWhereWithTheFollowingWhereMethod_ExpressionWithPredicate() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System; + using System.Linq; + using System.Linq.Expressions; + class Test + { + public Test(Expression> predicate) + { + IQueryable queryable = null!; + queryable.Where(x => x == 0).Where(predicate); + queryable.Where(predicate).Where(x => x == 0); + } + } + + """) + .ValidateAsync(); + } + + [Fact] + public async Task CombineWhereWithTheFollowingWhereMethod_IQueryable() + { + await CreateProjectBuilder() + .WithSourceCode(@"using System.Linq; +class Test +{ + public Test() + { + System.Linq.IQueryable enumerable = null; + [||]enumerable.Where(x => x == 0).Where(y => true); + } +} +") + .ShouldReportDiagnosticWithMessage($"Combine 'Where' with 'Where'") + .ShouldFixCodeWith(@"using System.Linq; +class Test +{ + public Test() + { + System.Linq.IQueryable enumerable = null; + enumerable.Where(x => x == 0 && true); + } +} +") + .ValidateAsync(); + } + [Fact] public async Task CombineWhereWithTheFollowingMethod_CombineLambdaWithNothing() {