refactor: eliminate DRY violations across analyzer pairs

[rjmurillo-bot]

Summary

Resolves #980. Extracts duplicated logic from three analyzer groups without changing behavior.

• Overridable member check: IsOverridableOrAllowedMockMember extracted to ISymbolExtensions, replacing identical private methods (IsPropertyOrMethod, IsOverridableOrTaskResultMember, IsAllowedMockMember) in SetupShouldBeUsedOnlyForOverridableMembersAnalyzer, SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer, and VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer
• MockBehavior reporting: Added TryReportMockBehaviorDiagnostic and TryHandleMissingMockBehaviorParameter overloads with messageArgs plus shared GetMockedTypeName to MockBehaviorDiagnosticAnalyzerBase, removing duplicate methods from SetExplicitMockBehaviorAnalyzer and SetStrictMockBehaviorAnalyzer
• Event argument validation: Replaced RaiseEventArgumentsShouldMatchEventSignatureAnalyzer.ValidateArgumentTypesWithEventName with existing EventSyntaxExtensions.ValidateEventArgumentTypes

Net result: 122 lines added, 285 lines removed (163 lines reduced).

Test plan

• All 2901 existing tests pass with zero failures
• Build succeeds with zero warnings
• No diagnostic IDs, messages, or categories changed
• No analyzer behavior changed, only structure

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Consolidated mock-behavior diagnostic reporting to a single path with support for formatted messages and overload-aware reporting.
  • Replaced many inlined helpers with shared reporting/handling flows for strict/explicit mock analyses.
  • Switched event-argument validation to context-based extension calls for consistency.
  • Centralized overridable/allowed-member checks into a single symbol extension used by setup, verify, and sequence analyzers.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Warning

Rate limit exceeded

@rjmurillo-bot has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 17 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 01b30bae-90db-4ef4-81c3-db5eb03febff

📥 Commits

Reviewing files that changed from the base of the PR and between 1d5024d and 7a3e91e.

📒 Files selected for processing (7)

• src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs
• src/Analyzers/RaisesEventArgumentsShouldMatchEventSignatureAnalyzer.cs
• src/Analyzers/SetStrictMockBehaviorAnalyzer.cs
• src/Analyzers/SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
• src/Common/EventSyntaxExtensions.cs
• src/Common/ISymbolExtensions.Moq.cs
• src/Common/ISymbolExtensions.cs

📝 Walkthrough

Walkthrough

Centralizes mock-behavior diagnostic/reporting and mocked-type extraction in the analyzer base, converts event-argument validators into context extension methods, and consolidates overridability checks into a shared ISymbol extension; callers and duplicate helpers were removed or updated accordingly.

Changes

Cohort / File(s)	Summary
Base analyzer infrastructure src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs	Added GetMockedTypeName(IOperation, IMethodSymbol), overloads of TryReportMockBehaviorDiagnostic(..., params object[] messageArgs) and TryHandleMissingMockBehaviorParameter(..., params object[] messageArgs) to unify diagnostic formatting, message-argument propagation, and overload-resolution reporting.
Mock behavior analyzers src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs, src/Analyzers/SetStrictMockBehaviorAnalyzer.cs	Replaced in-file mocked-type extraction and reporting with calls into the new base helpers; removed local helper implementations and unified diagnostic/code-fix metadata flow (uses Operation and messageArgs).
Overridable-member checks src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs, src/Analyzers/SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs, src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs	Replaced multiple private helper predicates with mockedMemberSymbol.IsOverridableOrAllowedMockMember(knownSymbols) and removed the duplicated helpers.
Event argument validation src/Analyzers/RaiseEventArgumentsShouldMatchEventSignatureAnalyzer.cs, src/Analyzers/RaisesEventArgumentsShouldMatchEventSignatureAnalyzer.cs	Switched to invoking ValidateEventArgumentTypes via the SyntaxNodeAnalysisContext extension (context.ValidateEventArgumentTypes(...)); removed the duplicated inline validation helper from the RaiseEvent analyzer.
Common extensions src/Common/EventSyntaxExtensions.cs, src/Common/ISymbolExtensions.cs	Made ValidateEventArgumentTypes an extension on SyntaxNodeAnalysisContext and added IsOverridableOrAllowedMockMember(this ISymbol, MoqKnownSymbols) to centralize overridability and Task/ValueTask Result-property checks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Add IOperation to DiagnosticExtensions and clean up overloads #233 — Adds IOperation-based diagnostic helpers and messageArgs-aware CreateDiagnostic overloads; aligns with the base analyzer reporting changes.
• Add new rule to enforce MockBehavior.Strict (Moq1410) #302 — Modifies MockBehaviorDiagnosticAnalyzerBase and related reporting logic; closely overlaps the refactor here.
• Refactor SetExplicitMockBehaviorAnalyzer to use KnownSymbols and add CodeFix #296 — Changes SetExplicit analyzer reporting and overload-resolution handling for missing MockBehavior parameters; directly related to call-site updates.

