Skip to content

refactor: convert NoMethodsInPropertySetupAnalyzer to IOperation #955

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rjmurillo merged 5 commits into from
Mar 1, 2026
Merged

refactor: convert NoMethodsInPropertySetupAnalyzer to IOperation #955

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
187a7c7
refactor: convert NoMethodsInPropertySetupAnalyzer to IOperation
rjmurillo Mar 1, 2026
db78020
fix: handle nullable SemanticModel defensively
rjmurillo Mar 1, 2026
681e023
fix: remove unrelated ILogger symbols from MoqKnownSymbols
rjmurillo Mar 1, 2026
56866d6
test: add coverage for Moq-not-referenced guard branch
rjmurillo Mar 1, 2026
962f7b5
test: suppress compiler diagnostics for Moq-not-referenced test
rjmurillo Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 61 additions & 11 deletions src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
using System.Diagnostics;

Check warning on line 1 in src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs#L1 

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

/// <summary>
Expand Down Expand Up @@ -28,23 +31,70 @@
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);

context.RegisterCompilationStartAction(RegisterCompilationStartAction);
}

private static void RegisterCompilationStartAction(CompilationStartAnalysisContext context)
{
MoqKnownSymbols knownSymbols = new(context.Compilation);

if (!knownSymbols.IsMockReferenced())
{
return;
}

ImmutableArray<IMethodSymbol> propertySetupMethods = ImmutableArray.CreateRange([
..knownSymbols.Mock1SetupGet,
..knownSymbols.Mock1SetupSet,
..knownSymbols.Mock1SetupProperty]);

if (propertySetupMethods.IsEmpty)
{
return;
}

context.RegisterOperationAction(
operationAnalysisContext => Analyze(operationAnalysisContext, propertySetupMethods),
OperationKind.Invocation);
}

private static void Analyze(SyntaxNodeAnalysisContext context)
private static void Analyze(OperationAnalysisContext context, ImmutableArray<IMethodSymbol> propertySetupMethods)

Check warning on line 62 in src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs#L62 

Method NoMethodsInPropertySetupAnalyzer::Analyze has a cyclomatic complexity of 9 (limit is 8)
{
InvocationExpressionSyntax setupGetOrSetInvocation = (InvocationExpressionSyntax)context.Node;
Debug.Assert(context.Operation is IInvocationOperation, "Expected IInvocationOperation");

if (context.Operation is not IInvocationOperation invocationOperation)
{
return;
}

IMethodSymbol targetMethod = invocationOperation.TargetMethod;
if (!targetMethod.IsInstanceOf(propertySetupMethods))
{
return;
}

// The lambda argument to SetupGet/SetupSet/SetupProperty contains the mocked member access.
// If the lambda body is an invocation (method call), that is invalid for property setup.
InvocationExpressionSyntax? mockedMethodCall =
(invocationOperation.Syntax as InvocationExpressionSyntax).FindMockedMethodInvocationFromSetupMethod();

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)
&& !string.Equals(setupGetOrSetMethod.Name.ToFullString(), "SetupProperty", StringComparison.Ordinal)) return;
if (mockedMethodCall == null)
{
return;
}

InvocationExpressionSyntax? mockedMethodCall = setupGetOrSetInvocation.FindMockedMethodInvocationFromSetupMethod();
if (mockedMethodCall == null) return;
SemanticModel? semanticModel = invocationOperation.SemanticModel;
if (semanticModel == null)
{
return;
}

