Skip to content

Commit

Permalink
[EC87] Use collection indexer (#41)
Browse files Browse the repository at this point in the history
  • Loading branch information
Djoums authored May 13, 2024
1 parent 7d012b4 commit 87857e1
Show file tree
Hide file tree
Showing 23 changed files with 745 additions and 45 deletions.
10 changes: 5 additions & 5 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
<PackageVersion Include="Microsoft.VSSDK.BuildTools" Version="17.9.3184" />
<PackageVersion Include="MSTest.TestAdapter" Version="3.3.1" />
<PackageVersion Include="MSTest.TestFramework" Version="3.3.1" />
<PackageVersion Include="Roslynator.Analyzers" Version="4.12.2" />
<PackageVersion Include="Roslynator.CodeAnalysis.Analyzers" Version="4.12.2" />
<PackageVersion Include="Roslynator.CodeFixes" Version="4.12.2" />
<PackageVersion Include="Roslynator.Formatting.Analyzers" Version="4.12.2" />
<PackageVersion Include="Roslynator.Analyzers" Version="4.12.3" />
<PackageVersion Include="Roslynator.CodeAnalysis.Analyzers" Version="4.12.3" />
<PackageVersion Include="Roslynator.CodeFixes" Version="4.12.3" />
<PackageVersion Include="Roslynator.Formatting.Analyzers" Version="4.12.3" />
</ItemGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion NuGet.Config
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
<package pattern="*" />
</packageSource>
</packageSourceMapping>
</configuration>
</configuration>
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ Both the EcoCode NuGet package and Visual Studio extension target .Net Standard
|[EC83](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC83/csharp/EC83.asciidoc)|Replace Enum ToString() with nameof|⚠️|✔️|✔️|
|[EC84](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC84/csharp/EC84.asciidoc)|Avoid async void methods|⚠️|✔️|✔️|
|[EC85](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC85/csharp/EC85.asciidoc)|Make type sealed|ℹ️|✔️|✔️|
|[EC86](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC86/csharp/EC86.asciidoc)|`GC.Collect` should not be called|⚠️|✔️||
|[EC87](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC87/csharp/EC87.asciidoc)|Use collection indexer|⚠️|✔️|✔️|

🤝 Contribution
---------------
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
namespace EcoCode.Analyzers;

/// <summary>The code fix provider for avoid async void methods.</summary>
/// <summary>EC86 fixer : GC Collect should not be called.</summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(GCCollectShouldNotBeCalledFixer)), Shared]
public sealed class GCCollectShouldNotBeCalledFixer: CodeFixProvider
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => [GCCollectShouldNotBeCalled.Descriptor.Id];
public override ImmutableArray<string> FixableDiagnosticIds => _fixableDiagnosticIds;
private static readonly ImmutableArray<string> _fixableDiagnosticIds = [GCCollectShouldNotBeCalled.Descriptor.Id];

/// <inheritdoc/>
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
using System.Linq;
namespace EcoCode.Analyzers;

namespace EcoCode.Analyzers;

/// <summary>Analyzer for avoid async void methods.</summary>
/// <summary>EC86 : GC Collect should not be called.</summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class GCCollectShouldNotBeCalled: DiagnosticAnalyzer
{
private static readonly ImmutableArray<SyntaxKind> SyntaxKinds = [SyntaxKind.InvocationExpression];

/// <summary>The diagnostic descriptor.</summary>
public static DiagnosticDescriptor Descriptor { get; } = new(
Rule.Ids.EC86_GCCollectShouldNotBeCalled,
Expand All @@ -18,30 +18,30 @@ public sealed class GCCollectShouldNotBeCalled: DiagnosticAnalyzer
helpLinkUri: Rule.GetHelpUri(Rule.Ids.EC86_GCCollectShouldNotBeCalled));

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Descriptor];
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;
private static readonly ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = [Descriptor];

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(static context => AnalyzeMethod(context), SyntaxKind.InvocationExpression);
context.RegisterSyntaxNodeAction(static context => AnalyzeMethod(context), SyntaxKinds);
}

