Refactor code to remove analyzer warnings

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

@@ -38,14 +38,14 @@ public override void Initialize(AnalysisContext context)
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
-        InvocationExpressionSyntax? callbackOrReturnsInvocation = (InvocationExpressionSyntax)context.Node;
+        InvocationExpressionSyntax callbackOrReturnsInvocation = (InvocationExpressionSyntax)context.Node;
 
         SeparatedSyntaxList<ArgumentSyntax> callbackOrReturnsMethodArguments = callbackOrReturnsInvocation.ArgumentList.Arguments;
 
         // Ignoring Callback() and Return() calls without lambda arguments
         if (callbackOrReturnsMethodArguments.Count == 0) return;
 
-        if (!Helpers.IsCallbackOrReturnInvocation(context.SemanticModel, callbackOrReturnsInvocation)) return;
+        if (!context.SemanticModel.IsCallbackOrReturnInvocation(callbackOrReturnsInvocation)) return;
 
         ParenthesizedLambdaExpressionSyntax? callbackLambda = callbackOrReturnsInvocation.ArgumentList.Arguments[0]?.Expression as ParenthesizedLambdaExpressionSyntax;
 
@@ -56,28 +56,25 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         SeparatedSyntaxList<ParameterSyntax> lambdaParameters = callbackLambda.ParameterList.Parameters;
         if (lambdaParameters.Count == 0) return;
 
-        InvocationExpressionSyntax? setupInvocation = Helpers.FindSetupMethodFromCallbackInvocation(context.SemanticModel, callbackOrReturnsInvocation, context.CancellationToken);
-        InvocationExpressionSyntax? mockedMethodInvocation = Helpers.FindMockedMethodInvocationFromSetupMethod(setupInvocation);
+        InvocationExpressionSyntax? setupInvocation = context.SemanticModel.FindSetupMethodFromCallbackInvocation(callbackOrReturnsInvocation, context.CancellationToken);
+        InvocationExpressionSyntax? mockedMethodInvocation = setupInvocation.FindMockedMethodInvocationFromSetupMethod();
         if (mockedMethodInvocation == null) return;
 
         SeparatedSyntaxList<ArgumentSyntax> mockedMethodArguments = mockedMethodInvocation.ArgumentList.Arguments;
 
         if (mockedMethodArguments.Count != lambdaParameters.Count)
         {
-            Diagnostic? diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
+            Diagnostic diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
             context.ReportDiagnostic(diagnostic);
         }
         else
         {
             for (int argumentIndex = 0; argumentIndex < mockedMethodArguments.Count; argumentIndex++)
             {
                 TypeSyntax? lambdaParameterTypeSyntax = lambdaParameters[argumentIndex].Type;
-                Debug.Assert(lambdaParameterTypeSyntax != null, nameof(lambdaParameterTypeSyntax) + " != null");
 
                 // TODO: Don't know if continue or break is the right thing to do here
-#pragma warning disable S2589 // Boolean expressions should not be gratuitous
                 if (lambdaParameterTypeSyntax is null) continue;
-#pragma warning restore S2589 // Boolean expressions should not be gratuitous
 
                 TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameterTypeSyntax, context.CancellationToken);
 
@@ -88,7 +85,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
 
                 if (!string.Equals(mockedMethodTypeName, lambdaParameterTypeName, StringComparison.Ordinal))
                 {
-                    Diagnostic? diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
+                    Diagnostic diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
                     context.ReportDiagnostic(diagnostic);
                 }
             }

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodCodeFix.cs

@@ -35,7 +35,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             return;
         }
 
-        Diagnostic? diagnostic = context.Diagnostics.First();
+        Diagnostic diagnostic = context.Diagnostics.First();
         TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
 
         // Find the type declaration identified by the diagnostic.
@@ -69,23 +69,23 @@ private async Task<Document> FixCallbackSignatureAsync(SyntaxNode root, Document
             return document;
         }
 
-        InvocationExpressionSyntax? setupMethodInvocation = Helpers.FindSetupMethodFromCallbackInvocation(semanticModel, callbackInvocation, cancellationToken);
+        InvocationExpressionSyntax? setupMethodInvocation = semanticModel.FindSetupMethodFromCallbackInvocation(callbackInvocation, cancellationToken);
         Debug.Assert(setupMethodInvocation != null, nameof(setupMethodInvocation) + " != null");
-        IMethodSymbol[] matchingMockedMethods = Helpers.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(semanticModel, setupMethodInvocation).ToArray();
+        IMethodSymbol[] matchingMockedMethods = semanticModel.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(setupMethodInvocation).ToArray();
 
         if (matchingMockedMethods.Length != 1)
         {
             return document;
         }
 
