Skip to content

Commit ff5ecb1

Browse files
v1.13.0 (#49)
* Add analyzer to check if the Times argument has been set explicitly for the Verify() methods (fixes: #37). * Updates the CI to display the tests. * Improving nullability tests * Add the PosInfoMoq1008 rule to check that static Mock.Verify()/VerifyAll() is called with at least one instance (fixes: #46). * Fix a bug with It.Is<T>() and It.IsAny<T>() parameters check with the PosInfo1006 rule when using inherited types (fixes #47). * Fix documentation PosInfoMoq1008 * Add the PosInfoMoq1009 rule (fixes: #48). * Checks the Times parameters in the Verifiable() methods. * Do not check Times parameters for the Verify() method with no arguments.
1 parent 74d3525 commit ff5ecb1

22 files changed

Lines changed: 908 additions & 55 deletions

.github/workflows/github-actions-ci.yaml

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,27 @@ jobs:
1616
uses: actions/setup-dotnet@v4
1717
with:
1818
dotnet-version: '8.x'
19-
20-
- name: Build
21-
run: dotnet build --property:Configuration=Debug "PosInformatique.Moq.Analyzers.sln"
2219

23-
- name: Test with the dotnet CLI
24-
run: dotnet test --property:Configuration=Debug "PosInformatique.Moq.Analyzers.sln"
20+
- name: Restore dependencies
21+
run: dotnet restore PosInformatique.Moq.Analyzers.sln
22+
23+
- name: Build solution
24+
run: |
25+
dotnet build PosInformatique.Moq.Analyzers.sln \
26+
--configuration Release \
27+
--no-restore
28+
29+
- name: Run tests
30+
run: |
31+
dotnet test PosInformatique.Moq.Analyzers.sln \
32+
--configuration Release \
33+
--no-build \
34+
--logger "trx;LogFileName=test_results.trx" \
35+
--results-directory ./TestResults
36+
37+
- name: Publish Test Results
38+
uses: EnricoMi/publish-unit-test-result-action@v2
39+
if: (!cancelled())
40+
with:
41+
files: |
42+
TestResults/**/*.trx

.github/workflows/github-actions-release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ on:
77
type: string
88
description: The version of the library
99
required: true
10-
default: 1.12.0
10+
default: 1.13.0
1111
VersionSuffix:
1212
type: string
1313
description: The version suffix of the library (for example rc.1)
1414

15-
run-name: ${{ inputs.VersionPrefix }}-${{ inputs.VersionSuffix }}
15+
run-name: ${{ inputs.VersionSuffix && format('{0}-{1}', inputs.VersionPrefix, inputs.VersionSuffix) || inputs.VersionPrefix }}
1616

1717
jobs:
1818
build:

PosInformatique.Moq.Analyzers.sln

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
3535
docs\Design\PosInfoMoq1004.md = docs\Design\PosInfoMoq1004.md
3636
docs\Design\PosInfoMoq1005.md = docs\Design\PosInfoMoq1005.md
3737
docs\Design\PosInfoMoq1006.md = docs\Design\PosInfoMoq1006.md
38+
docs\Design\PosInfoMoq1007.md = docs\Design\PosInfoMoq1007.md
39+
docs\Design\PosInfoMoq1008.md = docs\Design\PosInfoMoq1008.md
40+
docs\Design\PosInfoMoq1009.md = docs\Design\PosInfoMoq1009.md
3841
EndProjectSection
3942
EndProject
4043
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"

README.md

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,20 @@
1-
# PosInformatique.Moq.Analyzers
2-
PosInformatique.Moq.Analyzers is a library to verify syntax and code design when writing the unit tests using the [Moq](https://github.com/devlooped/moq) library.
1+
# PosInformatique.Moq.Analyzers
2+
<div align="center">
33

4-
## Installing from NuGet
4+
[![Nuget](https://img.shields.io/nuget/v/PosInformatique.Moq.Analyzers)](https://www.nuget.org/packages/PosInformatique.Moq.Analyzers/)
5+
[![NuGet downloads](https://img.shields.io/nuget/dt/PosInformatique.Moq.Analyzers)](https://www.nuget.org/packages/PosInformatique.Moq.Analyzers/)
6+
[![License](https://img.shields.io/github/license/Nonanti/MathFlow?style=flat-square)](LICENSE)
7+
[![Build Status](https://img.shields.io/github/actions/workflow/status/PosInformatique/PosInformatique.Moq.Analyzers/github-actions-ci.yaml?style=flat-square)](https://github.com/PosInformatique/PosInformatique.Moq.Analyzers/actions)
8+
[![.NET Standard 2.0](https://img.shields.io/badge/.NET%20Standard-2.0-512BD4?style=flat-square)](https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0)
9+
10+
</div>
11+
12+
PosInformatique.Moq.Analyzers is a set of analyzers to verify syntax and code design when writing the unit tests using the [Moq](https://github.com/devlooped/moq) library.
13+
14+
The analyzers are compiled against [.NET Standard 2.0](https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0),
15+
providing support for analyzing projects that target .NET Core or .NET Framework.
16+
17+
## 📦 Installing from NuGet
518
The [PosInformatique.Moq.Analyzers](https://www.nuget.org/packages/PosInformatique.FluentAssertions.Json/)
619
library is available directly on the
720
[![Nuget](https://img.shields.io/nuget/v/PosInformatique.Moq.Analyzers)](https://www.nuget.org/packages/PosInformatique.Moq.Analyzers/)
@@ -15,7 +28,7 @@ Install-Package PosInformatique.Moq.Analyzers
1528

1629
The analyzer is automatically added and activated with their default severity levels.
1730

18-
## Rules
31+
## 📋 Rules
1932

2033
This section describes the list of the rules analyzed by the library to improve code quality of the unit tests using
2134
the [Moq](https://github.com/devlooped/moq) library.
@@ -33,6 +46,9 @@ Design rules used to make your unit tests more strongly strict.
3346
| [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. |
3447
| [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. |
3548
| [PosInfoMoq1006: The `It.IsAny<T>()` or `It.Is<T>()` arguments must match the parameters of the mocked method.](docs/Design/PosInfoMoq1006.md) | When setting up a method using `It.IsAny<T>()` or `It.Is<T>()` as arguments, the type `T` must exactly match the parameters of the configured method. |
49+
| [PosInfoMoq1007: The `Verify()` method must specify the `Times` argument.](docs/Design/PosInfoMoq1007.md) | When calling the `Verify()` method, if the `Times` argument is not specified, Moq will assume `Times.AtLeastOnce()` by default. |
50+
| [PosInfoMoq1008: The `Mock.Verify()` and `Mock.VerifyAll()` methods must specify at least one mock.](docs/Design/PosInfoMoq1008.md) | When calling the static methods `Mock.Verify()` or `Mock.VerifyAll()` without providing any `Mock<T>` instances, no verification is performed. |
51+
| [PosInfoMoq1009: Avoid using `Verifiable()` method](docs/Design/PosInfoMoq1008.md) | A `Verify()` of an `Mock<T>` instance has not been called in the *Assert* phase of an unit test for `Verifiable()` setups. |
3652

3753
### Compilation
3854

docs/Design/PosInfoMoq1002.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99

1010
## Cause
1111

12-
A `Verify()` of an `Mock<T>` instance has not been called in the *Assert* phase
13-
of an unit test for `Verifiable()` setups.
12+
A `Verify()` of an `Mock<T>` instance has not been called in the *Assert* phase of an unit test for `Verifiable()` setups.
1413

1514
## Rule description
1615

@@ -43,4 +42,9 @@ on the `Mock<T>` instances, if some mocked members has been marked as `Verifiabl
4342

4443
## When to suppress warnings
4544

46-
Do not suppress a warning from this rule. Normally `Verifiable()` setup members must be call in the unit tests execution.
45+
Do not suppress a warning from this rule. Normally `Verifiable()` setup members must be call in the unit tests execution.
46+
47+
## Related rules
48+
49+
- [PosInfoMoq1007: The `Verify()` method must specify the `Times` argument](./PosInfoMoq1007.md)
50+
- [PosInfoMoq1009: Avoid using `Verifiable()` method](./PosInfoMoq1009.md)

docs/Design/PosInfoMoq1007.md

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# PosInfoMoq1007: The `Verify()` method must specify the `Times` argument
2+
3+
| Property | Value |
4+
|---------------------|-----------------------------------------------------------------------|
5+
| **Rule ID** | PosInfoMoq1007 |
6+
| **Title** | The `Verify()` method must specify the `Times` argument |
7+
| **Category** | Design |
8+
| **Default severity**| Warning |
9+
10+
## Cause
11+
12+
When calling the `Verify()` method, if the `Times` argument is not specified, Moq will assume `Times.AtLeastOnce()` by default.
13+
14+
## Rule description
15+
16+
Although Moq provides a default behavior (`Times.AtLeastOnce()`), relying on it can make test intentions unclear or ambiguous.
17+
Specifying the `Times` argument explicitly improves readability and ensures that test failures are interpreted correctly.
18+
19+
For example, the following code omits the `Times` argument:
20+
21+
```csharp
22+
[Fact]
23+
public void SaveUser_ShouldBeCalled()
24+
{
25+
var repository = new Mock<IRepository>();
26+
var service = new UserService(repository.Object);
27+
28+
service.Save(new User());
29+
30+
// Ambiguous: defaults internally to Times.AtLeastOnce()
31+
repository.Verify(x => x.Save(It.IsAny<User>()));
32+
}
33+
```
34+
35+
This code works the same as:
36+
37+
```csharp
38+
repository.Verify(x => x.Save(It.IsAny<User>()), Times.AtLeastOnce());
39+
```
40+
41+
However, omitting the `Times` argument makes it less obvious to a reader what the expected behavior is.
42+
43+
Instead, you should write the `Times` explicitly:
44+
45+
```csharp
46+
[Fact]
47+
public void SaveUser_ShouldBeCalledOnce()
48+
{
49+
var repository = new Mock<IRepository>();
50+
var service = new UserService(repository.Object);
51+
52+
service.Save(new User());
53+
54+
// Clear and explicit
55+
repository.Verify(x => x.Save(It.IsAny<User>()), Times.Once());
56+
}
57+
```
58+
59+
## How to fix violations
60+
61+
To fix a violation of this rule, **always specify the `Times` argument explicitly** when calling `Verify()` method.
62+
63+
Examples:
64+
- `Times.Once()`
65+
- `Times.Never()`
66+
- `Times.AtLeast(2)`
67+
- `Times.Exactly(3)`
68+
69+
## When to suppress warnings
70+
71+
You may suppress warnings from this rule if you are fine with the implicit default of `Times.AtLeastOnce()`.
72+
However, for readability and maintainability, it is strongly recommended to always be explicit with the `Times` argument.
73+
74+
## Related rules
75+
76+
- [PosInfoMoq1002: `Verify()` methods should be called when `Verifiable()` has been setup](./PosInfoMoq1002.md)

docs/Design/PosInfoMoq1008.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# PosInfoMoq1008: The `Mock.Verify()` and `Mock.VerifyAll()` methods must specify at least one mock
2+
3+
| Property | Value |
4+
|---------------------|------------------------------------------------------------------------------------------|
5+
| **Rule ID** | PosInfoMoq1008 |
6+
| **Title** | The `Mock.Verify()` and `Mock.VerifyAll()` methods must specify at least one mock |
7+
| **Category** | Design |
8+
| **Default severity**| Warning |
9+
10+
## Cause
11+
12+
When calling the static methods `Mock.Verify()` or `Mock.VerifyAll()` without providing any `Mock<T>` instances, no verification is performed.
13+
14+
## Rule description
15+
16+
The static methods `Mock.Verify()` and `Mock.VerifyAll()` are designed to verify multiple `Mock<T>` instances at once.
17+
However, calling these methods without specifying any `Mock<T>` instances results in no verification being performed,
18+
which makes the test ineffective.
19+
20+
For example, the following code calls `Mock.Verify()` without any arguments:
21+
22+
```csharp
23+
[Fact]
24+
public void ProcessData_ShouldVerifyMocks()
25+
{
26+
var repositoryMock = new Mock<IRepository>();
27+
var serviceMock = new Mock<IService>();
28+
29+
var processor = new DataProcessor(repositoryMock.Object, serviceMock.Object);
30+
processor.ProcessData();
31+
32+
// This does nothing - no mocks are verified
33+
Mock.Verify();
34+
}
35+
```
36+
37+
This code is ineffective because it appears to verify something, but actually performs no verification at all.
38+
39+
Instead, you should specify the `Mock<T>` instances you want to verify:
40+
41+
```csharp
42+
[Fact]
43+
public void ProcessData_ShouldVerifyMocks()
44+
{
45+
var repositoryMock = new Mock<IRepository>();
46+
var serviceMock = new Mock<IService>();
47+
48+
var processor = new DataProcessor(repositoryMock.Object, serviceMock.Object);
49+
processor.ProcessData();
50+
51+
// Clear and explicit - verifies both mocks
52+
Mock.Verify(repositoryMock, serviceMock);
53+
}
54+
```
55+
56+
The same applies to `Mock.VerifyAll()`:
57+
58+
```csharp
59+
// Wrong: no verification performed
60+
Mock.VerifyAll();
61+
62+
// Correct: verifies all setups on the specified mocks
63+
Mock.VerifyAll(repositoryMock, serviceMock);
64+
```
65+
66+
## How to fix violations
67+
68+
To fix a violation of this rule, **always provide at least one mock instance** when calling `Mock.Verify()` or `Mock.VerifyAll()`.
69+
70+
Examples:
71+
- `Mock.Verify(mockA)`
72+
- `Mock.Verify(mockA, mockB)`
73+
- `Mock.VerifyAll(mockA, mockB, mockC)`
74+
75+
## When to suppress warnings
76+
77+
You should generally not suppress warnings from this rule, as calling these methods without arguments serves no purpose.
78+
If you need to verify individual mocks, use the instance methods `mock.Verify()` or `mock.VerifyAll()` instead.

docs/Design/PosInfoMoq1009.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# PosInfoMoq1009: Avoid using `Verifiable()` method
2+
3+
| Property | Value |
4+
|---------------------|-----------------------------------------------------------------------|
5+
| **Rule ID** | PosInfoMoq1009 |
6+
| **Title** | Avoid using `Verifiable()` method |
7+
| **Category** | Design |
8+
| **Default severity**| Warning |
9+
10+
## Cause
11+
12+
Using the `Verifiable()` method in the *Arrange* phase of a unit test is anti-pattern.
13+
`Verifiable()` sets up expectations during the setup (*Arrange*) step, but the actual verification should occur in the *Assert* phase.
14+
15+
## Rule description
16+
17+
The `Verifiable()` method should be avoided because it is declared during the *Arrange* phase, which is not the correct place to assert test outcomes.
18+
Instead, you should explicitly call `Verify()` or `VerifyAll()` at the end of the test in the *Assert* phase to ensure mocked members have been correctly invoked.
19+
20+
```csharp
21+
[Fact]
22+
public void SendMail_ShouldCallSmtpService()
23+
{
24+
// Arrange
25+
var smtpService = new Mock<ISmtpService>();
26+
smtpService.Setup(s => s.SendMail("sender@domain.com", "Gilles"));
27+
var service = new CustomerService(smtpService.Object);
28+
29+
// Act
30+
service.SendMail("Gilles");
31+
32+
// Assert
33+
smtpService.VerifyAll(); // Explicit verification at the end of the test
34+
}
35+
```
36+
37+
## How to fix violations
38+
39+
Do **not** call `Verifiable()`.
40+
Instead, configure mocks without it, and perform explicit verifications using `Verify()` or `VerifyAll()` in the *Assert* phase.
41+
42+
## When to suppress warnings
43+
44+
Do not suppress warnings from this rule.
45+
Proper unit testing practices require verification in the *Assert* phase, not in the *Arrange* phase.
46+
47+
## Related rules
48+
49+
- [PosInfoMoq1002: `Verify()` methods should be called when `Verifiable()` has been setup](./PosInfoMoq1002.md)

src/Moq.Analyzers/AnalyzerReleases.Shipped.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
## Release 1.12.0
1+
## Release 1.13.0
2+
3+
### New Rules
4+
Rule ID | Category | Severity | Notes
5+
--------|----------|----------|-------
6+
PosInfoMoq1007 | Design | Warning | VerifyMustHaveTimesParameterAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq1007.html)
7+
PosInfoMoq1008 | Design | Warning | VerifyStaticMethodsRequiresMockParametersAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq1008.html)
8+
PosInfoMoq1009 | Design | Warning | VerifyShouldBeCalledForVerifiableSetupAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Design/PosInfoMoq1009.html)
9+
10+
## Release 1.12.0
211

312
### New Rules
413
Rule ID | Category | Severity | Notes

src/Moq.Analyzers/Analyzers/ItArgumentsMustMatchMockedMethodArgumentsAnalyzer.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,24 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
6464
var itIsAnyType = moqSymbols.GetItIsAnyType(invocationArgument.Symbol);
6565
if (itIsAnyType is not null)
6666
{
67-
if (!SymbolEqualityComparer.Default.Equals(itIsAnyType, invocationArgument.ParameterSymbol.Type))
67+
if (!itIsAnyType.IsOrInheritFrom(invocationArgument.ParameterSymbol.Type))
6868
{
6969
context.ReportDiagnostic(Rule, invocationArgument.Syntax.GetLocation());
70-
continue;
7170
}
71+
72+
continue;
7273
}
7374

7475
// Check if the parameter is "It.Is<xxx>()"
7576
var itIsType = moqSymbols.GetItIsType(invocationArgument.Symbol);
7677
if (itIsType is not null)
7778
{
78-
if (!SymbolEqualityComparer.Default.Equals(itIsType, invocationArgument.ParameterSymbol.Type))
79+
if (!itIsType.IsOrInheritFrom(invocationArgument.ParameterSymbol.Type))
7980
{
8081
context.ReportDiagnostic(Rule, invocationArgument.Syntax.GetLocation());
81-
continue;
8282
}
83+
84+
continue;
8385
}
8486
}
8587
}

0 commit comments

Comments
 (0)