test(Moq1302): comprehensive coverage for false positive fix (#1010)#1020
test(Moq1302): comprehensive coverage for false positive fix (#1010)#1020
Conversation
Add 10 new test scenarios covering real-world patterns that triggered false positives in Moq1302 (LinqToMocksExpressionShouldBeValid): - Exact reporter pattern: nested Mock.Of inside method argument - Static lambda with external constant - Captured local variable as comparison value - Method parameter as comparison value - Instance field (this member) as comparison value - Inequality operators (!=) with external constants - Chained external instance property (multi-hop) - Non-virtual method with external arguments (true positive) - String concatenation on right side of comparison - || with all external constants (no false positives) These complement the existing 22 tests from PR #1017 to ensure the IsRootedInLambdaParameter guard handles every common customer pattern. Closes #1010 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary of ChangesHello, I'm Gemini Code Assist1! 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 significantly enhances the test coverage for the Moq1302 analyzer, specifically targeting a fix for false positives introduced in a previous change. By adding a comprehensive set of new test cases, it validates that the analyzer accurately distinguishes between valid and invalid Moq expressions involving external constants, captured variables, and complex lambda structures, thereby improving the analyzer's reliability and preventing regressions. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request adds ten new test cases to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
 |
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene  |
Feedback
- Missing Async suffix across many test methods
- Task-returning tests repeatedly omit the Async suffix because tests were created or copied without following the naming convention; enforce a naming rule and apply a bulk rename/code-fix to stop future churn.
- Analyzer flags but not enforced or auto-fixed
- The analyzer reports the issue but is treated as a soft warning and lacks an automated fix; promote the rule to error in .editorconfig/CSProj and add a Roslyn CodeFix or “fix all” step to convert warnings into immediate, fixable failures.
- Test scaffolding/templates propagate the problem
- Generated test methods and templates produce nonconforming names, causing repeated occurrences; update templates/generators to append Async and include the naming rule in project templates so new code conforms by default.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 7, 2026 6:50a.m. | Review ↗ |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds excellent, comprehensive test coverage for the Moq1302 false positive fix. The new scenarios cover a wide range of common patterns, which significantly strengthens the analyzer's robustness. My review includes one suggestion to consolidate a few similar tests to improve maintainability by reducing boilerplate code, with an added note to consider potential downsides of such refactoring in test files.
tests/Moq.Analyzers.Test/LinqToMocksExpressionShouldBeValidAnalyzerTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Expands the Moq1302 (LinqToMocksExpressionShouldBeValid) analyzer test suite to prevent regressions of the #1010 false-positive scenario by adding coverage for common “external member/constant” patterns, running each scenario against both supported Moq versions.
Changes:
- Added 9 new “should not flag” regression tests covering external constants/values in various binary-expression shapes (
staticlambdas, captured locals, chained property access,!=,||, string concatenation, nestedMock.Ofin method args). - Added 1 new “should flag” regression test to ensure true positives remain (non-virtual method invocation rooted in the lambda parameter, even with external arguments).
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Response to Gemini Review ThreadRe: suggestion to consolidate three tests into a single data-driven Theory. These tests are intentionally separate rather than data-driven. Each exercises a distinct IOperation code path:
Consolidating them into a shared template would obscure which IOperation path is under test and make failures harder to diagnose. |
coderabbitai
bot
added
housekeeping
analyzers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…1020) ## Summary Adds 10 new test scenarios for Moq1302 (`LinqToMocksExpressionShouldBeValid`) to ensure the `IsRootedInLambdaParameter` guard from PR #1017 handles every common customer pattern. These tests run against both Moq 4.8.2 and 4.18.4. ### New test scenarios **False positive prevention (should NOT flag):** - Exact reporter pattern: nested `Mock.Of` inside method argument (#1010) - `static` lambda with external constant (reporter's exact syntax) - Captured local variable as comparison value - Method parameter as comparison value - Instance field (`this` member) as comparison value - Inequality operator (`!=`) with external constant - Chained external instance property (multi-hop: `settings.Service.DefaultName`) - String concatenation on right side (`Prefix.Value + "-default"`) - `||` with all external constants **True positive preservation (SHOULD flag):** - Non-virtual method with external arguments still flagged ### Test results - **Before**: 144 Moq1302 tests - **After**: 164 Moq1302 tests (+20, each runs against 2 Moq versions) - **Full suite**: 2892 tests, 0 failures ## Test plan - [x] All 164 Moq1302 tests pass - [x] Full suite (2892 tests) passes - [x] Build: 0 warnings, 0 errors Closes #1010 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded test coverage with comprehensive scenarios for analyzer validation, including edge cases with external variables, method parameters, instance fields, static lambdas, chained properties, and comparison operations to improve reliability across complex patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Richard Murillo <rjmurillo@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Adds 10 new test scenarios for Moq1302 (
LinqToMocksExpressionShouldBeValid) to ensure theIsRootedInLambdaParameterguard from PR #1017 handles every common customer pattern. These tests run against both Moq 4.8.2 and 4.18.4.New test scenarios
False positive prevention (should NOT flag):
Mock.Ofinside method argument (False positive in Moq1302: Invalid member 'StatusCodes.Status202Accepted' in LINQ to Mocks expression #1010)staticlambda with external constant (reporter's exact syntax)thismember) as comparison value!=) with external constantsettings.Service.DefaultName)Prefix.Value + "-default")||with all external constantsTrue positive preservation (SHOULD flag):
Test results
Test plan
Closes #1010
🤖 Generated with Claude Code
Summary by CodeRabbit