Skip to content

docs: document generic callback limitation in Moq1100#769

Merged
rjmurillo merged 2 commits intomainfrom
copilot/enhance-moq1100-validation
Oct 12, 2025
Merged

docs: document generic callback limitation in Moq1100#769
rjmurillo merged 2 commits intomainfrom
copilot/enhance-moq1100-validation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 11, 2025

Overview

This PR addresses issue #[issue-number] by documenting a known limitation in the CallbackSignatureShouldMatchMockedMethodAnalyzer (Moq1100) rather than implementing validation for generic callback syntax.

Background

The issue requested validation for .Callback<T>() generic type parameters to catch mismatches at compile time. However, maintainer analysis revealed:

  • Zero related issues in devlooped/moq about generic callback validation
  • Zero real-world code samples using .Callback<T>() syntax in open-source projects
  • The explicit generic syntax is rarely used in practice
  • An effective workaround exists: use lambda parameter inference

Given these findings, the decision was made to document the limitation rather than implement validation, following the principle of avoiding unnecessary complexity for edge cases with negligible real-world impact.

Changes

1. Documentation Update (docs/rules/Moq1100.md)

Added a new "Known Limitations" section that:

  • Explicitly documents that .Callback<T>() type parameter mismatches are NOT validated
  • Provides clear examples showing the limitation:
// This will NOT trigger a diagnostic (known limitation)
mock.Setup(x => x.DoWork("test")) // DoWork takes a string parameter
    .Callback<int>(wrongTypeParam => { }); // Using int instead - no warning!
  • Recommends best practices with full type validation:
// ✅ Recommended: Let the compiler infer parameter types
mock.Setup(x => x.DoWork("test"))
    .Callback(param => { }); // Type is inferred correctly as string

// ✅ Also recommended: Explicitly type the lambda parameter
mock.Setup(x => x.DoWork("test"))
    .Callback((string param) => { }); // Type validation works correctly
  • Explains the rationale backed by research findings

2. Test Documentation Update

Updated GenericCallbackValidation_CurrentLimitation_IsDocumented test to reflect this as an accepted limitation:

  • Changed language from "could be enhanced in a future version" to "accepted limitation"
  • Added detailed remarks explaining zero real-world usage
  • Referenced the documentation for best practices
  • Updated inline comments for clarity

Impact

Positive:

  • Developers are now explicitly informed about the limitation before encountering it
  • Clear guidance prevents confusion and provides actionable alternatives
  • No breaking changes or maintenance burden
  • Documentation can be easily updated if community demand emerges

Testing:

  • ✅ All 1,874 existing tests pass
  • ✅ Build succeeds with 0 warnings
  • ✅ Code formatting verified
  • ✅ No analyzer behavior changes

Alignment with Issue

While the original issue requested implementation of validation, this documentation-focused approach:

  1. Addresses the core concern (preventing developer surprise)
  2. Provides actionable guidance with working examples
  3. Avoids unnecessary complexity for a rarely-used pattern
  4. Can be revisited if community demand emerges

This aligns with best practices for analyzer development: focus implementation effort on patterns that have demonstrated real-world impact.

Related

Original prompt

This section details on the original issue you should resolve

<issue_title>Enhance Moq1100: Add validation for generic .Callback() type mismatches</issue_title>
<issue_description>## User Story
As a developer using Moq with callback setups, I want the analyzer to validate that generic type parameters match the mocked method signature, so that I can catch type mismatches at compile time instead of runtime.

Problem Statement

The CallbackSignatureShouldMatchMockedMethodAnalyzer (Moq1100) currently does not validate generic callback signatures when using . This can lead to runtime errors that should be caught during static analysis.

Current Gap

Acceptance Criteria

  • Analyzer detects type mismatches in where T doesn't match the mocked method parameters
  • Diagnostic Moq1100 is reported for generic callback type mismatches
  • Validation works for single and multiple type parameters
  • Performance impact is minimized given existing concerns with this analyzer

INVEST Criteria

  • Independent: Can be implemented without dependencies on other features
  • Negotiable: Implementation approach can be discussed (could be separate diagnostic if needed)
  • Valuable: Prevents runtime failures, improves developer experience
  • Estimable: Moderate complexity - extends existing callback validation logic
  • Small: Focused enhancement to existing analyzer
  • Testable: Clear pass/fail criteria with concrete test cases

Technical Notes

  • Extends existing CallbackSignatureShouldMatchMockedMethodAnalyzer
  • Current limitation documented in tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs:100
  • Consider performance impact given known issues with this analyzer

