From 2fa9b5ead5f56c9317fc68b1a75d74522da186c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ant=C3=A3o=20Almada?= Date: Sun, 19 Apr 2020 18:10:47 +0100 Subject: [PATCH] Refactor HLQ006 --- .../GetEnumeratorReturnTypeAnalyzerTests.cs | 130 ++++++++++++++++-- .../GetEnumeratorReturnTypeAnalyzer.cs | 4 +- .../Resources.Designer.cs | 6 +- NetFabric.Hyperlinq.Analyzer/Resources.resx | 6 +- .../HLQ006_GetEnumeratorReturnType.md | 14 +- 5 files changed, 136 insertions(+), 24 deletions(-) diff --git a/NetFabric.Hyperlinq.Analyzer.UnitTests/GetEnumeratorReturnTypeAnalyzerTests.cs b/NetFabric.Hyperlinq.Analyzer.UnitTests/GetEnumeratorReturnTypeAnalyzerTests.cs index c47bf32..214b020 100644 --- a/NetFabric.Hyperlinq.Analyzer.UnitTests/GetEnumeratorReturnTypeAnalyzerTests.cs +++ b/NetFabric.Hyperlinq.Analyzer.UnitTests/GetEnumeratorReturnTypeAnalyzerTests.cs @@ -108,7 +108,33 @@ public void Verify_Enumerable() using System.Collections; using System.Collections.Generic; -readonly struct Enumerable : IEnumerable +readonly struct Enumerable +{ + public Enumerator GetEnumerator() => new Enumerator(); + + public class Enumerator + { + public int Current => 0; + + public bool MoveNext() => false; + } +} + +readonly struct Enumerable2 : IEnumerable +{ + public Enumerator GetEnumerator() => new Enumerator(); + IEnumerator IEnumerable.GetEnumerator() => new Enumerator(); + IEnumerator IEnumerable.GetEnumerator() => new Enumerator(); + + public class Enumerator : IEnumerator + { + public int Current => 0; + + public bool MoveNext() => false; + } +} + +readonly struct Enumerable3 : IEnumerable { public IEnumerator GetEnumerator() => new Enumerator(); IEnumerator IEnumerable.GetEnumerator() => new Enumerator(); @@ -128,19 +154,38 @@ public void Dispose() { } } "; - var expected = new DiagnosticResult + var expected1 = new DiagnosticResult { Id = "HLQ006", - Message = "'GetEnumerator' returns an interface. Consider returning a value-type enumerator.", + Message = "'GetEnumerator' returns a reference type. Consider returning a value type.", Severity = DiagnosticSeverity.Warning, Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 12) }, }; - VerifyCSharpDiagnostic(test, expected); - } + var expected2 = new DiagnosticResult + { + Id = "HLQ006", + Message = "'GetEnumerator' returns a reference type. Consider returning a value type.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { + new DiagnosticResultLocation("Test0.cs", 20, 12) + }, + }; + var expected3 = new DiagnosticResult + { + Id = "HLQ006", + Message = "'GetEnumerator' returns a reference type. Consider returning a value type.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { + new DiagnosticResultLocation("Test0.cs", 34, 12) + }, + }; + + VerifyCSharpDiagnostic(test, expected1, expected2, expected3); + } [Fact] public void Verify_AsyncEnumerable() @@ -151,7 +196,44 @@ public void Verify_AsyncEnumerable() using System.Threading; using System.Threading.Tasks; -readonly struct AsyncEnumerable : IAsyncEnumerable +readonly struct AsyncEnumerable +{ + public Enumerator GetAsyncEnumerator() => new Enumerator(); + + public class Enumerator + { + public int Current => 0; + + public ValueTask MoveNextAsync() => new ValueTask(false); + } +} + +readonly struct AsyncEnumerable2 +{ + public Enumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator(); + + public class Enumerator + { + public int Current => 0; + + public ValueTask MoveNextAsync() => new ValueTask(false); + } +} + +readonly struct AsyncEnumerable3 : IAsyncEnumerable +{ + public Enumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator(); + IAsyncEnumerator IAsyncEnumerable.GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator(); + + public class Enumerator : IAsyncEnumerator + { + public int Current => 0; + + public ValueTask MoveNextAsync() => new ValueTask(false); + } +} + +readonly struct AsyncEnumerable4 : IAsyncEnumerable { public IAsyncEnumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator(); @@ -166,17 +248,47 @@ public struct Enumerator : IAsyncEnumerator } "; - var expected = new DiagnosticResult + var expected1 = new DiagnosticResult { Id = "HLQ006", - Message = "'GetAsyncEnumerator' returns an interface. Consider returning a value-type enumerator.", + Message = "'GetAsyncEnumerator' returns a reference type. Consider returning a value type.", Severity = DiagnosticSeverity.Warning, Locations = new[] { new DiagnosticResultLocation("Test0.cs", 9, 12) }, }; - VerifyCSharpDiagnostic(test, expected); + var expected2 = new DiagnosticResult + { + Id = "HLQ006", + Message = "'GetAsyncEnumerator' returns a reference type. Consider returning a value type.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { + new DiagnosticResultLocation("Test0.cs", 21, 12) + }, + }; + + var expected3 = new DiagnosticResult + { + Id = "HLQ006", + Message = "'GetAsyncEnumerator' returns a reference type. Consider returning a value type.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { + new DiagnosticResultLocation("Test0.cs", 33, 12) + }, + }; + + var expected4 = new DiagnosticResult + { + Id = "HLQ006", + Message = "'GetAsyncEnumerator' returns a reference type. Consider returning a value type.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { + new DiagnosticResultLocation("Test0.cs", 46, 12) + }, + }; + + VerifyCSharpDiagnostic(test, expected1, expected2, expected3, expected4); } } } diff --git a/NetFabric.Hyperlinq.Analyzer/GetEnumeratorReturnTypeAnalyzer.cs b/NetFabric.Hyperlinq.Analyzer/GetEnumeratorReturnTypeAnalyzer.cs index ee0d654..685cf9d 100644 --- a/NetFabric.Hyperlinq.Analyzer/GetEnumeratorReturnTypeAnalyzer.cs +++ b/NetFabric.Hyperlinq.Analyzer/GetEnumeratorReturnTypeAnalyzer.cs @@ -44,9 +44,9 @@ static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) var semanticModel = context.SemanticModel; - // check if it returns an interface + // check if it does not return a value type var returnType = methodDeclarationSyntax.ReturnType; - if (semanticModel.GetTypeInfo(returnType).Type.TypeKind != TypeKind.Interface) + if (semanticModel.GetTypeInfo(returnType).Type.TypeKind == TypeKind.Struct) return; // check if it's public diff --git a/NetFabric.Hyperlinq.Analyzer/Resources.Designer.cs b/NetFabric.Hyperlinq.Analyzer/Resources.Designer.cs index 327ca93..d4b90f6 100644 --- a/NetFabric.Hyperlinq.Analyzer/Resources.Designer.cs +++ b/NetFabric.Hyperlinq.Analyzer/Resources.Designer.cs @@ -115,7 +115,7 @@ internal static string AvoidSingle_Title { } /// - /// Looks up a localized string similar to Returning a value-type enumerator allows 'foreach' loops to perform better. Interfaces make method calls to be virtual.. + /// Looks up a localized string similar to Returning a value-type enumerator allows 'foreach' loops to perform better. It will allocate the enumerator on the heap and calls to enumerator methods are not virtual.. /// internal static string GetEnumeratorReturnType_Description { get { @@ -124,7 +124,7 @@ internal static string GetEnumeratorReturnType_Description { } /// - /// Looks up a localized string similar to '{0}' returns an interface. Consider returning a value-type enumerator.. + /// Looks up a localized string similar to '{0}' returns a reference type. Consider returning a value type.. /// internal static string GetEnumeratorReturnType_MessageFormat { get { @@ -133,7 +133,7 @@ internal static string GetEnumeratorReturnType_MessageFormat { } /// - /// Looks up a localized string similar to GetEnumerator() or GetAsyncEnumerator() return an interface.. + /// Looks up a localized string similar to GetEnumerator() or GetAsyncEnumerator() should return a value type.. /// internal static string GetEnumeratorReturnType_Title { get { diff --git a/NetFabric.Hyperlinq.Analyzer/Resources.resx b/NetFabric.Hyperlinq.Analyzer/Resources.resx index d8a0a4c..3a58e53 100644 --- a/NetFabric.Hyperlinq.Analyzer/Resources.resx +++ b/NetFabric.Hyperlinq.Analyzer/Resources.resx @@ -136,13 +136,13 @@ Avoid Single() and SingleOrDefault() - Returning a value-type enumerator allows 'foreach' loops to perform better. Interfaces make method calls to be virtual. + Returning a value-type enumerator allows 'foreach' loops to perform better. It will allocate the enumerator on the heap and calls to enumerator methods are not virtual. - '{0}' returns an interface. Consider returning a value-type enumerator. + '{0}' returns a reference type. Consider returning a value type. - GetEnumerator() or GetAsyncEnumerator() return an interface. + GetEnumerator() or GetAsyncEnumerator() should return a value type. Public methods should return highest admissible level interface. diff --git a/docs/reference/HLQ006_GetEnumeratorReturnType.md b/docs/reference/HLQ006_GetEnumeratorReturnType.md index adac7ae..8059399 100644 --- a/docs/reference/HLQ006_GetEnumeratorReturnType.md +++ b/docs/reference/HLQ006_GetEnumeratorReturnType.md @@ -1,17 +1,17 @@ -# HLQ006: `GetEnumerator()` or `GetAsyncEnumerator()` should not return an interface type +# HLQ006: `GetEnumerator()` or `GetAsyncEnumerator()` should return an instance of a value-typed enumerator -A `GetEnumerator()` or `GetAsyncEnumerator()` methods returns an interface type. +A `GetEnumerator()` or `GetAsyncEnumerator()` methods returns a reference type. ## Rule description Collections can be implemented so that the enumerator type returned by `GetEnumerator()` or `GetAsyncEnumerator()` is a value -type. The advantage is that the enumerator is allocated on the stack and method calls are not virtual. -Returning an interface type makes it always a reference type. - ## How to fix violations Change the return type of the method to return the enumerator type. This may require the addition of interface methods to be explicitly implemented. +Make sure the enumerator has a value type. + ## When to suppress warnings When it's not feasible to create a value-type enumerator. @@ -26,7 +26,7 @@ readonly struct Enumerable : IEnumerable public IEnumerator GetEnumerator() => new Enumerator(); IEnumerator IEnumerable.GetEnumerator() => new Enumerator(); - struct Enumerator : IEnumerator + class Enumerator : IEnumerator { ... } @@ -37,7 +37,7 @@ readonly struct AsyncEnumerable : IAsyncEnumerable public IAsyncEnumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator(); - struct Enumerator : IAsyncEnumerator + class Enumerator : IAsyncEnumerator { ... } @@ -46,7 +46,7 @@ readonly struct AsyncEnumerable : IAsyncEnumerable ## Example of how to fix -Change the enumerator type declaration to be `public` and change the method return type. Add the necessary interface explicit implementations: +Change the enumerator type declaration to be `public` and a `struct`. Change the `GetEnumerator()`and `GetAsyncEnumerator()` methods to return the enumerator type and not an interface. Add the necessary interface explicit method implementations: ```csharp readonly struct Enumerable : IEnumerable