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

Add Effective C# analyzers and update SquiggleCop baselines #182

Merged
merged 10 commits into from
Aug 26, 2024
21 changes: 21 additions & 0 deletions build/targets/codeanalysis/.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions build/targets/codeanalysis/CodeAnalysis.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
1 change: 1 addition & 0 deletions build/targets/codeanalysis/Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
8 changes: 6 additions & 2 deletions src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
53 changes: 53 additions & 0 deletions src/Moq.Analyzers/Common/ArrayExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using ArgumentOutOfRangeException = System.ArgumentOutOfRangeException;

Check failure on line 1 in src/Moq.Analyzers/Common/ArrayExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Moq.Analyzers/Common/ArrayExtensions.cs#L1

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected tabs found.


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;
}
}
2 changes: 2 additions & 0 deletions src/Moq.Analyzers/Common/DiagnosticCategory.cs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/Moq.Analyzers/Common/DiagnosticExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Moq.Analyzers/Common/DiagnosticIds.cs
Original file line number Diff line number Diff line change
@@ -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";
Expand Down
24 changes: 6 additions & 18 deletions src/Moq.Analyzers/Common/MoqMethodDescriptorBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
14 changes: 8 additions & 6 deletions src/Moq.Analyzers/Common/WellKnownTypeNames.cs
Original file line number Diff line number Diff line change
@@ -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);
}
26 changes: 14 additions & 12 deletions src/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -401,7 +403,7 @@ private static bool AnyConstructorsFound(
}

private static (bool IsEmpty, Location Location) ConstructorIsEmpty(
ImmutableArray<IMethodSymbol> constructors,
IMethodSymbol[] constructors,
ArgumentListSyntax? argumentList,
SyntaxNodeAnalysisContext context)
{
Expand All @@ -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(
Expand All @@ -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))
{
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Moq.Analyzers/MoqSetupMethodDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading