Remove false positive for Moq1200 when using parameterized lambda (#301)

When using Moq's expression features for type construction, the Moq1200
analyzer was firing unexpectedly indicating a matching constructor on
the type was not found. The `ConstructorArgumentsShouldMatchAnalyzer`
was extracting the parameters from the Mock type and comparing them to
the parameters of the constructed type as is, including the lambda and
the `MockBehavior`.

Example:

```csharp
_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Loose);
```

> See #234 for more details

This is incorrect for several reasons:

1. The parenthesized lambda is not itself a parameter for the target
type, but rather the body
2. The `MockBehavior` would not likely be a parameter on the target type

Correct analysis would be to drop the `MockBehavior` argument as with
other configurations of the `Mock<T>` type, and use the body of the
lambda. However, using the body of the lambda is not necessary. The
purpose of this analyzer is to detect errors that would not be caught at
compile time that would result in a runtime error. In this case, using a
constructor not available on the type would result in a compiler error.
As such, the constructor detection method has been updated to return a
tertiary result: `true` when there is a matching constructor found,
`false` when there is not, and `null` when the constructor is a lambda
(i.e., the constructor should be ignored).

Changes:

- Add unit test for issue #234
- Add support to ignore parameterized lambda arguments when performing
constructor detection
- Add support `MockBehavior` definitions to be in ordinal position 0 or
1

Fixes #234

Author: rjmurillo

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

@@ -99,9 +99,9 @@ private static bool IsExpressionMockBehavior(SyntaxNodeAnalysisContext context,
         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);
     }
@@ -267,10 +267,13 @@ private static void AnalyzeNewObject(SyntaxNodeAnalysisContext context, MoqKnown
     /// <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(
         IMethodSymbol[] constructors,
         ArgumentSyntax[] arguments,
         SyntaxNodeAnalysisContext context)
@@ -348,6 +351,24 @@ private static bool AnyConstructorsFound(
             }
         }
 
+        // 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;
     }
 
@@ -386,10 +407,18 @@ private static void VerifyMockAttempt(
         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))
+            {
+                // They passed a mock behavior as the second argument; ignore as Moq swallows it
+                arguments = arguments.RemoveAt(1);
+            }
         }
 
         switch (mockedClass.TypeKind)
@@ -433,7 +462,9 @@ private static void VerifyClassMockAttempt(
         }
 
         // 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);

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

@@ -0,0 +1,39 @@
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ConstructorArgumentsShouldMatchAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public partial class ConstructorArgumentsShouldMatchAnalyzerTests
+{
+    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);
+    }
+}