fix(linq-mutator): only mutate calls on IEnumerable<T>-shaped receivers - #3601
fix(linq-mutator): only mutate calls on IEnumerable<T>-shaped receivers#3601slang25 wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens LinqMutator so it only rewrites LINQ-operator-named member accesses when they are actually LINQ invocations, preventing false-positive mutations like IResponseCookies.Append being rewritten into a LINQ operator and breaking compilation.
Changes:
- Add semantic-model-based validation to confirm an invocation is LINQ (host type is
Enumerable/Queryable/ParallelEnumerableor receiver implementsIEnumerable<T>), with a legacy fallback when no semantic model is available. - Add unit tests covering the reported false-positive (
IResponseCookies.Append) and a real LINQ case (IEnumerable<int>.Append).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Stryker.Core/Stryker.Core/Mutators/LinqMutator.cs | Adds semantic-model filtering (IsLinqInvocation) before producing LINQ method mutations. |
| src/Stryker.Core/Stryker.Core.UnitTest/Mutators/LinqMutatorTest.cs | Adds semantic-model-backed tests to ensure non-LINQ Append isn’t mutated and LINQ Append is. |
| var invocation = node.Parent as InvocationExpressionSyntax ?? FindEnclosingInvocation(node); | ||
| if (invocation is null) | ||
| { | ||
| return true; |
There was a problem hiding this comment.
Reverted in the latest push: blocking mutation on non-invocation member access broke CsharpMutantOrchestratorTests.ShouldMutateConditionalMemberAccessProperly, which documents the longstanding behaviour of mutating test?.Other()?.Count → ?.Sum and relying on the CompileError filter downstream. Updating that test (and any other property/method-group shapes Stryker currently mutates) is out of scope for this PR — leaving the unresolve as a tracker for a follow-up.
LinqMutator was matching on method name alone, so any method whose name happens to coincide with a LINQ operator (e.g. IResponseCookies.Append) got rewritten to its LINQ counterpart, producing uncompilable mutations. Verify via the semantic model that the receiver implements IEnumerable<T> or that the method is declared on Enumerable/Queryable/ParallelEnumerable before substituting. Falls back to legacy name-only matching when no semantic model is available. Fixes stryker-mutator#3596
Address Copilot review feedback: 1. RequireArguments guard previously called FindEnclosingInvocation, which returns null for direct-invocation shape (items.Any()) because the MemberAccess parent is the InvocationExpressionSyntax itself, not a chained MemberAccess. As a result, Any() -> All() (and similar) could emit an uncompilable mutant with no predicate. Unified invocation lookup into FindInvocation so both guards use the same node. 2. IsLinqInvocation previously returned true when no enclosing invocation was found, which permitted mutation of method-group references like `Func<...> d = cookies.Append;` -> uncompilable. With a semantic model available, skip mutation when the member access is not part of an invocation. Adds unit tests for both cases.
After the RequireArguments guard in LinqMutator was tightened to also catch direct-invocation shape (commit fd28c30), 4 previously-uncompilable Any() -> All() (and similar) mutants are no longer generated. The dropped mutants were all CompileError status — they contribute nothing to mutation-testing quality but inflated the total count. All other status counts (ignored/survived/killed/timeout/nocoverage) are unchanged across every integration target; only the headline total drops by 4: - NetCore target projects: 660 -> 656 - MTPSolution: 670 -> 666
056b8af to
80ae7b5
Compare
…ion found The previous commit made IsLinqInvocation return false for member accesses that aren't part of an invocation (method-group references, property-shaped LINQ identifiers like `?.Count`). That broke CsharpMutantOrchestratorTests.ShouldMutateConditionalMemberAccessProperly, which documents Stryker's longstanding behaviour of mutating `test?.Other()?.Count` → `test?.Other()?.Sum` and relying on the CompileError filter to drop uncompilable mutants downstream. Restore the legacy `return true` path so that test (and any real-world shapes it represents) keeps working. The IResponseCookies.Append false positive — the PR's actual target — is still fixed via the receiver-type check, and the RequireArguments direct-invocation fix from the previous commit is retained.
80ae7b5 to
5b5dc53
Compare
Addresses Copilot review feedback. INamedTypeSymbol.ToDisplayString() defaults to MinimallyQualifiedFormat, which usually returns just "Enumerable" rather than "System.Linq.Enumerable", so the host-type check would never match for direct static calls like Enumerable.First(items, predicate). Compare ContainingNamespace explicitly against System.Linq so detection is stable regardless of which display format Roslyn picks.
Summary
LinqMutatorwas matching on method name alone, so any method whose name coincides with a LINQ operator (e.g.IResponseCookies.Append) got rewritten to its LINQ counterpart, producing uncompilable mutations.IEnumerable<T>, or that the method is declared onEnumerable/Queryable/ParallelEnumerable, before substituting.Fixes #3596. Split out from #3594.
Note: most effective alongside #3600, which ensures the semantic model can resolve types from implicit usings.
Test plan
LinqMutatorTestcases still pass (legacy name-only path undernullsemantic model)IResponseCookies.Append) and the real LINQ case (IEnumerable<int>.Append)