Add Effective C# analyzers and update SquiggleCop baselines

build/targets/codeanalysis/.globalconfig

@@ -7,3 +7,24 @@ is_global=true
 # AV2210 : Pass -warnaserror to the compiler or add <TreatWarningsAsErrors>True</TreatWarningsAsErrors> to your project file
 # This is set as part of the CI build. It is intentionally not set locally to allow for a fast inner dev loop.
 dotnet_diagnostic.AV2210.severity = none
+
+# Enable Effective C# Analyzers
+dotnet_diagnostic.ECS0100.severity = warning
+dotnet_diagnostic.ECS0200.severity = warning
+dotnet_diagnostic.ECS0300.severity = warning
+dotnet_diagnostic.ECS0400.severity = warning
+dotnet_diagnostic.ECS0500.severity = warning
+dotnet_diagnostic.ECS0600.severity = warning
+dotnet_diagnostic.ECS0700.severity = warning
+dotnet_diagnostic.ECS0800.severity = warning
+dotnet_diagnostic.ECS0900.severity = warning
+dotnet_diagnostic.ECS1000.severity = warning
+dotnet_diagnostic.ECS1100.severity = warning
+dotnet_diagnostic.ECS1200.severity = warning
+dotnet_diagnostic.ECS1300.severity = warning
+dotnet_diagnostic.ECS1400.severity = warning
+dotnet_diagnostic.ECS1500.severity = warning
+dotnet_diagnostic.ECS1600.severity = warning
+dotnet_diagnostic.ECS1700.severity = warning
+dotnet_diagnostic.ECS1800.severity = warning
+dotnet_diagnostic.ECS1900.severity = warning

build/targets/codeanalysis/CodeAnalysis.props

@@ -39,5 +39,9 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <PackageReference Include="ExhaustiveMatching.Analyzer" />
+    <PackageReference Include="EffectiveCSharp.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
   </ItemGroup>
 </Project>

build/targets/codeanalysis/Packages.props

@@ -8,5 +8,6 @@
     <PackageVersion Include="SquiggleCop.Tasks" Version="1.0.26" />
     <PackageVersion Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.11.20" />
     <PackageVersion Include="ExhaustiveMatching.Analyzer" Version="0.5.0" />
+    <PackageVersion Include="EffectiveCSharp.Analyzers" Version="0.1.0" />
   </ItemGroup>
 </Project>

src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs

