Skip to content

Commit

Permalink
Analyzer: Prefer .Length/Count/IsEmpty over Any() (#6236)
Browse files Browse the repository at this point in the history
* Add analyzer and fixer.

* Update src/NetAnalyzers/VisualBasic/Microsoft.NetCore.Analyzers/Performance/BasicPreferLengthCountIsEmptyOverAnyFixer.vb

* Extended type check for 'Length' and changed wording of message.

* Updated wording on message descriptions.

* Change 'GetReceiverType' to return an 'ITypeSymbol'.
  • Loading branch information
CollinAlpert authored Jan 30, 2023
1 parent 324147f commit 9f9d9fa
Show file tree
Hide file tree
Showing 29 changed files with 2,195 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,9 @@ public static void AnalyzeInvocationForIgnoredReturnValue(OperationAnalysisConte
return;
}

INamedTypeSymbol? type = invocation.GetReceiverType(context.Compilation, beforeConversion: false, context.CancellationToken);

// If we're not in one of the known immutable types, quit
if (type is not null && type.GetBaseTypesAndThis().Any(immutableTypeSymbols.Contains))
if (invocation.GetReceiverType(context.Compilation, beforeConversion: false, context.CancellationToken) is INamedTypeSymbol type
&& type.GetBaseTypesAndThis().Any(immutableTypeSymbols.Contains))
{
context.ReportDiagnostic(invocation.CreateDiagnostic(DoNotIgnoreReturnValueDiagnosticRule, type.Name, methodName));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.NetCore.Analyzers.Performance;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;

namespace Microsoft.NetCore.CSharp.Analyzers.Performance
{
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class CSharpPreferLengthCountIsEmptyOverAnyFixer : PreferLengthCountIsEmptyOverAnyFixer
{
protected override SyntaxNode? ReplaceAnyWithIsEmpty(SyntaxNode root, SyntaxNode node)
{
if (node is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccess } invocation)
{
return null;
}

var expression = memberAccess.Expression;
if (invocation.ArgumentList.Arguments.Count > 0)
{
expression = invocation.ArgumentList.Arguments[0].Expression;
}

var newMemberAccess = MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
expression,
IdentifierName(PreferLengthCountIsEmptyOverAnyAnalyzer.IsEmptyText)
);
if (invocation.Parent.IsKind(SyntaxKind.LogicalNotExpression))
{
return root.ReplaceNode(invocation.Parent, newMemberAccess.WithTriviaFrom(invocation.Parent));
}

var negatedExpression = PrefixUnaryExpression(
SyntaxKind.LogicalNotExpression,
newMemberAccess
);

return root.ReplaceNode(invocation, negatedExpression.WithTriviaFrom(invocation));
}

protected override SyntaxNode? ReplaceAnyWithLength(SyntaxNode root, SyntaxNode node)
{
return ReplaceAnyWithPropertyCheck(root, node, PreferLengthCountIsEmptyOverAnyAnalyzer.LengthText);
}

protected override SyntaxNode? ReplaceAnyWithCount(SyntaxNode root, SyntaxNode node)
{
return ReplaceAnyWithPropertyCheck(root, node, PreferLengthCountIsEmptyOverAnyAnalyzer.CountText);
}

private static SyntaxNode? ReplaceAnyWithPropertyCheck(SyntaxNode root, SyntaxNode node, string propertyName)
{
if (node is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccess } invocation)
{
return null;
}

var expression = memberAccess.Expression;
if (invocation.ArgumentList.Arguments.Count > 0)
{
// .Any() used like a normal static method and not like an extension method.
expression = invocation.ArgumentList.Arguments[0].Expression;
}

static BinaryExpressionSyntax GetBinaryExpression(ExpressionSyntax expression, string member, SyntaxKind expressionKind)
{
return BinaryExpression(
expressionKind,
MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
expression,
IdentifierName(member)
),
LiteralExpression(
SyntaxKind.NumericLiteralExpression,
Literal(0)
)
);
}

if (invocation.Parent.IsKind(SyntaxKind.LogicalNotExpression))
{
var binaryExpression = GetBinaryExpression(expression, propertyName, SyntaxKind.EqualsExpression);

return root.ReplaceNode(invocation.Parent, binaryExpression.WithTriviaFrom(invocation.Parent));
}

return root.ReplaceNode(invocation, GetBinaryExpression(expression, propertyName, SyntaxKind.NotEqualsExpression).WithTriviaFrom(invocation));
}
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ CA1856 | Performance | Error | ConstantExpectedAnalyzer, [Documentation](https:/
CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1857)
CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
CA1859 | Performance | Info | UseConcreteTypeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859)
CA1860 | Performance | Info | PreferLengthCountIsEmptyOverAnyAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1860)
CA2021 | Reliability | Info | DoNotCallEnumerableCastOrOfTypeWithIncompatibleTypesAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2021)