private static void AnalyzeMethod(SyntaxNodeAnalysisContext context)
{
var invocationExpression = (InvocationExpressionSyntax)context.Node;

//if the expression is not a method or method name is not GC.Collect or Containing type is not System.GC, return
if (context.SemanticModel.GetSymbolInfo(invocationExpression).Symbol is not IMethodSymbol methodSymbol
|| methodSymbol.Name != nameof(GC.Collect)
|| !SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType,
context.SemanticModel.Compilation.GetTypeByMetadataName("System.GC")))
if (context.SemanticModel.GetSymbolInfo(invocationExpression).Symbol is not IMethodSymbol methodSymbol ||
methodSymbol.Name != nameof(GC.Collect) ||
!SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType,
context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(GC).FullName)))
{
return;
}

//If there is no arguments or the "generation" argument is not 0, raise report
// If there is no arguments or the "generation" argument is not 0, report
bool report = !invocationExpression.ArgumentList.Arguments.Any();
if (!report)
{
Expand All @@ -51,13 +51,19 @@ private static void AnalyzeMethod(SyntaxNodeAnalysisContext context)
string firstParameterName = methodSymbol.Parameters[0].Name; // Parameter name from the method signature
if (firstArgument.NameColon.Name.Identifier.Text != firstParameterName)
{
firstArgument = invocationExpression.ArgumentList.Arguments
.First(arg => arg.NameColon?.Name.Identifier.Text == firstParameterName);
foreach (var argument in invocationExpression.ArgumentList.Arguments)
{
if (argument.NameColon?.Name.Identifier.Text == firstParameterName)
{
firstArgument = argument; // Should always be hit in this case
break;
}
}
}
}

var constantValue = context.SemanticModel.GetConstantValue(firstArgument.Expression);
if (constantValue.Value is not int intValue || intValue != 0)
report = true;
report = constantValue.Value is not int intValue || intValue != 0;
}

if(report)
Expand Down
125 changes: 125 additions & 0 deletions src/EcoCode.Core/Analyzers/EC87.UseListIndexer.Fixer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using System.Linq;
using System.Reflection;

namespace EcoCode.Analyzers;

/// <summary>EC87 fixer: Use list indexer.</summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(UseListIndexerFixer)), Shared]
public sealed class UseListIndexerFixer : CodeFixProvider
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => _fixableDiagnosticIds;
private static readonly ImmutableArray<string> _fixableDiagnosticIds = [UseListIndexer.Descriptor.Id];

/// <inheritdoc/>
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc/>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root is null) return;

foreach (var diagnostic in context.Diagnostics)
{
var parent = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent;
if (parent is null) continue;

foreach (var node in parent.AncestorsAndSelf())
{
if (node is not InvocationExpressionSyntax invocation || invocation.Expression is not MemberAccessExpressionSyntax memberAccess)
continue;

var refactorFunc = memberAccess.Name.Identifier.ValueText switch
{
nameof(Enumerable.First) => RefactorFirstAsync,
nameof(Enumerable.Last) =>
context.Document.GetLanguageVersion() >= LanguageVersion.CSharp8
? RefactorLastWithIndexAsync
: RefactorLastWithCountOrLengthAsync,
nameof(Enumerable.ElementAt) => RefactorElementAtAsync,
_ => default(Func<Document, InvocationExpressionSyntax, CancellationToken, Task<Document>>),
};

if (refactorFunc is null) continue;
context.RegisterCodeFix(
CodeAction.Create(
title: "Use collection indexer",
createChangedDocument: token => refactorFunc(context.Document, invocation, token),
equivalenceKey: "Use collection indexer"),
diagnostic);
break;
}
}
}

private static async Task<Document> UpdateDocument(Document document, InvocationExpressionSyntax invocation, ExpressionSyntax indexExpr, CancellationToken token)
{
if (await document.GetSyntaxRootAsync(token) is not SyntaxNode root)
return document;

var elementAccess = SyntaxFactory.ElementAccessExpression(
((MemberAccessExpressionSyntax)invocation.Expression).Expression,
SyntaxFactory.BracketedArgumentList(
SyntaxFactory.SingletonSeparatedList(
SyntaxFactory.Argument(indexExpr))));

return document.WithSyntaxRoot(root.ReplaceNode(invocation, elementAccess));
}

private static Task<Document> RefactorFirstAsync(Document document, InvocationExpressionSyntax invocationExpr, CancellationToken token) =>
UpdateDocument(document, invocationExpr, SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(0)), token);

