Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove false positive for Moq1200 when using parameterized lambda #301

Merged
merged 7 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@
return targetSymbol.IsInstanceOf(knownSymbols.MockBehavior);
}

private static bool IsFirstArgumentMockBehavior(SyntaxNodeAnalysisContext context, MoqKnownSymbols knownSymbols, ArgumentListSyntax? argumentList)
private static bool IsArgumentMockBehavior(SyntaxNodeAnalysisContext context, MoqKnownSymbols knownSymbols, ArgumentListSyntax? argumentList, uint argumentOrdinal)
{
ExpressionSyntax? expression = argumentList?.Arguments[0].Expression;
ExpressionSyntax? expression = argumentList?.Arguments.Count > argumentOrdinal ? argumentList.Arguments[(int)argumentOrdinal].Expression : null;

return IsExpressionMockBehavior(context, knownSymbols, expression);
}
Expand Down Expand Up @@ -267,10 +267,13 @@
/// <param name="constructors">The constructors.</param>
/// <param name="arguments">The arguments.</param>
/// <param name="context">The context.</param>
/// <returns><c>true</c> if a suitable constructor was found; otherwise <c>false</c>. </returns>
/// <returns>
/// <see langword="true" /> if a suitable constructor was found; otherwise <see langword="false" />.
/// If the construction method is a parenthesized lambda expression, <see langword="null" /> is returned.
/// </returns>
/// <remarks>Handles <see langword="params" /> and optional parameters.</remarks>
[SuppressMessage("Design", "MA0051:Method is too long", Justification = "This should be refactored; suppressing for now to enable TreatWarningsAsErrors in CI.")]
private static bool AnyConstructorsFound(
private static bool? AnyConstructorsFound(

Check failure on line 276 in src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs#L276

Refactor this method to reduce its Cognitive Complexity from 36 to the 15 allowed.

Check failure on line 276 in src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs#L276

The Cyclomatic Complexity of this method is 24 which is greater than 10 authorized.
IMethodSymbol[] constructors,
ArgumentSyntax[] arguments,
SyntaxNodeAnalysisContext context)
Expand Down Expand Up @@ -348,6 +351,24 @@
}
}

// Special case for Lambda expression syntax
// In Moq you can specify a Lambda expression that creates an instance
// of the specified type
// See https://github.com/devlooped/moq/blob/18dc7410ad4f993ce0edd809c5dfcaa3199f13ff/src/Moq/Mock%601.cs#L200
//
// The parenthesized lambda takes arguments as the first child node
// which may be empty or have args defined as part of a closure.
// Either way, we don't care about that, we only care that the
// constructor is valid.
//
// Since this does not use reflection through Castle, an invalid
// lambda here would cause the compiler to break, so no need to
// do additional checks.
if (arguments.Length == 1 && arguments[0].Expression.IsKind(SyntaxKind.ParenthesizedLambdaExpression))
{
return null;
}

return false;
}

Expand Down Expand Up @@ -386,10 +407,18 @@
ArgumentSyntax[] arguments = argumentList?.Arguments.ToArray() ?? [];
#pragma warning restore ECS0900 // Consider using an alternative implementation to avoid boxing and unboxing

if (hasMockBehavior && arguments.Length > 0 && IsFirstArgumentMockBehavior(context, knownSymbols, argumentList))
if (hasMockBehavior && arguments.Length > 0)
{
// They passed a mock behavior as the first argument; ignore as Moq swallows it
arguments = arguments.RemoveAt(0);
if (arguments.Length >= 1 && IsArgumentMockBehavior(context, knownSymbols, argumentList, 0))
{
// They passed a mock behavior as the first argument; ignore as Moq swallows it
arguments = arguments.RemoveAt(0);
}
else if (arguments.Length >= 2 && IsArgumentMockBehavior(context, knownSymbols, argumentList, 1))

Check failure on line 417 in src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs#L417

Add the missing 'else' clause with either the appropriate action or a suitable comment as to why no action is taken.
{
// They passed a mock behavior as the second argument; ignore as Moq swallows it
arguments = arguments.RemoveAt(1);
}
}

switch (mockedClass.TypeKind)
Expand Down Expand Up @@ -433,7 +462,9 @@
}

// We have constructors, now we need to check if the arguments match any of them
if (!AnyConstructorsFound(constructors, arguments, context))
// If the value is null it means we want to ignore and not create a diagnostic
bool? matchingCtorFound = AnyConstructorsFound(constructors, arguments, context);
if (matchingCtorFound.HasValue && !matchingCtorFound.Value)
{
Diagnostic diagnostic = constructorIsEmpty.Location.CreateDiagnostic(ClassMustHaveMatchingConstructor, argumentList);
context.ReportDiagnostic(diagnostic);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ConstructorArgumentsShouldMatchAnalyzer>;

Check failure on line 1 in tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Expressions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Expressions.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

namespace Moq.Analyzers.Test;

public partial class ConstructorArgumentsShouldMatchAnalyzerTests

Check notice on line 5 in tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Expressions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.Expressions.cs#L5

'partial' is gratuitous in this context.
{
public static IEnumerable<object[]> ExpressionTestData()
{
return new object[][]
{
["""_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Loose);"""],
["""_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Strict);"""],
["""_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Default);"""],
["""_ = new Mock<Calculator>(() => new Calculator());"""],
}.WithNamespaces().WithMoqReferenceAssemblyGroups();
}

[Theory]
[MemberData(nameof(ExpressionTestData))]
public async Task ShouldPassIfExpressionWithDefaultCtorIsUsedWithMockBehavior(string referenceAssemblyGroup, string @namespace, string mock)
{
await Verifier.VerifyAnalyzerAsync(
$@"
{@namespace}
public class Calculator
{{
public int Add(int a, int b) => a + b;
}}
internal class UnitTest
{{
private void Test()
{{
{mock}
}}
}}
",
referenceAssemblyGroup);
}
rjmurillo marked this conversation as resolved.
Show resolved Hide resolved
}
Loading