diff --git a/ClrHeapAllocationsAnalyzer.Test/AvoidAllocationWithArrayEmptyCodeFixTests.cs b/ClrHeapAllocationsAnalyzer.Test/AvoidAllocationWithArrayEmptyCodeFixTests.cs new file mode 100644 index 0000000..24ae768 --- /dev/null +++ b/ClrHeapAllocationsAnalyzer.Test/AvoidAllocationWithArrayEmptyCodeFixTests.cs @@ -0,0 +1,546 @@ +using System; +using System.Collections.Generic; +using ClrHeapAllocationAnalyzer; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using RoslynTestKit; + +namespace ClrHeapAllocationsAnalyzer.Test +{ + [TestClass] + public class AvoidAllocationWithArrayEmptyCodeFixTests: CodeFixTestFixture + { + protected override string LanguageName => LanguageNames.CSharp; + protected override CodeFixProvider CreateProvider() => new AvoidAllocationWithArrayEmptyCodeFix(); + + protected override IReadOnlyCollection CreateAdditionalAnalyzers() => new DiagnosticAnalyzer[] + { + new ExplicitAllocationAnalyzer(), + }; + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_when_return_from_method_ienumerable() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new List()|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_when_return_from_method_ireadonly_list() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IReadOnlyList DoSomething() + { + return [|new List()|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IReadOnlyList DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_when_return_from_method_ireadonly_collection() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IReadOnlyCollection DoSomething() + { + return [|new List()|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IReadOnlyCollection DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_when_return_from_method_array() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public int[] DoSomething() + { + return [|new int[0]|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public int[] DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewArrayRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_for_arrow_expression() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething => [|new List()|]; + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething => Array.Empty(); + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_creation_with_array_empty_for_readonly_property() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething { get {return [|new List()|];}} + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething { get {return Array.Empty();}} + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_list_with_creation_with_predefined_size_with_array_empty() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new List(10)|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_not_propose_code_fix_when_non_empty_list_created() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new List(){1, 2}|]; + } + } +}"; + + NoCodeFix(before, ExplicitAllocationAnalyzer.NewObjectRule.Id); + } + + [TestMethod] + public void should_not_propose_code_fix_when_return_type_inherit_form_enumerable() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public List DoSomething() + { + return [|new List()|]; + } + } +}"; + + NoCodeFix(before, ExplicitAllocationAnalyzer.NewObjectRule.Id); + } + + [TestMethod] + public void should_not_propose_code_fix_when_for_collection_creation_using_copy_constructor() + { + + var before = @" +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + var innerList = new List(){1, 2}; + return [|new ReadOnlyCollection(innerList)|]; + } + } +}"; + + NoCodeFix(before, ExplicitAllocationAnalyzer.NewObjectRule.Id); + } + + [TestMethod] + public void should_replace_empty_collection_creation_with_array_empty() + { + var before = @" +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new Collection()|]; + } + } +}"; + var after = @" +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_empty_array_creation_with_array_empty() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new int[0]|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewArrayRule.Id, 0); + } + + [TestMethod] + public void should_not_propose_code_fix_when_non_empty_array_creation() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new int[]{1, 2}|]; + } + } +}"; + NoCodeFix(before, ExplicitAllocationAnalyzer.NewArrayRule.Id); + } + + [TestMethod] + public void should_replace_empty_array_creation_with_init_block_with_array_empty() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return [|new int[] { }|]; + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public IEnumerable DoSomething() + { + return Array.Empty(); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewArrayRule.Id, 0); + } + + [TestMethod] + public void should_replace_list_creation_as_method_invocation_parameter_with_array_empty() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething() + { + Do([|new List()|]); + } + + private void Do(IEnumerable a) + { + + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething() + { + Do(Array.Empty()); + } + + private void Do(IEnumerable a) + { + + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewObjectRule.Id, 0); + } + + [TestMethod] + public void should_replace_array_creation_as_method_invocation_parameter_with_array_empty() + { + var before = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething() + { + Do([|new int[0]|]); + } + + private void Do(IEnumerable a) + { + + } + } +}"; + var after = @" +using System.Collections.Generic; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething() + { + Do(Array.Empty()); + } + + private void Do(IEnumerable a) + { + + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewArrayRule.Id, 0); + } + + [TestMethod] + public void should_replace_array_creation_as_delegate_invocation_parameter_with_array_empty() + { + var before = @" +using System.Collections.Generic; +using System; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething(Action> doSth) + { + doSth([|new int[0]|]); + } + } +}"; + var after = @" +using System.Collections.Generic; +using System; + +namespace SampleNamespace +{ + class SampleClass + { + public void DoSomething(Action> doSth) + { + doSth(Array.Empty()); + } + } +}"; + + TestCodeFix(before, after, ExplicitAllocationAnalyzer.NewArrayRule.Id, 0); + } + } +} diff --git a/ClrHeapAllocationsAnalyzer.Test/ClrHeapAllocationsAnalyzer.Test.csproj b/ClrHeapAllocationsAnalyzer.Test/ClrHeapAllocationsAnalyzer.Test.csproj index 48c0d28..68dad89 100644 --- a/ClrHeapAllocationsAnalyzer.Test/ClrHeapAllocationsAnalyzer.Test.csproj +++ b/ClrHeapAllocationsAnalyzer.Test/ClrHeapAllocationsAnalyzer.Test.csproj @@ -7,12 +7,15 @@ + + + diff --git a/ClrHeapAllocationsAnalyzer/AvoidAllocationWithArrayEmptyCodeFix.cs b/ClrHeapAllocationsAnalyzer/AvoidAllocationWithArrayEmptyCodeFix.cs new file mode 100644 index 0000000..d5b171c --- /dev/null +++ b/ClrHeapAllocationsAnalyzer/AvoidAllocationWithArrayEmptyCodeFix.cs @@ -0,0 +1,291 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ClrHeapAllocationAnalyzer +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AvoidAllocationWithArrayEmptyCodeFix)), Shared] + public class AvoidAllocationWithArrayEmptyCodeFix : CodeFixProvider + { + private const string RemoveUnnecessaryListCreation = "Avoid allocation by using Array.Empty<>()"; + + public sealed override ImmutableArray FixableDiagnosticIds + => ImmutableArray.Create(ExplicitAllocationAnalyzer.NewObjectRule.Id, ExplicitAllocationAnalyzer.NewArrayRule.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var diagnostic = context.Diagnostics.First(); + var diagnosticSpan = diagnostic.Location.SourceSpan; + var node = root.FindNode(diagnosticSpan); + + if (IsReturnStatement(node)) + { + await TryToRegisterCodeFixesForReturnStatement(context, node, diagnostic); + return; + } + + if (IsMethodInvocationParameter(node)) + { + await TryToRegisterCodeFixesForMethodInvocationParameter(context, node, diagnostic); + return; + } + } + + private async Task TryToRegisterCodeFixesForMethodInvocationParameter(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic) + { + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken); + if (IsExpectedParameterReadonlySequence(node, semanticModel) && node is ArgumentSyntax argument) + { + TryRegisterCodeFix(context, node, diagnostic, argument.Expression, semanticModel); + } + } + + private async Task TryToRegisterCodeFixesForReturnStatement(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic) + { + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken); + + if (IsInsideMemberReturningEnumerable(node, semanticModel)) + { + TryRegisterCodeFix(context, node, diagnostic, node, semanticModel); + } + } + + private void TryRegisterCodeFix(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic, SyntaxNode creationExpression, SemanticModel semanticModel) + { + switch (creationExpression) + { + case ObjectCreationExpressionSyntax objectCreation: + { + if (CanBeReplaceWithEnumerableEmpty(objectCreation, semanticModel)) + { + if (objectCreation.Type is GenericNameSyntax genericName) + { + var codeAction = CodeAction.Create(RemoveUnnecessaryListCreation, + token => Transform(context.Document, node, genericName.TypeArgumentList.Arguments[0], token), + RemoveUnnecessaryListCreation); + context.RegisterCodeFix(codeAction, diagnostic); + } + } + } + break; + case ArrayCreationExpressionSyntax arrayCreation: + { + if (CanBeReplaceWithEnumerableEmpty(arrayCreation)) + { + var codeAction = CodeAction.Create(RemoveUnnecessaryListCreation, + token => Transform(context.Document, node, arrayCreation.Type.ElementType, token), + RemoveUnnecessaryListCreation); + context.RegisterCodeFix(codeAction, diagnostic); + } + } + break; + } + } + + + private bool IsMethodInvocationParameter(SyntaxNode node) => node is ArgumentSyntax; + + private static bool IsReturnStatement(SyntaxNode node) + { + return node.Parent is ReturnStatementSyntax || node.Parent is YieldStatementSyntax || node.Parent is ArrowExpressionClauseSyntax; + } + + private bool IsInsideMemberReturningEnumerable(SyntaxNode node, SemanticModel semanticModel) + { + return IsInsideMethodReturningEnumerable(node, semanticModel) || + IsInsidePropertyDeclaration(node, semanticModel); + + } + + private bool IsInsidePropertyDeclaration(SyntaxNode node, SemanticModel semanticModel) + { + if(node.FindContainer() is PropertyDeclarationSyntax propertyDeclaration && IsPropertyTypeReadonlySequence(semanticModel, propertyDeclaration)) + { + return IsAutoPropertyWithGetter(node) || IsArrowExpression(node); + } + + return false; + } + + private bool IsAutoPropertyWithGetter(SyntaxNode node) + { + if(node.FindContainer() is AccessorDeclarationSyntax accessorDeclaration) + { + return accessorDeclaration.Keyword.Text == "get"; + } + + return false; + } + + private bool IsArrowExpression(SyntaxNode node) + { + return node.FindContainer() != null; + } + + private bool CanBeReplaceWithEnumerableEmpty(ArrayCreationExpressionSyntax arrayCreation) + { + return IsInitializationBlockEmpty(arrayCreation.Initializer); + } + + private bool CanBeReplaceWithEnumerableEmpty(ObjectCreationExpressionSyntax objectCreation, SemanticModel semanticModel) + { + return IsCollectionType(semanticModel, objectCreation) && + IsInitializationBlockEmpty(objectCreation.Initializer) && + IsCopyConstructor(semanticModel, objectCreation) == false; + } + + private static bool IsInsideMethodReturningEnumerable(SyntaxNode node, SemanticModel semanticModel) + { + if (node.FindContainer() is MethodDeclarationSyntax methodDeclaration) + { + if (IsReturnTypeReadonlySequence(semanticModel, methodDeclaration)) + { + return true; + } + } + + return false; + } + + private async Task Transform(Document contextDocument, SyntaxNode node, TypeSyntax typeArgument, CancellationToken cancellationToken) + { + var noAllocation = SyntaxFactory.ParseExpression($"Array.Empty<{typeArgument}>()"); + var newNode = ReplaceExpression(node, noAllocation); + if (newNode == null) + { + return contextDocument; + } + var syntaxRootAsync = await contextDocument.GetSyntaxRootAsync(cancellationToken); + var newSyntaxRoot = syntaxRootAsync.ReplaceNode(node.Parent, newNode); + return contextDocument.WithSyntaxRoot(newSyntaxRoot); + } + + private SyntaxNode ReplaceExpression(SyntaxNode node, ExpressionSyntax newExpression) + { + switch (node.Parent) + { + case ReturnStatementSyntax parentReturn: + return parentReturn.WithExpression(newExpression); + case ArrowExpressionClauseSyntax arrowStatement: + return arrowStatement.WithExpression(newExpression); + case ArgumentListSyntax argumentList: + var newArguments = argumentList.Arguments.Select(x => x == node ? SyntaxFactory.Argument(newExpression) : x); + return argumentList.WithArguments(SyntaxFactory.SeparatedList(newArguments)); + default: + return null; + } + } + + private bool IsCopyConstructor(SemanticModel semanticModel, ObjectCreationExpressionSyntax objectCreation) + { + if (objectCreation.ArgumentList == null || objectCreation.ArgumentList.Arguments.Count == 0) + { + return false; + } + + if (semanticModel.GetSymbolInfo(objectCreation).Symbol is IMethodSymbol methodSymbol) + { + if (methodSymbol.Parameters.Any(x=> x.Type is INamedTypeSymbol namedType && IsCollectionType(namedType))) + { + return true; + } + } + return false; + } + + private static bool IsInitializationBlockEmpty(InitializerExpressionSyntax initializer) + { + return initializer == null || initializer.Expressions.Count == 0; + } + + private bool IsCollectionType(SemanticModel semanticModel, ObjectCreationExpressionSyntax objectCreationExpressionSyntax) + { + return semanticModel.GetTypeInfo(objectCreationExpressionSyntax).Type is INamedTypeSymbol createdType && + (createdType.TypeKind == TypeKind.Array || IsCollectionType(createdType) ); + } + + private bool IsCollectionType(INamedTypeSymbol typeSymbol) + { + return typeSymbol.ConstructedFrom.Interfaces.Any(x => + x.IsGenericType && x.ToString().StartsWith("System.Collections.Generic.ICollection")); + } + + private static bool IsPropertyTypeReadonlySequence(SemanticModel semanticModel, PropertyDeclarationSyntax propertyDeclaration) + { + return IsTypeReadonlySequence(semanticModel, propertyDeclaration.Type); + } + + private static bool IsReturnTypeReadonlySequence(SemanticModel semanticModel, MethodDeclarationSyntax methodDeclarationSyntax) + { + var typeSyntax = methodDeclarationSyntax.ReturnType; + return IsTypeReadonlySequence(semanticModel, typeSyntax); + } + + private bool IsExpectedParameterReadonlySequence(SyntaxNode node, SemanticModel semanticModel) + { + if (node is ArgumentSyntax argument && node.Parent is ArgumentListSyntax argumentList) + { + var argumentIndex = argumentList.Arguments.IndexOf(argument); + if (semanticModel.GetSymbolInfo(argumentList.Parent).Symbol is IMethodSymbol methodSymbol) + { + if (methodSymbol.Parameters.Length > argumentIndex) + { + var parameterType = methodSymbol.Parameters[argumentIndex].Type; + if (IsTypeReadonlySequence(semanticModel, parameterType)) + { + return true; + } + } + } + } + + return false; + } + + private static bool IsTypeReadonlySequence(SemanticModel semanticModel, TypeSyntax typeSyntax) + { + var returnType = ModelExtensions.GetTypeInfo(semanticModel, typeSyntax).Type; + return IsTypeReadonlySequence(semanticModel, returnType); + } + + private static bool IsTypeReadonlySequence(SemanticModel semanticModel, ITypeSymbol type) + { + if (type.Kind == SymbolKind.ArrayType) + { + return true; + } + + if (type is INamedTypeSymbol namedType && namedType.IsGenericType) + { + foreach (var readonlySequence in GetReadonlySequenceTypes(semanticModel)) + { + if (readonlySequence.Equals(namedType.ConstructedFrom)) + { + return true; + } + } + } + + return false; + } + + private static IEnumerable GetReadonlySequenceTypes(SemanticModel semanticModel) + { + yield return semanticModel.Compilation.GetTypeByMetadataName("System.Collections.Generic.IEnumerable`1"); + yield return semanticModel.Compilation.GetTypeByMetadataName("System.Collections.Generic.IReadOnlyList`1"); + yield return semanticModel.Compilation.GetTypeByMetadataName("System.Collections.Generic.IReadOnlyCollection`1"); + } + } +} diff --git a/ClrHeapAllocationsAnalyzer/ClrHeapAllocationAnalyzer.csproj b/ClrHeapAllocationsAnalyzer/ClrHeapAllocationAnalyzer.csproj index dfeacc3..cc6fcc2 100644 --- a/ClrHeapAllocationsAnalyzer/ClrHeapAllocationAnalyzer.csproj +++ b/ClrHeapAllocationsAnalyzer/ClrHeapAllocationAnalyzer.csproj @@ -31,6 +31,7 @@ + diff --git a/ClrHeapAllocationsAnalyzer/SyntaxHelper.cs b/ClrHeapAllocationsAnalyzer/SyntaxHelper.cs new file mode 100644 index 0000000..995c207 --- /dev/null +++ b/ClrHeapAllocationsAnalyzer/SyntaxHelper.cs @@ -0,0 +1,17 @@ +using Microsoft.CodeAnalysis; + +namespace ClrHeapAllocationAnalyzer +{ + internal static class SyntaxHelper + { + public static T FindContainer(this SyntaxNode tokenParent) where T : SyntaxNode + { + if (tokenParent is T invocation) + { + return invocation; + } + + return tokenParent.Parent == null ? null : FindContainer(tokenParent.Parent); + } + } +} \ No newline at end of file