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’ll 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 all 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;
using Microsoft.CodeAnalysis.Operations;

namespace Moq.Analyzers;

/// <summary>
Expand Down Expand Up @@ -28,23 +31,70 @@ public override void Initialize(AnalysisContext context)
{
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)
{
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
Copy link
Copy Markdown
Contributor

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

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
15 changes: 15 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 @@ internal MoqKnownSymbols(Compilation compilation)
/// </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
9 changes: 9 additions & 0 deletions tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ namespace Moq.Analyzers.Test.Helpers;
/// </remarks>
public static class ReferenceAssemblyCatalog
{
/// <summary>
/// Gets the name of the reference assembly group for .NET 8.0 without Moq.
/// Used to test analyzer behavior when Moq is not referenced.
/// </summary>
public static string Net80 => nameof(Net80);

/// <summary>
/// Gets the name of the reference assembly group for .NET 8.0 with an older version of Moq (4.8.2).
/// </summary>
Expand All @@ -32,6 +38,9 @@ public static class ReferenceAssemblyCatalog
/// </remarks>
public static IReadOnlyDictionary<string, ReferenceAssemblies> Catalog { get; } = new Dictionary<string, ReferenceAssemblies>(StringComparer.Ordinal)
{
// .NET 8.0 without Moq, used to verify analyzers short-circuit when Moq is not referenced.
{ nameof(Net80), ReferenceAssemblies.Net.Net80 },

Comment on lines +41 to +43
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update Catalog XML remarks to include the new Net80 key.

After adding Net80 to the map, the Catalog remarks still list only two key names, so the public API docs are now stale.

📝 Suggested doc fix 
 /// <remarks>
-/// The key is the name of the reference assembly group (<see cref="Net80WithOldMoq"/> and <see cref="Net80WithNewMoq"/>).
+/// The key is the name of the reference assembly group
+/// (<see cref="Net80"/>, <see cref="Net80WithOldMoq"/>, and <see cref="Net80WithNewMoq"/>).
 /// </remarks>

As per coding guidelines, "All public APIs must have complete XML documentation with accurate, up-to-date content."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs` around lines 41
- 43, The XML <remarks> for the public Catalog (in ReferenceAssemblyCatalog) is
out of date after adding the new key nameof(Net80) =>
ReferenceAssemblies.Net.Net80; update the Catalog XML remarks to list the Net80
key alongside the existing keys so the public API docs accurately reflect the
available map entries (mention the symbol Net80 and the map entry nameof(Net80)
/ ReferenceAssemblies.Net.Net80 when editing the remarks).

// 4.8.2 was one of the first popular versions of Moq. Ensure this version is prior to 4.13.1, as it changed the internal
// implementation of `.As<T>()` (see https://github.com/devlooped/moq/commit/b552aeddd82090ee0f4743a1ab70a16f3e6d2d11).
{ nameof(Net80WithOldMoq), ReferenceAssemblies.Net.Net80.AddPackages([new PackageIdentity("Moq", "4.8.2")]) },
Expand Down
28 changes: 28 additions & 0 deletions tests/Moq.Analyzers.Test/NoMethodsInPropertySetupAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Microsoft.CodeAnalysis.Testing;

Check warning on line 1 in tests/Moq.Analyzers.Test/NoMethodsInPropertySetupAnalyzerTests.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Test/NoMethodsInPropertySetupAnalyzerTests.cs#L1 

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

using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.NoMethodsInPropertySetupAnalyzer>;

namespace Moq.Analyzers.Test;
Expand Down Expand Up @@ -61,6 +63,32 @@
ReferenceAssemblyCatalog.Net80WithNewMoq);
}

[Fact]
public async Task ShouldNotAnalyzeWhenMoqNotReferenced()
{
await Verifier.VerifyAnalyzerAsync(
"""
namespace Test
{
public interface IFoo
{
string Prop1 { get; set; }
string Method();
}

public class UnitTest
{
private void Test()
{
var x = new object();
}
}
}
""",
ReferenceAssemblyCatalog.Net80,
CompilerDiagnostics.None);
}

[Fact]
public async Task ShouldIncludeMethodNameInDiagnosticMessage()
{
Expand Down
Loading