Suggested reviewers

• MattKotsenas
• rjmurillo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.52% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: refactoring to eliminate DRY violations across analyzer pairs, which is the core objective of the PR.
Description check	✅ Passed	The description comprehensively covers the motivation (issue #980), changes made across three analyzer groups with specific details, type of change (Refactoring), test results, and measurable improvements (163 lines reduced).
Linked Issues check	✅ Passed	All coding requirements from issue #980 are met: IsOverridableOrAllowedMockMember extracted [#980], MockBehavior reporting helpers centralized [#980], and RaiseEvent validation refactored [#980].
Out of Scope Changes check	✅ Passed	All changes are within scope: method extractions to base classes, extension method conversions, and private helper consolidations directly address the three DRY violations identified in #980.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/980-dry-analyzer-pairs

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[deepsource-io]

DeepSource Code Review

We reviewed changes in 5bc448e...7a3e91e on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Focus Area: Hygiene

Security   Reliability   Complexity   Hygiene

Feedback

• Inconsistent public API parameter ordering
  • The parameter-order mistake in the extension indicates a wider lack of API conventions; callers can accidentally pass swapped arguments. Enforce a canonical parameter order for event extensions and add compile-time checks to prevent regressions.
• Missing static analysis for API shape
  • This slipped because no analyzer enforces signature rules across files; minor ordering bugs survive routine edits. Introduce Roslyn rules to validate extension 'this' placement and parameter ordering during the build.
• Tests don't assert public API contracts
  • Behavioural tests miss API-shape regressions because no reflection-based assertions exist for signatures. Add unit tests that assert extension method parameter order and fail loudly when the public contract deviates.

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
C#		Mar 8, 2026 6:31p.m.	Review ↗

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Analyzers/SetStrictMockBehaviorAnalyzer.cs (1)

34-34: 🧹 Nitpick | 🔵 Trivial

Consider aligning variable naming with SetExplicitMockBehaviorAnalyzer.

This file uses mockedTypeName while SetExplicitMockBehaviorAnalyzer uses typeName. Consider unifying to one naming convention across both analyzers for consistency. mockedTypeName is more descriptive; alternatively, both could use typeName for brevity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Analyzers/SetStrictMockBehaviorAnalyzer.cs` at line 34, The variable name
mockedTypeName in SetStrictMockBehaviorAnalyzer should be renamed to match the
convention used in SetExplicitMockBehaviorAnalyzer (typeName) for consistency;
update the declaration and every usage of mockedTypeName within
SetStrictMockBehaviorAnalyzer (including references in GetMockedTypeName calls,
diagnostic messages, and any symbolic comparisons) to typeName, and run
tests/compile to ensure no references remain to mockedTypeName; alternatively,
if you prefer the more descriptive name, change typeName in
SetExplicitMockBehaviorAnalyzer to mockedTypeName in the same consistent way
across both analyzers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs`:
- Around line 17-18: Update the XML doc comments in
MockBehaviorDiagnosticAnalyzerBase so type references use <see cref="..."/>
instead of plain text: change the <param name="operation"> and <param
name="target"> descriptions to reference their types with <see cref="..."/>
(e.g., <see cref="IOperation"/> or the actual symbol type and <see
cref="IMethodSymbol"/> or the actual method symbol type) and ensure the tags
reference the exact symbol names used in the code (operation, target) for proper
IntelliSense linking.

In `@src/Common/ISymbolExtensions.cs`:
- Around line 279-300: The switch in
ISymbolExtensions.IsOverridableOrAllowedMockMember currently ignores
IEventSymbol so virtual events (surfaced by
MoqVerificationHelpers.TryGetMockedMemberSymbol for SetupAdd/SetupRemove) are
treated as non-overridable; update IsOverridableOrAllowedMockMember to handle
IEventSymbol (call eventSymbol.IsOverridable()) in the switch alongside
IPropertySymbol/IMethodSymbol and leave other cases false, and add a regression
unit test that exercises SetupAdd/SetupRemove on a virtual event to ensure the
analyzer no longer reports a false positive.

---

Outside diff comments:
In `@src/Analyzers/SetStrictMockBehaviorAnalyzer.cs`:
- Line 34: The variable name mockedTypeName in SetStrictMockBehaviorAnalyzer
should be renamed to match the convention used in
SetExplicitMockBehaviorAnalyzer (typeName) for consistency; update the
declaration and every usage of mockedTypeName within
SetStrictMockBehaviorAnalyzer (including references in GetMockedTypeName calls,
diagnostic messages, and any symbolic comparisons) to typeName, and run
tests/compile to ensure no references remain to mockedTypeName; alternatively,
if you prefer the more descriptive name, change typeName in
SetExplicitMockBehaviorAnalyzer to mockedTypeName in the same consistent way
across both analyzers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 862198cb-5e27-4007-abdd-5c6f23e7c5e0

📥 Commits

Reviewing files that changed from the base of the PR and between e8a1411 and ab1d7ab.

📒 Files selected for processing (10)

• src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs
• src/Analyzers/RaiseEventArgumentsShouldMatchEventSignatureAnalyzer.cs
• src/Analyzers/RaisesEventArgumentsShouldMatchEventSignatureAnalyzer.cs
• src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs
• src/Analyzers/SetStrictMockBehaviorAnalyzer.cs
• src/Analyzers/SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
• src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
• src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
• src/Common/EventSyntaxExtensions.cs
• src/Common/ISymbolExtensions.cs

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs (1)

45-54: ⚠️ Potential issue | 🔴 Critical

Fix member ordering to resolve SA1202 build failure.

The build is failing due to SA1202: internal members (starting at line 65) are placed after the private protected member AnalyzeCore at line 54. StyleCop requires accessibility ordering: public → internal → private protected → private.

🔧 Proposed fix: Reorder members

Move AnalyzeCore below all internal methods. The correct order should be:

1. public override void Initialize (public)
2. internal static string GetMockedTypeName (internal static)
3. internal bool TryReportMockBehaviorDiagnostic overloads (internal)
4. internal bool TryHandleMissingMockBehaviorParameter overloads (internal)
5. private protected abstract void AnalyzeCore (private protected)
6. private void RegisterCompilationStartAction (private)
7. private void AnalyzeObjectCreation (private)
8. private void AnalyzeInvocation (private)

     /// <inheritdoc />
     public override void Initialize(AnalysisContext context)
     {
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
         context.EnableConcurrentExecution();

         context.RegisterCompilationStartAction(RegisterCompilationStartAction);
     }

-    private protected abstract void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols);
-
     /// <summary>
     /// Attempts to report a diagnostic for a MockBehavior parameter issue.
     /// </summary>
     // ... TryReportMockBehaviorDiagnostic methods ...
     // ... TryHandleMissingMockBehaviorParameter methods ...

