From 438d8747c959cadb01d2831442a272aabdb3d0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9rald=20Barr=C3=A9?= Date: Tue, 22 Sep 2026 21:10:39 -0400 Subject: [PATCH] Fix the LINQ optimization rules on predicates, casts, ordering and collection interfaces --- docs/Rules/MA0029.md | 2 + docs/Rules/MA0030.md | 9 + docs/Rules/MA0078.md | 2 + docs/Rules/MA0112.md | 9 +- .../Rules/OptimizeLinqUsageFixer.cs | 353 ++++++---- .../Rules/OptimizeLinqUsageAnalyzer.cs | 157 ++++- .../Rules/OptimizeLinqUsageAnalyzerTests.cs | 660 ++++++++++++++++++ 7 files changed, 1030 insertions(+), 162 deletions(-) diff --git a/docs/Rules/MA0029.md b/docs/Rules/MA0029.md index 9bd0aea56..5ecdf4088 100644 --- a/docs/Rules/MA0029.md +++ b/docs/Rules/MA0029.md @@ -11,3 +11,5 @@ new int[0].Where(x => true).Any(); // Should be new int[0].Any(x => true); ``` + +The rule does not report the calls whose predicate cannot be combined, such as `Where((x, index) => ...)`, or `FirstOrDefault(defaultValue)` whose argument is not a predicate. diff --git a/docs/Rules/MA0030.md b/docs/Rules/MA0030.md index f4678e8d0..871c0e9e1 100644 --- a/docs/Rules/MA0030.md +++ b/docs/Rules/MA0030.md @@ -12,3 +12,12 @@ new int[0].OrderBy(x => x).OrderBy(x => x); new int[0].OrderBy(x => x); new int[0].OrderBy(x => x).ThenBy(x => x); ``` + +`Order()` and `OrderDescending()` have no key selector, so they are replaced with `ThenBy(x => x)` and `ThenByDescending(x => x)`: + +```csharp +new int[0].OrderBy(x => -x).Order(); + +// Should be +new int[0].OrderBy(x => -x).ThenBy(x => x); +``` diff --git a/docs/Rules/MA0078.md b/docs/Rules/MA0078.md index 049153a6d..997017e3f 100644 --- a/docs/Rules/MA0078.md +++ b/docs/Rules/MA0078.md @@ -18,3 +18,5 @@ var stringsAsObjects = strings.Cast(); ``` The rule only reports a selector whose body is a single cast of its element. A selector that contains other statements, such as `str => { count++; return (object)str; }`, is not reported, as `Cast()` would remove them. + +`Cast()` boxes each element and casts the object to `T`, so the rule only reports the conversions that `Cast()` supports: reference conversions, boxing and unboxing conversions, and the conversions between a value type, its nullable type, and the enums with the same underlying type. The other conversions, such as `x => (long)x` or `x => (long?)x` on a sequence of `int`, would throw an `InvalidCastException` with `Cast()` and are not reported. diff --git a/docs/Rules/MA0112.md b/docs/Rules/MA0112.md index 571f95f79..9597b1fd3 100644 --- a/docs/Rules/MA0112.md +++ b/docs/Rules/MA0112.md @@ -6,8 +6,15 @@ Sources: [OptimizeLinqUsageAnalyzer.cs](https://github.com/meziantou/Meziantou.A For performance reasons, use the `Count` property instead of `Any()`. ````c# -var collection = new int[10]; +var collection = new List(); if(collection.Any()) { } // report diagnostic if(collection.Count > 0) { } // ok + +var array = new int[10]; +if(array.Any()) { } // report diagnostic + +if(array.Length > 0) { } // ok ```` + +The rule does not report a collection whose `Count` property cannot be used, such as a type that implements `ICollection.Count` explicitly. diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs index 57d87a58a..369fc9db2 100644 --- a/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/OptimizeLinqUsageFixer.cs @@ -37,7 +37,19 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) if (diagnostic.Id == RuleIdentifiers.OptimizeEnumerable_UseCountInsteadOfAny) { const string CodeFixTitle = "Optimize linq usage"; - context.RegisterCodeFix(CodeAction.Create(CodeFixTitle, ct => UseCountGreaterThanZero(context.Document, nodeToFix, ct), equivalenceKey: CodeFixTitle), context.Diagnostics); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel?.GetOperation(nodeToFix, context.CancellationToken) is not IInvocationOperation { Arguments: [var source] }) + return; + + // The Count property of arrays is an explicit implementation of ICollection + var propertyName = source.Value.GetActualType() is { TypeKind: TypeKind.Array } ? "Length" : "Count"; + + // 'items?.Any()' cannot be replaced by a comparison, as the result of the conditional access is nullable + var countExpression = CreatePropertyAccess(SyntaxGenerator.GetGenerator(context.Document), nodeToFix, propertyName, allowConditionalAccess: false); + if (countExpression is null) + return; + + context.RegisterCodeFix(CodeAction.Create(CodeFixTitle, ct => UseCountGreaterThanZero(context.Document, nodeToFix, countExpression, ct), equivalenceKey: CodeFixTitle), context.Diagnostics); return; } @@ -53,15 +65,20 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) switch (data) { case OptimizeLinqUsageData.UseLengthProperty: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseLengthProperty(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics); - break; - case OptimizeLinqUsageData.UseLongLengthProperty: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseLongLengthProperty(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics); - break; - case OptimizeLinqUsageData.UseCountProperty: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseCountProperty(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics); + var propertyName = data switch + { + OptimizeLinqUsageData.UseLengthProperty => "Length", + OptimizeLinqUsageData.UseLongLengthProperty => "LongLength", + _ => "Count", + }; + + var propertyAccess = CreatePropertyAccess(SyntaxGenerator.GetGenerator(context.Document), nodeToFix, propertyName, allowConditionalAccess: true); + if (propertyAccess is null) + return; + + context.RegisterCodeFix(CodeAction.Create(title, ct => ReplaceNode(context.Document, nodeToFix, propertyAccess.WithTriviaFrom(nodeToFix), ct), equivalenceKey: title), context.Diagnostics); break; case OptimizeLinqUsageData.UseFindMethod: @@ -113,7 +130,24 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) if (!diagnostic.Properties.TryGetValue(OptimizeLinqUsageAnalyzerCommon.MethodNameKey, out var methodName) || methodName is null) return; - context.RegisterCodeFix(CodeAction.Create("Use " + expectedMethodName, ct => UseThenBy(context.Document, orderByLastSpan, expectedMethodName, ct), equivalenceKey: "UseThenBy"), context.Diagnostics); + if (root?.FindNode(orderByLastSpan, getInnermostNodeForTie: true) is InvocationExpressionSyntax lastInvocation && GetMethodNameSyntax(lastInvocation) is { } lastMethodName) + { + // Order() and OrderDescending() have no key selector, so ThenBy(x => x) must use the element as the key + string? keySelectorParameterName = null; + if (methodName is "Order" or "OrderDescending") + { + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel is null) + return; + + keySelectorParameterName = GetUniqueParameterName(semanticModel, lastInvocation.SpanStart, _ => true); + if (keySelectorParameterName is null) + return; + } + + context.RegisterCodeFix(CodeAction.Create("Use " + expectedMethodName, ct => UseThenBy(context.Document, lastInvocation, lastMethodName, expectedMethodName, keySelectorParameterName, ct), equivalenceKey: "UseThenBy"), context.Diagnostics); + } + context.RegisterCodeFix(CodeAction.Create("Remove " + methodName, ct => RemoveDuplicatedOrderBy(context.Document, orderByFirstSpan, orderByLastSpan, ct), equivalenceKey: "RemoveOrderBy"), context.Diagnostics); break; @@ -127,7 +161,20 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) } else { - context.RegisterCodeFix(CodeAction.Create(title, ct => CombineWhereWithNextMethod(context.Document, whereFirstSpan, whereLastSpan, ct), equivalenceKey: title), context.Diagnostics); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + var whereNode = root?.FindNode(whereFirstSpan, getInnermostNodeForTie: true); + var nextNode = root?.FindNode(whereLastSpan, getInnermostNodeForTie: true); + if (semanticModel is null || whereNode is null || nextNode is null) + return; + + if (semanticModel.GetOperation(whereNode, context.CancellationToken) is not IInvocationOperation whereOperation || semanticModel.GetOperation(nextNode, context.CancellationToken) is not IInvocationOperation nextOperation) + return; + + var combinedExpression = CombineWhereWithNextMethod(SyntaxGenerator.GetGenerator(context.Document), semanticModel, whereOperation, nextOperation, context.CancellationToken); + if (combinedExpression is null) + return; + + context.RegisterCodeFix(CodeAction.Create(title, ct => ReplaceNode(context.Document, nextOperation.Syntax, combinedExpression, ct), equivalenceKey: title), context.Diagnostics); } break; @@ -169,7 +216,18 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) break; case OptimizeLinqUsageData.UseOrder: - context.RegisterCodeFix(CodeAction.Create(title, ct => UseOrderInsteadOfOrderBy(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics); + if (nodeToFix is not InvocationExpressionSyntax orderByInvocation || GetMethodNameSyntax(orderByInvocation) is not { } orderByMethodName) + return; + + var orderBySemanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (orderBySemanticModel?.GetOperation(orderByInvocation, context.CancellationToken) is not IInvocationOperation orderByOperation) + return; + + // The key selector is the first argument of 'items.OrderBy(x => x)', but the second one of 'Enumerable.OrderBy(items, x => x)' + if (orderByOperation.Arguments.FirstOrDefault(argument => argument.Parameter?.Ordinal == 1)?.Syntax is not ArgumentSyntax keySelectorArgument) + return; + + context.RegisterCodeFix(CodeAction.Create(title, ct => UseOrderInsteadOfOrderBy(context.Document, orderByMethodName, keySelectorArgument, ct), equivalenceKey: title), context.Diagnostics); break; } } @@ -242,17 +300,10 @@ private static async Task UseAny(Document document, TextSpan countOper return editor.GetChangedDocument(); } - private static async Task UseCountGreaterThanZero(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + private static async Task UseCountGreaterThanZero(Document document, SyntaxNode nodeToFix, ExpressionSyntax countExpression, CancellationToken cancellationToken) { var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - if (editor.SemanticModel.GetOperation(nodeToFix, cancellationToken) is not IInvocationOperation invocation) - return document; - - if (invocation.Arguments.Length != 1) - return document; - var generator = editor.Generator; - var countExpression = generator.MemberAccessExpression(invocation.Arguments[0].Syntax, "Count"); // The invocation may be the operand of an operator or the target of a member access, both of which bind // tighter than '!=', so the comparison must be parenthesized. Simplifier removes the useless parentheses. @@ -423,63 +474,71 @@ private static async Task UseConstantValue(Document document, SyntaxNo return editor.GetChangedDocument(); } - private static async Task UseOrderInsteadOfOrderBy(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + private static async Task UseOrderInsteadOfOrderBy(Document document, SimpleNameSyntax methodName, ArgumentSyntax keySelectorArgument, CancellationToken cancellationToken) { - if (nodeToFix is not InvocationExpressionSyntax invocation) - return document; - - var member = GetMemberAccessExpression(nodeToFix); - if (member is null) - return document; - - var newName = member.Name.Identifier.ValueText is "OrderBy" ? "Order" : "OrderDescending"; + var newName = methodName.Identifier.ValueText is "OrderBy" ? "Order" : "OrderDescending"; var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - editor.RemoveNode(invocation.ArgumentList.Arguments.First()); - editor.ReplaceNode(member, member.WithName(IdentifierName(newName))); + editor.RemoveNode(keySelectorArgument); + editor.ReplaceNode(methodName, IdentifierName(newName).WithTriviaFrom(methodName)); return editor.GetChangedDocument(); } - private static async Task UseLengthProperty(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + private static async Task ReplaceNode(Document document, SyntaxNode nodeToReplace, SyntaxNode newNode, CancellationToken cancellationToken) { - var expression = GetParentMemberExpression(nodeToFix); - if (expression is null) - return document; - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var generator = editor.Generator; - var propertyAccess = generator.MemberAccessExpression(expression, "Length"); - - editor.ReplaceNode(nodeToFix, propertyAccess); + editor.ReplaceNode(nodeToReplace, newNode); return editor.GetChangedDocument(); } - private static async Task UseLongLengthProperty(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + /// + /// Creates the access to the property replacing a method without argument, such as 'items.Count' for 'items.Count()', + /// 'Enumerable.Count(items)', or 'items?.Count()'. The trivia of the invocation are not copied. + /// + private static ExpressionSyntax? CreatePropertyAccess(SyntaxGenerator generator, SyntaxNode nodeToFix, string propertyName, bool allowConditionalAccess) { - var expression = GetParentMemberExpression(nodeToFix); - if (expression is null) - return document; + if (nodeToFix is not InvocationExpressionSyntax invocation) + return null; - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var generator = editor.Generator; - var propertyAccess = generator.MemberAccessExpression(expression, "LongLength"); + return invocation switch + { + // Enumerable.Count(items) + { ArgumentList.Arguments: [var source] } => (ExpressionSyntax)generator.MemberAccessExpression(source.Expression, propertyName), - editor.ReplaceNode(nodeToFix, propertyAccess); - return editor.GetChangedDocument(); + // items.Count() + { ArgumentList.Arguments: [], Expression: MemberAccessExpressionSyntax memberAccess } => memberAccess.WithName(IdentifierName(propertyName).WithTriviaFrom(memberAccess.Name)), + + // items?.Count() + { ArgumentList.Arguments: [], Expression: MemberBindingExpressionSyntax memberBinding } when allowConditionalAccess => memberBinding.WithName(IdentifierName(propertyName).WithTriviaFrom(memberBinding.Name)), + + _ => null, + }; } - private static async Task UseCountProperty(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + private static SimpleNameSyntax? GetMethodNameSyntax(InvocationExpressionSyntax invocation) { - var expression = GetParentMemberExpression(nodeToFix); - if (expression is null) - return document; + return invocation.Expression switch + { + MemberAccessExpressionSyntax memberAccess => memberAccess.Name, + MemberBindingExpressionSyntax memberBinding => memberBinding.Name, + SimpleNameSyntax name => name, + _ => null, + }; + } - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var generator = editor.Generator; - var propertyAccess = generator.MemberAccessExpression(expression, "Count"); + /// + /// Returns 'x', or 'x1', 'x2', ... when a symbol named 'x' is in scope, as a lambda parameter could hide it or conflict with it. + /// + private static string? GetUniqueParameterName(SemanticModel semanticModel, int position, Func isValid) + { + for (var i = 0; i < 1000; i++) + { + var name = i == 0 ? "x" : "x" + i.ToString(CultureInfo.InvariantCulture); + if (semanticModel.LookupSymbols(position, name: name).IsEmpty && isValid(name)) + return name; + } - editor.ReplaceNode(nodeToFix, propertyAccess); - return editor.GetChangedDocument(); + return null; } private static async Task UseListMethod(Document document, SyntaxNode nodeToFix, string methodName, bool convertPredicate, CancellationToken cancellationToken) @@ -647,104 +706,138 @@ private static async Task RemoveDuplicatedOrderBy(Document document, T return editor.GetChangedDocument(); } - private static async Task UseThenBy(Document document, TextSpan lastOperationSpan, string expectedMethodName, CancellationToken cancellationToken) + private static async Task UseThenBy(Document document, InvocationExpressionSyntax invocation, SimpleNameSyntax methodName, string expectedMethodName, string? keySelectorParameterName, CancellationToken cancellationToken) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var nodeToFix = root?.FindNode(lastOperationSpan, getInnermostNodeForTie: true); - if (nodeToFix is null) - return document; - - var expression = GetMemberAccessExpression(nodeToFix); - if (expression is null) - return document; - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + var generator = editor.Generator; - var newExpression = expression.WithName(IdentifierName(expectedMethodName)); + var newInvocation = invocation.ReplaceNode(methodName, IdentifierName(expectedMethodName).WithTriviaFrom(methodName)); + if (keySelectorParameterName is not null) + { + var keySelector = (ExpressionSyntax)generator.ValueReturningLambdaExpression(keySelectorParameterName, generator.IdentifierName(keySelectorParameterName)); + newInvocation = newInvocation.AddArgumentListArguments(Argument(keySelector)); + } - editor.ReplaceNode(expression, newExpression); + editor.ReplaceNode(invocation, newInvocation); return editor.GetChangedDocument(); } - private static async Task CombineWhereWithNextMethod(Document document, TextSpan firstOperationSpan, TextSpan lastOperationSpan, CancellationToken cancellationToken) + private static SyntaxNode? CombineWhereWithNextMethod(SyntaxGenerator generator, SemanticModel semanticModel, IInvocationOperation whereOperation, IInvocationOperation nextOperation, CancellationToken cancellationToken) { - // enumerable.Where(x=> x).C() => enumerable.C(x=> x) - // enumerable.Where(x=> x).C(y=>y) => enumerable.C(y=> y && y) - // enumerable.Where(Condition).C(y=>y) => enumerable.C(y=> Condition(y) && y) - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var firstNode = root?.FindNode(firstOperationSpan, getInnermostNodeForTie: true); - var lastNode = root?.FindNode(lastOperationSpan, getInnermostNodeForTie: true); - if (firstNode is null || lastNode is null) - return document; + // enumerable.Where(x => x).C() => enumerable.C(x => x) + // enumerable.Where(x => x).C(y => y) => enumerable.C(x => x && x) + // enumerable.Where(Condition).C(y => y) => enumerable.C(y => Condition(y) && y) + // enumerable.Where(condition1).C(condition2) => enumerable.C(x => condition1(x) && condition2(x)) + if (whereOperation.Arguments.Length != 2) + return null; - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var semanticModel = editor.SemanticModel; - if (semanticModel?.GetOperation(firstNode, cancellationToken) is not IInvocationOperation firstOperation || semanticModel?.GetOperation(lastNode, cancellationToken) is not IInvocationOperation lastOperation) - return document; + var method = generator.MemberAccessExpression(GetSourceSyntax(whereOperation), nextOperation.TargetMethod.Name); + var whereArgument = whereOperation.Arguments[1]; + if (nextOperation.Arguments.Length == 1) + return generator.InvocationExpression(method, whereArgument.Syntax); - var generator = editor.Generator; - var method = generator.MemberAccessExpression(firstOperation.Arguments[0].Syntax, lastOperation.TargetMethod.Name); - var argument = CombineArguments(firstOperation.Arguments.ElementAtOrDefault(1), lastOperation.Arguments.ElementAtOrDefault(1)); - var newExpression = argument is null ? generator.InvocationExpression(method) : generator.InvocationExpression(method, argument); + if (nextOperation.Arguments.Length != 2) + return null; - editor.ReplaceNode(lastOperation.Syntax, newExpression); - return editor.GetChangedDocument(); + var wherePredicate = GetCombinablePredicate(whereArgument.Value); + var nextPredicate = GetCombinablePredicate(nextOperation.Arguments[1].Value); + if (wherePredicate is null || nextPredicate is null) + return null; + + var parameterName = GetCombinedParameterName(semanticModel, nextOperation.Syntax.SpanStart, wherePredicate, nextPredicate, cancellationToken); + if (parameterName is null) + return null; + + var left = CreatePredicateBody(generator, wherePredicate, parameterName); + var right = CreatePredicateBody(generator, nextPredicate, parameterName); + return generator.InvocationExpression(method, generator.ValueReturningLambdaExpression(parameterName, generator.LogicalAndExpression(left, right))); - SyntaxNode? CombineArguments(IArgumentOperation? argument1, IArgumentOperation? argument2) + static SyntaxNode GetSourceSyntax(IInvocationOperation operation) { - if (argument2 is null) - return argument1?.Syntax; + // 'Enumerable.Where(items, predicate)' uses an argument, while 'items.Where(predicate)' uses the expression + var syntax = operation.Arguments[0].Syntax; + return syntax is ArgumentSyntax argument ? argument.Expression : syntax; + } - if (argument1 is null) - return argument2?.Syntax; - if (argument1.Value is IDelegateCreationOperation value1 && argument2.Value is IDelegateCreationOperation value2) - { - var anonymousMethod1 = value1.Target as IAnonymousFunctionOperation; - var anonymousMethod2 = value2.Target as IAnonymousFunctionOperation; + static CombinablePredicate? GetCombinablePredicate(IOperation operation) + { + // Queryable methods convert the lambda to an Expression> + operation = operation.UnwrapImplicitConversions(); - var newParameterName = - anonymousMethod1?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - anonymousMethod2?.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - "x"; + var lambda = operation switch + { + IDelegateCreationOperation { Target: IAnonymousFunctionOperation anonymousFunction } => anonymousFunction, + IAnonymousFunctionOperation anonymousFunction => anonymousFunction, + _ => null, + }; - var left = PrepareSyntaxNode(generator, value1, newParameterName); - var right = PrepareSyntaxNode(generator, value2, newParameterName); + if (lambda is not null) + { + // The body of 'x => { return x > 0; }' or 'delegate (int x) { return x > 0; }' cannot be combined with another condition + if (lambda.Symbol.Parameters.Length != 1 || lambda.Syntax is not LambdaExpressionSyntax { ExpressionBody: { } body }) + return null; - return generator.ValueReturningLambdaExpression(newParameterName, - generator.LogicalAndExpression(left, right)); + return new CombinablePredicate(lambda, body); } - else if (argument1.Value.UnwrapConversions() is IAnonymousFunctionOperation anonymousMethod1 && argument2.Value.UnwrapImplicitConversions() is IAnonymousFunctionOperation anonymousMethod2) - { - var newParameterName = - anonymousMethod1.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - anonymousMethod2.Symbol.Parameters.ElementAtOrDefault(0)?.Name ?? - "x"; - var left = ReplaceParameter(anonymousMethod1, newParameterName); - var right = ReplaceParameter(anonymousMethod2, newParameterName); + if (operation.Syntax is not ExpressionSyntax expression) + return null; - return generator.ValueReturningLambdaExpression(newParameterName, - generator.LogicalAndExpression(left, right)); + // 'Filter' => 'Filter(x)'. The instance of the method is evaluated for each element instead of once. + if (operation is IDelegateCreationOperation { Target: IMethodReferenceOperation methodReference }) + { + if (methodReference.Instance is not null && !CanBeEvaluatedTwice(methodReference.Instance)) + return null; + + return new CombinablePredicate(Lambda: null, expression); } + // 'predicate' => 'predicate(x)'. The delegate is evaluated for each element instead of once. + if (operation.Type is { TypeKind: TypeKind.Delegate } && CanBeEvaluatedTwice(operation)) + return new CombinablePredicate(Lambda: null, expression); + return null; } - static SyntaxNode PrepareSyntaxNode(SyntaxGenerator generator, IDelegateCreationOperation delegateCreationOperation, string parameterName) + static string? GetCombinedParameterName(SemanticModel semanticModel, int position, CombinablePredicate predicate1, CombinablePredicate predicate2, CancellationToken cancellationToken) { - if (delegateCreationOperation.Target is IAnonymousFunctionOperation anonymousMethod) + // Prefer the names chosen by the user. They are already valid at this location, as the lambdas are declared in the same scope. + foreach (var predicate in new[] { predicate1, predicate2 }) { - return ReplaceParameter(anonymousMethod, parameterName); + var name = predicate.Lambda?.Symbol.Parameters[0].Name; + if (name is not null and not "_" && IsValid(name)) + return name; } - if (delegateCreationOperation.Target is IMethodReferenceOperation) + return GetUniqueParameterName(semanticModel, position, IsValid); + + bool IsValid(string name) => !UsesName(predicate1, name) && !UsesName(predicate2, name); + + // The new parameter must not hide a symbol used by one of the predicates, nor conflict with a local declared in one of them + bool UsesName(CombinablePredicate predicate, string name) { - return generator.InvocationExpression( - delegateCreationOperation.Syntax, - generator.IdentifierName("x")); + foreach (var token in predicate.Syntax.DescendantTokens()) + { + if (!token.IsKind(SyntaxKind.IdentifierToken) || token.ValueText != name) + continue; + + // The references to the parameter of the lambda are renamed + if (predicate.Lambda is not null && token.Parent is IdentifierNameSyntax identifierName && semanticModel.GetSymbolInfo(identifierName, cancellationToken).Symbol.IsEqualTo(predicate.Lambda.Symbol.Parameters[0])) + continue; + + return true; + } + + return false; } + } - return delegateCreationOperation.Syntax; + static SyntaxNode CreatePredicateBody(SyntaxGenerator generator, CombinablePredicate predicate, string parameterName) + { + if (predicate.Lambda is not null) + return ReplaceParameter(predicate.Lambda, predicate.Syntax, parameterName); + + return generator.InvocationExpression(predicate.Syntax, generator.IdentifierName(parameterName)); } } @@ -774,11 +867,14 @@ private static async Task ReorderWhereBeforeOrderBy(Document document, return editor.GetChangedDocument(); } - private static SyntaxNode ReplaceParameter(IAnonymousFunctionOperation method, string newParameterName) + private static SyntaxNode ReplaceParameter(IAnonymousFunctionOperation method, ExpressionSyntax body, string newParameterName) { var semanticModel = method.SemanticModel!; var parameterSymbol = method.Symbol.Parameters[0]; - return new ParameterRewriter(semanticModel, parameterSymbol, newParameterName).Visit(method.Body.Syntax); + if (parameterSymbol.Name == newParameterName) + return body; + + return new ParameterRewriter(semanticModel, parameterSymbol, newParameterName).Visit(body); } private static MemberAccessExpressionSyntax? GetMemberAccessExpression(SyntaxNode invocationExpressionSyntax) @@ -798,6 +894,11 @@ private static SyntaxNode ReplaceParameter(IAnonymousFunctionOperation method, s return memberAccessExpression.Expression; } + /// + /// A predicate that can be inlined in a lambda: the expression body of a lambda, or a delegate to invoke. + /// + private sealed record CombinablePredicate(IAnonymousFunctionOperation? Lambda, ExpressionSyntax Syntax); + private sealed class ParameterRewriter(SemanticModel semanticModel, IParameterSymbol parameterSymbol, string newParameterName) : CSharpSyntaxRewriter { public override SyntaxNode? VisitIdentifierName(IdentifierNameSyntax node) @@ -805,7 +906,7 @@ private sealed class ParameterRewriter(SemanticModel semanticModel, IParameterSy var symbol = semanticModel.GetSymbolInfo(node).Symbol; if (symbol is not null && symbol.IsEqualTo(parameterSymbol)) { - return IdentifierName(newParameterName); + return IdentifierName(newParameterName).WithTriviaFrom(node); } return base.VisitIdentifierName(node); diff --git a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs index 361bf8bf4..c2d94e51e 100755 --- a/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs @@ -278,29 +278,16 @@ private void UseCountPropertyInsteadOfMethod(OperationAnalysisContext context, I return; } - if (actualType.AllInterfaces.Any(i => i.OriginalDefinition.IsEqualTo(ICollectionOfTSymbol) || i.OriginalDefinition.IsEqualTo(IReadOnlyCollectionOfTSymbol))) + // The type itself is one of the interfaces when the expression is typed as ICollection or IReadOnlyCollection + if (actualType.GetAllInterfacesIncludingSelf().Any(i => i.OriginalDefinition.IsEqualTo(ICollectionOfTSymbol) || i.OriginalDefinition.IsEqualTo(IReadOnlyCollectionOfTSymbol))) { - // Ensure the Count property is not an explicit implementation - if (HasNonExplicitCountMethod(actualType)) + // Ensure the Count property can be used, i.e. it is not an explicit implementation nor ambiguous + if (HasCountProperty(operation, actualType)) { var properties = CreateProperties(OptimizeLinqUsageData.UseCountProperty); context.ReportDiagnostic(ListMethodsRule, properties, operation, DiagnosticInvocationReportOptions.ReportOnMember, "Count", operation.TargetMethod.Name); return; } - - static bool HasNonExplicitCountMethod(ITypeSymbol type) - { - foreach (var member in type.GetMembers("Count")) - { - if (member.Kind != SymbolKind.Property) - continue; - - if (((IPropertySymbol)member).ExplicitInterfaceImplementations.Length == 0) - return true; - } - - return false; - } } } else if (operation.TargetMethod.Name == nameof(Enumerable.LongCount)) @@ -453,10 +440,22 @@ private void UseIndexerInsteadOfElementAt(OperationAnalysisContext context, IInv if (actualType is null) return; - if (actualType.AllInterfaces.Any(i => i.OriginalDefinition.IsEqualTo(IListOfTSymbol) || i.OriginalDefinition.IsEqualTo(IReadOnlyListOfTSymbol))) + // The type itself is one of the interfaces when the expression is typed as IList or IReadOnlyList + if (!actualType.GetAllInterfacesIncludingSelf().Any(i => i.OriginalDefinition.IsEqualTo(IListOfTSymbol) || i.OriginalDefinition.IsEqualTo(IReadOnlyListOfTSymbol))) + return; + + // Ensure the members used by the code fix can be used, i.e. they are not explicit implementations nor ambiguous + if (actualType.TypeKind != TypeKind.Array) { - context.ReportDiagnostic(IndexerInsteadOfElementAtRule, properties, operation, DiagnosticInvocationReportOptions.ReportOnMember, "[]", operation.TargetMethod.Name); + if (!HasInt32Indexer(operation, actualType)) + return; + + // list[^1] and list[list.Count - 1] use the Count property + if (operation.TargetMethod.Name == nameof(Enumerable.Last) && !HasCountProperty(operation, actualType)) + return; } + + context.ReportDiagnostic(IndexerInsteadOfElementAtRule, properties, operation, DiagnosticInvocationReportOptions.ReportOnMember, "[]", operation.TargetMethod.Name); } private static readonly HashSet CombinableLinqMethods = new(StringComparer.Ordinal) @@ -474,7 +473,7 @@ private void CombineWhereWithNextMethod(OperationAnalysisContext context, IInvoc if (operation.TargetMethod.Name == nameof(Enumerable.Where)) { // Cannot replace Where when using Func - if (IsIndexedPredicateOverload(operation.TargetMethod)) + if (!IsPredicateOverload(operation)) return; // Check parent methods @@ -483,6 +482,11 @@ private void CombineWhereWithNextMethod(OperationAnalysisContext context, IInvoc { if (CombinableLinqMethods.Contains(parent.TargetMethod.Name)) { + // The next method must only take the source, or the source and a predicate. The predicate cannot be combined + // with the other overloads, such as FirstOrDefault(TSource defaultValue) or Where(Func). + if (parent.Arguments.Length != 1 && !IsPredicateOverload(parent)) + return; + // Do not report on IQueryable since combining Where clauses has no performance benefit // (the generated query is identical) and splitting them can improve readability if (QueryableSymbol is not null && operation.TargetMethod.ContainingType.IsEqualTo(QueryableSymbol) && parent.TargetMethod.ContainingType.IsEqualTo(QueryableSymbol)) @@ -502,6 +506,34 @@ private void CombineWhereWithNextMethod(OperationAnalysisContext context, IInvoc } } + /// + /// Detects the overloads taking the source and a predicate, such as Where(Func<TSource,bool>) or Any(Expression<Func<TSource,bool>>). + /// The parameters of the original definition are used, so FirstOrDefault(TSource defaultValue) is not a predicate overload even when TSource is a delegate type. + /// + private bool IsPredicateOverload(IInvocationOperation operation) + { + if (operation.Arguments.Length != 2) + return false; + + var parameters = operation.TargetMethod.OriginalDefinition.Parameters; + if (parameters.Length != 2) + return false; + + if (parameters[1].Type is not INamedTypeSymbol type) + return false; + + // Queryable methods take an Expression> + if (type.OriginalDefinition.IsEqualTo(ExpressionOfTSymbol)) + { + if (type.TypeArguments is not [INamedTypeSymbol delegateType]) + return false; + + type = delegateType; + } + + return type.DelegateInvokeMethod is { Parameters.Length: 1, ReturnType.SpecialType: SpecialType.System_Boolean }; + } + /// /// Detects the overloads taking a predicate with the index of the element, such as Where(Func<TSource,int,bool>). /// Returns when the shape of the method is unknown, so the callers stay on the safe side. @@ -906,22 +938,33 @@ private static void UseCastInsteadOfSelect(OperationAnalysisContext context, IIn var castType = castOp.Type.ToMinimalDisplayString(semanticModel, nullableFlowState, operation.Syntax.SpanStart); context.ReportDiagnostic(OptimizeLinqUsageAnalyzer.UseCastInsteadOfSelect, properties, operation, DiagnosticInvocationReportOptions.ReportOnMember, castType); - bool CanReplaceByCast(IConversionOperation op) + static bool CanReplaceByCast(IConversionOperation op) { - if (op.Conversion.IsUserDefined || op.Conversion.IsNumeric) + // Cast() boxes the value and casts the object to T, so it only supports the conversions + // that do not change the representation of the value + var conversion = op.GetConversion(); + if (conversion.IsUserDefined) return false; - // Handle enums: source.Select(item => (byte)item); - // Using Cast is only possible when the enum underlying type is the same as the conversion type - var operandActualType = op.Operand.GetActualType(context.CancellationToken); - var enumerationType = operandActualType.GetEnumUnderlyingType(); - if (enumerationType is not null) + if (conversion.IsIdentity || conversion.IsReference || conversion.IsBoxing || conversion.IsUnboxing) + return true; + + // Unboxing supports the conversions between an enum, its underlying type, and their nullable types: + // source.Select(item => (byte)item) or source.Select(item => (int?)item); + if (conversion.IsNullable || conversion.IsEnumeration) + return GetUnboxingType(op.Operand.Type).IsEqualTo(GetUnboxingType(op.Type)); + + return false; + + static ITypeSymbol? GetUnboxingType(ITypeSymbol? type) { - if (!enumerationType.IsEqualTo(op.Type)) - return false; - } + if (type is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T, TypeArguments: [var underlyingType] }) + { + type = underlyingType; + } - return true; + return type.GetEnumUnderlyingType() ?? type; + } } } @@ -941,11 +984,56 @@ private void UseCountInsteadOfAny(OperationAnalysisContext context, IInvocationO var implementedInterfaces = operandType.GetAllInterfacesIncludingSelf().Select(i => i.OriginalDefinition); if (implementedInterfaces.Any(i => i.IsEqualTo(ICollectionOfTSymbol) || i.IsEqualTo(ICollectionSymbol) || i.IsEqualTo(IReadOnlyCollectionOfTSymbol))) { + // The code fix uses the Length property of arrays, and the Count property of the other types, + // which may be an explicit implementation of the interface or be ambiguous + if (operandType.TypeKind != TypeKind.Array && !HasCountProperty(operation, operandType)) + return; + context.ReportDiagnostic(OptimizeLinqUsageAnalyzer.UseCountInsteadOfAny, operation); } } } + /// + /// Indicates whether expression.Count binds to an accessible property of the type. The member lookup + /// finds the members inherited from the base types and the base interfaces, and excludes the explicit implementations. + /// + private static bool HasCountProperty(IOperation operation, ITypeSymbol type) + { + var semanticModel = operation.SemanticModel; + if (semanticModel is null) + return false; + + var position = operation.Syntax.SpanStart; + return semanticModel.LookupSymbols(position, type, "Count") is [IPropertySymbol { IsStatic: false, IsIndexer: false, Type.SpecialType: SpecialType.System_Int32, GetMethod: { } getMethod }] + && semanticModel.IsAccessible(position, getMethod); + } + + /// + /// Indicates whether expression[int] binds to a single accessible indexer of the type. + /// + private static bool HasInt32Indexer(IOperation operation, ITypeSymbol type) + { + var semanticModel = operation.SemanticModel; + if (semanticModel is null) + return false; + + var position = operation.Syntax.SpanStart; + IPropertySymbol? result = null; + foreach (var symbol in semanticModel.LookupSymbols(position, type, WellKnownMemberNames.Indexer)) + { + if (symbol is not IPropertySymbol { IsIndexer: true, IsStatic: false, Parameters: [{ Type.SpecialType: SpecialType.System_Int32 }], GetMethod: { } getMethod } indexer) + continue; + + if (result is not null || !semanticModel.IsAccessible(position, getMethod)) + return false; + + result = indexer; + } + + return result is not null; + } + private static IInvocationOperation? GetParentLinqOperation(IOperation op) { var parent = op.Parent; @@ -957,10 +1045,9 @@ private void UseCountInsteadOfAny(OperationAnalysisContext context, IInvocationO if (parent is IInvocationOperation invocationOperation) return invocationOperation; - if (parent is IArgumentOperation) - { - return GetParentLinqOperation(parent); - } + // Only follow the source of the extension method: 'items.Where(...).Any()', but not 'seqs.FirstOrDefault(items.Where(...))' + if (parent is IArgumentOperation { Parameter.Ordinal: 0, Parent: IInvocationOperation { TargetMethod.IsExtensionMethod: true } parentInvocation }) + return parentInvocation; return null; } diff --git a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerTests.cs index 6f4cfc887..0ab3fb62f 100644 --- a/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/OptimizeLinqUsageAnalyzerTests.cs @@ -2611,4 +2611,664 @@ public Test() return test.RunAsync(); } + + [Fact] + public Task CombineWhereWithAny_DelegateVariables() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System; + using System.Linq; + class Test + { + bool M(int[] items, Func f, Func g) => {|MA0029:items.Where(f).Any(g)|}; + } + """; + test.FixedCode = """ + using System; + using System.Linq; + class Test + { + bool M(int[] items, Func f, Func g) => items.Any(x => f(x) && g(x)); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithAny_DelegateVariables_ParameterNameConflict() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System; + using System.Linq; + class Test + { + bool M(int[] items, Func x, Func x1) => {|MA0029:items.Where(x).Any(x1)|}; + } + """; + test.FixedCode = """ + using System; + using System.Linq; + class Test + { + bool M(int[] items, Func x, Func x1) => items.Any(x2 => x(x2) && x1(x2)); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithAny_DelegateReturnedByMethod_NoCodeFix() + { + var code = """ + using System; + using System.Linq; + class Test + { + bool M(int[] items) => {|MA0029:items.Where(GetFilter()).Any(x => x > 0)|}; + + static Func GetFilter() => _ => true; + } + """; + var test = new CodeFixTest(); + test.TestCode = code; + test.FixedCode = code; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithAny_LambdaWithBlockBody_NoCodeFix() + { + var code = """ + using System.Linq; + class Test + { + bool M(int[] items) => {|MA0029:items.Where(x => { return x > 0; }).Any(x => x < 10)|}; + } + """; + var test = new CodeFixTest(); + test.TestCode = code; + test.FixedCode = code; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithAny_LambdaParameterShadowsOuterVariable() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + bool M(int[] items, int x) => {|MA0029:items.Where(x => x > 0).Any(y => y > x)|}; + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + bool M(int[] items, int x) => items.Any(y => y > 0 && y > x); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithAny_MethodGroupWithLambda_UsesLambdaParameterName() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + bool M(int[] items) => {|MA0029:items.Where(Filter).Any(item => item > 0)|}; + + static bool Filter(int value) => true; + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + bool M(int[] items) => items.Any(item => Filter(item) && item > 0); + + static bool Filter(int value) => true; + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("FirstOrDefault")] + [InlineData("LastOrDefault")] + [InlineData("SingleOrDefault")] + public Task CombineWhereWithOrDefault_DefaultValue_NoDiagnostic(string methodName) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + int M(int[] items) => items.Where(x => x > 10).{{methodName}}(5); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhereWithIndexedPredicate_NoDiagnostic() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items) => items.Where(x => x > 0).Where((x, i) => i % 2 == 0); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task CombineWhere_WhereIsNotTheSourceOfTheNextMethod_NoDiagnostic() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + IEnumerable M(IEnumerable> seqs, int[] items) => seqs.FirstOrDefault(items.Where(x => x > 0)); + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("OrderBy(x => -x)", "Order", "ThenBy")] + [InlineData("OrderByDescending(x => -x)", "OrderDescending", "ThenByDescending")] + [InlineData("Order()", "Order", "ThenBy")] + [InlineData("OrderDescending()", "OrderDescending", "ThenByDescending")] + public Task OrderFollowedByOrder_FixWithThenBy(string first, string second, string expectedMethod) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + void M(int[] items) => _ = {|MA0030:items.{{first}}.{{second}}()|}; + } + """; + test.FixedCode = $$""" + using System.Linq; + class Test + { + void M(int[] items) => _ = items.{{first}}.{{expectedMethod}}(x => x); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task OrderFollowedByOrder_FixWithThenBy_ParameterNameConflict() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items, int x) => _ = {|MA0030:items.OrderBy(y => -y).Order()|}; + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(int[] items, int x) => _ = items.OrderBy(y => -y).ThenBy(x1 => x1); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task OrderFollowedByOrderBy_FixWithThenBy() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = {|MA0030:items.Order().OrderBy(x => -x)|}; + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = items.Order().ThenBy(x => -x); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task OrderFollowedByOrder_FixRemoveDuplicate() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = {|MA0030:items.OrderBy(x => -x).Order()|}; + } + """; + test.CodeActionIndex = 1; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = items.Order(); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task UseOrder_StaticInvocation() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = Enumerable.{|MA0159:OrderBy|}(items, x => x); + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = Enumerable.Order(items); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task UseOrderDescending_StaticInvocation() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = Enumerable.{|MA0159:OrderByDescending|}(items, x => x); + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(int[] items) => _ = Enumerable.OrderDescending(items); + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("int[]", "Count", "Length")] + [InlineData("int[]", "LongCount", "LongLength")] + [InlineData("System.Collections.Generic.List", "Count", "Count")] + public Task CountProperty_StaticInvocation(string type, string method, string property) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + long M({{type}} items) => Enumerable.{|MA0020:{{method}}|}(items); + } + """; + test.FixedCode = $$""" + using System.Linq; + class Test + { + long M({{type}} items) => items.{{property}}; + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("int[]", "Count", "Length")] + [InlineData("int[]", "LongCount", "LongLength")] + [InlineData("System.Collections.Generic.List", "Count", "Count")] + public Task CountProperty_ConditionalAccess(string type, string method, string property) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + long? M({{type}} items) => items?{|MA0020:.{{method}}()|}; + } + """; + test.FixedCode = $$""" + using System.Linq; + class Test + { + long? M({{type}} items) => items?.{{property}}; + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task Any_Array_CodeFix() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + bool M(int[] items) => {|MA0112:items.Any()|}; + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + bool M(int[] items) => items.Length != 0; + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task Any_StaticInvocation_CodeFix() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + bool M(List items) => {|MA0112:Enumerable.Any(items)|}; + } + """; + test.FixedCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + bool M(List items) => items.Count != 0; + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task Any_ConditionalAccess_NoCodeFix() + { + var code = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + bool? M(List items) => items?{|MA0112:.Any()|}; + } + """; + var test = new CodeFixTest(); + test.TestCode = code; + test.FixedCode = code; + + return test.RunAsync(); + } + + [Fact] + public Task Any_ICollectionExplicitImplementation_NoDiagnostic() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections; + using System.Collections.Generic; + using System.Linq; + class Test + { + bool M(Collection items) => items.Any(); + + class Collection : ICollection + { + int ICollection.Count => throw null; + bool ICollection.IsReadOnly => throw null; + void ICollection.Add(T item) => throw null; + void ICollection.Clear() => throw null; + bool ICollection.Contains(T item) => throw null; + void ICollection.CopyTo(T[] array, int arrayIndex) => throw null; + IEnumerator IEnumerable.GetEnumerator() => throw null; + IEnumerator IEnumerable.GetEnumerator() => throw null; + bool ICollection.Remove(T item) => throw null; + } + } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("long?")] + [InlineData("long")] + [InlineData("E")] + [InlineData("E?")] + public Task OptimizeLinq_CastChangesTheRepresentation_NoDiagnostic(string type) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + void M(int[] items) => _ = items.Select(x => ({{type}})x); + } + + enum E : byte { } + """; + + return test.RunAsync(); + } + + [Theory] + [InlineData("int?")] + [InlineData("E")] + [InlineData("E?")] + public Task OptimizeLinq_CastKeepsTheRepresentation(string type) + { + var test = new CodeFixTest(); + test.TestCode = $$""" + using System.Linq; + class Test + { + void M(int[] items) => _ = items.{|MA0078:Select|}(x => ({{type}})x); + } + + enum E { } + """; + test.FixedCode = $$""" + using System.Linq; + class Test + { + void M(int[] items) => _ = items.Cast<{{type}}>(); + } + + enum E { } + """; + + return test.RunAsync(); + } + + [Fact] + public Task OptimizeLinq_Unboxing() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Linq; + class Test + { + void M(object[] items) => _ = items.{|MA0078:Select|}(x => (int)x); + } + """; + test.FixedCode = """ + using System.Linq; + class Test + { + void M(object[] items) => _ = items.Cast(); + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task InterfaceReceivers() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + void M(IList a, ICollection b, IReadOnlyList c, IReadOnlyCollection d) + { + _ = a.{|MA0098:First|}(); + _ = b.{|MA0020:Count|}(); + _ = c.{|MA0098:Last|}(); + _ = a.{|MA0020:Count|}(); + _ = c.{|MA0098:ElementAt|}(1); + _ = d.{|MA0020:Count|}(); + } + } + """; + test.FixedCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + void M(IList a, ICollection b, IReadOnlyList c, IReadOnlyCollection d) + { + _ = a[0]; + _ = b.Count; + _ = c[^1]; + _ = a.Count; + _ = c[1]; + _ = d.Count; + } + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task InterfaceReceivers_CSharp7_3() + { + var test = new CodeFixTest(); + test.LanguageVersion = LanguageVersion.CSharp7_3; + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + void M(IList a, IReadOnlyList b) + { + _ = a.{|MA0098:Last|}(); + _ = b.{|MA0098:Last|}(); + } + } + """; + test.FixedCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + void M(IList a, IReadOnlyList b) + { + _ = a[a.Count - 1]; + _ = b[b.Count - 1]; + } + } + """; + + return test.RunAsync(); + } + + [Fact] + public Task InterfaceReceivers_AmbiguousMembers_NoDiagnostic() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + void M(IMyList a) + { + _ = a.First(); + _ = a.Last(); + _ = a.ElementAt(1); + _ = a.Count(); + } + } + + interface IMyList : IList, IReadOnlyList { } + """; + + return test.RunAsync(); + } + + [Fact] + public Task Count_ClassDerivedFromList() + { + var test = new CodeFixTest(); + test.TestCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + int M(MyList items) => items.{|MA0020:Count|}(); + } + + class MyList : List { } + """; + test.FixedCode = """ + using System.Collections.Generic; + using System.Linq; + class Test + { + int M(MyList items) => items.Count; + } + + class MyList : List { } + """; + + return test.RunAsync(); + } }