@@ -8,8 +8,8 @@ namespace Moq.Analyzers;
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public class AsShouldBeUsedOnlyForInterfaceAnalyzer : DiagnosticAnalyzer
 {
-    private const string Title = "Moq: Invalid As type parameter";
-    private const string Message = "Mock.As() should take interfaces only";
+    private static readonly LocalizableString Title = "Moq: Invalid As type parameter";
+    private static readonly LocalizableString Message = "Mock.As() should take interfaces only";
 
     private static readonly DiagnosticDescriptor Rule = new(
         DiagnosticIds.AsShouldOnlyBeUsedForInterfacesRuleId,
@@ -42,11 +42,13 @@ private static void RegisterCompilationStartAction(CompilationStartAnalysisConte
         }
 
         // Look for the Mock.As() method and provide it to Analyze to avoid looking it up multiple times.
+#pragma warning disable ECS0900 // Minimize boxing and unboxing
         ImmutableArray<IMethodSymbol> asMethods = mockTypes
             .SelectMany(mockType => mockType.GetMembers(WellKnownTypeNames.As))
             .OfType<IMethodSymbol>()
             .Where(method => method.IsGenericMethod)
             .ToImmutableArray();
+#pragma warning restore ECS0900 // Minimize boxing and unboxing
 
         if (asMethods.IsEmpty)
         {
@@ -66,10 +68,12 @@ private static void Analyze(OperationAnalysisContext context, ImmutableArray<IMe
         }
 
         IMethodSymbol targetMethod = invocationOperation.TargetMethod;
+#pragma warning disable ECS0900 // Minimize boxing and unboxing
         if (!targetMethod.IsInstanceOf(wellKnownAsMethods))
         {
             return;
         }
+#pragma warning restore ECS0900 // Minimize boxing and unboxing
 
         ImmutableArray<ITypeSymbol> typeArguments = targetMethod.TypeArguments;
         if (typeArguments.Length != 1)

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

@@ -6,24 +6,20 @@ namespace Moq.Analyzers;
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public class CallbackSignatureShouldMatchMockedMethodAnalyzer : DiagnosticAnalyzer
 {
-    internal const string RuleId = DiagnosticIds.BadCallbackParameters;
-    private const string Title = "Moq: Bad callback parameters";
-    private const string Message = "Callback signature must match the signature of the mocked method";
+    private static readonly LocalizableString Title = "Moq: Bad callback parameters";
+    private static readonly LocalizableString Message = "Callback signature must match the signature of the mocked method";
 
     private static readonly DiagnosticDescriptor Rule = new(
-        RuleId,
+        DiagnosticIds.BadCallbackParameters,
         Title,
         Message,
         DiagnosticCategory.Moq,
         DiagnosticSeverity.Warning,
         isEnabledByDefault: true,
-        helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{RuleId}.md");
+        helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.BadCallbackParameters}.md");
 
     /// <inheritdoc />
-    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
-    {
-        get { return ImmutableArray.Create(Rule); }
-    }
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
     /// <inheritdoc />
     public override void Initialize(AnalysisContext context)

src/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodCodeFix.cs

@@ -14,16 +14,10 @@ namespace Moq.Analyzers;
 public class CallbackSignatureShouldMatchMockedMethodCodeFix : CodeFixProvider
 {
     /// <inheritdoc />
-    public sealed override ImmutableArray<string> FixableDiagnosticIds
-    {
-        get { return ImmutableArray.Create(CallbackSignatureShouldMatchMockedMethodAnalyzer.RuleId); }
-    }
+    public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.BadCallbackParameters);
 
     /// <inheritdoc />
-    public sealed override FixAllProvider GetFixAllProvider()
-    {
-        return WellKnownFixAllProviders.BatchFixer;
-    }
+    public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
 
     /// <inheritdoc />
     public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)

src/Moq.Analyzers/Common/ArrayExtensions.cs

@@ -0,0 +1,53 @@
+using ArgumentOutOfRangeException = System.ArgumentOutOfRangeException;
+
+namespace Moq.Analyzers.Common;
+
+internal static class ArrayExtensions
+{
+    /// <summary>
+    /// Returns an array with the element at the specified position removed.
+    /// </summary>
+    /// <typeparam name="T">The array type.</typeparam>
+    /// <param name="array">The array.</param>
+    /// <param name="index">The 0-based index into the array for the element to omit from the returned array.</param>
+    /// <returns>The new array.</returns>
+    internal static T[] RemoveAt<T>(this T[] array, int index)
+    {
+        return RemoveRange(array, index, 1);
+    }
+
+    /// <summary>
+    /// Returns an array with the elements at the specified position removed.
+    /// </summary>
+    /// <typeparam name="T">The array type.</typeparam>
+    /// <param name="array">The array.</param>
+    /// <param name="index">The 0-based index into the array for the element to omit from the returned array.</param>
+    /// <param name="length">The number of elements to remove.</param>
+    /// <returns>The new array.</returns>
+    private static T[] RemoveRange<T>(this T[] array, int index, int length)
+    {
+        // Range check
+        if (index < 0 || index >= array.Length)
+        {
+            throw new ArgumentOutOfRangeException(nameof(index));
+        }
+
+        if (length < 0 || index + length > array.Length)
+        {
+            throw new ArgumentOutOfRangeException(nameof(length));
+        }
+
+#pragma warning disable S2583 // Change condition so it doesn't always evaluate to false
+        if (array.Length == 0)
+#pragma warning restore S2583
+        {
+            return array;
+        }
+
+        T[] tmp = new T[array.Length - length];
+        Array.Copy(array, tmp, index);
+        Array.Copy(array, index + length, tmp, index, array.Length - index - length);
+
+        return tmp;
+    }
+}

src/Moq.Analyzers/Common/DiagnosticCategory.cs

@@ -1,5 +1,7 @@
 namespace Moq.Analyzers.Common;
 
+#pragma warning disable ECS0200 // Consider using readonly instead of const for flexibility
+
 internal static class DiagnosticCategory
 {
     internal const string Moq = nameof(Moq);

src/Moq.Analyzers/Common/DiagnosticExtensions.cs

@@ -13,12 +13,12 @@ public static Diagnostic CreateDiagnostic(
         DiagnosticDescriptor rule,
         ImmutableDictionary<string, string?>? properties,
         params object?[]? messageArgs)
-        => node.CreateDiagnostic(rule, additionalLocations: ImmutableArray<Location>.Empty, properties, messageArgs);
+        => node.CreateDiagnostic(rule, additionalLocations: Array.Empty<Location>(), properties, messageArgs);
 
     public static Diagnostic CreateDiagnostic(
         this SyntaxNode node,
         DiagnosticDescriptor rule,
-        ImmutableArray<Location> additionalLocations,
+        IEnumerable<Location>? additionalLocations,
         ImmutableDictionary<string, string?>? properties,
         params object?[]? messageArgs)
         => node
@@ -44,12 +44,12 @@ public static Diagnostic CreateDiagnostic(
         DiagnosticDescriptor rule,
         ImmutableDictionary<string, string?>? properties,
         params object?[]? messageArgs)
-        => location.CreateDiagnostic(rule, ImmutableArray<Location>.Empty, properties, messageArgs);
+        => location.CreateDiagnostic(rule, Array.Empty<Location>(), properties, messageArgs);
 
     public static Diagnostic CreateDiagnostic(
         this Location location,
         DiagnosticDescriptor rule,
-        ImmutableArray<Location> additionalLocations,
+        IEnumerable<Location>? additionalLocations,
         ImmutableDictionary<string, string?>? properties,
         params object?[]? messageArgs)
     {

src/Moq.Analyzers/Common/DiagnosticIds.cs

@@ -1,5 +1,7 @@
 namespace Moq.Analyzers.Common;
 
+#pragma warning disable ECS0200 // Consider using readonly instead of const for flexibility
+
 internal static class DiagnosticIds
 {
     internal const string SealedClassCannotBeMocked = "Moq1000";

src/Moq.Analyzers/Common/MoqMethodDescriptorBase.cs

@@ -11,28 +11,16 @@
 /// </remarks>
 internal abstract class MoqMethodDescriptorBase
 {
-    private const string ContainingNamespace = WellKnownTypeNames.Moq;
-    private const string ContainingType = WellKnownTypeNames.MockName;
+    private static readonly string ContainingNamespace = WellKnownTypeNames.Moq;
+    private static readonly string ContainingType = WellKnownTypeNames.MockName;
 
     public abstract bool IsMatch(SemanticModel semanticModel, MemberAccessExpressionSyntax memberAccessSyntax, CancellationToken cancellationToken);
 
-    protected static bool IsFastMatch(MemberAccessExpressionSyntax memberAccessSyntax, ReadOnlySpan<char> methodName)
-    {
-        return memberAccessSyntax.Name.Identifier.Text.AsSpan().SequenceEqual(methodName);
-    }
+    protected static bool IsFastMatch(MemberAccessExpressionSyntax memberAccessSyntax, ReadOnlySpan<char> methodName) => memberAccessSyntax.Name.Identifier.Text.AsSpan().SequenceEqual(methodName);
 
-    protected static bool IsContainedInMockType(IMethodSymbol methodSymbol)
-    {
-        return IsInMoqNamespace(methodSymbol) && IsInMockType(methodSymbol);
-    }
+    protected static bool IsContainedInMockType(IMethodSymbol methodSymbol) => IsInMoqNamespace(methodSymbol) && IsInMockType(methodSymbol);
 
-    private static bool IsInMoqNamespace(ISymbol symbol)
-    {
-        return symbol.ContainingNamespace.Name.AsSpan().SequenceEqual(ContainingNamespace.AsSpan());
-    }
+    private static bool IsInMoqNamespace(ISymbol symbol) => symbol.ContainingNamespace.Name.AsSpan().SequenceEqual(ContainingNamespace.AsSpan());
 
-    private static bool IsInMockType(ISymbol symbol)
-    {
-        return symbol.ContainingType.Name.AsSpan().SequenceEqual(ContainingType.AsSpan());
-    }
+    private static bool IsInMockType(ISymbol symbol) => symbol.ContainingType.Name.AsSpan().SequenceEqual(ContainingType.AsSpan());
 }

src/Moq.Analyzers/Common/WellKnownTypeNames.cs

@@ -1,16 +1,18 @@
 namespace Moq.Analyzers.Common;
 
+#pragma warning disable ECS0200 // Consider using readonly instead of const for flexibility
+
 internal static class WellKnownTypeNames
 {
-    internal const string Moq = "Moq";
+    internal const string Moq = nameof(Moq);
     internal const string MockName = "Mock";
-    internal const string MockBehavior = "MockBehavior";
-    internal const string MockFactory = "MockFactory";
+    internal const string MockBehavior = nameof(MockBehavior);
+    internal const string MockFactory = nameof(MockFactory);
     internal const string MoqMock = $"{Moq}.{MockName}";
     internal const string MoqMock1 = $"{MoqMock}`1";
     internal const string MoqBehavior = $"{Moq}.{MockBehavior}";
     internal const string MoqRepository = $"{Moq}.MockRepository";
-    internal const string As = "As";
-    internal const string Create = "Create";
-    internal const string Of = "Of";
+    internal const string As = nameof(As);
+    internal const string Create = nameof(Create);
+    internal const string Of = nameof(Of);
 }

src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

@@ -132,7 +132,7 @@ private static bool IsFirstArgumentMockBehavior(SyntaxNodeAnalysisContext contex
     private static void VerifyDelegateMockAttempt(
     SyntaxNodeAnalysisContext context,
     ArgumentListSyntax? argumentList,
-    ImmutableArray<ArgumentSyntax> arguments)
+    ArgumentSyntax[] arguments)
     {
         if (arguments.Length == 0)
         {
@@ -149,7 +149,7 @@ private static void VerifyDelegateMockAttempt(
     private static void VerifyInterfaceMockAttempt(
         SyntaxNodeAnalysisContext context,
         ArgumentListSyntax? argumentList,
-        ImmutableArray<ArgumentSyntax> arguments)
+        ArgumentSyntax[] arguments)
     {
         // Interfaces and delegates don't have ctors, so bail out early
         if (arguments.Length == 0)
@@ -322,16 +322,18 @@ private static void AnalyzeNewObject(SyntaxNodeAnalysisContext context)
     /// <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(
-        ImmutableArray<IMethodSymbol> constructors,
-        ImmutableArray<ArgumentSyntax> arguments,
+        IMethodSymbol[] constructors,
+        ArgumentSyntax[] arguments,
         SyntaxNodeAnalysisContext context)
     {
         for (int constructorIndex = 0; constructorIndex < constructors.Length; constructorIndex++)
         {
             IMethodSymbol constructor = constructors[constructorIndex];
             bool hasParams = constructor.Parameters.Length > 0 && constructor.Parameters[^1].IsParams;
             int fixedParametersCount = hasParams ? constructor.Parameters.Length - 1 : constructor.Parameters.Length;
+#pragma warning disable ECS0900 // Consider using an alternative implementation to avoid boxing and unboxing
             int requiredParameters = constructor.Parameters.Count(parameterSymbol => !parameterSymbol.IsOptional);
+#pragma warning restore ECS0900 // Consider using an alternative implementation to avoid boxing and unboxing
             bool allParametersMatch = true;
 
             // Check if the number of arguments is valid considering params
@@ -401,7 +403,7 @@ private static bool AnyConstructorsFound(
     }
 
     private static (bool IsEmpty, Location Location) ConstructorIsEmpty(
-        ImmutableArray<IMethodSymbol> constructors,
+        IMethodSymbol[] constructors,
         ArgumentListSyntax? argumentList,
         SyntaxNodeAnalysisContext context)
     {
@@ -416,7 +418,7 @@ private static (bool IsEmpty, Location Location) ConstructorIsEmpty(
             location = context.Node.GetLocation();
         }
 
-        return (constructors.IsEmpty, location);
+        return (constructors.Length == 0, location);
     }
 
     private static void VerifyMockAttempt(
@@ -430,9 +432,9 @@ private static void VerifyMockAttempt(
             return;
         }
 
-        ImmutableArray<ArgumentSyntax> arguments =
-                                        argumentList?.Arguments.ToImmutableArray()
-                                        ?? ImmutableArray<ArgumentSyntax>.Empty;
+#pragma warning disable ECS0900 // Consider using an alternative implementation to avoid boxing and unboxing
+        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, argumentList))
         {
@@ -463,13 +465,13 @@ private static void VerifyClassMockAttempt(
         SyntaxNodeAnalysisContext context,
         ITypeSymbol mockedClass,
         ArgumentListSyntax? argumentList,
-        ImmutableArray<ArgumentSyntax> arguments)
+        ArgumentSyntax[] arguments)
     {
-        ImmutableArray<IMethodSymbol> constructors = mockedClass
+        IMethodSymbol[] constructors = mockedClass
             .GetMembers()
             .OfType<IMethodSymbol>()
             .Where(methodSymbol => methodSymbol.IsConstructor())
-            .ToImmutableArray();
+            .ToArray();
 
         // Bail out early if there are no arguments on constructors or no constructors at all
         (bool IsEmpty, Location Location) constructorIsEmpty = ConstructorIsEmpty(constructors, argumentList, context);

src/Moq.Analyzers/MoqSetupMethodDescriptor.cs

@@ -6,7 +6,7 @@
 /// </summary>
 internal class MoqSetupMethodDescriptor : MoqMethodDescriptorBase
 {
-    private const string MethodName = "Setup";
+    private static readonly string MethodName = "Setup";
 
     [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")]
     public override bool IsMatch(SemanticModel semanticModel, MemberAccessExpressionSyntax memberAccessSyntax, CancellationToken cancellationToken)