### Removed Rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public override void Initialize(AnalysisContext context)
return;
}
var receiverType = invocation.GetReceiverType(operationContext.Compilation, beforeConversion: true, cancellationToken: operationContext.CancellationToken);
var receiverType = (INamedTypeSymbol?)invocation.GetReceiverType(operationContext.Compilation, beforeConversion: true, cancellationToken: operationContext.CancellationToken);
if (receiverType != null &&
receiverType.DerivesFromOrImplementsAnyConstructionOf(immutableCollectionType))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2013,4 +2013,28 @@ Widening and user defined conversions are not supported with generic types.</val
<data name="UseConcreteTypeForParameterMessage" xml:space="preserve">
<value>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</value>
</data>
</root>
<data name="PreferLengthOverAnyCodeFixTitle" xml:space="preserve">
<value>Use 'Length' check instead of 'Any()'</value>
</data>
<data name="PreferLengthOverAnyMessage" xml:space="preserve">
<value>Prefer comparing 'Length' to 0 rather than using 'Any()', both for clarity and for performance</value>
</data>
<data name="PreferCountOverAnyCodeFixTitle" xml:space="preserve">
<value>Use 'Count' check instead of 'Any()'</value>
</data>
<data name="PreferLengthCountIsEmptyOverAnyTitle" xml:space="preserve">
<value>Avoid using 'Enumerable.Any()' extension method</value>
</data>
<data name="PreferCountOverAnyMessage" xml:space="preserve">
<value>Prefer comparing 'Count' to 0 rather than using 'Any()', both for clarity and for performance</value>
</data>
<data name="PreferLengthCountIsEmptyOverAnyDescription" xml:space="preserve">
<value>Prefer using 'IsEmpty', 'Count' or 'Length' properties whichever available, rather than calling 'Enumerable.Any()'. The intent is clearer and it is more performant than using 'Enumerable.Any()' extension method.</value>
</data>
<data name="PreferIsEmptyOverAnyCodeFixTitle" xml:space="preserve">
<value>Use 'IsEmpty' check instead of 'Any()'</value>
</data>
<data name="PreferIsEmptyOverAnyMessage" xml:space="preserve">
<value>Prefer an 'IsEmpty' check rather than using 'Any()', both for clarity and for performance</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Microsoft.NetCore.Analyzers.Performance
{
public abstract class PreferLengthCountIsEmptyOverAnyFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(PreferLengthCountIsEmptyOverAnyAnalyzer.RuleId);

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var node = root.FindNode(context.Span, getInnermostNodeForTie: true);

foreach (var diagnostic in context.Diagnostics)
{
var (newRoot, codeFixTitle) = diagnostic.Properties[PreferLengthCountIsEmptyOverAnyAnalyzer.DiagnosticPropertyKey] switch
{
PreferLengthCountIsEmptyOverAnyAnalyzer.IsEmptyText => (ReplaceAnyWithIsEmpty(root, node), MicrosoftNetCoreAnalyzersResources.PreferIsEmptyOverAnyCodeFixTitle),
PreferLengthCountIsEmptyOverAnyAnalyzer.LengthText => (ReplaceAnyWithLength(root, node), MicrosoftNetCoreAnalyzersResources.PreferLengthOverAnyCodeFixTitle),
PreferLengthCountIsEmptyOverAnyAnalyzer.CountText => (ReplaceAnyWithCount(root, node), MicrosoftNetCoreAnalyzersResources.PreferCountOverAnyCodeFixTitle),
_ => throw new NotSupportedException()
};
if (newRoot is null)
{
continue;
}

var codeAction = CodeAction.Create(codeFixTitle, _ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot)), codeFixTitle);
context.RegisterCodeFix(codeAction, diagnostic);
}
}

protected abstract SyntaxNode? ReplaceAnyWithIsEmpty(SyntaxNode root, SyntaxNode node);
protected abstract SyntaxNode? ReplaceAnyWithLength(SyntaxNode root, SyntaxNode node);
protected abstract SyntaxNode? ReplaceAnyWithCount(SyntaxNode root, SyntaxNode node);

public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Immutable;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using static Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources;

