Enable warnings as errors

.editorconfig

@@ -82,13 +82,17 @@ dotnet_style_readonly_field = true:warning
 
 # Parameter preferences
 dotnet_code_quality_unused_parameters = all:suggestion
+# AV1561: Signature contains too many parameters
+dotnet_diagnostic.AV1561.severity = suggestion
 
 # Suppression preferences
 dotnet_remove_unnecessary_suppression_exclusions = none
 
 # XMLDocs preferences
 # SA1600: Elements should be documented. We disable this it requires xmldocs for _all_ members. CS1591 already covers documenting public members.
 dotnet_diagnostic.SA1600.severity = silent
+# AV2305: Missing XML comment for internally visible type, member or parameter
+dotnet_diagnostic.AV2305.severity = silent
 
 #### C# Coding Conventions ####
 [*.cs]
@@ -381,7 +385,27 @@ dotnet_naming_style.s_camelcase.required_suffix =
 dotnet_naming_style.s_camelcase.word_separator =
 dotnet_naming_style.s_camelcase.capitalization = camel_case
 
+# AV1580: Method argument calls a nested method
+# Because debugger breakpoints cannot be set inside expressions, avoid overuse of nested method calls.
+# Example: string result = ConvertToXml(ApplyTransforms(ExecuteQuery(GetConfigurationSettings(source))));
+# requires extra steps to inspect intermediate method return values. On the other hard, were this expression broken into intermediate variables, setting a breakpoint on one of them would be sufficient.
+#
+# This is moved to silent because it's flagging foo.AsSpan()
+dotnet_diagnostic.AV1580.severity = silent
+
 # MA0040: Forward the CancellationToken parameter to methods that take one
 dotnet_diagnostic.MA0040.severity = error
 # Async analyzer
-dotnet_diagnostic.CA2016.severity = error
+dotnet_diagnostic.CA2016.severity = error
+
+#### Handling TODOs ####
+# This is a popular rule in analyzers. Everyone has an opinion and
+# some of the severity levels conflict. We don't need all of these
+# to fire, only one. Pick one and mark it as informational so we
+# don't lose track.
+# S1135: Track uses of "TODO" tags
+dotnet_diagnostic.S1135.severity = suggestion
+# AV2318: Work-tracking TODO comment should be removed
+dotnet_diagnostic.AV2318.severity = none
+# MA0026: Fix TODO comment
+dotnet_diagnostic.MA0026.severity = none

Source/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodCodeFixTests.cs

