Skip to content

Commit

Permalink
Refactor Moq1300 to use IOperation-based analysis (#125)
Browse files Browse the repository at this point in the history
Refactor Moq1300 to use `IOperation`-based analysis instead of syntax
analysis. This method results in a slightly less specific diagnostic (it
uses the location of the entire `.As<T>()` method instead of only the
type parameter, however it has several benefits:

1. About double the performance / speed
2. Cuts the memory allocations in half
3. I believe a bit more robust to "interesting" syntax trees

Here's a BenchmarkDotNet run comparing the old and new analyzers:
```
| Method                    | Mean     | Error   | StdDev   | Ratio | RatioSD | Gen0      | Gen1      | Gen2      | Allocated | Alloc Ratio |
|-------------------------- |---------:|--------:|---------:|------:|--------:|----------:|----------:|----------:|----------:|------------:|
| OldMoq1300WithDiagnostics | 204.2 ms | 5.01 ms | 14.44 ms |  1.72 |    0.13 | 8000.0000 | 4000.0000 | 1000.0000 |  69.35 MB |        1.36 |
| NewMoq1300WithDiagnostics | 162.3 ms | 3.21 ms |  3.15 ms |  1.38 |    0.06 | 6000.0000 | 2000.0000 |         - |  62.42 MB |        1.22 |
| Moq1300Baseline           | 118.0 ms | 2.35 ms |  5.12 ms |  1.00 |    0.00 | 5000.0000 | 2000.0000 |         - |  50.97 MB |        1.00 |
```
  • Loading branch information
MattKotsenas authored Jun 25, 2024
1 parent be45e11 commit 2dcad69
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 58 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ dotnet_diagnostic.CA2016.severity = error
# Disabled because it's common to use a named argument when passing `null` or bool arguments to make the parameter's purpose clear
dotnet_diagnostic.AV1555.severity = none

# AV1500: Methods should not exceed 7 statements
dotnet_diagnostic.AV1500.severity = silent

#### Handling TODOs ####
# This is a popular rule in analyzers. Everyone has an opinion and
# some of the severity levels conflict. We don't need all of these
Expand Down
59 changes: 37 additions & 22 deletions src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

/// <summary>
Expand All @@ -10,8 +12,6 @@ public class AsShouldBeUsedOnlyForInterfaceAnalyzer : DiagnosticAnalyzer
private const string Title = "Moq: Invalid As type parameter";
private const string Message = "Mock.As() should take interfaces only";

private static readonly MoqMethodDescriptorBase MoqAsMethodDescriptor = new MoqAsMethodDescriptor();

private static readonly DiagnosticDescriptor Rule = new(
RuleId,
Title,
Expand All @@ -29,43 +29,58 @@ public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
private static void Analyze(SyntaxNodeAnalysisContext context)
{
if (context.Node is not InvocationExpressionSyntax invocationExpression)
context.RegisterCompilationStartAction(static context =>
{
return;
}
// Ensure Moq is referenced in the compilation
ImmutableArray<INamedTypeSymbol> mockTypes = context.Compilation.GetMoqMock();
if (mockTypes.IsEmpty)
{
return;
}

if (invocationExpression.Expression is not MemberAccessExpressionSyntax memberAccessSyntax)
{
return;
}
// Look for the Mock.As() method and provide it to Analyze to avoid looking it up multiple times.
ImmutableArray<IMethodSymbol> asMethods = mockTypes
.SelectMany(mockType => mockType.GetMembers("As"))
.OfType<IMethodSymbol>()
.Where(method => method.IsGenericMethod)
.ToImmutableArray();
if (asMethods.IsEmpty)
{
return;
}

context.RegisterOperationAction(context => Analyze(context, asMethods), OperationKind.Invocation);
});
}

if (!MoqAsMethodDescriptor.IsMatch(context.SemanticModel, memberAccessSyntax, context.CancellationToken))
private static void Analyze(OperationAnalysisContext context, ImmutableArray<IMethodSymbol> wellKnownAsMethods)
{
if (context.Operation is not IInvocationOperation invocationOperation)
{
return;
}

if (!memberAccessSyntax.Name.TryGetGenericArguments(out SeparatedSyntaxList<TypeSyntax> typeArguments))
IMethodSymbol targetMethod = invocationOperation.TargetMethod;
if (!targetMethod.IsInstanceOf(wellKnownAsMethods))
{
return;
}

if (typeArguments.Count != 1)
ImmutableArray<ITypeSymbol> typeArguments = targetMethod.TypeArguments;
if (typeArguments.Length != 1)
{
return;
}

TypeSyntax typeArgument = typeArguments[0];
SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(typeArgument, context.CancellationToken);

if (symbolInfo.Symbol is ITypeSymbol { TypeKind: not TypeKind.Interface })
if (typeArguments[0] is ITypeSymbol { TypeKind: not TypeKind.Interface })
{
context.ReportDiagnostic(Diagnostic.Create(Rule, typeArgument.GetLocation()));
// Try to locate the type argument in the syntax tree to report the diagnostic at the correct location.
// If that fails for any reason, report the diagnostic on the operation itself.
NameSyntax? memberName = context.Operation.Syntax.DescendantNodes().OfType<MemberAccessExpressionSyntax>().Select(mae => mae.Name).DefaultIfNotSingle();
Location location = memberName?.GetLocation() ?? invocationOperation.Syntax.GetLocation();

context.ReportDiagnostic(Diagnostic.Create(Rule, location));
}
}
}
41 changes: 41 additions & 0 deletions src/Moq.Analyzers/CompilationExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

internal static class CompilationExtensions
{
/// <summary>
/// An extension method that performs <see cref="Compilation.GetTypeByMetadataName(string)"/> for multiple metadata names.
/// </summary>
/// <param name="compilation">The <see cref="Compilation"/> to inspect.</param>
/// <param name="metadataNames">A list of type names to query.</param>
/// <returns><see langword="null"/> if the type can't be found or there was an ambiguity during lookup.</returns>
public static ImmutableArray<INamedTypeSymbol> GetTypesByMetadataNames(this Compilation compilation, ReadOnlySpan<string> metadataNames)
{
ImmutableArray<INamedTypeSymbol>.Builder builder = ImmutableArray.CreateBuilder<INamedTypeSymbol>(metadataNames.Length);

foreach (string metadataName in metadataNames)
{
INamedTypeSymbol? type = compilation.GetTypeByMetadataName(metadataName);
if (type is not null)
{
builder.Add(type);
}
}

return builder.ToImmutable();
}

/// <summary>
/// Get the Moq.Mock and Moq.Mock`1 type symbols (if part of the compilation).
/// </summary>
/// <param name="compilation">The <see cref="Compilation"/> to inspect.</param>
/// <returns>
/// <see cref="INamedTypeSymbol"/>s for the Moq.Mock symbols that are part of the compilation.
/// An empty array if none (never <see langword="null"/>).
/// </returns>
public static ImmutableArray<INamedTypeSymbol> GetMoqMock(this Compilation compilation)
{
return compilation.GetTypesByMetadataNames([WellKnownTypeNames.MoqMock, WellKnownTypeNames.MoqMock1]);
}
}
43 changes: 43 additions & 0 deletions src/Moq.Analyzers/EnumerableExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
namespace Moq.Analyzers;

internal static class EnumerableExtensions
{
/// <inheritdoc cref="DefaultIfNotSingle{TSource}(IEnumerable{TSource}, Func{TSource, bool})"/>
public static TSource? DefaultIfNotSingle<TSource>(this IEnumerable<TSource> source)
{
return source.DefaultIfNotSingle(_ => true);
}

/// <summary>
/// Returns the only element of a sequence that satisfies a specified condition or default if no such element exists or more than one element satisfies the condition.
/// </summary>
/// <typeparam name="TSource">The type of the <paramref name="source"/> collection.</typeparam>
/// <param name="source">The collection to enumerate.</param>
/// <param name="predicate">A function to test each element for a condition.</param>
/// <returns>
/// The single element that satisfies the condition, or default if no such element exists or more than one element satisfies the condition.
/// </returns>
/// <remarks>
/// This should be equivalent to calling <see cref="Enumerable.SingleOrDefault{TSource}(IEnumerable{TSource}, Func{TSource, bool})"/>
/// combined with a catch that returns <see langword="null"/>.
/// </remarks>
public static TSource? DefaultIfNotSingle<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
bool isFound = false;
TSource? item = default;

foreach (TSource element in source.Where(predicate))
{
if (isFound)
{
// We already found an element, thus there's multiple matches; return default.
return default;
}

isFound = true;
item = element;
}

return item;
}
}
33 changes: 0 additions & 33 deletions src/Moq.Analyzers/MoqAsMethodDescriptor.cs

