Skip to content

Commit

Permalink
Add the PosInfoMoq1003 and PosInfoMoq1004 implementations.
Browse files Browse the repository at this point in the history
  • Loading branch information
GillesTourreau committed Jul 1, 2024
1 parent fc643f3 commit 568a3e2
Show file tree
Hide file tree
Showing 16 changed files with 591 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/github-actions-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
type: string
description: The version of the library
required: true
default: 1.7.0
default: 1.8.0
VersionSuffix:
type: string
description: The version suffix of the library (for example rc.1)
Expand Down
2 changes: 2 additions & 0 deletions PosInformatique.Moq.Analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
docs\design\PosInfoMoq1000.md = docs\design\PosInfoMoq1000.md
docs\design\PosInfoMoq1001.md = docs\design\PosInfoMoq1001.md
docs\Design\PosInfoMoq1002.md = docs\Design\PosInfoMoq1002.md
docs\Design\PosInfoMoq1003.md = docs\Design\PosInfoMoq1003.md
docs\Design\PosInfoMoq1004.md = docs\Design\PosInfoMoq1004.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Design rules used to make your unit tests more strongly strict.
| [PosInfoMoq1000: `VerifyAll()` methods should be called when instantiate a `Mock<T>` instances](docs/Design/PosInfoMoq1000.md) | When instantiating a `Mock<T>` in the *Arrange* phase of an unit test, `VerifyAll()` method should be called in the *Assert* phase to check the setup methods has been called. |
| [PosInfoMoq1001: The `Mock<T>` instance behavior should be defined to Strict mode](docs/Design/PosInfoMoq1001.md) | When instantiating a `Mock<T>` instance, the `MockBehavior` of the `Mock` instance should be defined to `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. |

### Compilation

Expand Down
4 changes: 2 additions & 2 deletions docs/Design/PosInfoMoq1000.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ should be called in the *Assert* phase to check the setup methods has been calle

```csharp
[Fact]
public void GetCustomer_ShouldCallRepository()
public void SendMail_ShouldCallSmtpService()
{
// Arrange
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"));

var service = new CustomerService(repository.Object);
var service = new CustomerService(smtpService.Object);

// Act
service.SendMail("Gilles");
Expand Down
4 changes: 2 additions & 2 deletions docs/Design/PosInfoMoq1002.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ In the *Arrange* phase of an unit test, when `Verifiable()` method has been setu

```csharp
[Fact]
public void GetCustomer_ShouldCallRepository()
public void SendMail_ShouldCallSmtpService()
{
// Arrange
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
.Verifiable();

var service = new CustomerService(repository.Object);
var service = new CustomerService(smtpService.Object);

// Act
service.SendMail("Gilles");
Expand Down
93 changes: 93 additions & 0 deletions docs/Design/PosInfoMoq1003.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# PosInfoMoq1003: The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments

| Property | Value |
|-------------------------------------|---------------------------------------------------------------------------------------------------------------------|
| **Rule ID** | PosInfoMoq1003 |
| **Title** | The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments |
| **Category** | Design |
| **Default severity** | Warning |

## Cause

A method has been setup with `It.IsAny<T>()` arguments without checking the parameters in a `Callback()`
method.

## Rule description

When setup a method using `It.IsAny<T>()` arguments, the parameters should be check in the `Callback()` method.

For example if we have the following code to test:

```csharp
[Fact]
public class CustomerService
{
private readonly ISmtpService smtpService;

public CustomerService(ISmtpService smtpService)
{
this.smtpService = smtpService;
}

public void SendMail(string emailAddress)
{
this.smtpService.SendMail("[email protected]", emailAddress);
}
}
```

If we mock the `ISmtpService.SendMail()` with a `It.IsAny<string>()` for the `emailAddress` argument,
we can not check if the `CustomerService.SendMail()` has propagate correctly the value of the argument to the
parameter of the `ISmtpService.SendMail()` method.

```csharp
[Fact]
public void SendMail_ShouldCallSmtpService()
{
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>())); // With It.IsAny<string>() we can not check that emailAddress has been correctly passed in the CustomerService.SendMail() method.
var service = new CustomerService(smtpService.Object);

service.SendMail("Gilles");
}
```

The `emailAddress` parameter passed to the `ISmtpService.SendMail()` method should be tested
with the `Callback()` method, when mocking the `ISmtpService.SendMail()` method with a `It.IsAny<T>()` argument.

```csharp
[Fact]
public void SendMail_ShouldCallSmtpService()
{
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>())) // With It.IsAny() we should test the arguments if correctly propagated in the Callback() method.
.Callback((string _, string emailAddress) =>
{
Assert.AreEqual("Gilles", em); // Check the emailAddress parameter.
});

