HLQ006: GetEnumerator() or GetAsyncEnumerator() should not return an …

…interface type (#22)

* Add GetEnumeratorReturnTypeAnalyzer

* Add documentation

NetFabric.Hyperlinq.Analyzer.UnitTests/GetEnumeratorReturnTypeAnalyzerTests.cs

@@ -0,0 +1,182 @@
+using System;
+using Xunit;
+using TestHelper;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace NetFabric.Hyperlinq.Analyzer.UnitTests
+{
+    public class GetEnumeratorReturnTypeAnalyzerTests : CodeFixVerifier
+    {
+        protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() =>
+            new GetEnumeratorReturnTypeAnalyzer();
+
+        [Fact]
+        public void Verify_NoDiagnostics()
+        {
+            var test = @"
+using System;
+using System.Collections;
+using System.Collections.Generic;
+
+interface ICustomEnumerable
+{
+    IEnumerator GetEnumerator();
+}   
+
+interface IValueEnumerable<T, TEnumerator> : IEnumerable<T>
+    where TEnumerator : struct, IEnumerator<T>
+{
+    new Enumerator GetEnumerator();
+}   
+
+readonly struct Enumerable
+{
+    public Enumerator GetEnumerator() => new Enumerator();
+
+    public struct Enumerator
+    {
+        public int Current => 0;
+
+        public bool MoveNext() => false;
+    }
+}
+
+readonly struct Enumerable2 : IEnumerable<int>
+{
+    public Enumerator GetEnumerator() => new Enumerator();
+    IEnumerator<int> IEnumerable<int>.GetEnumerator() => new Enumerator();
+    IEnumerator IEnumerable.GetEnumerator() => new Enumerator();
+
+    public struct Enumerator : IEnumerator<int>
+    {
+        public int Current => 0;
+
+        public bool MoveNext() => false;
+    }
+}
+
+readonly struct AsyncEnumerable
+{
+    public Enumerator GetAsyncEnumerator() => new Enumerator();
+
+    public struct Enumerator
+    {
+        public int Current => 0;
+
+        public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(false);
+    }
+}
+
+readonly struct AsyncEnumerable2
+{
+    public Enumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator();
+
+    public struct Enumerator
+    {
+        public int Current => 0;
+
+        public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(false);
+    }
+}
+
+readonly struct AsyncEnumerable3 : IAsyncEnumerable<int>
+{
+    public Enumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator();
+    IAsyncEnumerator<int> IAsyncEnumerable<int>.GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator();
+
+    public struct Enumerator : IAsyncEnumerator<int>
+    {
+        public int Current => 0;
+
+        public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(false);
+    }
+}
+";
+
+            VerifyCSharpDiagnostic(test);
+        }
+
+        [Fact]
+        public void Verify_Enumerable()
+        {
+            var test = @"
+using System;
+using System.Collections;
+using System.Collections.Generic;
+
+readonly struct Enumerable : IEnumerable<int>
+{
+    public IEnumerator<int> GetEnumerator() => new Enumerator();
+    IEnumerator IEnumerable.GetEnumerator() => new Enumerator();
+
+    public struct Enumerator : IEnumerator<int>
+    {
+        public int Current => 0;
+
+        object IEnumerator.Current => throw new NotImplementedException();
+
+        public bool MoveNext() => false;
+
+        public void Reset() => throw new NotImplementedException();
+
+        public void Dispose() { }
+    }
+}
+";
+
+            var expected = new DiagnosticResult
+            {
+                Id = "HLQ006",
+                Message = "'GetEnumerator' returns an interface. Consider returning a value-type enumerator.",
+                Severity = DiagnosticSeverity.Warning,
+                Locations = new[] {
+                    new DiagnosticResultLocation("Test0.cs", 8, 12)
+                },
+            };
+
+            VerifyCSharpDiagnostic(test, expected);
+        }
+
+
+        [Fact]
+        public void Verify_AsyncEnumerable()
+        {
+            var test = @"
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+
+readonly struct AsyncEnumerable : IAsyncEnumerable<int>
+{
+    public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator();
+
+    public struct Enumerator : IAsyncEnumerator<int>
+    {
+        public int Current => 0;
+
+        public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(false);
+
+        public ValueTask DisposeAsync() => new ValueTask();
+    }
+}
+";
+
+            var expected = new DiagnosticResult
+            {
+                Id = "HLQ006",
+                Message = "'GetAsyncEnumerator' returns an interface. Consider returning a value-type enumerator.",
+                Severity = DiagnosticSeverity.Warning,
+                Locations = new[] {
+                    new DiagnosticResultLocation("Test0.cs", 9, 12)
+                },
+            };
+
+            VerifyCSharpDiagnostic(test, expected);
+        }
+    }
+}

NetFabric.Hyperlinq.Analyzer/DiagnosticIds.cs

@@ -7,5 +7,6 @@ static class DiagnosticIds
         public const string HighestLevelInterfaceId = "HLQ003";
         public const string RefEnumerationVariableId = "HLQ004";
         public const string AvoidSingleId = "HLQ005";
+        public const string GetEnumeratorReturnTypeId = "HLQ006";
     }
 }