-        ParameterListSyntax? newParameters = SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(matchingMockedMethods[0].Parameters.Select(
+        ParameterListSyntax newParameters = SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(matchingMockedMethods[0].Parameters.Select(
             parameterSymbol =>
             {
-                TypeSyntax? type = SyntaxFactory.ParseTypeName(parameterSymbol.Type.ToMinimalDisplayString(semanticModel, oldParameters.SpanStart));
+                TypeSyntax type = SyntaxFactory.ParseTypeName(parameterSymbol.Type.ToMinimalDisplayString(semanticModel, oldParameters.SpanStart));
                 return SyntaxFactory.Parameter(default, SyntaxFactory.TokenList(), type, SyntaxFactory.Identifier(parameterSymbol.Name), null);
             })));
 
-        SyntaxNode? newRoot = root.ReplaceNode(oldParameters, newParameters);
+        SyntaxNode newRoot = root.ReplaceNode(oldParameters, newParameters);
         return document.WithSyntaxRoot(newRoot);
     }
 }

src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

@@ -39,74 +39,101 @@ public override void Initialize(AnalysisContext context)
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "MA0051:Method is too long", Justification = "Tracked in #90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
-        ObjectCreationExpressionSyntax? objectCreation = (ObjectCreationExpressionSyntax)context.Node;
+        ObjectCreationExpressionSyntax objectCreation = (ObjectCreationExpressionSyntax)context.Node;
 
         GenericNameSyntax? genericName = GetGenericNameSyntax(objectCreation.Type);
         if (genericName == null) return;
 
-        if (!IsMockGenericType(genericName)) return;
+        if (!IsMockGenericType(genericName))
+        {
+            return;
+        }
 
         // Full check that we are calling new Mock<T>()
         IMethodSymbol? constructorSymbol = GetConstructorSymbol(context, objectCreation);
 
-        Debug.Assert(constructorSymbol != null, nameof(constructorSymbol) + " != null");
-
-#pragma warning disable S2589 // Boolean expressions should not be gratuitous
-        if (constructorSymbol is null) return;
-#pragma warning restore S2589 // Boolean expressions should not be gratuitous
+        // If constructorSymbol is null, we should have caught that earlier (and we cannot proceed)
+        if (constructorSymbol == null)
+        {
+            return;
+        }
 
-            // Vararg parameter is the one that takes all arguments for mocked class constructor
+        // Vararg parameter is the one that takes all arguments for mocked class constructor
         IParameterSymbol? varArgsConstructorParameter = constructorSymbol.Parameters.FirstOrDefault(parameterSymbol => parameterSymbol.IsParams);
 
         // Vararg parameter are not used, so there are no arguments for mocked class constructor
-        if (varArgsConstructorParameter == null) return;
+        if (varArgsConstructorParameter == null)
+        {
+            return;
+        }
 
         int varArgsConstructorParameterIndex = constructorSymbol.Parameters.IndexOf(varArgsConstructorParameter);
 
         // Find mocked type
         INamedTypeSymbol? mockedTypeSymbol = GetMockedSymbol(context, genericName);
-        if (mockedTypeSymbol == null) return;
+        if (mockedTypeSymbol == null)
+        {
+            return;
+        }
 
         // Skip first argument if it is not vararg - typically it is MockingBehavior argument
-        ArgumentSyntax[]? constructorArguments = objectCreation.ArgumentList?.Arguments.Skip(varArgsConstructorParameterIndex == 0 ? 0 : 1).ToArray();
+        IEnumerable<ArgumentSyntax>? constructorArguments = objectCreation.ArgumentList?.Arguments.Skip(varArgsConstructorParameterIndex == 0 ? 0 : 1);
 
         if (!mockedTypeSymbol.IsAbstract)
         {
-            if (constructorArguments != null
-                && IsConstructorMismatch(context, objectCreation, genericName, constructorArguments)
-                && objectCreation.ArgumentList != null)
-            {
-                Diagnostic? diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
-                context.ReportDiagnostic(diagnostic);
-            }
+            AnalyzeConcrete(context, constructorArguments, objectCreation, genericName);
         }
         else
         {
             // Issue #1: Currently detection does not work well for abstract classes because they cannot be instantiated
 
             // The mocked symbol is abstract, so we need to check if the constructor arguments match the abstract class constructor
+            AnalyzeAbstract(context, constructorArguments, mockedTypeSymbol, objectCreation);
+        }
+    }
 