ISymbol? mockedMethodSymbol = context.SemanticModel.GetSymbolInfo(mockedMethodCall, context.CancellationToken).Symbol;
if (mockedMethodSymbol == null) return;
ISymbol? mockedMethodSymbol = semanticModel.GetSymbolInfo(mockedMethodCall, context.CancellationToken).Symbol;
if (mockedMethodSymbol == null)
{
return;
Comment on lines +77 to +96

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

This implementation for finding the mocked method call mixes IOperation with SyntaxNode analysis by falling back to FindMockedMethodInvocationFromSetupMethod. This partially defeats the purpose of refactoring to IOperation.

Additionally, the FindMockedMethodInvocationFromSetupMethod helper only works for expression-bodied lambdas (x => x.Method()) and will fail for block-bodied lambdas (x => { return x.Method(); }), as LambdaExpressionSyntax.Body would be a BlockSyntax.

You can implement this logic purely using the IOperation tree, which will be more robust and align better with the refactoring goal. This also provides an opportunity to correctly handle block-bodied lambdas.

        // The lambda argument to SetupGet/SetupSet/SetupProperty contains the mocked member access.
        // If the lambda body is an invocation (method call), that is invalid for property setup.
        if (invocationOperation.Arguments.Length == 0)
        {
            return;
        }

        IOperation argument = invocationOperation.Arguments[0].Value.WalkDownImplicitConversion();
        if (argument is not IAnonymousFunctionOperation lambda)
        {
            return;
        }

        IOperation? body = lambda.Body;
        if (body is IBlockOperation block && block.Operations.Length == 1 && block.Operations[0] is IReturnOperation returnOp)
        {
            body = returnOp.ReturnedValue;
        }

        if (body is IInvocationOperation mockedMethodInvocation)
        {
            IMethodSymbol mockedMethodSymbol = mockedMethodInvocation.TargetMethod;
            Diagnostic diagnostic = mockedMethodInvocation.Syntax.CreateDiagnostic(Rule, mockedMethodSymbol.Name);
            context.ReportDiagnostic(diagnostic);
        }

}

Diagnostic diagnostic = mockedMethodCall.CreateDiagnostic(Rule, mockedMethodSymbol.Name);
context.ReportDiagnostic(diagnostic);
Expand Down
26 changes: 26 additions & 0 deletions src/Common/WellKnown/MoqKnownSymbols.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@
/// </summary>
internal ImmutableArray<IMethodSymbol> Mock1Setup => Mock1?.GetMembers("Setup").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.Mock{T}.SetupGet</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> Mock1SetupGet => Mock1?.GetMembers("SetupGet").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.Mock{T}.SetupSet</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> Mock1SetupSet => Mock1?.GetMembers("SetupSet").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.Mock{T}.SetupProperty</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> Mock1SetupProperty => Mock1?.GetMembers("SetupProperty").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.Mock{T}.SetupAdd</c>.
/// </summary>
Expand Down Expand Up @@ -403,4 +418,15 @@
/// Gets the methods for <c>Moq.Times.Exactly</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> TimesExactly => Times?.GetMembers("Exactly").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the interface <c>Microsoft.Extensions.Logging.ILogger</c>.
/// </summary>
internal INamedTypeSymbol? ILogger => TypeProvider.GetOrCreateTypeByMetadataName("Microsoft.Extensions.Logging.ILogger");

/// <summary>
/// Gets the interface <c>Microsoft.Extensions.Logging.ILogger{T}</c>.
/// </summary>
internal INamedTypeSymbol? ILogger1 => TypeProvider.GetOrCreateTypeByMetadataName("Microsoft.Extensions.Logging.ILogger`1");

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These additions for ILogger and ILogger<T> seem unrelated to the primary goal of this pull request, which is to refactor NoMethodsInPropertySetupAnalyzer. To maintain clean and focused commits, it would be better to move these changes to a separate pull request.


}

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (ubuntu-24.04-arm)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (ubuntu-24.04-arm)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (ubuntu-24.04-arm)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (ubuntu-24.04-arm)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / build

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / build

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / build

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / build

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (windows-latest)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (windows-latest)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (windows-latest)

Check failure on line 432 in src/Common/WellKnown/MoqKnownSymbols.cs

View workflow job for this annotation

GitHub Actions / test (windows-latest)

Loading