+    private protected abstract void AnalyzeCore(OperationAnalysisContext context, IMethodSymbol target, ImmutableArray<IArgumentOperation> arguments, MoqKnownSymbols knownSymbols);
+
     private void RegisterCompilationStartAction(CompilationStartAnalysisContext context)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs` around lines 45 - 54,
SA1202 is triggered because the private protected member AnalyzeCore is declared
before internal members; move the declaration of AnalyzeCore (private protected
abstract void AnalyzeCore(OperationAnalysisContext, IMethodSymbol,
ImmutableArray<IArgumentOperation>, MoqKnownSymbols)) so it appears after the
internal members (internal static string GetMockedTypeName, the internal
TryReportMockBehaviorDiagnostic overloads, and the internal
TryHandleMissingMockBehaviorParameter overloads) and before the private methods
(RegisterCompilationStartAction, AnalyzeObjectCreation, AnalyzeInvocation) to
restore the required accessibility ordering.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs`:
- Around line 45-54: SA1202 is triggered because the private protected member
AnalyzeCore is declared before internal members; move the declaration of
AnalyzeCore (private protected abstract void
AnalyzeCore(OperationAnalysisContext, IMethodSymbol,
ImmutableArray<IArgumentOperation>, MoqKnownSymbols)) so it appears after the
internal members (internal static string GetMockedTypeName, the internal
TryReportMockBehaviorDiagnostic overloads, and the internal
TryHandleMissingMockBehaviorParameter overloads) and before the private methods
(RegisterCompilationStartAction, AnalyzeObjectCreation, AnalyzeInvocation) to
restore the required accessibility ordering.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9416b927-9061-49bb-a27d-2d7026b9fbe0

📥 Commits

Reviewing files that changed from the base of the PR and between 503330a and 1d5024d.

📒 Files selected for processing (6)

• src/Analyzers/MockBehaviorDiagnosticAnalyzerBase.cs
• src/Analyzers/RaisesEventArgumentsShouldMatchEventSignatureAnalyzer.cs
• src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs
• src/Analyzers/SetStrictMockBehaviorAnalyzer.cs
• src/Analyzers/SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs
• src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.29% (target: -1.00%)	❌ 79.73% (target: 95.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (5bc448e)	2531	2282	90.16%
Head commit (7a3e91e)	2488 (-43)	2236 (-46)	89.87% (-0.29%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#1044)	74	59	79.73%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences