Skip to content

Commit

Permalink
Refactor HLQ006
Browse files Browse the repository at this point in the history
  • Loading branch information
aalmada committed Apr 19, 2020
1 parent 79ed574 commit 2fa9b5e
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,33 @@ public void Verify_Enumerable()
using System.Collections;
using System.Collections.Generic;
readonly struct Enumerable : IEnumerable<int>
readonly struct Enumerable
{
public Enumerator GetEnumerator() => new Enumerator();
public class 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 class Enumerator : IEnumerator<int>
{
public int Current => 0;
public bool MoveNext() => false;
}
}
readonly struct Enumerable3 : IEnumerable<int>
{
public IEnumerator<int> GetEnumerator() => new Enumerator();
IEnumerator IEnumerable.GetEnumerator() => new Enumerator();
Expand All @@ -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()
Expand All @@ -151,7 +196,44 @@ public void Verify_AsyncEnumerable()
using System.Threading;
using System.Threading.Tasks;
readonly struct AsyncEnumerable : IAsyncEnumerable<int>
readonly struct AsyncEnumerable
{
public Enumerator GetAsyncEnumerator() => new Enumerator();
public class 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 class 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 class Enumerator : IAsyncEnumerator<int>
{
public int Current => 0;
public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(false);
}
}
readonly struct AsyncEnumerable4 : IAsyncEnumerable<int>
{
public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken cancellationToken = default) => new Enumerator();
Expand All @@ -166,17 +248,47 @@ public struct Enumerator : IAsyncEnumerator<int>
}
";

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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions NetFabric.Hyperlinq.Analyzer/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions NetFabric.Hyperlinq.Analyzer/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@
<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>
<value>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.</value>
</data>
<data name="GetEnumeratorReturnType_MessageFormat" xml:space="preserve">
<value>'{0}' returns an interface. Consider returning a value-type enumerator.</value>
<value>'{0}' returns a reference type. Consider returning a value type.</value>
</data>
<data name="GetEnumeratorReturnType_Title" xml:space="preserve">
<value>GetEnumerator() or GetAsyncEnumerator() return an interface.</value>
<value>GetEnumerator() or GetAsyncEnumerator() should return a value type.</value>
</data>
<data name="HighestLevelInterface_Description" xml:space="preserve">
<value>Public methods should return highest admissible level interface.</value>
Expand Down
14 changes: 7 additions & 7 deletions docs/reference/HLQ006_GetEnumeratorReturnType.md
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -26,7 +26,7 @@ readonly struct Enumerable<T> : IEnumerable<T>
public IEnumerator<T> GetEnumerator() => new Enumerator();
IEnumerator IEnumerable.GetEnumerator() => new Enumerator();

struct Enumerator : IEnumerator<T>
class Enumerator : IEnumerator<T>
{
...
}
Expand All @@ -37,7 +37,7 @@ readonly struct AsyncEnumerable<T> : IAsyncEnumerable<T>
public IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = default)
=> new Enumerator();

struct Enumerator : IAsyncEnumerator<T>
class Enumerator : IAsyncEnumerator<T>
{
...
}
Expand All @@ -46,7 +46,7 @@ readonly struct AsyncEnumerable<T> : IAsyncEnumerable<T>

## 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<T> : IEnumerable<T>
Expand Down

0 comments on commit 2fa9b5e

Please sign in to comment.