Skip to content

Commit

Permalink
v1.7.0 (#25)
Browse files Browse the repository at this point in the history
* Refactoring of the unit tests to use Moq library from NuGet packages (fixes #23).
* Changes the PosInfoMoq1000 rule to check only VerifyAll() method call (#fixes #22).
* Add the PosInfoMoq1002 rule (fixes #22).
* Improving the PosInfoMoq2005 rules unit tests to check when no explicit constructors.
* Improves the PosInfoMoq1002 rules to check the Verifiable() expression setup method depending of the expressions (#22).
* Add unit test to check that custom Setup() method is not impacted by the analyzer (fixes #12).
  • Loading branch information
GillesTourreau authored Jun 28, 2024
1 parent da2f494 commit fc643f3
Show file tree
Hide file tree
Showing 36 changed files with 1,043 additions and 605 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.6.0
default: 1.7.0
VersionSuffix:
type: string
description: The version suffix of the library (for example rc.1)
Expand Down
1 change: 1 addition & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<!-- Add the default using directive for all the code -->
<ItemGroup>
<Using Include="System" />
<Using Include="System.Threading.Tasks" />
</ItemGroup>

</Project>
2 changes: 2 additions & 0 deletions PosInformatique.Moq.Analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{DC2CDF77-88A9-490D-84ED-34832943104A}"
ProjectSection(SolutionItems) = preProject
tests\.editorconfig = tests\.editorconfig
tests\Directory.Build.props = tests\Directory.Build.props
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{3C20D95F-AB5F-44EC-8FB6-CB9827B7FD63}"
Expand All @@ -29,6 +30,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
ProjectSection(SolutionItems) = preProject
docs\design\PosInfoMoq1000.md = docs\design\PosInfoMoq1000.md
docs\design\PosInfoMoq1001.md = docs\design\PosInfoMoq1001.md
docs\Design\PosInfoMoq1002.md = docs\Design\PosInfoMoq1002.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ Design rules used to make your unit tests more strongly strict.

| Rule | Description |
| - | - |
| [PosInfoMoq1000: `Verify()` and `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, `Verify()` or `VerifyAll()` method should be called in the *Assert* phase to check the setup methods has been called. |
| [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. |

### Compilation

Expand Down
12 changes: 6 additions & 6 deletions docs/Design/PosInfoMoq1000.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# PosInfoMoq1000: `Verify()` and `VerifyAll()` methods should be called when instantiate a `Mock<T>` instances
# PosInfoMoq1000: ``VerifyAll()` method should be called when instantiate a `Mock<T>` instances

| Property | Value |
|-------------------------------------|--------------------------------------------------------------------------------------------|
| **Rule ID** | PosInfoMoq1000 |
| **Title** | `Verify()` and `VerifyAll()` methods should be called when instantiate a Mock<T> instances |
| **Rule ID** | PosInfoMoq1000 |
| **Title** | `VerifyAll()` methods should be called when instantiate a Mock<T> instances |
| **Category** | Design |
| **Default severity** | Warning |

## Cause

A `Verify()` or `VerifyAll()` of an `Mock<T>` instance has not been called in the *Assert* phase
A `VerifyAll()` of an `Mock<T>` instance has not been called in the *Assert* phase
of an unit test.

## Rule description

When instantiating a `Mock<T>` in the *Arrange* phase of an unit test, `Verify()` or `VerifyAll()` method
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.

```csharp
Expand All @@ -37,7 +37,7 @@ public void GetCustomer_ShouldCallRepository()

## How to fix violations

To fix a violation of this rule, call the `Verify()` or `VerifyAll()` in the *Assert* phase
To fix a violation of this rule, call the `VerifyAll()` in the *Assert* phase
on the `Mock<T>` instances created during the *Arrange* phase.

## When to suppress warnings
Expand Down
46 changes: 46 additions & 0 deletions docs/Design/PosInfoMoq1002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# PosInfoMoq1002: `Verify()` methods should be called when `Verifiable()` has been setup

| Property | Value |
|-------------------------------------|------------------------------------------------------------------------|
| **Rule ID** | PosInfoMoq1002 |
| **Title** | `Verify()` methods should be called when `Verifiable()` has been setup |
| **Category** | Design |
| **Default severity** | Warning |

## Cause

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

## Rule description

In the *Arrange* phase of an unit test, when `Verifiable()` method has been setup to mocked member, the
`Verify()` method should be called in the *Assert* phase to check the setup member has been called.

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

var service = new CustomerService(repository.Object);

// Act
service.SendMail("Gilles");

// Assert
smtpService.Verify(); // The Verify() will check that the mocked ISmtpService.SendMail() has been called (because marked with the ".Verifiable()" method).
}
```

## How to fix violations

To fix a violation of this rule, call the `Verify()` in the *Assert* phase
on the `Mock<T>` instances, if some mocked members has been marked as `Verifiable()` in the *Arrange* phase.

## When to suppress warnings

Do not suppress a warning from this rule. Normally `Verifiable()` setup members must be call in the unit tests execution.
12 changes: 10 additions & 2 deletions src/Moq.Analyzers/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
## Release 1.6.0
## Release 1.7.0

### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
PosInfoMoq1002 | Design | Warning | `Verify()` methods should be called when `Verifiable()` has been setup.

## Release 1.6.0

### New Rules

Expand Down Expand Up @@ -48,5 +56,5 @@ PosInfoMoq2000 | Compilation | Error | The `Returns()` or `ReturnsAsync()` met

Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
PosInfoMoq1000 | Design | Warning | `Verify()` and `VerifyAll()` methods should be called when instantiate a Mock<T> instances.
PosInfoMoq1000 | Design | Warning | `VerifyAll()` method should be called when instantiate a Mock<T> instances.
PosInfoMoq1001 | Design | Warning | The `Mock<T>` instance behavior should be defined to Strict mode.
Original file line number Diff line number Diff line change
Expand Up @@ -64,58 +64,46 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
return;
}

// Check each CallBack() method for the following calls.
var followingMethods = invocationExpression.DescendantNodes().OfType<InvocationExpressionSyntax>();
// Extracts the setup method from the Callback() method call.
var setupMethod = moqExpressionAnalyzer.ExtractSetupMethod(invocationExpression, context.CancellationToken);

foreach (var followingMethod in followingMethods)
if (setupMethod is null)
{
// Find the symbol of the mocked method (if not symbol found, it is mean we Setup() method that not currently compile)
// so we skip the analysis.
if (!moqExpressionAnalyzer.IsMockSetupMethod(followingMethod, out var _, context.CancellationToken))
{
continue;
}

var mockedMethod = moqExpressionAnalyzer.ExtractSetupMethod(followingMethod, out var _, context.CancellationToken);

if (mockedMethod is null)
{
continue;
}
return;
}

// Compare the parameters between the mocked method and lambda expression in the CallBack() method.
// 1- Compare the number of the parameters
if (callBackLambdaExpressionSymbol.Parameters.Length != mockedMethod.Parameters.Length)
{
var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.GetLocation());
context.ReportDiagnostic(diagnostic);
// Compare the parameters between the mocked method and lambda expression in the CallBack() method.
// 1- Compare the number of the parameters
if (callBackLambdaExpressionSymbol.Parameters.Length != setupMethod.InvocationArguments.Count)
{
var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.GetLocation());
context.ReportDiagnostic(diagnostic);

continue;
}
return;
}

// 2- Iterate for each parameter
for (var i = 0; i < callBackLambdaExpressionSymbol.Parameters.Length; i++)
// 2- Iterate for each parameter
for (var i = 0; i < callBackLambdaExpressionSymbol.Parameters.Length; i++)
{
// Special case, if the argument is IsAnyType
if (moqSymbols.IsAnyType(setupMethod.InvocationArguments[i].ParameterSymbol.Type))
{
// Special case, if the argument is IsAnyType
if (moqSymbols.IsAnyType(mockedMethod.Parameters[i].Type))
{
// The callback parameter associated must be an object.
if (callBackLambdaExpressionSymbol.Parameters[i].Type.SpecialType != SpecialType.System_Object)
{
var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
context.ReportDiagnostic(diagnostic);

continue;
}
}
else if (!SymbolEqualityComparer.Default.Equals(callBackLambdaExpressionSymbol.Parameters[i].Type, mockedMethod.Parameters[i].Type))
// The callback parameter associated must be an object.
if (callBackLambdaExpressionSymbol.Parameters[i].Type.SpecialType != SpecialType.System_Object)
{
var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
context.ReportDiagnostic(diagnostic);

continue;
}
}
else if (!SymbolEqualityComparer.Default.Equals(callBackLambdaExpressionSymbol.Parameters[i].Type, setupMethod.InvocationArguments[i].ParameterSymbol.Type))
{
var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
context.ReportDiagnostic(diagnostic);

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

// Extracts the method in the lambda expression of the Setup() method
var members = moqExpressionAnalyzer.ExtractChainedMembersInvocationFromLambdaExpression(invocationExpression, context.CancellationToken);
var setupMethod = moqExpressionAnalyzer.ExtractChainedMembersInvocationFromLambdaExpression(invocationExpression, context.CancellationToken);

if (setupMethod is null)
{
return;
}

// Check if the member is overridable.
foreach (var member in members)
foreach (var member in setupMethod.Members)
{
if (!moqSymbols.IsOverridable(member.Symbol))
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//-----------------------------------------------------------------------
// <copyright file="VerifyShouldBeCalledAnalyzer.cs" company="P.O.S Informatique">
// <copyright file="VerifyAllShouldBeCalledAnalyzer.cs" company="P.O.S Informatique">
// Copyright (c) P.O.S Informatique. All rights reserved.
// </copyright>
//-----------------------------------------------------------------------
Expand All @@ -13,16 +13,16 @@ namespace PosInformatique.Moq.Analyzers
using Microsoft.CodeAnalysis.Diagnostics;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class VerifyShouldBeCalledAnalyzer : DiagnosticAnalyzer
public class VerifyAllShouldBeCalledAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
"PosInfoMoq1000",
"Verify() and VerifyAll() methods should be called when instantiate a Mock<T> instances",
"The Verify() or VerifyAll() method should be called at the end of the unit test",
"VerifyAll() method should be called when instantiate a Mock<T> instances",
"The VerifyAll() method should be called at the end of the unit test",
"Design",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: "VerifyAll() or VerifyAll() methods should be called at the end of the unit test method.",
description: "VerifyAll() method should be called at the end of the unit test method.",
helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Design/PosInfoMoq1000.html");

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
Expand Down Expand Up @@ -105,14 +105,14 @@ private static bool IsMockVerifyAllInvocation(InvocationExpressionSyntax invocat
return false;
}

if (!moqSymbols.IsVerifyMethod(verifyMethod.Symbol) && !moqSymbols.IsVerifyAllMethod(verifyMethod.Symbol))
if (!moqSymbols.IsVerifyAllMethod(verifyMethod.Symbol))
{
if (!moqSymbols.IsVerifyStaticMethod(verifyMethod.Symbol) && !moqSymbols.IsVerifyAllStaticMethod(verifyMethod.Symbol))
if (!moqSymbols.IsVerifyAllStaticMethod(verifyMethod.Symbol))
{
return false;
}

// Special case, the static method Verify() or VerifyAll() has been called.
// Special case, the static method VerifyAll() has been called.
// In this case, iterate on each arguments of the method called and check if the variableNameSymbol has been passed.
foreach (var argument in invocation.ArgumentList.Arguments)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,15 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
}

// Extracts the method in the lambda expression of the Setup() method
var members = moqExpressionAnalyzer.ExtractChainedMembersInvocationFromLambdaExpression(invocationExpression, context.CancellationToken);
var verifyMethod = moqExpressionAnalyzer.ExtractChainedMembersInvocationFromLambdaExpression(invocationExpression, context.CancellationToken);

if (verifyMethod is null)
{
return;
}

// Check if the member is overridable.
foreach (var member in members)
foreach (var member in verifyMethod.Members)
{
if (!moqSymbols.IsOverridable(member.Symbol))
{
Expand Down
Loading

0 comments on commit fc643f3

Please sign in to comment.