Skip to content

docs(Moq1300): add guidance for generic type parameter scenarios#762

Merged
rjmurillo merged 3 commits intomainfrom
docs/756-Moq1300-FP
Oct 11, 2025
Merged

docs(Moq1300): add guidance for generic type parameter scenarios#762
rjmurillo merged 3 commits intomainfrom
docs/756-Moq1300-FP

Conversation

@rjmurillo
Copy link
Copy Markdown
Owner

@rjmurillo rjmurillo commented Oct 10, 2025

Summary

Enhances the Moq1300 documentation to provide clear guidance for handling generic type parameters constrained to interfaces, addressing the scenario reported in #756.

Changes

Documentation Updates

  • Added new "Advanced Scenarios: Generic Type Parameters" section to docs/rules/Moq1300.md
  • Explains why Moq1300 fires for generic type parameters (C# cannot enforce "interface-only" constraints)
  • Provides clear criteria for when suppression is appropriate vs. when to avoid it
  • Includes practical suppression example with inline comments
  • Documents the maintainer's reconsideration criteria for potential future changes

Key Improvements

  1. Clarified the C# language limitation: Explains that where T : IFoo allows both interfaces and classes implementing IFoo, which creates the diagnostic scenario
  2. Safety-first approach: Maintains the analyzer's error severity while providing guidance for legitimate use cases
  3. Structured guidance: Clear "When Suppression is Appropriate" and "When to Avoid Suppression" sections
  4. Transparency: References issue Mock.As<T> triggers Moq1300 (Mock.As<T> should be interface) in generic method #756 and documents conditions under which this guidance may be reconsidered

Rationale

This documentation-first approach was chosen over implementing a separate diagnostic due to:

  1. Pattern rarity: Single reported instance of an admittedly unusual scenario
  2. Resource allocation: For a resource-constrained OSS project, enhanced documentation is more efficient than multi-file analyzer changes
  3. Language limitation: C# constraints cannot provide compile-time safety for "interface-only" regardless of diagnostic severity
  4. Existing precedent: Similar scenarios are already handled via documented suppression guidance
  5. Safety trade-off: Error severity prevents runtime failures while suppression provides flexibility for controlled scenarios

Closes #756

CC @andrewimcclement

Summary by CodeRabbit

  • Documentation
    • Added an "Advanced Scenarios" subsection covering generic type parameters constrained to interfaces and when the rule may trigger.
    • Added rationale and guidance on appropriate suppression vs. avoiding suppression.
    • Included an example showing suppression with preprocessor directives and notes on per-file and configuration-based suppression.
    • Added a reference note to a related issue.

Copilot AI review requested due to automatic review settings October 10, 2025 21:50
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 10, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Added an "Advanced Scenarios: Generic Type Parameters" subsection to docs/rules/Moq1300.md explaining Moq1300 behavior for generic type parameters constrained to interfaces, expanded suppression guidance (source-file and config options) with a pragma example, and referenced the linked issue. No API changes.

Changes

Cohort / File(s) Summary
Docs: Moq1300 rule
docs/rules/Moq1300.md
Appended an "Advanced Scenarios: Generic Type Parameters" subsection describing Moq1300 triggering for generic type parameters constrained to interfaces and guidance for suppression; expanded "Suppress a warning" with source-file and configuration approaches; added a #pragma warning suppression example and referenced linked issue; documentation-only change.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The pull request only adds documentation guidance for generic type parameter scenarios and does not implement the requested analyzer behavior change to ignore or produce a separate diagnostic for Mock.As when T is a generic type parameter, so it does not satisfy the coding requirements of issue #756. To address issue #756, the Moq1300 analyzer should be updated to suppress or create a distinct diagnostic for Mock.As calls with generic type parameters, and this behavior should be documented in the rule guidance.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary documentation change, specifying that new guidance is being added for generic type parameter scenarios in the Moq1300 rule.
Out of Scope Changes Check ✅ Passed All changes are confined to the Moq1300 rule documentation and focus solely on the generic type parameter scenario discussed in the linked issue, with no unrelated code or doc modifications.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2236d09 and 5f5c18e.

📒 Files selected for processing (1)
  • docs/rules/Moq1300.md (1 hunks)

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

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 enhances the Moq1300 analyzer documentation to address a specific scenario where the diagnostic fires for generic type parameters constrained to interfaces, even when the constraint should theoretically ensure safety.

  • Adds comprehensive guidance for handling generic type parameters in Moq1300 scenarios
  • Explains the C# language limitation that prevents "interface-only" constraints
  • Provides clear criteria for when diagnostic suppression is appropriate vs. when to avoid it

@rjmurillo rjmurillo self-assigned this Oct 10, 2025
@rjmurillo rjmurillo added this to the vNext milestone Oct 10, 2025
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Oct 10, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.02% (target: -1.00%) (target: 95.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (8645380) 1996 1769 88.63%
Head commit (5f5c18e) 1999 (+3) 1772 (+3) 88.64% (+0.02%)

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 (#762) 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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8645380 and 2236d09.

📒 Files selected for processing (1)
  • docs/rules/Moq1300.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

All documentation and markdown reports must pass formatting checks. Use a markdown linter if available.

**/*.md: The generated Explainer/PRD must follow a specific Markdown structure with sections: Introduction/Overview, Goals, Non-Goals, User Stories, Functional Requirements, Design Considerations (Optional), Technical Considerations (Optional), Success Metrics, and Open Questions.
All user stories included in the Explainer/PRD must be validated for compliance with the INVEST mnemonic; if not compliant, the AI must rewrite them or ask the user for clarification.
The Explainer/PRD must be written for a junior developer audience, using explicit, unambiguous language, avoiding jargon, and targeting a grade 9 reading level.
The Explainer/PRD must explicitly list any open questions or assumptions at the end of the document; if there are none, it must state 'None'.
The Explainer/PRD must be formatted in Markdown (.md).

The generated task list must be in Markdown format (.md)

When editing documentation, read markdown.instructions.md before proceeding

**/*.md: Read this instruction file before editing any Markdown (.md) file
Cross-reference related instruction files (csharp.instructions.md, project.instructions.md, text.instructions.md) before making Markdown changes
Complete the Validation Checklist before submitting Markdown changes
Stop and request help if uncertain about any Markdown requirement
Validate all links in Markdown before committing
Update the table of contents in Markdown when adding new sections
Ensure Markdown formatting and linting requirements are met (use a markdown linter if available)
Use clear, unambiguous language in all Markdown documentation
Provide concrete examples in Markdown for concepts and procedures
Create step-by-step guides in Markdown for complex processes
Include troubleshooting sections in Markdown documentation
Use consistent formatting across all Markdown documentation
Address and resolve all automated formatting/linting bot...

Files:

  • docs/rules/Moq1300.md
docs/rules/**/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

All documentation updates for analyzers must update docs/rules/.

For a new analyzer or code fix, add or modify rule documentation under docs/rules/

Update rule documentation under docs/rules/ for analyzer changes

Files:

  • docs/rules/Moq1300.md

⚙️ CodeRabbit configuration file

Evaluate the markdown files against the standards for Roslyn Code Analysis rules

Files:

  • docs/rules/Moq1300.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.111Z
Learning: If a Moq feature has known limitations or edge cases (e.g., explicit interface implementation, event setup), document this in the test or analyzer code.
📚 Learning: 2025-08-04T08:54:29.141Z
Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-08-04T08:54:29.141Z
Learning: Applies to tests/**/*.cs : Setups for explicit interface implementations must use the correct cast syntax in Moq analyzer tests.

Applied to files:

  • docs/rules/Moq1300.md
📚 Learning: 2025-07-26T15:57:15.819Z
Learnt from: rjmurillo
PR: rjmurillo/moq.analyzers#580
File: tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs:116-116
Timestamp: 2025-07-26T15:57:15.819Z
Learning: The CallbackSignatureShouldMatchMockedMethodAnalyzer (Moq1100) currently has a gap where it does not validate generic .Callback<T>() type parameters against the mocked method signature. This can lead to runtime errors when generic callback types don't match the expected method parameters, documented in CallbackSignatureShouldMatchMockedMethodAnalyzerTests.GenericCallbackValidation_CurrentLimitation_IsDocumented(). According to Moq documentation, .Callback<T> should provide compile-time type safety where T must match the argument type of the mocked method.

Applied to files:

  • docs/rules/Moq1300.md
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (ubuntu-24.04-arm)
  • GitHub Check: build (windows-11-arm)

@rjmurillo rjmurillo merged commit 5877902 into main Oct 11, 2025
18 of 19 checks passed
@rjmurillo rjmurillo deleted the docs/756-Moq1300-FP branch October 11, 2025 15:32
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.

Mock.As<T> triggers Moq1300 (Mock.As<T> should be interface) in generic method

3 participants