From 87857e1d5d29a59ba6edc36a0c84eb08e8a8354b Mon Sep 17 00:00:00 2001 From: Vianney de Bellabre Date: Mon, 13 May 2024 21:06:52 +0200 Subject: [PATCH] [EC87] Use collection indexer (#41) --- Directory.Packages.props | 10 +- NuGet.Config | 2 +- README.md | 2 + ... EC86.GCCollectShouldNotBeCalled.Fixer.cs} | 5 +- ....cs => EC86.GCCollectShouldNotBeCalled.cs} | 38 +- .../Analyzers/EC87.UseListIndexer.Fixer.cs | 125 ++++++ .../Analyzers/EC87.UseListIndexer.cs | 81 ++++ .../Extensions/CompilationExtensions.cs | 6 + .../Extensions/DocumentExtensions.cs | 11 + .../Extensions/MethodSymbolExtension.cs | 21 + .../Extensions/TypeSymbolExtensions.cs | 17 + src/EcoCode.Core/Models/Rule.cs | 1 + src/EcoCode.Tests/TestRunner.cs | 55 ++- ...ontCallFunctionsInLoopConditions.Tests.cs} | 2 +- ...72.DontExecuteSqlCommandsInLoops.Tests.cs} | 2 +- ...75.DontConcatenateStringsInLoops.Tests.cs} | 2 +- ...s.cs => EC81.SpecifyStructLayout.Tests.cs} | 2 +- ...> EC82.VariableCanBeMadeConstant.Tests.cs} | 2 +- ...83.ReplaceEnumToStringWithNameOf.Tests.cs} | 2 +- ...cs => EC84.AvoidAsyncVoidMethods.Tests.cs} | 2 +- ...tTests.cs => EC85.MakeTypeSealed.Tests.cs} | 2 +- ... EC86.GCCollectShouldNotBeCalled.Tests.cs} | 3 +- .../Tests/EC87.UseListIndexer.Tests.cs | 397 ++++++++++++++++++ 23 files changed, 745 insertions(+), 45 deletions(-) rename src/EcoCode.Core/Analyzers/{EC86_GCCollectShouldNotBeCalled.Fixer.cs => EC86.GCCollectShouldNotBeCalled.Fixer.cs} (71%) rename src/EcoCode.Core/Analyzers/{EC86_GCCollectShouldNotBeCalled.cs => EC86.GCCollectShouldNotBeCalled.cs} (62%) create mode 100644 src/EcoCode.Core/Analyzers/EC87.UseListIndexer.Fixer.cs create mode 100644 src/EcoCode.Core/Analyzers/EC87.UseListIndexer.cs create mode 100644 src/EcoCode.Core/Extensions/DocumentExtensions.cs create mode 100644 src/EcoCode.Core/Extensions/MethodSymbolExtension.cs create mode 100644 src/EcoCode.Core/Extensions/TypeSymbolExtensions.cs rename src/EcoCode.Tests/Tests/{EC69_DontCallFunctionsInLoopConditionsUnitTests.cs => EC69.DontCallFunctionsInLoopConditions.Tests.cs} (97%) rename src/EcoCode.Tests/Tests/{EC72_DontExecuteSqlCommandsInLoopsUnitTests.cs => EC72.DontExecuteSqlCommandsInLoops.Tests.cs} (94%) rename src/EcoCode.Tests/Tests/{EC75_DontConcatenateStringsInLoopsUnitTests.cs => EC75.DontConcatenateStringsInLoops.Tests.cs} (97%) rename src/EcoCode.Tests/Tests/{EC81_SpecifyStructLayoutUnitTests.cs => EC81.SpecifyStructLayout.Tests.cs} (98%) rename src/EcoCode.Tests/Tests/{EC82_VariableCanBeMadeConstantUnitTests.cs => EC82.VariableCanBeMadeConstant.Tests.cs} (98%) rename src/EcoCode.Tests/Tests/{EC83_ReplaceEnumToStringWithNameOfUnitTests.cs => EC83.ReplaceEnumToStringWithNameOf.Tests.cs} (98%) rename src/EcoCode.Tests/Tests/{EC84_AvoidAsyncVoidMethodsUnitTests.cs => EC84.AvoidAsyncVoidMethods.Tests.cs} (98%) rename src/EcoCode.Tests/Tests/{EC85_MakeTypeSealedUnitTests.cs => EC85.MakeTypeSealed.Tests.cs} (99%) rename src/EcoCode.Tests/Tests/{EC86_GCCollectShouldNotBeCalledUnitTests.cs => EC86.GCCollectShouldNotBeCalled.Tests.cs} (98%) create mode 100644 src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 0a82f873..1b9f92e2 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -16,9 +16,9 @@ - - - - + + + + - + \ No newline at end of file diff --git a/NuGet.Config b/NuGet.Config index df9a12a0..85ebb4c5 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -5,4 +5,4 @@ - \ No newline at end of file + diff --git a/README.md b/README.md index 058df71b..212e5281 100644 --- a/README.md +++ b/README.md @@ -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 --------------- diff --git a/src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.Fixer.cs b/src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.Fixer.cs similarity index 71% rename from src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.Fixer.cs rename to src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.Fixer.cs index 18429142..66713fc8 100644 --- a/src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.Fixer.cs +++ b/src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.Fixer.cs @@ -1,11 +1,12 @@ namespace EcoCode.Analyzers; -/// The code fix provider for avoid async void methods. +/// EC86 fixer : GC Collect should not be called. [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(GCCollectShouldNotBeCalledFixer)), Shared] public sealed class GCCollectShouldNotBeCalledFixer: CodeFixProvider { /// - public override ImmutableArray FixableDiagnosticIds => [GCCollectShouldNotBeCalled.Descriptor.Id]; + public override ImmutableArray FixableDiagnosticIds => _fixableDiagnosticIds; + private static readonly ImmutableArray _fixableDiagnosticIds = [GCCollectShouldNotBeCalled.Descriptor.Id]; /// public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; diff --git a/src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.cs b/src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.cs similarity index 62% rename from src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.cs rename to src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.cs index dda63082..7d2df175 100644 --- a/src/EcoCode.Core/Analyzers/EC86_GCCollectShouldNotBeCalled.cs +++ b/src/EcoCode.Core/Analyzers/EC86.GCCollectShouldNotBeCalled.cs @@ -1,11 +1,11 @@ -using System.Linq; +namespace EcoCode.Analyzers; -namespace EcoCode.Analyzers; - -/// Analyzer for avoid async void methods. +/// EC86 : GC Collect should not be called. [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class GCCollectShouldNotBeCalled: DiagnosticAnalyzer { + private static readonly ImmutableArray SyntaxKinds = [SyntaxKind.InvocationExpression]; + /// The diagnostic descriptor. public static DiagnosticDescriptor Descriptor { get; } = new( Rule.Ids.EC86_GCCollectShouldNotBeCalled, @@ -18,30 +18,30 @@ public sealed class GCCollectShouldNotBeCalled: DiagnosticAnalyzer helpLinkUri: Rule.GetHelpUri(Rule.Ids.EC86_GCCollectShouldNotBeCalled)); /// - public override ImmutableArray SupportedDiagnostics => [Descriptor]; + public override ImmutableArray SupportedDiagnostics => _supportedDiagnostics; + private static readonly ImmutableArray _supportedDiagnostics = [Descriptor]; /// 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) { @@ -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) diff --git a/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.Fixer.cs b/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.Fixer.cs new file mode 100644 index 00000000..f90b7ebb --- /dev/null +++ b/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.Fixer.cs @@ -0,0 +1,125 @@ +using System.Linq; +using System.Reflection; + +namespace EcoCode.Analyzers; + +/// EC87 fixer: Use list indexer. +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(UseListIndexerFixer)), Shared] +public sealed class UseListIndexerFixer : CodeFixProvider +{ + /// + public override ImmutableArray FixableDiagnosticIds => _fixableDiagnosticIds; + private static readonly ImmutableArray _fixableDiagnosticIds = [UseListIndexer.Descriptor.Id]; + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + 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>), + }; + + 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 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 RefactorFirstAsync(Document document, InvocationExpressionSyntax invocationExpr, CancellationToken token) => + UpdateDocument(document, invocationExpr, SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(0)), token); + + private static Task 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 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.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 RefactorElementAtAsync(Document document, InvocationExpressionSyntax invocation, CancellationToken token) => + UpdateDocument(document, invocation, invocation.ArgumentList.Arguments[0].Expression, token); +} diff --git a/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.cs b/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.cs new file mode 100644 index 00000000..d17f5464 --- /dev/null +++ b/src/EcoCode.Core/Analyzers/EC87.UseListIndexer.cs @@ -0,0 +1,81 @@ +using System.Collections; +using System.Linq; + +namespace EcoCode.Analyzers; + +/// EC87: Use list indexer. +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class UseListIndexer : DiagnosticAnalyzer +{ + private static readonly ImmutableArray SyntaxKinds = [SyntaxKind.InvocationExpression]; + + /// The diagnostic descriptor. + 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 or IReadOnlyList, should use their indexers instead of Linq methods for improved performance.", + helpLinkUri: Rule.GetHelpUri(Rule.Ids.EC87_UseCollectionIndexer)); + + /// + public override ImmutableArray SupportedDiagnostics => _supportedDiagnostics; + private static readonly ImmutableArray _supportedDiagnostics = [Descriptor]; + + /// + 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; + } + } +} diff --git a/src/EcoCode.Core/Extensions/CompilationExtensions.cs b/src/EcoCode.Core/Extensions/CompilationExtensions.cs index b83425a5..d4063c96 100644 --- a/src/EcoCode.Core/Extensions/CompilationExtensions.cs +++ b/src/EcoCode.Core/Extensions/CompilationExtensions.cs @@ -3,6 +3,12 @@ /// Extension methods for . public static class CompilationExtensions { + /// Returns the LINQ Enumerable symbol. + /// The compilation. + /// The LINQ Enumerable symbol, null if not found. + public static INamedTypeSymbol? GetLinqEnumerableSymbol(this Compilation compilation) => + compilation.GetTypeByMetadataName(typeof(System.Linq.Enumerable).FullName); + /// /// Gets a type by its metadata name to use for code analysis within a . This method /// attempts to find the "best" symbol to use for code analysis, which is the symbol matching the first of the diff --git a/src/EcoCode.Core/Extensions/DocumentExtensions.cs b/src/EcoCode.Core/Extensions/DocumentExtensions.cs new file mode 100644 index 00000000..bbbd363b --- /dev/null +++ b/src/EcoCode.Core/Extensions/DocumentExtensions.cs @@ -0,0 +1,11 @@ +namespace EcoCode.Extensions; + +/// Extension methods for . +public static class DocumentExtensions +{ + /// Returns the language version of the document. + /// The document. + /// The language version. + public static LanguageVersion GetLanguageVersion(this Document document) => + document.Project.ParseOptions is CSharpParseOptions options ? options.LanguageVersion : LanguageVersion.Latest; +} diff --git a/src/EcoCode.Core/Extensions/MethodSymbolExtension.cs b/src/EcoCode.Core/Extensions/MethodSymbolExtension.cs new file mode 100644 index 00000000..ecfa6b67 --- /dev/null +++ b/src/EcoCode.Core/Extensions/MethodSymbolExtension.cs @@ -0,0 +1,21 @@ +namespace EcoCode.Extensions; + +/// Extensions methods for . +public static class MethodSymbolExtensions +{ + /// Returns whether the method is a LINQ extension method. + /// The method symbol. + /// The compilation. + /// True if the method is a LINQ extension method, false otherwise. + public static bool IsLinqMethod(this IMethodSymbol methodSymbol, Compilation compilation) => + methodSymbol.IsExtensionMethod && + SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, compilation.GetLinqEnumerableSymbol()); + + /// Returns whether the method is a LINQ extension method. + /// The method symbol. + /// The LINQ Enumerable symbol. + /// True if the method is a LINQ extension method, false otherwise. + public static bool IsLinqMethod(this IMethodSymbol methodSymbol, INamedTypeSymbol? linqEnumerableSymbol) => + methodSymbol.IsExtensionMethod && + SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, linqEnumerableSymbol); +} diff --git a/src/EcoCode.Core/Extensions/TypeSymbolExtensions.cs b/src/EcoCode.Core/Extensions/TypeSymbolExtensions.cs new file mode 100644 index 00000000..0d4ccad8 --- /dev/null +++ b/src/EcoCode.Core/Extensions/TypeSymbolExtensions.cs @@ -0,0 +1,17 @@ +namespace EcoCode.Extensions; + +internal static class TypeSymbolExtensions +{ + /// Returns whether the type is a primitive number type. + /// The type. + /// True if the type is a primitive number type, false otherwise. + 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; +} diff --git a/src/EcoCode.Core/Models/Rule.cs b/src/EcoCode.Core/Models/Rule.cs index f84d80b7..9226ed8d 100644 --- a/src/EcoCode.Core/Models/Rule.cs +++ b/src/EcoCode.Core/Models/Rule.cs @@ -23,5 +23,6 @@ public static class Ids public const string EC84_AvoidAsyncVoidMethods = "EC84"; public const string EC85_MakeTypeSealed = "EC85"; public const string EC86_GCCollectShouldNotBeCalled = "EC86"; + public const string EC87_UseCollectionIndexer = "EC87"; } } diff --git a/src/EcoCode.Tests/TestRunner.cs b/src/EcoCode.Tests/TestRunner.cs index 482c253b..0305c561 100644 --- a/src/EcoCode.Tests/TestRunner.cs +++ b/src/EcoCode.Tests/TestRunner.cs @@ -1,24 +1,57 @@ -namespace EcoCode.Tests; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis; -public delegate Task AnalyzerDlg(string source); -public delegate Task CodeFixerDlg(string source, string? fixedSource = null); +namespace EcoCode.Tests; + +public delegate Task AnalyzerDlg(string source, LanguageVersion languageVersion = default); +public delegate Task CodeFixerDlg(string source, string? fixedSource = null, LanguageVersion languageVersion = default); internal static class TestRunner { - public static async Task VerifyAsync(string source) + // Analyzer + + public static Task VerifyAsync(string source, LanguageVersion languageVersion = default) + where TAnalyzer : DiagnosticAnalyzer, new() => + new CustomAnalyzerVerifier(source, languageVersion).RunAsync(); + + private sealed class CustomAnalyzerVerifier : CSharpAnalyzerTest where TAnalyzer : DiagnosticAnalyzer, new() { - var test = new CSharpAnalyzerTest { TestCode = source }; - await test.RunAsync().ConfigureAwait(false); + private readonly LanguageVersion _languageVersion; + + public CustomAnalyzerVerifier(string source, LanguageVersion languageVersion) + { + TestCode = source; + _languageVersion = languageVersion; + } + + protected override ParseOptions CreateParseOptions() => _languageVersion == default + ? base.CreateParseOptions() + : ((CSharpParseOptions)base.CreateParseOptions()).WithLanguageVersion(_languageVersion); } - public static async Task VerifyAsync(string source, string? fixedSource = null) + // CodeFix + + public static Task VerifyAsync(string source, string? fixedSource = null, LanguageVersion languageVersion = default) + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() => + new CustomCodeFixVerifier(source, fixedSource, languageVersion).RunAsync(); + + private sealed class CustomCodeFixVerifier : CSharpCodeFixTest where TAnalyzer : DiagnosticAnalyzer, new() where TCodeFix : CodeFixProvider, new() { - var test = new CSharpCodeFixTest { TestCode = source }; - if (fixedSource is not null) - test.FixedCode = fixedSource; - await test.RunAsync().ConfigureAwait(false); + private readonly LanguageVersion _languageVersion; + + public CustomCodeFixVerifier(string source, string? fixedSource, LanguageVersion languageVersion) + { + TestCode = source; + if (fixedSource is not null) FixedCode = fixedSource; + _languageVersion = languageVersion; + } + + protected override ParseOptions CreateParseOptions() => _languageVersion == default + ? base.CreateParseOptions() + : ((CSharpParseOptions)base.CreateParseOptions()).WithLanguageVersion(_languageVersion); } } diff --git a/src/EcoCode.Tests/Tests/EC69_DontCallFunctionsInLoopConditionsUnitTests.cs b/src/EcoCode.Tests/Tests/EC69.DontCallFunctionsInLoopConditions.Tests.cs similarity index 97% rename from src/EcoCode.Tests/Tests/EC69_DontCallFunctionsInLoopConditionsUnitTests.cs rename to src/EcoCode.Tests/Tests/EC69.DontCallFunctionsInLoopConditions.Tests.cs index 06af9907..2a90fbb4 100644 --- a/src/EcoCode.Tests/Tests/EC69_DontCallFunctionsInLoopConditionsUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC69.DontCallFunctionsInLoopConditions.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class DontCallFunctionsInLoopConditionsUnitTests +public sealed class DontCallFunctionsInLoopConditionsTests { private static readonly AnalyzerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC72_DontExecuteSqlCommandsInLoopsUnitTests.cs b/src/EcoCode.Tests/Tests/EC72.DontExecuteSqlCommandsInLoops.Tests.cs similarity index 94% rename from src/EcoCode.Tests/Tests/EC72_DontExecuteSqlCommandsInLoopsUnitTests.cs rename to src/EcoCode.Tests/Tests/EC72.DontExecuteSqlCommandsInLoops.Tests.cs index f3e5821f..aee13113 100644 --- a/src/EcoCode.Tests/Tests/EC72_DontExecuteSqlCommandsInLoopsUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC72.DontExecuteSqlCommandsInLoops.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class DontExecuteSqlCommandsInLoopsUnitTests +public sealed class DontExecuteSqlCommandsInLoopsTests { private static readonly AnalyzerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC75_DontConcatenateStringsInLoopsUnitTests.cs b/src/EcoCode.Tests/Tests/EC75.DontConcatenateStringsInLoops.Tests.cs similarity index 97% rename from src/EcoCode.Tests/Tests/EC75_DontConcatenateStringsInLoopsUnitTests.cs rename to src/EcoCode.Tests/Tests/EC75.DontConcatenateStringsInLoops.Tests.cs index 31d520f7..69c765e8 100644 --- a/src/EcoCode.Tests/Tests/EC75_DontConcatenateStringsInLoopsUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC75.DontConcatenateStringsInLoops.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class DontConcatenateStringsInLoopsUnitTests +public sealed class DontConcatenateStringsInLoopsTests { private static readonly AnalyzerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC81_SpecifyStructLayoutUnitTests.cs b/src/EcoCode.Tests/Tests/EC81.SpecifyStructLayout.Tests.cs similarity index 98% rename from src/EcoCode.Tests/Tests/EC81_SpecifyStructLayoutUnitTests.cs rename to src/EcoCode.Tests/Tests/EC81.SpecifyStructLayout.Tests.cs index deb25949..375cb59b 100644 --- a/src/EcoCode.Tests/Tests/EC81_SpecifyStructLayoutUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC81.SpecifyStructLayout.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class SpecifyStructLayoutUnitTests +public sealed class SpecifyStructLayoutTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC82_VariableCanBeMadeConstantUnitTests.cs b/src/EcoCode.Tests/Tests/EC82.VariableCanBeMadeConstant.Tests.cs similarity index 98% rename from src/EcoCode.Tests/Tests/EC82_VariableCanBeMadeConstantUnitTests.cs rename to src/EcoCode.Tests/Tests/EC82.VariableCanBeMadeConstant.Tests.cs index e51e46e0..d0cbed78 100644 --- a/src/EcoCode.Tests/Tests/EC82_VariableCanBeMadeConstantUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC82.VariableCanBeMadeConstant.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class VariableCanBeMadeConstantUnitTests +public sealed class VariableCanBeMadeConstantTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC83_ReplaceEnumToStringWithNameOfUnitTests.cs b/src/EcoCode.Tests/Tests/EC83.ReplaceEnumToStringWithNameOf.Tests.cs similarity index 98% rename from src/EcoCode.Tests/Tests/EC83_ReplaceEnumToStringWithNameOfUnitTests.cs rename to src/EcoCode.Tests/Tests/EC83.ReplaceEnumToStringWithNameOf.Tests.cs index d3a5ce1e..6831655e 100644 --- a/src/EcoCode.Tests/Tests/EC83_ReplaceEnumToStringWithNameOfUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC83.ReplaceEnumToStringWithNameOf.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class ReplaceEnumToStringWithNameOfUnitTests +public sealed class ReplaceEnumToStringWithNameOfTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC84_AvoidAsyncVoidMethodsUnitTests.cs b/src/EcoCode.Tests/Tests/EC84.AvoidAsyncVoidMethods.Tests.cs similarity index 98% rename from src/EcoCode.Tests/Tests/EC84_AvoidAsyncVoidMethodsUnitTests.cs rename to src/EcoCode.Tests/Tests/EC84.AvoidAsyncVoidMethods.Tests.cs index aef09f03..31c81bed 100644 --- a/src/EcoCode.Tests/Tests/EC84_AvoidAsyncVoidMethodsUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC84.AvoidAsyncVoidMethods.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class AvoidAsyncVoidMethodsUnitTests +public sealed class AvoidAsyncVoidMethodsTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC85_MakeTypeSealedUnitTests.cs b/src/EcoCode.Tests/Tests/EC85.MakeTypeSealed.Tests.cs similarity index 99% rename from src/EcoCode.Tests/Tests/EC85_MakeTypeSealedUnitTests.cs rename to src/EcoCode.Tests/Tests/EC85.MakeTypeSealed.Tests.cs index 81de24a7..805a156f 100644 --- a/src/EcoCode.Tests/Tests/EC85_MakeTypeSealedUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC85.MakeTypeSealed.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public sealed class MakeTypeSealed +public sealed class MakeTypeSealedTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; diff --git a/src/EcoCode.Tests/Tests/EC86_GCCollectShouldNotBeCalledUnitTests.cs b/src/EcoCode.Tests/Tests/EC86.GCCollectShouldNotBeCalled.Tests.cs similarity index 98% rename from src/EcoCode.Tests/Tests/EC86_GCCollectShouldNotBeCalledUnitTests.cs rename to src/EcoCode.Tests/Tests/EC86.GCCollectShouldNotBeCalled.Tests.cs index 914d12da..149e67ca 100644 --- a/src/EcoCode.Tests/Tests/EC86_GCCollectShouldNotBeCalledUnitTests.cs +++ b/src/EcoCode.Tests/Tests/EC86.GCCollectShouldNotBeCalled.Tests.cs @@ -1,7 +1,7 @@ namespace EcoCode.Tests; [TestClass] -public class GCCollectShouldNotBeCalledUnitTests +public class GCCollectShouldNotBeCalledTests { private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync< GCCollectShouldNotBeCalled, @@ -117,5 +117,4 @@ public static async void Main() } } """).ConfigureAwait(false); - } diff --git a/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs b/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs new file mode 100644 index 00000000..81b9bc4e --- /dev/null +++ b/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs @@ -0,0 +1,397 @@ +using Microsoft.CodeAnalysis.CSharp; + +namespace EcoCode.Tests; + +[TestClass] +public sealed class UseListIndexerTests +{ + private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; + + [TestMethod] + public async Task EmptyCodeAsync() => await VerifyAsync("").ConfigureAwait(false); + + #region First + + [TestMethod, TestCategory("First")] + public Task RefactorFirstAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine([|arr.First|]()); + + var list = new List { 1, 2, 3 }; + Console.WriteLine([|list.First|]()); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine([|coll.First|]()); + } + } + """, """ + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr[0]); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list[0]); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll[0]); + } + } + """); + + [TestMethod] + public Task DontRefactorFirstWithNoIndexerAsync() => VerifyAsync(""" + using System.Linq; + public static class Test + { + public static int GetFirstValue() + { + var enumerable = Enumerable.Range(0, 10); + return enumerable.First(); + } + } + """); + + [TestMethod] + public Task DontRefactorFirstWithPredicateAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr.First(x => x != 1)); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list.First(x => x != 1)); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll.First(x => x != 1)); + } + } + """); + + [TestMethod] + public Task DontRefactorFirstIfNotLinqAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + public static class Test + { + private sealed class MyCollection : List + { + public T First() => this[0]; + } + + public static void Run() + { + var myCollection = new MyCollection { 1, 2, 3 }; + Console.WriteLine(myCollection.First()); + } + } + """); + + [TestMethod] + public Task DontRefactorFirstNonListsAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + public static void Run() + { + var set = new HashSet { 1, 2, 3 }; + Console.WriteLine(set.First()); + + var dic = new Dictionary { { 1, 1 }, { 2, 2 }, { 3, 3 } }; + Console.WriteLine(dic.First()); + } + } + """); + + #endregion + + #region Last + + [TestMethod] + public Task RefactorLastWithIndexerAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine([|arr.Last|]()); + + var list = new List { 1, 2, 3 }; + Console.WriteLine([|list.Last|]()); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine([|coll.Last|]()); + } + } + """, """ + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr[^1]); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list[^1]); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll[^1]); + } + } + """); + + [TestMethod] + public Task RefactorLastWithCountOrLengthAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List + { + } + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine([|arr.Last|]()); + + var list = new List { 1, 2, 3 }; + Console.WriteLine([|list.Last|]()); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine([|coll.Last|]()); + } + } + """, """ + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List + { + } + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr[arr.Length - 1]); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list[list.Count - 1]); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll[coll.Count - 1]); + } + } + """, languageVersion: LanguageVersion.CSharp7); + + [TestMethod] + public Task DontRefactorLastWithNoIndexerAsync() => VerifyAsync(""" + using System.Linq; + public static class Test + { + public static int GetFirstValue() + { + var enumerable = Enumerable.Range(0, 10); + return enumerable.Last(); + } + } + """); + + [TestMethod] + public Task DontRefactorLastWithPredicateAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr.Last(x => x != 1)); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list.Last(x => x != 1)); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll.Last(x => x != 1)); + } + } + """); + + [TestMethod] + public Task DontRefactorLastIfNotLinqAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + public static class Test + { + private sealed class MyCollection : List + { + public T Last() => this[^1]; + } + + public static void Run() + { + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll.Last()); + } + } + """); + + [TestMethod] + public Task DontRefactorLastNonListsAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + public static void Run() + { + var set = new HashSet { 1, 2, 3 }; + Console.WriteLine(set.Last()); + + var dic = new Dictionary { { 1, 1 }, { 2, 2 }, { 3, 3 } }; + Console.WriteLine(dic.Last()); + } + } + """); + + #endregion + + #region ElementAt + + [TestMethod] + public Task UseIndexerInsteadOfElementAtAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine([|arr.ElementAt|](2)); + + var list = new List { 1, 2, 3 }; + Console.WriteLine([|list.ElementAt|](2)); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine([|coll.ElementAt|](2)); + } + } + """, """ + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + private sealed class MyCollection : List; + + public static void Run() + { + var arr = new int[] { 1, 2, 3 }; + Console.WriteLine(arr[2]); + + var list = new List { 1, 2, 3 }; + Console.WriteLine(list[2]); + + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll[2]); + } + } + """); + + [TestMethod] + public Task DontRefactorElementAtWithNoIndexerAsync() => VerifyAsync(""" + using System; + using System.Linq; + public static class Test + { + public static void Run() + { + var enumerable = Enumerable.Range(0, 10); + Console.WriteLine(enumerable.ElementAt(2)); + } + } + """); + + [TestMethod] + public Task DontRefactorElementAtIfNotLinqAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + public static class Test + { + private sealed class MyCollection : List + { + public T ElementAt(int index) => this[index]; + } + + public static void Run() + { + var coll = new MyCollection { 1, 2, 3 }; + Console.WriteLine(coll.ElementAt(2)); + } + } + """); + + [TestMethod] + public Task DontRefactorElementAtNonListsAsync() => VerifyAsync(""" + using System; + using System.Collections.Generic; + using System.Linq; + public static class Test + { + public static void Run() + { + var set = new HashSet { 1, 2, 3 }; + Console.WriteLine(set.ElementAt(2)); + + var dic = new Dictionary { { 1, 1 }, { 2, 2 }, { 3, 3 } }; + Console.WriteLine(dic.ElementAt(2)); + } + } + """); + + #endregion +}