This file was deleted.

44 changes: 44 additions & 0 deletions src/Moq.Analyzers/SymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
namespace Moq.Analyzers;

internal static class SymbolExtensions
{
/// <summary>
/// Determines whether the symbol is an instance of the specified symbol.
/// </summary>
/// <typeparam name="TSymbol">The type of the <see cref="ISymbol"/> to compare.</typeparam>
/// <param name="symbol">The symbol to compare.</param>
/// <param name="other">The symbol to compare to.</param>
/// <param name="symbolEqualityComparer">The <see cref="SymbolEqualityComparer"/> to use for equality.</param>
/// <returns>
/// <see langword="true"/> if <paramref name="symbol"/> is an instance of <paramref name="other"/>, either as a direct match,
/// or as a specialaization; otherwise, <see langword="false"/>.
/// </returns>
/// <remarks>
/// As an example, <c>Type.Method&lt;int&gt;()</c> is an instance of <c>Type.Method&lt;T&gt;()</c>.
/// </remarks>
public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, TSymbol other, SymbolEqualityComparer? symbolEqualityComparer = null)
where TSymbol : class, ISymbol
{
symbolEqualityComparer ??= SymbolEqualityComparer.Default;

return symbol switch
{
IMethodSymbol methodSymbol => symbolEqualityComparer.Equals(methodSymbol.OriginalDefinition, other),
_ => symbolEqualityComparer.Equals(symbol, other),
};
}

