Skip to content

Commit

Permalink
Refactor CA1826 to read option only when needed and use SpecialType c…
Browse files Browse the repository at this point in the history
…hecks
  • Loading branch information
Youssef1313 committed Aug 20, 2023
1 parent 3587540 commit 5682065
Showing 1 changed file with 21 additions and 20 deletions.
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,21 @@ 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)
{
return context.Options.GetBoolOptionValue(
EditorConfigOptionNames.ExcludeOrDefaultMethods, Rule, context.Operation.Syntax.SyntaxTree,
context.Compilation, defaultValue: false);
}
}
}

0 comments on commit 5682065

Please sign in to comment.