-            // Extract types of arguments passed in the constructor call
-            if (constructorArguments != null)
-            {
-                ITypeSymbol[] argumentTypes = constructorArguments
-                    .Select(arg => context.SemanticModel.GetTypeInfo(arg.Expression, context.CancellationToken).Type)
-                    .ToArray()!;
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
+    private static void AnalyzeAbstract(
+        SyntaxNodeAnalysisContext context,
+        IEnumerable<ArgumentSyntax>? constructorArguments,
+        INamedTypeSymbol mockedTypeSymbol,
+        ObjectCreationExpressionSyntax objectCreation)
+    {
+        // Extract types of arguments passed in the constructor call
+        if (constructorArguments != null)
+        {
+            ITypeSymbol[] argumentTypes = constructorArguments
+                .Select(arg => context.SemanticModel.GetTypeInfo(arg.Expression, context.CancellationToken).Type)
+                .ToArray()!;
 
-                // Check all constructors of the abstract type
-                for (int constructorIndex = 0; constructorIndex < mockedTypeSymbol.Constructors.Length; constructorIndex++)
+            // Check all constructors of the abstract type
+            for (int constructorIndex = 0; constructorIndex < mockedTypeSymbol.Constructors.Length; constructorIndex++)
+            {
+                IMethodSymbol constructor = mockedTypeSymbol.Constructors[constructorIndex];
+                if (AreParametersMatching(constructor.Parameters, argumentTypes))
                 {
-                    IMethodSymbol constructor = mockedTypeSymbol.Constructors[constructorIndex];
-                    if (AreParametersMatching(constructor.Parameters, argumentTypes))
-                    {
-                        return; // Found a matching constructor
-                    }
+                    return;
                 }
             }
+        }
 
-            Debug.Assert(objectCreation.ArgumentList != null, "objectCreation.ArgumentList != null");
+        Debug.Assert(objectCreation.ArgumentList != null, "objectCreation.ArgumentList != null");
 
-            Diagnostic? diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList?.GetLocation());
+        Diagnostic diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList?.GetLocation());
+        context.ReportDiagnostic(diagnostic);
+    }
+
+    private static void AnalyzeConcrete(
+        SyntaxNodeAnalysisContext context,
+        IEnumerable<ArgumentSyntax>? constructorArguments,
+        ObjectCreationExpressionSyntax objectCreation,
+        GenericNameSyntax genericName)
+    {
+        if (constructorArguments != null
+            && IsConstructorMismatch(context, objectCreation, genericName, constructorArguments)
+            && objectCreation.ArgumentList != null)
+        {
+            Diagnostic diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
             context.ReportDiagnostic(diagnostic);
         }
     }
@@ -122,18 +149,20 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         return mockedTypeSymbol;
     }
 