/// <inheritdoc cref="IsInstanceOf{TSymbol}(ISymbol, TSymbol, SymbolEqualityComparer?)"/>
/// <param name="symbol">The symbol to compare.</param>
/// <param name="others">
/// The symbols to compare to. Returns <see langword="true"/> if <paramref name="symbol"/> matches any of others.
/// </param>
/// <param name="symbolEqualityComparer">The <see cref="SymbolEqualityComparer"/> to use for equality.</param>
public static bool IsInstanceOf<TSymbol>(this ISymbol symbol, IEnumerable<TSymbol> others, SymbolEqualityComparer? symbolEqualityComparer = null)
where TSymbol : class, ISymbol
{
symbolEqualityComparer ??= SymbolEqualityComparer.Default;

return others.Any(other => symbol.IsInstanceOf(other, symbolEqualityComparer));
}
}
7 changes: 7 additions & 0 deletions src/Moq.Analyzers/WellKnownTypeNames.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Moq.Analyzers;

internal static class WellKnownTypeNames
{
public const string MoqMock = "Moq.Mock";
public const string MoqMock1 = "Moq.Mock`1";
}
1 change: 1 addition & 0 deletions tests/Moq.Analyzers.Benchmarks/Moq1300Benchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal class {name}
private void Test()
{{
new Mock<SampleClass{index}>().As<SampleClass{index}>();
_ = new SampleClass{index}().Calculate(); // Add an expression that looks similar but does not match
}}
}}
"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Microsoft.CodeAnalysis.Testing;
using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.AsShouldBeUsedOnlyForInterfaceAnalyzer>;

namespace Moq.Analyzers.Test;
Expand All @@ -9,8 +8,8 @@ public static IEnumerable<object[]> TestData()
{
return new object[][]
{
["""new Mock<BaseSampleClass>().As<{|Moq1300:BaseSampleClass|}>();"""],
["""new Mock<BaseSampleClass>().As<{|Moq1300:SampleClass|}>();"""],
["""new Mock<BaseSampleClass>().{|Moq1300:As<BaseSampleClass>|}();"""],
["""new Mock<BaseSampleClass>().{|Moq1300:As<SampleClass>|}();"""],
["""new Mock<SampleClass>().As<ISampleInterface>();"""],
["""new Mock<SampleClass>().As<ISampleInterface>().Setup(x => x.Calculate(It.IsAny<int>(), It.IsAny<int>())).Returns(10);"""],
}.WithNamespaces().WithReferenceAssemblyGroups();
Expand Down

0 comments on commit 2dcad69

Please sign in to comment.