Skip to content

Commit

Permalink
Update analyzer, refactor code, and add abstract class support
Browse files Browse the repository at this point in the history
This commit includes several updates and improvements. The `ConstructorArgumentsShouldMatchAnalyzerTests.ShouldFailIfClassParametersDoNotMatch.approved.txt` file has been updated to reflect changes in the source code. The `FooAbstract` class and its usage have been removed from `ConstructorArgumentsShouldMatch.cs`. Defensive programming practices have been added to `DiagnosticVerifier.Helper.cs` to ensure non-null arguments. A new file `AbstractClass.cs` has been added to `Moq.Analyzers.Test.csproj`. The `Analyze` method in `ConstructorArgumentsShouldMatchAnalyzer.cs` has been refactored for better readability and maintainability, and now supports abstract classes. The `Moq.Analyzers.csproj` now uses the latest C# language version and the assembly version has been updated to `0.0.6.0`. New test class `AbstractClassTests` and related files have been added to verify the functionality of the updated analyzer.

Fixes #1
  • Loading branch information
rjmurillo committed May 30, 2024
1 parent c1b1d42 commit 21226ac
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 49 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Diagnostic 1
Id: Moq1002
Location: SourceFile(Test0.cs[1438..1444))
Highlight: ("42")
Lines: var mock = new Mock<AbstractClassWithCtor>("42");
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

29 changes: 29 additions & 0 deletions Source/Moq.Analyzers.Test/AbstractClassTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using Microsoft.CodeAnalysis.Diagnostics;