NetFabric.Hyperlinq.Analyzer/GetEnumeratorReturnTypeAnalyzer.cs

@@ -0,0 +1,81 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NetFabric.CodeAnalysis;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+
+namespace NetFabric.Hyperlinq.Analyzer
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class GetEnumeratorReturnTypeAnalyzer : DiagnosticAnalyzer
+    {
+        const string DiagnosticId = DiagnosticIds.GetEnumeratorReturnTypeId;
+
+        static readonly LocalizableString Title =
+            new LocalizableResourceString(nameof(Resources.GetEnumeratorReturnType_Title), Resources.ResourceManager, typeof(Resources));
+        static readonly LocalizableString MessageFormat =
+            new LocalizableResourceString(nameof(Resources.GetEnumeratorReturnType_MessageFormat), Resources.ResourceManager, typeof(Resources));
+        static readonly LocalizableString Description =
+            new LocalizableResourceString(nameof(Resources.GetEnumeratorReturnType_Description), Resources.ResourceManager, typeof(Resources));
+        const string Category = "Performance";
+
+        static readonly DiagnosticDescriptor rule =
+            new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Warning,
+                isEnabledByDefault: true, description: Description,
+                helpLinkUri: "https://github.com/NetFabric/NetFabric.Hyperlinq.Analyzer/tree/master/docs/reference/HLQ006_GetEnumeratorReturnType.md");
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+            ImmutableArray.Create(rule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.EnableConcurrentExecution();
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+            context.RegisterSyntaxNodeAction(AnalyzeMethodDeclaration, SyntaxKind.MethodDeclaration);
+        }
+
+        static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context)
+        {
+            if (!(context.Node is MethodDeclarationSyntax methodDeclarationSyntax))
+                return;
+
+            var semanticModel = context.SemanticModel;
+
+            // check if it returns an interface
+            var returnType = methodDeclarationSyntax.ReturnType;
+            if (semanticModel.GetTypeInfo(returnType).Type.TypeKind != TypeKind.Interface)
+                return;
+
+            // check if it's public
+            if (!methodDeclarationSyntax.Modifiers.Any(modifier => modifier.IsKind(SyntaxKind.PublicKeyword)))
+                return;
+
+            // check if it's "GetEnumerator" or "GetAsyncEnumerator"
+            var identifier = methodDeclarationSyntax.Identifier.ValueText;
+            if (identifier == "GetEnumerator" && methodDeclarationSyntax.ParameterList.Parameters.Count == 0)
+            {
+                // check if it returns an enumerator
+                var type = semanticModel.GetTypeInfo(returnType).Type;
+                if (!type.IsEnumerator(semanticModel.Compilation, out var enumerableSymbols))
+                    return;
+            }
+            else if (identifier == "GetAsyncEnumerator" && methodDeclarationSyntax.ParameterList.Parameters.Count < 2)
+            {
+                // check if it returns an async enumerator
+                var type = semanticModel.GetTypeInfo(returnType).Type;
+                if (!type.IsAsyncEnumerator(semanticModel.Compilation, out var enumerableSymbols))
+                    return;
+            }
+            else
+            {
+                return;
+            }
+
+            var diagnostic = Diagnostic.Create(rule, methodDeclarationSyntax.ReturnType.GetLocation(), identifier);
+            context.ReportDiagnostic(diagnostic);
+        }
+    }
+}

NetFabric.Hyperlinq.Analyzer/Resources.resx

@@ -118,7 +118,7 @@
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
   <data name="AssignmentBoxing_Description" xml:space="preserve">
-    <value>This collection has a value type enumerator. Assigning it to an interface cause it to be boxed and method calls to be virtual, affecting peformance.</value>
+    <value>This collection has a value type enumerator. Assigning it to an interface cause it to be boxed and method calls to be virtual, affecting performance.</value>
   </data>
   <data name="AssignmentBoxing_MessageFormat" xml:space="preserve">
     <value>'{0}' has a value type enumerator. Assigning it to '{1}' causes boxing of the enumerator.</value>
@@ -135,6 +135,15 @@
   <data name="AvoidSingle_Title" xml:space="preserve">
     <value>Avoid Single() and SingleOrDefault()</value>
   </data>
+  <data name="GetEnumeratorReturnType_Description" xml:space="preserve">
+    <value>Returning a value-type enumerator allows 'foreach' loops to perform better. Interfaces make method calls to be virtual.</value>
+  </data>
+  <data name="GetEnumeratorReturnType_MessageFormat" xml:space="preserve">
+    <value>'{0}' returns an interface. Consider returning a value-type enumerator.</value>
+  </data>
+  <data name="GetEnumeratorReturnType_Title" xml:space="preserve">
+    <value>GetEnumerator() or GetAsyncEnumerator() return an interface.</value>
+  </data>
   <data name="HighestLevelInterface_Description" xml:space="preserve">
     <value>Public methods should return highest admissible level interface.</value>
   </data>