fix: Moq1203 false positive when Setup call is wrapped in parentheses

[rjmurillo]

Summary

• Fix false positive Moq1203 when Setup(...) call is wrapped in parentheses
• Add GetParentSkippingParentheses extension to SyntaxNodeExtensions for reuse
• Add 9 regression test cases covering single/nested parentheses, Callback chaining, ReturnsAsync, and Throws

Fixes #887

Root cause

The chain walk in HasReturnValueSpecification checked current?.Parent is MemberAccessExpressionSyntax. When Setup was wrapped in parentheses like (mock.Setup(...)).Returns(42), the parent was ParenthesizedExpressionSyntax, so the walk exited early and missed .Returns().

Fix

One-line change: current?.Parent becomes current?.GetParentSkippingParentheses(), where the new extension walks through any ParenthesizedExpressionSyntax wrappers before returning the logical parent.

Test plan

• 9 new test cases (single/nested parens with Returns, Throws, ReturnsAsync, Callback chain, intermediate chain parens, diagnostic cases for sync and async)
• All 1964 tests pass
• PR review workflow (code-reviewer, silent-failure-hunter, test-analyzer, code-simplifier) passed locally

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Analyzer now correctly ignores extra parentheses when evaluating Setup expressions, preventing false positives about missing return-value specifications.

• Tests

  • Added regression tests covering parenthesized Setup expressions (both with and without return-value specifications) to ensure correct detection and prevent regressions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

Fixes a false positive in MethodSetupShouldSpecifyReturnValueAnalyzer by skipping parenthesized expression wrappers when traversing member-access chains so return-value specifications (e.g., .Returns()) are correctly detected for parenthesized Setup calls.

Changes

Cohort / File(s)	Summary
Analyzer Change src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs	Replaced parent traversal in HasReturnValueSpecification to use GetParentSkippingParentheses() so the member-access chain continues past ParenthesizedExpressionSyntax.
Syntax Helpers src/Common/SyntaxNodeExtensions.cs	Added internal static SyntaxNode? GetParentSkippingParentheses(this SyntaxNode node) to walk ancestors and skip ParenthesizedExpressionSyntax wrappers.
Tests tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs	Added two data providers and two tests (Issue887_ParenthesizedSetupTestData, Issue887_ParenthesizedSetupWithDiagnosticTestData, ShouldHandleParenthesizedSetupExpressions, ShouldFlagParenthesizedSetupWithoutReturnValue) covering parenthesized Setup scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: Moq1203 false positives for ReturnsAsync and Callback chaining #886: Modifies the same HasReturnValueSpecification traversal logic and return-value detection.

Suggested reviewers

• MattKotsenas

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing a false positive for Moq1203 when Setup calls are wrapped in parentheses, which matches the core objective of the PR.
Linked Issues check	✅ Passed	The PR fully addresses issue #887 by implementing GetParentSkippingParentheses to traverse parenthesized expressions and updating HasReturnValueSpecification to use it, with comprehensive regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly focused on fixing the false positive for parenthesized Setup calls with appropriate tests; no unrelated modifications are present.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/887-moq1203-parenthesized-setup

---

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

[gemini-code-assist]

Summary of Changes

Hello @rjmurillo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a false positive in the Moq1203 analyzer, which incorrectly flagged Setup calls wrapped in parentheses as missing a return value specification. The core issue stemmed from the analyzer's syntax tree traversal logic failing to correctly identify subsequent method calls when an intermediate ParenthesizedExpressionSyntax was present. The solution involves a new extension method that intelligently skips these parentheses, ensuring the analyzer accurately processes the Setup chain.

Highlights

• Analyzer Fix: Resolved a false positive for Moq1203 diagnostic when Setup(...) calls are enclosed in parentheses.
• New Utility Method: Introduced GetParentSkippingParentheses extension method in SyntaxNodeExtensions for robust syntax tree traversal.
• Test Coverage: Added 9 new regression test cases to validate the fix across various scenarios including single/nested parentheses, Callback chaining, ReturnsAsync, and Throws.

Changelog

• src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs
  • Updated the HasReturnValueSpecification method to use the new GetParentSkippingParentheses extension when traversing the syntax tree to find member access expressions.