private static Task<Document> RefactorLastWithIndexAsync(Document document, InvocationExpressionSyntax invocation, CancellationToken token) =>
UpdateDocument(document, invocation,
SyntaxFactory.PrefixUnaryExpression(SyntaxKind.IndexExpression,
SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(1))),
token);

private static async Task<Document> RefactorLastWithCountOrLengthAsync(Document document, InvocationExpressionSyntax invocation, CancellationToken token)
{
if (await document.GetSemanticModelAsync(token).ConfigureAwait(false) is not { } semanticModel)
return document;

var memberAccess = (MemberAccessExpressionSyntax)invocation.Expression;
if (semanticModel.GetTypeInfo(memberAccess.Expression, token).Type is not { } memberType ||
GetCountOrLength(memberType, invocation.SpanStart, semanticModel) is not { } countOrLength)
{
return document;
}

var property = SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
memberAccess.Expression,
SyntaxFactory.IdentifierName(countOrLength.Name));

var oneLiteral = SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(1));

var indexExpression = SyntaxFactory.BinaryExpression(SyntaxKind.SubtractExpression, property, oneLiteral);

return await UpdateDocument(document, invocation, indexExpression, token);

static ISymbol? GetCountOrLength(ITypeSymbol type, int position, SemanticModel semanticModel)
{
do
{
foreach (var member in type.GetMembers())
{
if (member.Name is nameof(IReadOnlyList<int>.Count) or nameof(Array.Length) &&
member is IPropertySymbol prop &&
prop.Type.IsPrimitiveNumber() &&
semanticModel.IsAccessible(position, prop))
{
return prop;
}
}
type = type.BaseType!;
} while (type is not null);

return null;
}
}

private static Task<Document> RefactorElementAtAsync(Document document, InvocationExpressionSyntax invocation, CancellationToken token) =>
UpdateDocument(document, invocation, invocation.ArgumentList.Arguments[0].Expression, token);
}
81 changes: 81 additions & 0 deletions src/EcoCode.Core/Analyzers/EC87.UseListIndexer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
using System.Collections;
using System.Linq;

namespace EcoCode.Analyzers;

/// <summary>EC87: Use list indexer.</summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UseListIndexer : DiagnosticAnalyzer
{
private static readonly ImmutableArray<SyntaxKind> SyntaxKinds = [SyntaxKind.InvocationExpression];

/// <summary>The diagnostic descriptor.</summary>
public static DiagnosticDescriptor Descriptor { get; } = new(
Rule.Ids.EC87_UseCollectionIndexer,
title: "Use list indexer",
messageFormat: "A list indexer should be used instead of a Linq method",
Rule.Categories.Performance,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: "Collections that implement IList, IList<T> or IReadOnlyList<T>, should use their indexers instead of Linq methods for improved performance.",
helpLinkUri: Rule.GetHelpUri(Rule.Ids.EC87_UseCollectionIndexer));

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;
private static readonly ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = [Descriptor];

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(static context => AnalyzeInvocationExpression(context), SyntaxKinds);
}

// TODO: analysis can be improved by including scenarios with method chains
// For example: myList.Skip(5).First() should be refactored to myList[5]

