Skip to content

Commit

Permalink
Update code to remove warnings from backlog (#73)
Browse files Browse the repository at this point in the history 
This reduces the backlog of warnings by about half, removing many cases
for passing null references to methods and null dereference. The
remaining warnings are from multiple sources: TODO, diagnostic best
practices, and some tricker items we'll need to look at

Related to #26
  • Loading branch information
@rjmurillo
rjmurillo authored Jun 7, 2024
1 parent 5c29516 commit 7a45b15
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 150 deletions.
18 changes: 9 additions & 9 deletions Source/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
{
var asInvocation = (InvocationExpressionSyntax)context.Node;

if (asInvocation.Expression is MemberAccessExpressionSyntax memberAccessExpression && Helpers.IsMoqAsMethod(context.SemanticModel, memberAccessExpression))
if (asInvocation.Expression is MemberAccessExpressionSyntax memberAccessExpression
&& Helpers.IsMoqAsMethod(context.SemanticModel, memberAccessExpression)
&& memberAccessExpression.Name is GenericNameSyntax genericName
&& genericName.TypeArgumentList.Arguments.Count == 1)
{
if (memberAccessExpression.Name is GenericNameSyntax genericName && genericName.TypeArgumentList.Arguments.Count == 1)
var typeArgument = genericName.TypeArgumentList.Arguments[0];
var symbolInfo = context.SemanticModel.GetSymbolInfo(typeArgument, context.CancellationToken);
if (symbolInfo.Symbol is ITypeSymbol typeSymbol && typeSymbol.TypeKind != TypeKind.Interface)
{
var typeArgument = genericName.TypeArgumentList.Arguments[0];
var symbolInfo = context.SemanticModel.GetSymbolInfo(typeArgument, context.CancellationToken);
if (symbolInfo.Symbol != null && symbolInfo.Symbol is ITypeSymbol typeSymbol && typeSymbol.TypeKind != TypeKind.Interface)
{
var diagnostic = Diagnostic.Create(Rule, typeArgument.GetLocation());
context.ReportDiagnostic(diagnostic);
}
var diagnostic = Diagnostic.Create(Rule, typeArgument.GetLocation());
context.ReportDiagnostic(diagnostic);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
{
var mockedMethodArgumentType = context.SemanticModel.GetTypeInfo(mockedMethodArguments[i].Expression, context.CancellationToken);
var lambdaParameterType = context.SemanticModel.GetTypeInfo(lambdaParameters[i].Type, context.CancellationToken);

Check warning on line 62 in Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build (windows-2022) 

Possible null reference argument for parameter 'expression' in 'TypeInfo CSharpExtensions.GetTypeInfo(SemanticModel? semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken = default(CancellationToken))'.

Check warning on line 62 in Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build (ubuntu-22.04) 

Possible null reference argument for parameter 'expression' in 'TypeInfo CSharpExtensions.GetTypeInfo(SemanticModel? semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken = default(CancellationToken))'.
string mockedMethodTypeName = mockedMethodArgumentType.ConvertedType.ToString();
string lambdaParameterTypeName = lambdaParameterType.ConvertedType.ToString();
if (mockedMethodTypeName != lambdaParameterTypeName)
string? mockedMethodTypeName = mockedMethodArgumentType.ConvertedType?.ToString();
string? lambdaParameterTypeName = lambdaParameterType.ConvertedType?.ToString();
if (!string.Equals(mockedMethodTypeName, lambdaParameterTypeName, StringComparison.Ordinal))
{
var diagnostic = Diagnostic.Create(Rule, callbackLambda.ParameterList.GetLocation());
context.ReportDiagnostic(diagnostic);
Expand Down
62 changes: 41 additions & 21 deletions Source/Moq.Analyzers/CallbackSignatureShouldMatchMockedMethodCodeFix.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Composition;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;

Expand All @@ -22,44 +23,63 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

if (root == null)
{
return;
}

var diagnostic = context.Diagnostics.First();
var diagnosticSpan = diagnostic.Location.SourceSpan;

// Find the type declaration identified by the diagnostic.
var badArgumentListSyntax = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType<ParameterListSyntax>().First();
ParameterListSyntax? badArgumentListSyntax = root.FindToken(diagnosticSpan.Start)
.Parent?
.AncestorsAndSelf()
.OfType<ParameterListSyntax>()
.First();

// Register a code action that will invoke the fix.
context.RegisterCodeFix(
CodeAction.Create(
title: "Fix Moq callback signature",
createChangedDocument: c => FixCallbackSignature(root, context.Document, badArgumentListSyntax, c),
createChangedDocument: c => FixCallbackSignatureAsync(root, context.Document, badArgumentListSyntax, c),
equivalenceKey: "Fix Moq callback signature"),
diagnostic);
}

private async Task<Document> FixCallbackSignature(SyntaxNode root, Document document, ParameterListSyntax oldParameters, CancellationToken cancellationToken)
private async Task<Document> FixCallbackSignatureAsync(SyntaxNode root, Document document, ParameterListSyntax? oldParameters, CancellationToken cancellationToken)
{
var semanticModel = await document.GetSemanticModelAsync(cancellationToken);
var callbackInvocation = oldParameters?.Parent?.Parent?.Parent?.Parent as InvocationExpressionSyntax;
if (callbackInvocation != null)
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

Debug.Assert(semanticModel != null, nameof(semanticModel) + " != null");

if (semanticModel == null)
{
var setupMethodInvocation = Helpers.FindSetupMethodFromCallbackInvocation(semanticModel, callbackInvocation);
var matchingMockedMethods = Helpers.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(semanticModel, setupMethodInvocation).ToArray();
return document;
}

if (matchingMockedMethods.Length == 1)
{
var newParameters = SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(matchingMockedMethods[0].Parameters.Select(
p =>
{
var type = SyntaxFactory.ParseTypeName(p.Type.ToMinimalDisplayString(semanticModel, oldParameters.SpanStart));
return SyntaxFactory.Parameter(default(SyntaxList<AttributeListSyntax>), SyntaxFactory.TokenList(), type, SyntaxFactory.Identifier(p.Name), null);
})));

var newRoot = root.ReplaceNode(oldParameters, newParameters);
return document.WithSyntaxRoot(newRoot);
}
if (oldParameters?.Parent?.Parent?.Parent?.Parent is not InvocationExpressionSyntax callbackInvocation)
{
return document;
}

return document;
var setupMethodInvocation = Helpers.FindSetupMethodFromCallbackInvocation(semanticModel, callbackInvocation);
Debug.Assert(setupMethodInvocation != null, nameof(setupMethodInvocation) + " != null");
var matchingMockedMethods = Helpers.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(semanticModel, setupMethodInvocation).ToArray();

if (matchingMockedMethods.Length != 1 || oldParameters == null)
{
return document;
}

var newParameters = SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(matchingMockedMethods[0].Parameters.Select(
p =>
{
var type = SyntaxFactory.ParseTypeName(p.Type.ToMinimalDisplayString(semanticModel, oldParameters.SpanStart));
return SyntaxFactory.Parameter(default, SyntaxFactory.TokenList(), type, SyntaxFactory.Identifier(p.Name), null);
})));

var newRoot = root.ReplaceNode(oldParameters, newParameters);
return document.WithSyntaxRoot(newRoot);
}
}
63 changes: 42 additions & 21 deletions Source/Moq.Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Diagnostics;

namespace Moq.Analyzers;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
Expand Down Expand Up @@ -36,22 +38,32 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
var constructorSymbol = GetConstructorSymbol(context, objectCreation);

// Vararg parameter is the one that takes all arguments for mocked class constructor
var varArgsConstructorParameter = constructorSymbol.Parameters.FirstOrDefault(x => x.IsParams);
var varArgsConstructorParameter = constructorSymbol?.Parameters.FirstOrDefault(x => x.IsParams);

// Vararg parameter are not used, so there are no arguments for mocked class constructor
if (varArgsConstructorParameter == null) return;

Debug.Assert(constructorSymbol != null, nameof(constructorSymbol) + " != null");

if (constructorSymbol == null)
{
return;
}

var varArgsConstructorParameterIdx = constructorSymbol.Parameters.IndexOf(varArgsConstructorParameter);

// Find mocked type
var mockedTypeSymbol = GetMockedSymbol(context, genericName);
if (mockedTypeSymbol == null) return;

// Skip first argument if it is not vararg - typically it is MockingBehavior argument
var constructorArguments = objectCreation.ArgumentList.Arguments.Skip(varArgsConstructorParameterIdx == 0 ? 0 : 1).ToArray();
var constructorArguments = objectCreation.ArgumentList?.Arguments.Skip(varArgsConstructorParameterIdx == 0 ? 0 : 1).ToArray();

if (!mockedTypeSymbol.IsAbstract)
{
if (IsConstructorMismatch(context, objectCreation, genericName, constructorArguments))
if (constructorArguments != null
&& IsConstructorMismatch(context, objectCreation, genericName, constructorArguments)
&& objectCreation.ArgumentList != null)
{
var diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
context.ReportDiagnostic(diagnostic);
Expand All @@ -64,33 +76,38 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
// The mocked symbol is abstract, so we need to check if the constructor arguments match the abstract class constructor

// Extract types of arguments passed in the constructor call
var argumentTypes = constructorArguments
.Select(arg => context.SemanticModel.GetTypeInfo(arg.Expression, context.CancellationToken).Type)
.ToArray();

// Check all constructors of the abstract type
foreach (var constructor in mockedTypeSymbol.Constructors)
if (constructorArguments != null)
{
if (AreParametersMatching(constructor.Parameters, argumentTypes))
ITypeSymbol[] argumentTypes = constructorArguments
.Select(arg => context.SemanticModel.GetTypeInfo(arg.Expression, context.CancellationToken).Type)
.ToArray()!;

// Check all constructors of the abstract type
for (int i = 0; i < mockedTypeSymbol.Constructors.Length; i++)
{
return; // Found a matching constructor
IMethodSymbol constructor = mockedTypeSymbol.Constructors[i];
if (AreParametersMatching(constructor.Parameters, argumentTypes))
{
return; // Found a matching constructor
}
}
}

var diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList.GetLocation());
Debug.Assert(objectCreation.ArgumentList != null, "objectCreation.ArgumentList != null");

var diagnostic = Diagnostic.Create(Rule, objectCreation.ArgumentList?.GetLocation());
context.ReportDiagnostic(diagnostic);
}
}

private static INamedTypeSymbol GetMockedSymbol(
private static INamedTypeSymbol? GetMockedSymbol(
SyntaxNodeAnalysisContext context,
GenericNameSyntax genericName)
{
var typeArguments = genericName.TypeArgumentList.Arguments;
if (typeArguments == null || typeArguments.Count != 1) return null;
if (typeArguments.Count != 1) return null;
var mockedTypeSymbolInfo = context.SemanticModel.GetSymbolInfo(typeArguments[0], context.CancellationToken);
var mockedTypeSymbol = mockedTypeSymbolInfo.Symbol as INamedTypeSymbol;
if (mockedTypeSymbol == null || mockedTypeSymbol.TypeKind != TypeKind.Class) return null;
if (mockedTypeSymbolInfo.Symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class } mockedTypeSymbol) return null;
return mockedTypeSymbol;
}

Expand All @@ -105,7 +122,7 @@ private static bool AreParametersMatching(ImmutableArray<IParameterSymbol> const
// Check if each parameter type matches in order
for (int i = 0; i < constructorParameters.Length; i++)
{
if (!constructorParameters[i].Type.Equals(argumentTypes2[i]))
if (!constructorParameters[i].Type.Equals(argumentTypes2[i], SymbolEqualityComparer.IncludeNullability))
{
return false;
}
Expand All @@ -114,7 +131,7 @@ private static bool AreParametersMatching(ImmutableArray<IParameterSymbol> const
return true;
}

private static GenericNameSyntax GetGenericNameSyntax(TypeSyntax typeSyntax)
private static GenericNameSyntax? GetGenericNameSyntax(TypeSyntax typeSyntax)
{
if (typeSyntax is GenericNameSyntax genericNameSyntax)
{
Expand All @@ -131,15 +148,19 @@ private static GenericNameSyntax GetGenericNameSyntax(TypeSyntax typeSyntax)

private static bool IsMockGenericType(GenericNameSyntax genericName)
{
return genericName?.Identifier.Text == "Mock" && genericName.TypeArgumentList.Arguments.Count == 1;
return string.Equals(genericName.Identifier.Text, "Mock", StringComparison.Ordinal)
&& genericName.TypeArgumentList.Arguments.Count == 1;
}

private static IMethodSymbol GetConstructorSymbol(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreation)
private static IMethodSymbol? GetConstructorSymbol(SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreation)
{
var constructorSymbolInfo = context.SemanticModel.GetSymbolInfo(objectCreation, context.CancellationToken);
var constructorSymbol = constructorSymbolInfo.Symbol as IMethodSymbol;
return constructorSymbol?.MethodKind == MethodKind.Constructor &&
constructorSymbol.ContainingType?.ConstructedFrom.ToDisplayString() == "Moq.Mock<T>"
string.Equals(
constructorSymbol.ContainingType?.ConstructedFrom.ToDisplayString(),
"Moq.Mock<T>",
StringComparison.Ordinal)
? constructorSymbol
: null;
}
Expand Down
1 change: 0 additions & 1 deletion Source/Moq.Analyzers/Diagnostics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ internal static class Diagnostics
internal const string NoMethodsInPropertySetupTitle = "Moq: Property setup used for a method";
internal const string NoMethodsInPropertySetupMessage = "SetupGet/SetupSet should be used for properties, not for methods.";


internal const string SetupShouldBeUsedOnlyForOverridableMembersId = "Moq1200";
internal const string SetupShouldBeUsedOnlyForOverridableMembersTitle = "Moq: Invalid setup parameter";
internal const string SetupShouldBeUsedOnlyForOverridableMembersMessage = "Setup should be used only for overridable members.";
Expand Down
Loading

0 comments on commit 7a45b15

Please sign in to comment.