Skip to content

Commit

Permalink
Add the PosInfoMoq1005 rule to check usage of the SetupSet<T>() method (
Browse files Browse the repository at this point in the history
fixes #42).
  • Loading branch information
GillesTourreau committed Oct 24, 2024
1 parent 7595618 commit a06e5b9
Show file tree
Hide file tree
Showing 8 changed files with 300 additions and 2 deletions.
1 change: 1 addition & 0 deletions PosInformatique.Moq.Analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
docs\Design\PosInfoMoq1002.md = docs\Design\PosInfoMoq1002.md
docs\Design\PosInfoMoq1003.md = docs\Design\PosInfoMoq1003.md
docs\Design\PosInfoMoq1004.md = docs\Design\PosInfoMoq1004.md
docs\Design\PosInfoMoq1005.md = docs\Design\PosInfoMoq1005.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Design rules used to make your unit tests more strongly strict.
| [PosInfoMoq1002: `Verify()` methods should be called when `Verifiable()` has been setup](docs/Design/PosInfoMoq1002.md) | When a mocked member has been setup with the `Verifiable()` method, the `Verify()` method must be called at the end of the unit test. |
| [PosInfoMoq1003: The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments](docs/Design/PosInfoMoq1003.md) | When a mocked method contains a `It.IsAny<T>()` argument, the related parameter should be checked in the `Callback()` method. |
| [PosInfoMoq1004: The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument](docs/Design/PosInfoMoq1004.md) | When a mocked method contains a `It.IsAny<T>()` argument, the related parameter should not be ignored in the `Callback()` method. |
| [PosInfoMoq1005: Defines the generic argument of the `SetupSet()` method with the type of the mocked property](docs/Design/PosInfoMoq1005.md) | When mocking the setter of a property, use the `SetupSet<TProperty>()` method version. |

### Compilation

Expand Down
86 changes: 86 additions & 0 deletions docs/Design/PosInfoMoq1005.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# PosInfoMoq1005: Defines the generic argument of the `SetupSet()` method with the type of the mocked property.

| Property | Value |
|-------------------------------------|------------------------------------------------------------------------------------------------------|
| **Rule ID** | PosInfoMoq1005 |
| **Title** | Defines the generic argument of the `SetupSet()` method with the type of the mocked property. |
| **Category** | Design |
| **Default severity** | Warning |

## Cause

A property setter has been set up using `SetupSet()` without a generic argument that represents the type of the mocked property.

## Rule description

Moq provides two methods to mock a property setter:
- `Mock<T>.SetupSet(Action<T>)`
- `Mock<T>.SetupSet<TProperty>(Action<T, TProperty>)`

When setting up a property setter, use `Mock<T>.SetupSet<TProperty>(Action<T, TProperty>)` by explicitly defining the type of the property to mock.
This overload of the `SetupSet()` method allows you to define a typed `Callback()` and avoid exceptions if the delegate argument in the `Callback()`
does not match the property type.

For example, consider the following code to test:

```csharp
[Fact]
public interface Customer
{
string Name { get; set; }
}
```

If you mock the setter of the `Customer.Name` property, you should set up the property with the `SetupSet<string>()` method:

```csharp
[Fact]
public void SetNameOfCustomer()
{
var customer = new Mock<Customer>();
customer.SetupSet<string>(c => c.Name = "Gilles") // The SetupSet<string>() version is used.
.Callback((string value) =>
{
// Called when the setter of the property is set.
});
}
```

The following code violates the rule because the `SetupSet()` method has no generic argument:

```csharp
[Fact]
public void SetNameOfCustomer()
{
var customer = new Mock<Customer>();
customer.SetupSet(c => c.Name = "Gilles") // The SetupSet() has been used without set the generic argument.
.Callback((string value) =>
{
// Called when the setter of the property is set.
});
}
```

If the non-generic version of the `SetupSet()` method is used, the delegate in the `Callback()` method cannot be checked at compile time,
an exception will occur during the execution of the unit test:

```csharp
[Fact]
public void SetNameOfCustomer()
{
var customer = new Mock<Customer>();
customer.SetupSet(c => c.Name = "Gilles")
.Callback((int value) => // The code compiles, but during the execution of the unit test
{ // an ArgumentException will be thrown.
});
}
```

## How to fix violations

To fix a violation of this rule, use the `SetupSet<TProperty>()` method with the type of the mocked property as the generic argument.

## When to suppress warnings

Do not suppress a warning from this rule. Using the `SetupSet<T>()` method ensures that the delegate argument in the `Callback()`
method matches the type of the property.
2 changes: 2 additions & 0 deletions src/Moq.Analyzers/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
### New Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
PosInfoMoq1005 | Compilation | Error | Defines the generic argument of the `SetupSet()` method with the type of the mocked property.
PosInfoMoq2014 | Compilation | Error | The `Callback()` delegate expression must not return a value.
PosInfoMoq2015 | Compilation | Error | The `Protected().Setup()` method must match the return type of the mocked method.
PosInfoMoq2016 | Compilation | Error | `Mock<T>` constructor with factory lambda expression can be used only with classes.

## Release 1.10.0

### New Rules
Expand Down
68 changes: 68 additions & 0 deletions src/Moq.Analyzers/Analyzers/SetupSetAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//-----------------------------------------------------------------------
// <copyright file="SetupSetAnalyzer.cs" company="P.O.S Informatique">
// Copyright (c) P.O.S Informatique. All rights reserved.
// </copyright>
//-----------------------------------------------------------------------

namespace PosInformatique.Moq.Analyzers
{
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class SetupSetAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor UseSetupSetWithGenericArgumentRule = new DiagnosticDescriptor(
"PosInfoMoq1005",
"Defines the generic argument of the SetupSet() method with the type of the mocked property",
"Defines the generic argument of the SetupSet() method with the type of the mocked property",
"Compilation",
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "The SetupSet<T>() method must be used when setting up a mocked property.",
helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq1005.html");

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(UseSetupSetWithGenericArgumentRule);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
}

private static void Analyze(SyntaxNodeAnalysisContext context)
{
var invocationExpression = (InvocationExpressionSyntax)context.Node;

var moqSymbols = MoqSymbols.FromCompilation(context.Compilation);

if (moqSymbols is null)
{
return;
}

var methodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken);

// Check if not obsolete extension
if (moqSymbols.IsObsoleteMockExtension(methodSymbol.Symbol))
{
return;
}

// Check is Setup() method.
if (!moqSymbols.IsSetupSetMethodWithoutGenericArgument(methodSymbol.Symbol))
{
var nameSyntax = ((MemberAccessExpressionSyntax)invocationExpression.Expression).Name;

context.ReportDiagnostic(UseSetupSetWithGenericArgumentRule, nameSyntax.GetLocation());

return;
}
}
}
}
6 changes: 4 additions & 2 deletions src/Moq.Analyzers/Moq.Analyzers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@
<PackageReleaseNotes>
1.11.0
- Add new rules:
- PosInfoMoq1005: Defines the generic argument of the SetupSet() method with the type of the mocked property.
- PosInfoMoq2014: The delegate in the argument of the Returns() method must return a value with same type of the mocked method.
- PosInfoMoq2015: The Protected().Setup() method must match the return type of the mocked method.
- PosInfoMoq2016: Mock&lt;T&gt; constructor with factory lambda expression can be used only with classes.
- Fix the PosInfoMoq1001 rule for the Mock&lt;T&gt; instantiation with the lambda expression to create mock instances.
- Fix the PosInfoMoq2004 rule for the Mock&lt;T&gt; instantiation with the lambda expression to create mock instances.
- Fix the PosInfoMoq1001 fixer when using the Mock.Of&lt;T&gt; as argument in the constructor call.