namespace Moq.Analyzers.Test
{
using System.IO;

using ApprovalTests;
using ApprovalTests.Reporters;

using TestHelper;

using Xunit;


[UseReporter(typeof(DiffReporter))]
public class AbstractClassTests : DiagnosticVerifier
{
[Fact]
public void ShouldPassIfGoodParameters()
{
Approvals.Verify(VerifyCSharpDiagnostic(File.ReadAllText("Data/AbstractClass.cs")));
}

protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
{
return new ConstructorArgumentsShouldMatchAnalyzer();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
Diagnostic 1
Id: Moq1002
Location: SourceFile(Test0.cs[1359..1368))
Location: SourceFile(Test0.cs[1260..1269))
Highlight: (1, true)
Lines: var mock1 = new Moq.Mock<Foo>(1, true);
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 2
Id: Moq1002
Location: SourceFile(Test0.cs[1440..1449))
Location: SourceFile(Test0.cs[1341..1350))
Highlight: (2, true)
Lines: var mock2 = new Mock<ConstructorArgumentsShouldMatch.Foo>(2, true);
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 3
Id: Moq1002
Location: SourceFile(Test0.cs[1521..1529))
Location: SourceFile(Test0.cs[1422..1430))
Highlight: ("1", 3)
Lines: var mock3 = new Mock<ConstructorArgumentsShouldMatch.Foo>("1", 3);
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 4
Id: Moq1002
Location: SourceFile(Test0.cs[1601..1624))
Location: SourceFile(Test0.cs[1502..1525))
Highlight: (new int[] { 1, 2, 3 })
Lines: var mock4 = new Mock<ConstructorArgumentsShouldMatch.Foo>(new int[] { 1, 2, 3 });
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 5
Id: Moq1002
Location: SourceFile(Test0.cs[1721..1751))
Location: SourceFile(Test0.cs[1622..1652))
Highlight: (MockBehavior.Strict, 4, true)
Lines: var mock1 = new Mock<Foo>(MockBehavior.Strict, 4, true);
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 6
Id: Moq1002
Location: SourceFile(Test0.cs[1827..1856))
Location: SourceFile(Test0.cs[1728..1757))
Highlight: (MockBehavior.Loose, 5, true)
Lines: var mock2 = new Moq.Mock<ConstructorArgumentsShouldMatch.Foo>(MockBehavior.Loose, 5, true);
Severity: Warning
Message: Parameters provided into mock do not match any existing constructors.

Diagnostic 7
Id: Moq1002
Location: SourceFile(Test0.cs[1932..1960))
Location: SourceFile(Test0.cs[1833..1861))
Highlight: (MockBehavior.Loose, "2", 6)
Lines: var mock3 = new Moq.Mock<ConstructorArgumentsShouldMatch.Foo>(MockBehavior.Loose, "2", 6);
Severity: Warning
Expand Down
53 changes: 53 additions & 0 deletions Source/Moq.Analyzers.Test/Data/AbstractClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using System.Runtime.CompilerServices;

namespace Moq.Analyzers.Test.Data
{
internal abstract class AbstractClassDefaultCtor
{
protected AbstractClassDefaultCtor()
{
}
}

internal abstract class AbstractClassWithCtor
{
protected AbstractClassWithCtor(int a)
{
}
}

internal class MyUnitTests
{
// Base case that we can handle abstract types
private void TestForBaseNoArgs()
{
var mock = new Mock<AbstractClassDefaultCtor>();
mock.As<AbstractClassDefaultCtor>();

var mock2 = new Mock<AbstractClassWithCtor>();
var mock3 = new Mock<AbstractClassDefaultCtor>(MockBehavior.Default);
}

// This is syntatically not allowed by C#, but you can do it with Moq
private void TestForBaseWithArgsNonePassed()
{
var mock = new Mock<AbstractClassWithCtor>();
mock.As<AbstractClassWithCtor>();
}

private void TestForBaseWithArgsPassed()
{
var mock2 = new Mock<AbstractClassWithCtor>(42);
}

private void TestForBaseWithArgsPassedAndBehavior()
{
var mock3 = new Mock<AbstractClassWithCtor>(MockBehavior.Default, 42);
}

private void TestBad()
{
var mock = new Mock<AbstractClassWithCtor>("42");
}
}
}
11 changes: 0 additions & 11 deletions Source/Moq.Analyzers.Test/Data/ConstructorArgumentsShouldMatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ public Foo(params DateTime[] dates) { }
public Foo(List<string> l, string s = "A") { }
}

internal abstract class FooAbstract
{
public FooAbstract(string s) { }
}

internal class MyUnitTests
#pragma warning restore SA1402 // File may only contain a single class
{
Expand Down Expand Up @@ -72,11 +67,5 @@ private void TestGood1()
var mock15 = new Mock<Foo>(MockBehavior.Default, new List<string>(), "8");
var mock16 = new Mock<Foo>(MockBehavior.Default, new List<string>());
}

private void TestAbstractGood()
{
var mock1 = new Mock<FooAbstract>("9");
var mock2 = new Mock<FooAbstract>(MockBehavior.Default, "10");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace TestHelper
using System.Diagnostics;

namespace TestHelper
{
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -42,6 +44,8 @@ public abstract partial class DiagnosticVerifier
/// <returns>An IEnumerable of Diagnostics that surfaced in the source code, sorted by Location.</returns>
protected static Diagnostic[] GetSortedDiagnosticsFromDocuments(DiagnosticAnalyzer analyzer, Document[] documents)
{
Debug.Assert(documents != null, nameof(documents) + " != null");

var projects = new HashSet<Project>();
foreach (var document in documents)
{
Expand All @@ -51,6 +55,7 @@ protected static Diagnostic[] GetSortedDiagnosticsFromDocuments(DiagnosticAnalyz
var diagnostics = new List<Diagnostic>();
foreach (var project in projects)
{
Debug.Assert(analyzer != null, nameof(analyzer) + " != null");
var compilationWithAnalyzers = project.GetCompilationAsync().Result.WithAnalyzers(ImmutableArray.Create(analyzer));
var diags = compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync().Result;
foreach (var diag in diags)
Expand Down
3 changes: 3 additions & 0 deletions Source/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
</ProjectReference>
</ItemGroup>
<ItemGroup>
<Compile Update="Data\AbstractClass.cs">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Compile>
<Compile Update="Data\CallbackSignatureShouldMatchMockedMethod.cs">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Compile>
Expand Down
137 changes: 108 additions & 29 deletions Source/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,13 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
{
var objectCreation = (ObjectCreationExpressionSyntax)context.Node;

// TODO Think how to make this piece more elegant while fast
GenericNameSyntax genericName = objectCreation.Type as GenericNameSyntax;
if (objectCreation.Type is QualifiedNameSyntax)
{
var qualifiedName = objectCreation.Type as QualifiedNameSyntax;
genericName = qualifiedName.Right as GenericNameSyntax;
}
var genericName = GetGenericNameSyntax(objectCreation.Type);
if (genericName == null) return;

if (genericName?.Identifier == null || genericName.TypeArgumentList == null) return;

// Quick and dirty check that we are calling new Mock<T>()
if (genericName.Identifier.ToFullString() != "Mock") return;
if (!IsMockGenericType(genericName)) return;

// Full check that we are calling new Mock<T>()
var constructorSymbolInfo = context.SemanticModel.GetSymbolInfo(objectCreation);
var constructorSymbol = constructorSymbolInfo.Symbol as IMethodSymbol;
if (constructorSymbol == null || constructorSymbol.ContainingType == null || constructorSymbol.ContainingType.ConstructedFrom == null) return;
if (constructorSymbol.MethodKind != MethodKind.Constructor) return;
if (constructorSymbol.ContainingType.ConstructedFrom.ToDisplayString() != "Moq.Mock<T>") return;
if (constructorSymbol.Parameters == null || constructorSymbol.Parameters.Length == 0) return;
var constructorSymbol = GetConstructorSymbol(context, objectCreation);

// Vararg parameter is the one that takes all arguments for mocked class constructor
var varArgsConstructorParameter = constructorSymbol.Parameters.FirstOrDefault(x => x.IsParams);
Expand All @@ -61,26 +48,118 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
var varArgsConstructorParameterIdx = constructorSymbol.Parameters.IndexOf(varArgsConstructorParameter);

// Find mocked type
var typeArguments = genericName.TypeArgumentList.Arguments;
if (typeArguments == null || typeArguments.Count != 1) return;
var mockedTypeSymbolInfo = context.SemanticModel.GetSymbolInfo(typeArguments[0]);
var mockedTypeSymbol = mockedTypeSymbolInfo.Symbol as INamedTypeSymbol;
if (mockedTypeSymbol == null || mockedTypeSymbol.TypeKind != TypeKind.Class) return;

// TODO: Currently detection does not work well for abstract classes because they cannot be instantiated
if (mockedTypeSymbol.IsAbstract) return;
var mockedTypeSymbol = GetMockedSymbol(context, genericName);
if (mockedTypeSymbol == null) return;

// Skip first argument if it is not vararg - typically it is MockingBehavior argument
var constructorArguments = objectCreation.ArgumentList.Arguments.Skip(varArgsConstructorParameterIdx == 0 ? 0 : 1).ToArray();

// Build fake constructor call to understand how it is resolved
var fakeConstructorCall = SyntaxFactory.ObjectCreationExpression(typeArguments[0], SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(constructorArguments)), null);
var mockedClassConstructorSymbolInfo = context.SemanticModel.GetSpeculativeSymbolInfo(objectCreation.GetLocation().SourceSpan.Start, fakeConstructorCall, SpeculativeBindingOption.BindAsExpression);
if (mockedClassConstructorSymbolInfo.Symbol == null)
if (!mockedTypeSymbol.IsAbstract)
{
if (IsConstructorMismatch(context, objectCreation, genericName, constructorArguments))
{
var diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
context.ReportDiagnostic(diagnostic);
}
}
else
{
// Issue #1: Currently detection does not work well for abstract classes because they cannot be instantiated

// The mocked symbol is generic, so we need to check if the constructor arguments match the abstract class constructor

// Extract types of arguments passed in the constructor call
var argumentTypes = constructorArguments
.Select(arg => context.SemanticModel.GetTypeInfo(arg.Expression).Type)
.ToArray();

// Check all constructors of the abstract type
foreach (var constructor in mockedTypeSymbol.Constructors)
{
if (AreParametersMatching(constructor.Parameters, argumentTypes))
{
return; // Found a matching constructor
}
}

var diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
context.ReportDiagnostic(diagnostic);
}
}

private static INamedTypeSymbol GetMockedSymbol(
SyntaxNodeAnalysisContext context,
GenericNameSyntax genericName)
{
var typeArguments = genericName.TypeArgumentList.Arguments;
if (typeArguments == null || typeArguments.Count != 1) return null;
var mockedTypeSymbolInfo = context.SemanticModel.GetSymbolInfo(typeArguments[0]);
var mockedTypeSymbol = mockedTypeSymbolInfo.Symbol as INamedTypeSymbol;
if (mockedTypeSymbol == null || mockedTypeSymbol.TypeKind != TypeKind.Class) return null;
return mockedTypeSymbol;
}

private static bool AreParametersMatching(ImmutableArray<IParameterSymbol> constructorParameters, ITypeSymbol[] argumentTypes2)
{
// Check if the number of parameters matches
if (constructorParameters.Length != argumentTypes2.Length)
{
return false;
}

// Check if each parameter type matches in order
for (int i = 0; i < constructorParameters.Length; i++)
{
if (!constructorParameters[i].Type.Equals(argumentTypes2[i]))
{
return false;
}
}

return true;
}

private static GenericNameSyntax GetGenericNameSyntax(TypeSyntax typeSyntax)
{
if (typeSyntax is GenericNameSyntax genericNameSyntax)
{
return genericNameSyntax;
}

if (typeSyntax is QualifiedNameSyntax qualifiedNameSyntax)
{
return qualifiedNameSyntax.Right as GenericNameSyntax;
}

return null;
}

private static bool IsMockGenericType(GenericNameSyntax genericName)
{
return genericName?.Identifier.Text == "Mock" && genericName.TypeArgumentList.Arguments.Count == 1;
}

private static IMethodSymbol GetConstructorSymbol(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreation)
{
var constructorSymbolInfo = context.SemanticModel.GetSymbolInfo(objectCreation);
var constructorSymbol = constructorSymbolInfo.Symbol as IMethodSymbol;
return constructorSymbol?.MethodKind == MethodKind.Constructor &&
constructorSymbol.ContainingType?.ConstructedFrom.ToDisplayString() == "Moq.Mock<T>"
? constructorSymbol
: null;
}

private static bool IsConstructorMismatch(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreation, GenericNameSyntax genericName, ArgumentSyntax[] constructorArguments)
{
var fakeConstructorCall = SyntaxFactory.ObjectCreationExpression(
genericName.TypeArgumentList.Arguments.First(),
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(constructorArguments)),
null);

var mockedClassConstructorSymbolInfo = context.SemanticModel.GetSpeculativeSymbolInfo(
objectCreation.SpanStart, fakeConstructorCall, SpeculativeBindingOption.BindAsExpression);

return mockedClassConstructorSymbolInfo.Symbol == null;
}
}
}
3 changes: 2 additions & 1 deletion Source/Moq.Analyzers/Moq.Analyzers.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<MinimumVisualStudioVersion>11.0</MinimumVisualStudioVersion>
<Configuration Condition="'$(Configuration)' == ''">Debug</Configuration>
Expand Down Expand Up @@ -52,6 +52,7 @@
<Version>0.0.5</Version>
<AssemblyVersion>0.0.6.0</AssemblyVersion>
<FileVersion>0.0.6.0</FileVersion>
<LangVersion>latest</LangVersion>
</PropertyGroup>

<ItemGroup>
Expand Down

0 comments on commit 21226ac

Please sign in to comment.