Skip to content

Refactor CA1826 to read option only when needed and use SpecialType checks #6872

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Refactor CA1826 to read option only when needed and use SpecialType checks #6872

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions ...Runtime/DoNotUseEnumerableMethodsOnIndexableCollectionsInsteadUseTheCollectionDirectly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down Expand Up @@ -46,29 +46,23 @@ public override void Initialize(AnalysisContext context)

private static void OnCompilationStart(CompilationStartAnalysisContext context)
{
var listType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericIList1);
var readonlyListType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericIReadOnlyList1);
var enumerableType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemLinqEnumerable);
if (readonlyListType == null || enumerableType == null || listType == null)
if (enumerableType == null)
{
return;
}

context.RegisterOperationAction(operationContext =>
context.RegisterOperationAction(context =>
{
var invocation = (IInvocationOperation)operationContext.Operation;
var invocation = (IInvocationOperation)context.Operation;

var excludeOrDefaultMethods = operationContext.Options.GetBoolOptionValue(
EditorConfigOptionNames.ExcludeOrDefaultMethods, Rule, invocation.Syntax.SyntaxTree,
operationContext.Compilation, defaultValue: false);

if (!IsPossibleLinqInvocation(invocation, excludeOrDefaultMethods))
if (!IsPossibleLinqInvocation(invocation, context))
{
return;
}

var methodSymbol = invocation.TargetMethod.ReducedFrom ?? invocation.TargetMethod;
var targetType = invocation.GetReceiverType(operationContext.Compilation, beforeConversion: true, cancellationToken: operationContext.CancellationToken);
var targetType = invocation.GetReceiverType(context.Compilation, beforeConversion: true, cancellationToken: context.CancellationToken);
if (methodSymbol == null || targetType == null)
{
return;
Expand All @@ -79,13 +73,13 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context)
return;
}

if (!IsTypeWithInefficientLinqMethods(targetType, readonlyListType, listType))
if (!IsTypeWithInefficientLinqMethods(targetType))
{
return;
}

var properties = new Dictionary<string, string?> { [MethodPropertyKey] = invocation.TargetMethod.Name }.ToImmutableDictionary();
operationContext.ReportDiagnostic(invocation.CreateDiagnostic(Rule, properties));
context.ReportDiagnostic(invocation.CreateDiagnostic(Rule, properties));
}, OperationKind.Invocation);
}

Expand All @@ -97,10 +91,10 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context)
/// At this point it only identifies <see cref="IReadOnlyList{T}"/> directly but could easily be extended to support
/// any type which has an index and count property.
/// </summary>
private static bool IsTypeWithInefficientLinqMethods(ITypeSymbol targetType, ITypeSymbol readonlyListType, ITypeSymbol listType)
private static bool IsTypeWithInefficientLinqMethods(ITypeSymbol targetType)
{
// If this type is simply IReadOnlyList<T> then no further checking is needed.
if (targetType.TypeKind == TypeKind.Interface && targetType.OriginalDefinition.Equals(readonlyListType))
if (targetType.TypeKind == TypeKind.Interface && targetType.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_IReadOnlyList_T)
{
return true;
}
Expand All @@ -109,12 +103,12 @@ private static bool IsTypeWithInefficientLinqMethods(ITypeSymbol targetType, ITy
bool implementsList = false;
foreach (var current in targetType.AllInterfaces)
{
if (current.OriginalDefinition.Equals(readonlyListType))
if (current.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_IReadOnlyList_T)
{
implementsReadOnlyList = true;
}

if (current.OriginalDefinition.Equals(listType))
if (current.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_IList_T)
{
implementsList = true;
}
Expand All @@ -139,14 +133,19 @@ private static bool IsSingleParameterLinqMethod(IMethodSymbol methodSymbol, ITyp
methodSymbol.Parameters.Length == 1;
}

private static bool IsPossibleLinqInvocation(IInvocationOperation invocation, bool excludeOrDefaultMethods)
private static bool IsPossibleLinqInvocation(IInvocationOperation invocation, OperationAnalysisContext context)
{
return invocation.TargetMethod.Name switch
{
"Last" or "First" or "Count" => true,
"LastOrDefault" or "FirstOrDefault" => !excludeOrDefaultMethods,
"LastOrDefault" or "FirstOrDefault" => !ShouldExcludeOrDefaultMethods(context),
_ => false,
};
}

private static bool ShouldExcludeOrDefaultMethods(OperationAnalysisContext context)
=> context.Options.GetBoolOptionValue(
EditorConfigOptionNames.ExcludeOrDefaultMethods, Rule, context.Operation.Syntax.SyntaxTree,
context.Compilation, defaultValue: false);
}
}