• src/Common/SyntaxNodeExtensions.cs
  • Added a new internal static extension method GetParentSkippingParentheses which returns the logical parent of a SyntaxNode by skipping any intermediate ParenthesizedExpressionSyntax nodes.
• tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs
  • Added Issue887_ParenthesizedSetupTestData which provides test cases for Setup expressions wrapped in parentheses that should not trigger the Moq1203 diagnostic.
  • Added Issue887_ParenthesizedSetupWithDiagnosticTestData which provides test cases for parenthesized Setup expressions without a return value specification that should trigger the Moq1203 diagnostic.
  • Added a new [Theory] test method ShouldHandleParenthesizedSetupExpressions to run the positive test cases.
  • Added a new [Theory] test method ShouldFlagParenthesizedSetupWithoutReturnValue to run the diagnostic test cases, ignoring compiler errors.

Activity

• The author added 9 new test cases to cover the regression.
• All 1964 existing tests passed after the changes.
• The PR review workflow (code-reviewer, silent-failure-hunter, test-analyzer, code-simplifier) passed locally.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[deepsource-io]

DeepSource Code Review

DeepSource reviewed changes in the commit range 38943ac..5e2e1fb on this pull request. Below is the summary for the review, and you can see the individual issues we found as review comments.

For detailed review results, please see the PR on DeepSource ↗

PR Report Card

Security	× 0 issues		Overall PR Quality    Focus Area: Hygiene Guidance Fix naming convention issues in `tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs` for methods with return type `Task`.
Reliability	× 0 issues
Complexity	× 0 issues
Hygiene	× 2 issues

Code Review Summary

Analyzer	Status	Summary	Details
C#		2 new issues detected.	Review ↗

How are these analyzer statuses calculated?

Administrators can configure which issue categories are reported and cause analysis to be marked as failed when detected. This helps prevent bad and insecure code from being introduced in the codebase. If you're an administrator, you can modify this in the repository's settings.

[gemini-code-assist]

Code Review

The pull request effectively addresses the false positive for Moq1203 when Setup calls are wrapped in parentheses. The introduction of the GetParentSkippingParentheses extension method is a clean and reusable solution to correctly traverse the syntax tree. The added regression tests are comprehensive, covering various scenarios with single and nested parentheses, callback chaining, ReturnsAsync, and Throws, which thoroughly validates the fix. The changes are well-implemented and maintain code quality.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.02% (target: -1.00%)	✅ 100.00% (target: 95.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (38943ac)	2045	1819	88.95%
Head commit (5e2e1fb)	2049 (+4)	1823 (+4)	88.97% (+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 (#895)	5	5	100.00%

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

[Copilot]

Pull request overview

Fixes a Moq1203 false positive by ensuring the analyzer’s fluent-call chain walk can traverse through parenthesized Setup(...) expressions (e.g., (mock.Setup(...)).Returns(...)).

Changes:

• Add GetParentSkippingParentheses to centralize “skip ParenthesizedExpressionSyntax wrappers” logic.
• Update MethodSetupShouldSpecifyReturnValueAnalyzer.HasReturnValueSpecification to use the new helper when walking the chain.
• Add regression test cases for parenthesized/nested-parenthesized fluent setups, including chaining scenarios.

Reviewed changes

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

File	Description
tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs	Adds regression data + theories for issue #887 (parenthesized fluent setups).
src/Common/SyntaxNodeExtensions.cs	Introduces GetParentSkippingParentheses helper extension.
src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs	Uses GetParentSkippingParentheses during member-access chain traversal to avoid premature exit on parentheses.

[rjmurillo]

@MattKotsenas FYI - TIL that the order by which the analyzer walks (top down vs bottom up) matters.

In 1203 the analyzer walks UP (setup -> parent -> returns). Parenthesis break this upward walk.

When you walk down (so we'd trigger this analyzer on Returns/Callback) and down into .Expression and find the Setup call. When parenthesis wrap Setup, memberAccess.Expression is ParenthesizedExpressionSyntax instead of InvocationExpressionSyntax. So the analyzers return null and skip.

Spot checking looks like 1206 and 1100 also are impacted by this. 1203 is the worst because it's a FP whereas 1206 and 1100 are FN so less noticed.