var service = new CustomerService(smtpService.Object);

service.SendMail("Gilles");
}
```

### Remarks
- If the parameters of mocked methods are very simple (primitive values for example), pass directly the expected value in the arguments of the method. For example
```csharp
smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
```
Instead of
```csharp
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
```
- Use the `Callback()` to assert complex parameters.

## How to fix violations

To fix a violation of this rule, use the `Callback()` method to check the `It.IsAny<T>()` arguments.

## When to suppress warnings

Do not suppress a warning from this rule. Normally `It.IsAny<T>()` arguments should be check and asserted in the `Callback()` methods.
93 changes: 93 additions & 0 deletions docs/Design/PosInfoMoq1004.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# PosInfoMoq1004: The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument.

| Property | Value |
|-------------------------------------|------------------------------------------------------------------------------------------------------|
| **Rule ID** | PosInfoMoq1004 |
| **Title** | The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument. |
| **Category** | Design |
| **Default severity** | Warning |

## Cause

A method has been setup with `It.IsAny<T>()` arguments and the parameter in the `Callback()` has not be used.

## Rule description

When setup a method using `It.IsAny<T>()` arguments, the parameters should be check in the `Callback()` method.

For example if we have the following code to test:

```csharp
[Fact]
public class CustomerService
{
private readonly ISmtpService smtpService;

public CustomerService(ISmtpService smtpService)
{
this.smtpService = smtpService;
}

public void SendMail(string emailAddress)
{
this.smtpService.SendMail("[email protected]", emailAddress);
}
}
```

If we mock the `ISmtpService.SendMail()` with a `It.IsAny<string>()` for the `emailAddress` argument,
we can not check if the `CustomerService.SendMail()` has propagate correctly the value of the argument to the
parameter of the `ISmtpService.SendMail()` method.

```csharp
[Fact]
public void SendMail_ShouldCallSmtpService()
{
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
.Callback((string _, string _) => { ... }); // The second parameter (emailAddress) should not be ignored and should be tested.
var service = new CustomerService(smtpService.Object);

service.SendMail("Gilles");
}
```

The `emailAddress` parameter passed to the `ISmtpService.SendMail()` method should be tested
with the `Callback()` method, when mocking the `ISmtpService.SendMail()` method with a `It.IsAny<T>()` argument.

```csharp
[Fact]
public void SendMail_ShouldCallSmtpService()
{
var smtpService = new Mock<ISmtpService>();
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>())) // With It.IsAny() we should test the arguments if correctly propagated in the Callback() method.
.Callback((string _, string emailAddress) =>
{
Assert.AreEqual("Gilles", em); // Check the emailAddress parameter.
});

var service = new CustomerService(smtpService.Object);

service.SendMail("Gilles");
}
```

### Remarks
- If the parameters of mocked methods are very simple (primitive values for example), pass directly the expected value in the arguments of the method. For example
```csharp
smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
```
Instead of
```csharp
smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
```
- Use the `Callback()` to assert complex parameters.

## How to fix violations

To fix a violation of this rule, use the `Callback()` method to check the `It.IsAny<T>()` arguments.

## When to suppress warnings

Do not suppress a warning from this rule. Normally `It.IsAny<T>()` arguments should be check and asserted in the `Callback()` methods.
11 changes: 10 additions & 1 deletion src/Moq.Analyzers/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
## Release 1.7.0
## Release 1.8.0

### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
PosInfoMoq1003 | Design | Warning | The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments.
PosInfoMoq1004 | Design | Warning | The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument.

## Release 1.7.0

### New Rules

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
return;
}

// Try to determine if the invocation expression is a Callback() expression.
var methodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken);

if (!moqSymbols.IsCallback(methodSymbol.Symbol))
{
return;
}

// If yes, we extract the lambda expression of it.
// Extract the lambda expression.
var moqExpressionAnalyzer = new MoqExpressionAnalyzer(moqSymbols, context.SemanticModel);

var callBackLambdaExpressionSymbol = moqExpressionAnalyzer.ExtractCallBackLambdaExpressionMethod(invocationExpression, out var lambdaExpression, context.CancellationToken);
Expand Down
Loading

0 comments on commit 568a3e2

Please sign in to comment.