private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context)
{
var invocationExpr = (InvocationExpressionSyntax)context.Node;
var memberAccess = (MemberAccessExpressionSyntax)invocationExpr.Expression;

if (context.SemanticModel.GetSymbolInfo(invocationExpr).Symbol is not IMethodSymbol method ||
!method.IsExtensionMethod ||
!SymbolEqualityComparer.Default.Equals(method.ContainingType, context.Compilation.GetTypeByMetadataName(typeof(Enumerable).FullName)))
{
return;
}

bool report = method.Name switch
{
nameof(Enumerable.First) => method.Parameters.Length == 0,
nameof(Enumerable.Last) => method.Parameters.Length == 0,
nameof(Enumerable.ElementAt) => method.Parameters.Length == 1,
_ => false,
};

if (report && IsList(context.SemanticModel.GetTypeInfo(memberAccess.Expression).Type, context.Compilation))
context.ReportDiagnostic(Diagnostic.Create(Descriptor, memberAccess.GetLocation()));

static bool IsList(ITypeSymbol? type, Compilation compilation)
{
if (type is null) return false;

var iReadOnlyListT = compilation.GetTypeByMetadataName(typeof(IReadOnlyList<>).FullName);
var iListT = compilation.GetTypeByMetadataName(typeof(IList<>).FullName);
var iList = compilation.GetTypeByMetadataName(typeof(IList).FullName);

foreach (var iface in type.AllInterfaces)
{
if (SymbolEqualityComparer.Default.Equals(iface.OriginalDefinition, iReadOnlyListT) ||
SymbolEqualityComparer.Default.Equals(iface.OriginalDefinition, iListT) ||
SymbolEqualityComparer.Default.Equals(iface.OriginalDefinition, iList))
{
return true;
}
}
return false;
}
}
}
6 changes: 6 additions & 0 deletions src/EcoCode.Core/Extensions/CompilationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
/// <summary>Extension methods for <see cref="Compilation"/>.</summary>
public static class CompilationExtensions
{
/// <summary>Returns the LINQ Enumerable symbol.</summary>
/// <param name="compilation">The compilation.</param>
/// <returns>The LINQ Enumerable symbol, null if not found.</returns>
public static INamedTypeSymbol? GetLinqEnumerableSymbol(this Compilation compilation) =>
compilation.GetTypeByMetadataName(typeof(System.Linq.Enumerable).FullName);

/// <summary>
/// Gets a type by its metadata name to use for code analysis within a <see cref="Compilation"/>. This method
/// attempts to find the "best" symbol to use for code analysis, which is the symbol matching the first of the
Expand Down
11 changes: 11 additions & 0 deletions src/EcoCode.Core/Extensions/DocumentExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace EcoCode.Extensions;

/// <summary>Extension methods for <see cref="Document"/>.</summary>
public static class DocumentExtensions
{
/// <summary>Returns the language version of the document.</summary>
/// <param name="document">The document.</param>
/// <returns>The language version.</returns>
public static LanguageVersion GetLanguageVersion(this Document document) =>
document.Project.ParseOptions is CSharpParseOptions options ? options.LanguageVersion : LanguageVersion.Latest;
}
21 changes: 21 additions & 0 deletions src/EcoCode.Core/Extensions/MethodSymbolExtension.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace EcoCode.Extensions;

/// <summary>Extensions methods for <see cref="IMethodSymbol"/>.</summary>
public static class MethodSymbolExtensions
{
/// <summary>Returns whether the method is a LINQ extension method.</summary>
/// <param name="methodSymbol">The method symbol.</param>
/// <param name="compilation">The compilation.</param>
/// <returns>True if the method is a LINQ extension method, false otherwise.</returns>
public static bool IsLinqMethod(this IMethodSymbol methodSymbol, Compilation compilation) =>
methodSymbol.IsExtensionMethod &&
SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, compilation.GetLinqEnumerableSymbol());

/// <summary>Returns whether the method is a LINQ extension method.</summary>
/// <param name="methodSymbol">The method symbol.</param>
/// <param name="linqEnumerableSymbol">The LINQ Enumerable symbol.</param>
/// <returns>True if the method is a LINQ extension method, false otherwise.</returns>
public static bool IsLinqMethod(this IMethodSymbol methodSymbol, INamedTypeSymbol? linqEnumerableSymbol) =>
methodSymbol.IsExtensionMethod &&
SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, linqEnumerableSymbol);
}
17 changes: 17 additions & 0 deletions src/EcoCode.Core/Extensions/TypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
namespace EcoCode.Extensions;

internal static class TypeSymbolExtensions
{
/// <summary>Returns whether the type is a primitive number type.</summary>
/// <param name="type">The type.</param>
/// <returns>True if the type is a primitive number type, false otherwise.</returns>
public static bool IsPrimitiveNumber(this ITypeSymbol type) => type.SpecialType is
SpecialType.System_Int32 or
SpecialType.System_Int64 or
SpecialType.System_UInt32 or
SpecialType.System_UInt64 or
SpecialType.System_Int16 or
SpecialType.System_UInt16 or
SpecialType.System_Byte or
SpecialType.System_SByte;
}
Loading

0 comments on commit 87857e1

Please sign in to comment.