-    private static bool AreParametersMatching(ImmutableArray<IParameterSymbol> constructorParameters, ITypeSymbol[] argumentTypes2)
+    private static bool AreParametersMatching(
+        ImmutableArray<IParameterSymbol> constructorParameters,
+        ITypeSymbol[] argumentTypes)
     {
         // Check if the number of parameters matches
-        if (constructorParameters.Length != argumentTypes2.Length)
+        if (constructorParameters.Length != argumentTypes.Length)
         {
             return false;
         }
 
         // Check if each parameter type matches in order
         for (int constructorParameterIndex = 0; constructorParameterIndex < constructorParameters.Length; constructorParameterIndex++)
         {
-            if (!constructorParameters[constructorParameterIndex].Type.Equals(argumentTypes2[constructorParameterIndex], SymbolEqualityComparer.IncludeNullability))
+            if (!constructorParameters[constructorParameterIndex].Type.Equals(argumentTypes[constructorParameterIndex], SymbolEqualityComparer.IncludeNullability))
             {
                 return false;
             }
@@ -144,6 +173,8 @@ private static bool AreParametersMatching(ImmutableArray<IParameterSymbol> const
 
     private static GenericNameSyntax? GetGenericNameSyntax(TypeSyntax typeSyntax)
     {
+        // REVIEW: Switch and ifs are equal in this case, but switch causes AV1535 to trigger
+        // The switch expression adds more instructions to do the same, so stick with ifs
         if (typeSyntax is GenericNameSyntax genericNameSyntax)
         {
             return genericNameSyntax;
@@ -177,12 +208,16 @@ private static bool IsMockGenericType(GenericNameSyntax genericName)
             : null;
     }
 
-    private static bool IsConstructorMismatch(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreation, GenericNameSyntax genericName, ArgumentSyntax[] constructorArguments)
+    private static bool IsConstructorMismatch(
+        SyntaxNodeAnalysisContext context,
+        ObjectCreationExpressionSyntax objectCreation,
+        GenericNameSyntax genericName,
+        IEnumerable<ArgumentSyntax> constructorArguments)
     {
-        ObjectCreationExpressionSyntax? fakeConstructorCall = SyntaxFactory.ObjectCreationExpression(
+        ObjectCreationExpressionSyntax fakeConstructorCall = SyntaxFactory.ObjectCreationExpression(
             genericName.TypeArgumentList.Arguments.First(),
             SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(constructorArguments)),
-            null);
+            initializer: null);
 
         SymbolInfo mockedClassConstructorSymbolInfo = context.SemanticModel.GetSpeculativeSymbolInfo(
             objectCreation.SpanStart, fakeConstructorCall, SpeculativeBindingOption.BindAsExpression);

src/Moq.Analyzers/InvocationExpressionSyntaxExtensions.cs

@@ -0,0 +1,19 @@
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Extension methods for <see cref="InvocationExpressionSyntax"/>s.
+/// </summary>
+internal static class InvocationExpressionSyntaxExtensions
+{
+    internal static InvocationExpressionSyntax? FindMockedMethodInvocationFromSetupMethod(this InvocationExpressionSyntax? setupInvocation)
+    {
+        LambdaExpressionSyntax? setupLambdaArgument = setupInvocation?.ArgumentList.Arguments[0].Expression as LambdaExpressionSyntax;
+        return setupLambdaArgument?.Body as InvocationExpressionSyntax;
+    }
+
+    internal static ExpressionSyntax? FindMockedMemberExpressionFromSetupMethod(this InvocationExpressionSyntax? setupInvocation)
+    {
+        LambdaExpressionSyntax? setupLambdaArgument = setupInvocation?.ArgumentList.Arguments[0].Expression as LambdaExpressionSyntax;
+        return setupLambdaArgument?.Body as ExpressionSyntax;
+    }
+}

src/Moq.Analyzers/SyntaxExtensions.cs → src/Moq.Analyzers/NameSyntaxExtensions.cs

@@ -3,9 +3,9 @@
 namespace Moq.Analyzers;
 
 /// <summary>
-/// Extensions methods for <see cref="SyntaxNode"/>s.
+/// Extensions methods for <see cref="NameSyntax"/>s.
 /// </summary>
-internal static class SyntaxExtensions
+internal static class NameSyntaxExtensions
 {
     /// <summary>
     /// Tries to get the generic arguments of a given <see cref="NameSyntax"/>.

src/Moq.Analyzers/NoConstructorArgumentsForInterfaceMockAnalyzer.cs

@@ -38,7 +38,7 @@ public override void Initialize(AnalysisContext context)
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
-        ObjectCreationExpressionSyntax? objectCreation = (ObjectCreationExpressionSyntax)context.Node;
+        ObjectCreationExpressionSyntax objectCreation = (ObjectCreationExpressionSyntax)context.Node;
 
         // TODO Think how to make this piece more elegant while fast
         GenericNameSyntax? genericName = objectCreation.Type as GenericNameSyntax;
@@ -84,7 +84,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         {
             Debug.Assert(objectCreation.ArgumentList != null, "objectCreation.ArgumentList != null");
 
-            Diagnostic? diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList?.GetLocation());
+            Diagnostic diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList?.GetLocation());
             context.ReportDiagnostic(diagnostic);
         }
     }

src/Moq.Analyzers/NoMethodsInPropertySetupAnalyzer.cs

@@ -36,19 +36,19 @@ public override void Initialize(AnalysisContext context)
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
-        InvocationExpressionSyntax? setupGetOrSetInvocation = (InvocationExpressionSyntax)context.Node;
+        InvocationExpressionSyntax setupGetOrSetInvocation = (InvocationExpressionSyntax)context.Node;
 
         if (setupGetOrSetInvocation.Expression is not MemberAccessExpressionSyntax setupGetOrSetMethod) return;
         if (!string.Equals(setupGetOrSetMethod.Name.ToFullString(), "SetupGet", StringComparison.Ordinal)
             && !string.Equals(setupGetOrSetMethod.Name.ToFullString(), "SetupSet", StringComparison.Ordinal)) return;
 
-        InvocationExpressionSyntax? mockedMethodCall = Helpers.FindMockedMethodInvocationFromSetupMethod(setupGetOrSetInvocation);
+        InvocationExpressionSyntax? mockedMethodCall = setupGetOrSetInvocation.FindMockedMethodInvocationFromSetupMethod();
         if (mockedMethodCall == null) return;
 
         ISymbol? mockedMethodSymbol = context.SemanticModel.GetSymbolInfo(mockedMethodCall, context.CancellationToken).Symbol;
         if (mockedMethodSymbol == null) return;
 
-        Diagnostic? diagnostic = Diagnostic.Create(Rule, mockedMethodCall.GetLocation());
+        Diagnostic diagnostic = Diagnostic.Create(Rule, mockedMethodCall.GetLocation());
         context.ReportDiagnostic(diagnostic);
     }
 }