namespace Microsoft.NetCore.Analyzers.Performance
{
/// <summary>
/// Prefer using 'IsEmpty' or comparing 'Count' / 'Length' property to 0 rather than using 'Any()', both for clarity and for performance.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class PreferLengthCountIsEmptyOverAnyAnalyzer : DiagnosticAnalyzer
{
private const string AnyText = nameof(Enumerable.Any);

internal const string IsEmptyText = nameof(ImmutableArray<dynamic>.IsEmpty);
internal const string LengthText = nameof(Array.Length);
internal const string CountText = nameof(ICollection.Count);

internal const string RuleId = "CA1860";
internal const string DiagnosticPropertyKey = nameof(DiagnosticPropertyKey);

internal static readonly DiagnosticDescriptor IsEmptyDescriptor = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyTitle)),
CreateLocalizableResourceString(nameof(PreferIsEmptyOverAnyMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyDescription)),
isPortedFxCopRule: false,
isDataflowRule: false
);

internal static readonly DiagnosticDescriptor LengthDescriptor = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyTitle)),
CreateLocalizableResourceString(nameof(PreferLengthOverAnyMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyDescription)),
isPortedFxCopRule: false,
isDataflowRule: false
);

internal static readonly DiagnosticDescriptor CountDescriptor = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyTitle)),
CreateLocalizableResourceString(nameof(PreferCountOverAnyMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(PreferLengthCountIsEmptyOverAnyDescription)),
isPortedFxCopRule: false,
isDataflowRule: false
);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
LengthDescriptor,
CountDescriptor,
IsEmptyDescriptor
);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(ctx =>
{
var typeProvider = WellKnownTypeProvider.GetOrCreate(ctx.Compilation);
var iEnumerable = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsIEnumerable);
var iEnumerableOfT = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericIEnumerable1);
var anyMethod = typeProvider
.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemLinqEnumerable)
?.GetMembers(AnyText)
.OfType<IMethodSymbol>()
.FirstOrDefault(m => m.IsExtensionMethod && m.Parameters.Length == 1);
if (iEnumerable is not null && iEnumerableOfT is not null && anyMethod is not null)
{
ctx.RegisterOperationAction(c => OnInvocationAnalysis(c, iEnumerable, iEnumerableOfT, anyMethod), OperationKind.Invocation);
}
});
}

private static void OnInvocationAnalysis(OperationAnalysisContext context, INamedTypeSymbol iEnumerable, INamedTypeSymbol iEnumerableOfT, IMethodSymbol anyMethod)
{
var invocation = (IInvocationOperation)context.Operation;
var originalMethod = invocation.TargetMethod.OriginalDefinition;
if (originalMethod.MethodKind == MethodKind.ReducedExtension)
{
originalMethod = originalMethod.ReducedFrom;
}

if (originalMethod.Equals(anyMethod, SymbolEqualityComparer.Default))
{
var type = invocation.GetReceiverType(context.Compilation, beforeConversion: true, context.CancellationToken);
if (type is null || (!type.AllInterfaces.Contains(iEnumerable, SymbolEqualityComparer.Default) && !type.AllInterfaces.Contains(iEnumerableOfT)))
{
return;
}

if (HasEligibleIsEmptyProperty(type))
{
var properties = ImmutableDictionary.CreateBuilder<string, string?>();
properties.Add(DiagnosticPropertyKey, IsEmptyText);
context.ReportDiagnostic(invocation.CreateDiagnostic(IsEmptyDescriptor, properties: properties.ToImmutable()));
}
else if (HasEligibleLengthProperty(type))
{
var properties = ImmutableDictionary.CreateBuilder<string, string?>();
properties.Add(DiagnosticPropertyKey, LengthText);
context.ReportDiagnostic(invocation.CreateDiagnostic(LengthDescriptor, properties: properties.ToImmutable()));
}

else if (HasEligibleCountProperty(type))
{
var properties = ImmutableDictionary.CreateBuilder<string, string?>();
properties.Add(DiagnosticPropertyKey, CountText);
context.ReportDiagnostic(invocation.CreateDiagnostic(CountDescriptor, properties: properties.ToImmutable()));
}
}
}

private static bool HasEligibleIsEmptyProperty(ITypeSymbol typeSymbol)
{
return typeSymbol.GetMembers(IsEmptyText)
.OfType<IPropertySymbol>()
.Any(property => property.Type.SpecialType == SpecialType.System_Boolean);
}

private static bool HasEligibleLengthProperty(ITypeSymbol typeSymbol)
{
if (typeSymbol is IArrayTypeSymbol)
{
return true;
}

return typeSymbol.GetMembers(LengthText)
.OfType<IPropertySymbol>()
.Any(property => property.Type.SpecialType is SpecialType.System_Int32 or SpecialType.System_UInt32);
}

private static bool HasEligibleCountProperty(ITypeSymbol typeSymbol)
{
return typeSymbol.GetMembers(CountText)
.OfType<IPropertySymbol>()
.Any(property => property.Type.SpecialType is SpecialType.System_Int32 or SpecialType.System_UInt32);
}
}
}
Loading

0 comments on commit 9f9d9fa

Please sign in to comment.