References

Documentation Links

<agent_instructions>User Impact & Necessity
Real-World Usage Evidence:
After searching the devlooped/moq repository (the correct upstream Moq project) and broader open-source code:

Zero related issues found in devlooped/moq discussing generic callback type parameter validation
Zero real-world code samples found using .Callback() syntax with Moq in open-source projects
The search found only 3 global code samples for .Callback< syntax, none of which were Moq-related
Issue #445 in devlooped/moq addressed callback signature validation at a high level (parameter count/types), but was closed in 2017 with Moq 4.8.0
No evidence of user pain or demand for this specific enhancement
Severity Assessment:

Current limitation is documented in your own test suite (lines 97-127)
Runtime failure mode is clear: InvalidCastException at test runtime
Workaround exists: Use lambda parameter inference (.Callback(x => { })) instead of explicit generic syntax (.Callback(x => { }))
Who is Affected:

Developers who explicitly use .Callback() syntax and provide wrong type parameters
This appears to be an edge case—the preferred pattern (lambda inference) already works correctly
Implementation & Maintenance
Complexity Assessment (Moderate):
Based on analyzer source code review:

Implementation location: Lines 45-50 in CallbackSignatureShouldMatchMockedMethodAnalyzer.cs
Required work: Extract generic type arguments from GenericNameSyntax and validate against lambda parameter types
Estimated effort: 3-5 days (including tests, documentation, code review)
Test expansion needed: Add data-driven tests to CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs following existing patterns
Maintenance Burden:

Low ongoing maintenance once implemented
Follows established analyzer patterns
Well-tested via data-driven test infrastructure (58+ callback scenarios already in place from PR #580)
Alignment with Project Goals
Current Project Status:

Issue #434 (parent analyzer evaluation project) is 93% complete (14/15 sub-issues done)
PR #580 (merged July 27, 2025) added comprehensive callback validation tests
This enhancement is the final 7% to complete issue #434
Priority Assessment:

Completes a major milestone (issue #434)
However, zero evidence of user demand despite Moq4's large user base
Alternative solution exists (lambda parameter inference)
Trade-offs & Risks
Benefits:

Completes issue #434 (comprehensive callback validation)
Provides earlier failure detection (compile-time vs runtime)
Consistency with other Moq analyzers

Complexity Assessment:

From analyzing CallbackSignatureShouldMatchMockedMethodAnalyzer.cs:

Current analyzer validates lambda parameter types and ref/out/in modifiers (lines 145-175)
Missi...

Fixes #622


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 11, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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

Co-authored-by: rjmurillo <6811113+rjmurillo@users.noreply.github.com>
Copilot AI changed the title [WIP] Add validation for generic .Callback<T>() type mismatches docs: document generic callback limitation in Moq1100 Oct 11, 2025
Copilot AI requested a review from rjmurillo October 11, 2025 15:07
@rjmurillo rjmurillo marked this pull request as ready for review October 11, 2025 15:39
Copilot AI review requested due to automatic review settings October 11, 2025 15:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR documents a known limitation in the Moq1100 analyzer regarding generic callback validation rather than implementing the feature. The decision was made after research revealed zero real-world usage of the .Callback<T>() syntax in open-source projects, making documentation a more pragmatic approach than implementation.

Key Changes:

  • Added "Known Limitations" section to Moq1100 documentation with examples and best practices
  • Updated test comments to reflect this as an accepted limitation rather than future enhancement
  • Provided clear guidance on recommended alternatives using lambda parameter inference

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
docs/rules/Moq1100.md Added comprehensive "Known Limitations" section documenting the generic callback validation gap with examples and best practices
tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs Updated test documentation to reflect accepted limitation status with detailed remarks

@codacy-production
Copy link
Copy Markdown

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) (target: 95.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (3b16ea4) 1999 1772 88.64%
Head commit (b18d2d7) 1999 (+0) 1772 (+0) 88.64% (+0.00%)

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 (#769) 0 0 ∅ (not applicable)

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

@rjmurillo rjmurillo merged commit 8f0f043 into main Oct 12, 2025
26 checks passed
@rjmurillo rjmurillo deleted the copilot/enhance-moq1100-validation branch October 12, 2025 16:55
@rjmurillo rjmurillo added this to the vNext milestone Oct 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhance Moq1100: Add validation for generic .Callback<T>() type mismatches

3 participants