1.10.0
- Add new rules:
- PosInfoMoq2012: The delegate in the argument of the Returns() method must return a value with same type of the mocked method.
- PosInfoMoq2013: The delegate in the argument of the Returns()/ReturnsAsync() method must have the same parameter types of the mocked method/property.
- PosInfoMoq2012: The delegate in the argument of the Returns() method must return a value with same type of the mocked method.
- PosInfoMoq2013: The delegate in the argument of the Returns()/ReturnsAsync() method must have the same parameter types of the mocked method/property.

1.9.3
- Fix the PosInfoMoq2006 when Setup() a method/property in inherited class.
Expand Down
36 changes: 36 additions & 0 deletions src/Moq.Analyzers/MoqSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ internal sealed class MoqSymbols

private readonly Lazy<IMethodSymbol> mockConstructorWithFactory;

private readonly Lazy<IMethodSymbol> setupSetMethodWithoutGenericArgument;

private readonly Lazy<INamedTypeSymbol> obsoleteExtensionsClass;

private MoqSymbols(INamedTypeSymbol mockGenericClass, Compilation compilation)
{
this.mockGenericClass = mockGenericClass;
Expand All @@ -53,6 +57,7 @@ private MoqSymbols(INamedTypeSymbol mockGenericClass, Compilation compilation)
this.isAnyTypeClass = new Lazy<INamedTypeSymbol>(() => compilation.GetTypeByMetadataName("Moq.It+IsAnyType")!);
this.isAnyMethod = new Lazy<ISymbol>(() => compilation.GetTypeByMetadataName("Moq.It")!.GetMembers("IsAny").Single());
this.verifiesInterface = new Lazy<INamedTypeSymbol>(() => compilation.GetTypeByMetadataName("Moq.Language.IVerifies")!);
this.obsoleteExtensionsClass = new Lazy<INamedTypeSymbol>(() => compilation.GetTypeByMetadataName("Moq.ObsoleteMockExtensions")!);

this.setupMethods = new Lazy<IReadOnlyList<IMethodSymbol>>(() => mockGenericClass.GetMembers("Setup").Concat(setupConditionResultInterface.Value.GetMembers("Setup")).OfType<IMethodSymbol>().ToArray());
this.mockBehaviorStrictField = new Lazy<ISymbol>(() => this.mockBehaviorEnum.Value.GetMembers("Strict").First());
Expand All @@ -68,6 +73,7 @@ private MoqSymbols(INamedTypeSymbol mockGenericClass, Compilation compilation)
this.mockOfMethods = new Lazy<IReadOnlyList<IMethodSymbol>>(() => mockGenericClass.BaseType!.GetMembers("Of").Where(m => m.IsStatic).OfType<IMethodSymbol>().ToArray());

this.mockConstructorWithFactory = new Lazy<IMethodSymbol>(() => mockGenericClass.Constructors.Single(c => c.Parameters.Length == 2 && c.Parameters[0].Type.Name == "Expression"));
this.setupSetMethodWithoutGenericArgument = new Lazy<IMethodSymbol>(() => mockGenericClass.GetMembers("SetupSet").OfType<IMethodSymbol>().Single(c => c.TypeArguments.Length == 1));
}