@@ -4,6 +4,7 @@ namespace Moq.Analyzers.Test;
 
 public class CallbackSignatureShouldMatchMockedMethodCodeFixTests
 {
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "MA0051:Method is too long", Justification = "Contains test data")]
     public static IEnumerable<object[]> TestData()
     {
         return new object[][]

Source/Moq.Analyzers.Test/ConstructorArgumentsShouldMatchAnalyzerTests.cs

@@ -35,6 +35,7 @@ public static IEnumerable<object[]> TestData()
             ["""new Mock<AbstractGenericClassDefaultCtor<object>>{|Moq1002:(42)|};"""],
             ["""new Mock<AbstractGenericClassDefaultCtor<object>>();"""],
             ["""new Mock<AbstractGenericClassDefaultCtor<object>>(MockBehavior.Default);"""],
+
             // TODO: "I think this _should_ fail, but currently passes. Tracked by #55."
             // ["""new Mock<AbstractClassWithCtor>();"""],
             ["""new Mock<AbstractClassWithCtor>{|Moq1002:("42")|};"""],

Source/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj

@@ -7,7 +7,6 @@
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
-    <NoWarn>1701;1702;SA1600;SA1402</NoWarn>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">

Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

@@ -1,3 +1,5 @@
+using System.Diagnostics;
+
 namespace Moq.Analyzers;
 
 /// <summary>
@@ -68,10 +70,21 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         {
             for (int i = 0; i < mockedMethodArguments.Count; i++)
             {
+                TypeSyntax? lambdaParameterTypeSyntax = lambdaParameters[i].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);
+
                 TypeInfo mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(mockedMethodArguments[i].Expression, context.CancellationToken);
-                TypeInfo lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameters[i].Type, context.CancellationToken);
+
                 string? mockedMethodTypeName = mockedMethodArgumentType.ConvertedType?.ToString();
                 string? lambdaParameterTypeName = lambdaParameterType.ConvertedType?.ToString();
+
                 if (!string.Equals(mockedMethodTypeName, lambdaParameterTypeName, StringComparison.Ordinal))
                 {
                     Diagnostic? diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());

Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodCodeFix.cs

@@ -60,10 +60,12 @@ private async Task<Document> FixCallbackSignatureAsync(SyntaxNode root, Document
 
         Debug.Assert(semanticModel != null, nameof(semanticModel) + " != null");
 
+#pragma warning disable S2583 // Conditionally executed code should be reachable
         if (semanticModel == null)
         {
             return document;
         }
+#pragma warning restore S2583 // Conditionally executed code should be reachable
 
         if (oldParameters?.Parent?.Parent?.Parent?.Parent is not InvocationExpressionSyntax callbackInvocation)
         {
@@ -72,9 +74,9 @@ private async Task<Document> FixCallbackSignatureAsync(SyntaxNode root, Document
 
         InvocationExpressionSyntax? setupMethodInvocation = Helpers.FindSetupMethodFromCallbackInvocation(semanticModel, callbackInvocation, cancellationToken);
         Debug.Assert(setupMethodInvocation != null, nameof(setupMethodInvocation) + " != null");
-        IMethodSymbol[]? matchingMockedMethods = Helpers.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(semanticModel, setupMethodInvocation).ToArray();
+        IMethodSymbol[] matchingMockedMethods = Helpers.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(semanticModel, setupMethodInvocation).ToArray();
 
-        if (matchingMockedMethods.Length != 1 || oldParameters == null)
+        if (matchingMockedMethods.Length != 1)
         {
             return document;
         }

Source/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

@@ -35,6 +35,7 @@ public override void Initialize(AnalysisContext context)
         context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.ObjectCreationExpression);
     }
 
+    [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;
@@ -47,18 +48,17 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         // Full check that we are calling new Mock<T>()
         IMethodSymbol? constructorSymbol = GetConstructorSymbol(context, objectCreation);
 
-        // Vararg parameter is the one that takes all arguments for mocked class constructor
-        IParameterSymbol? varArgsConstructorParameter = constructorSymbol?.Parameters.FirstOrDefault(x => x.IsParams);
+        Debug.Assert(constructorSymbol != null, nameof(constructorSymbol) + " != null");
 
-        // Vararg parameter are not used, so there are no arguments for mocked class constructor
-        if (varArgsConstructorParameter == null) return;
+#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
 
-        Debug.Assert(constructorSymbol != null, nameof(constructorSymbol) + " != null");
+            // Vararg parameter is the one that takes all arguments for mocked class constructor
+        IParameterSymbol? varArgsConstructorParameter = constructorSymbol.Parameters.FirstOrDefault(x => x.IsParams);
 
-        if (constructorSymbol == null)
-        {
-            return;
-        }
+        // Vararg parameter are not used, so there are no arguments for mocked class constructor
+        if (varArgsConstructorParameter == null) return;
 
         int varArgsConstructorParameterIdx = constructorSymbol.Parameters.IndexOf(varArgsConstructorParameter);
 
@@ -166,6 +166,7 @@ private static bool IsMockGenericType(GenericNameSyntax genericName)
     {
         SymbolInfo constructorSymbolInfo = context.SemanticModel.GetSymbolInfo(objectCreation, context.CancellationToken);
         IMethodSymbol? constructorSymbol = constructorSymbolInfo.Symbol as IMethodSymbol;
+
         return constructorSymbol?.MethodKind == MethodKind.Constructor &&
                string.Equals(
                    constructorSymbol.ContainingType?.ConstructedFrom.ToDisplayString(),

Source/Moq.Analyzers/Helpers.cs

@@ -1,4 +1,5 @@
 using System.Diagnostics;
+using System.Linq.Expressions;
 
 namespace Moq.Analyzers;
 
@@ -17,12 +18,14 @@ internal static bool IsCallbackOrReturnInvocation(SemanticModel semanticModel, I
 
         Debug.Assert(callbackOrReturnsMethod != null, nameof(callbackOrReturnsMethod) + " != null");
 
+#pragma warning disable S2583 // Conditionally executed code should be reachable
         if (callbackOrReturnsMethod == null)
         {
             return false;
         }
+#pragma warning restore S2583 // Conditionally executed code should be reachable
 
-        string? methodName = callbackOrReturnsMethod.Name.ToString();
+        string methodName = callbackOrReturnsMethod.Name.ToString();
 
         // First fast check before walking semantic model
         if (!string.Equals(methodName, "Callback", StringComparison.Ordinal)
@@ -65,24 +68,32 @@ internal static IEnumerable<IMethodSymbol> GetAllMatchingMockedMethodSymbolsFrom
         LambdaExpressionSyntax? setupLambdaArgument = setupMethodInvocation?.ArgumentList.Arguments[0].Expression as LambdaExpressionSyntax;
         InvocationExpressionSyntax? mockedMethodInvocation = setupLambdaArgument?.Body as InvocationExpressionSyntax;
 
-        return GetAllMatchingSymbols<IMethodSymbol>(semanticModel, mockedMethodInvocation);
+        return mockedMethodInvocation == null
+            ? []
+            : GetAllMatchingSymbols<IMethodSymbol>(semanticModel, mockedMethodInvocation);
     }
 
-    internal static IEnumerable<T> GetAllMatchingSymbols<T>(SemanticModel semanticModel, ExpressionSyntax? expression)
+    internal static IEnumerable<T> GetAllMatchingSymbols<T>(SemanticModel semanticModel, ExpressionSyntax expression)
         where T : class
     {
-        List<T>? matchingSymbols = new List<T>();
-        if (expression != null)
+        List<T> matchingSymbols = new();
+
+        SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(expression);
+        if (symbolInfo is { CandidateReason: CandidateReason.None, Symbol: T })
         {
-            SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(expression);
-            if (symbolInfo is { CandidateReason: CandidateReason.None, Symbol: T })
-            {
-                matchingSymbols.Add(symbolInfo.Symbol as T);
-            }
-            else if (symbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure)
+            T? value = symbolInfo.Symbol as T;
+            Debug.Assert(value != null, "Value should not be null.");
+
+#pragma warning disable S2589 // Boolean expressions should not be gratuitous
+            if (value != default(T))
             {
-                matchingSymbols.AddRange(symbolInfo.CandidateSymbols.OfType<T>());
+                matchingSymbols.Add(value);
             }
+#pragma warning restore S2589 // Boolean expressions should not be gratuitous
+        }
+        else if (symbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure)
+        {
+            matchingSymbols.AddRange(symbolInfo.CandidateSymbols.OfType<T>());
         }
 
         return matchingSymbols;

Source/Moq.Analyzers/NoConstructorArgumentsForInterfaceMockAnalyzer.cs

@@ -53,7 +53,13 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
 
         // Full check
         SymbolInfo constructorSymbolInfo = context.SemanticModel.GetSymbolInfo(objectCreation, context.CancellationToken);
-        if (constructorSymbolInfo.Symbol is not IMethodSymbol constructorSymbol || constructorSymbol.ContainingType == null || constructorSymbol.ContainingType.ConstructedFrom == null) return;
+        if (constructorSymbolInfo.Symbol is not IMethodSymbol constructorSymbol
+            || constructorSymbol.ContainingType == null
+            || constructorSymbol.ContainingType.ConstructedFrom == null)
+        {
+            return;
+        }
+
         if (constructorSymbol.MethodKind != MethodKind.Constructor) return;
         if (!string.Equals(
                 constructorSymbol.ContainingType.ConstructedFrom.ToDisplayString(),
@@ -63,7 +69,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
             return;
         }
 
-        if (constructorSymbol.Parameters == null || constructorSymbol.Parameters.Length == 0) return;
+        if (constructorSymbol.Parameters.Length == 0) return;
         if (!constructorSymbol.Parameters.Any(x => x.IsParams)) return;
 
         // Find mocked type

build/targets/codeanalysis/CodeAnalysis.props

@@ -5,6 +5,8 @@
     <AnalysisMode>preview</AnalysisMode>
     <WarningLevel>9999</WarningLevel>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+    <MSBuildTreatWarningsAsErrors>true</MSBuildTreatWarningsAsErrors>
   </PropertyGroup>
 
   <ItemGroup>