public static MoqSymbols? FromCompilation(Compilation compilation)
Expand Down Expand Up @@ -122,6 +128,21 @@ public bool IsMock(ISymbol? symbol)
return true;
}

public bool IsObsoleteMockExtension(ISymbol? symbol)
{
if (symbol is null)
{
return false;
}

if (!SymbolEqualityComparer.Default.Equals(symbol.ContainingType, this.obsoleteExtensionsClass.Value))
{
return false;
}

return true;
}

public bool IsSetupMethod(ISymbol? symbol)
{
if (symbol is null)
Expand Down Expand Up @@ -162,6 +183,21 @@ public bool IsSetupProtectedMethod(ISymbol? symbol)
return false;
}

public bool IsSetupSetMethodWithoutGenericArgument(ISymbol? symbol)
{
if (symbol is null)
{
return false;
}

if (!SymbolEqualityComparer.Default.Equals(symbol.OriginalDefinition, this.setupSetMethodWithoutGenericArgument.Value))
{
return false;
}

return true;
}

public bool IsVerifiableMethod(ISymbol? symbol)
{
if (symbol is null)
Expand Down
102 changes: 102 additions & 0 deletions tests/Moq.Analyzers.Tests/Analyzers/SetupSetAnalyzerTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
//-----------------------------------------------------------------------
// <copyright file="SetupSetAnalyzerTest.cs" company="P.O.S Informatique">
// Copyright (c) P.O.S Informatique. All rights reserved.
// </copyright>
//-----------------------------------------------------------------------

namespace PosInformatique.Moq.Analyzers.Tests
{
using Verifier = MoqCSharpAnalyzerVerifier<SetupSetAnalyzer>;

public class SetupSetAnalyzerTest
{
[Fact]
public async Task SetupSetWithGenericArgument_NoDiagnosticReported()
{
var source = @"
namespace ConsoleApplication1
{
using Moq;
using System;
public class TestClass
{
public void TestMethod()
{
var mock1 = new Mock<I>();
mock1.SetupSet<int>(i => i.TestProperty = 1234);
mock1.SetupSet(i => i.TestProperty); // Ignored because Obsolete by Moq
}
}
public interface I
{
int TestProperty { get; set; }
}
}";

await Verifier.VerifyAnalyzerAsync(source);
}

[Fact]
public async Task SetupSetWithNoGenericArgument_DiagnosticReported()
{
var source = @"
namespace ConsoleApplication1
{
using Moq;
using System;
public class TestClass
{
public void TestMethod()
{
var mock1 = new Mock<I>();
mock1.[|SetupSet|](i => i.TestProperty = 1234);
}
}
public interface I
{
int TestProperty { get; set; }
}
}";

await Verifier.VerifyAnalyzerAsync(source);
}

[Fact]
public async Task NoMoqLibrary()
{
var source = @"
namespace ConsoleApplication1
{
using OtherNamespace;
public class TestClass
{
public void TestMethod()
{
var mock1 = new Mock<I>();
mock1.SetupSet(i => i.Property = 1234);
}
}
public interface I
{
int Property { get; set; }
}
}
namespace OtherNamespace
{
public class Mock<T>
{
public void SetupSet(System.Action<T> _) { }
}
}";

await Verifier.VerifyAnalyzerWithNoMoqLibraryAsync(source);
}
}
}

0 comments on commit a06e5b9

Please sign in to comment.