feat: migrate NSubstitute property Received gets and sets#51
Conversation
There was a problem hiding this comment.
Pull request overview
Adds NSubstitute-to-Mockolate code fix support for property-style Received/DidNotReceive verifications (property gets/sets), along with new verification tests.
Changes:
- Add code-fix rewrite for property-set verifications like
sub.Received().Prop = value/sub.DidNotReceive().Prop = value. - Add code-fix rewrite for property-get verifications expressed as discard assignments like
_ = sub.Received().Prop. - Extend NSubstitute code-fix tests to cover the new property get/set scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Tests/Mockolate.Migration.Tests/NSubstituteCodeFixProviderTests.VerifyTests.cs | Adds new tests asserting property get/set Received/DidNotReceive rewrites. |
| Source/Mockolate.Migration.Analyzers.CodeFixers/NSubstituteCodeFixProvider.cs | Implements property get/set verification rewrite logic and ensures Mockolate.Verify using is added when needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…uple Sonar S107 flagged the method's eight parameters. Returning a nullable named tuple drops it to four and lets callers pattern-match the result.
The property-set verification migration emitted the RHS verbatim, so sub.Received().Name = Arg.Any<string>() became sub.Mock.Verify.Name.Set(Arg.Any<string>()).Never() — the NSubstitute matcher was never translated. Now BuildPropertyVerifySetArgs runs the shared Arg.* → It.* rewrite and wraps non-matcher values in It.Is(...), matching the convention used by the Moq VerifySet migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
FindAndBuildPropertyVerifyReplacements rewrites any matching SimpleAssignmentExpression regardless of where the assignment appears. Since assignment expressions can be used as sub-expressions (not just statements), this code fix could change semantics or produce non-compiling code when the assignment’s value is used. Consider restricting rewrites to assignments whose parent is an ExpressionStatementSyntax (and, for the get case, a discard assignment used as a statement) to ensure only verification-style usages are migrated.
| if (assignment.Parent is not ExpressionStatementSyntax) | |
| { | |
| continue; | |
| } |
| /// Translates property-style verifications: | ||
| /// <list type="bullet"> | ||
| /// <item><c>_ = sub.Received().Prop</c> → <c>sub.Mock.Verify.Prop.Got().AtLeastOnce()</c></item> | ||
| /// <item><c>sub.Received().Prop = v</c> → <c>sub.Mock.Verify.Prop.Set(v).AtLeastOnce()</c></item> |
There was a problem hiding this comment.
The XML doc example for property sets shows sub.Mock.Verify.Prop.Set(v)..., but the implementation wraps non-matcher values in It.Is(...) (e.g., Set(It.Is("x"))). Consider updating the example/comment to reflect the actual output so the documentation matches behavior.
| /// <item><c>sub.Received().Prop = v</c> → <c>sub.Mock.Verify.Prop.Set(v).AtLeastOnce()</c></item> | |
| /// <item><c>sub.Received().Prop = "x"</c> → <c>sub.Mock.Verify.Prop.Set(It.Is("x")).AtLeastOnce()</c></item> |
| if (expression is not MemberAccessExpressionSyntax propertyAccess || | ||
| propertyAccess.Expression is not InvocationExpressionSyntax receivedInvocation || | ||
| receivedInvocation.Expression is not MemberAccessExpressionSyntax receivedAccess) | ||
| { | ||
| return null; |
There was a problem hiding this comment.
TryExtractReceivedPropertyAccess only checks the syntax shape sub.Received().X but doesn’t confirm that X resolves to an actual property. This can mis-rewrite cases like method-group accesses (e.g., _ = sub.Received().SomeMethod;) into Verify.SomeMethod.Got() which likely won’t compile. Consider using the semantic model to ensure propertyAccess (or propertyAccess.Name) binds to an IPropertySymbol before treating it as a property verification.
|
This is addressed in release v0.5.0. |
This pull request extends the NSubstitute-to-Mockolate migration code fixer to support property verification conversions, in addition to existing method call verifications. It introduces logic to translate property "get" and "set" verifications (e.g.,
sub.Received().Propandsub.Received().Prop = v) into the corresponding Mockolate verification chains, and adds comprehensive tests to ensure correct behavior.Enhancements to property verification migration:
NSubstituteCodeFixProvider.csto identify and convert property verification expressions, such as property gets (_ = sub.Received().Prop) and sets (sub.Received().Prop = v), to Mockolate's.Mock.Verify.Prop.Got().AtLeastOnce()and.Mock.Verify.Prop.Set(v).AtLeastOnce()respectively. Negative cases likeDidNotReceiveare mapped to.Never().using Mockolate.Verify;directive is added when necessary.Test coverage improvements:
NSubstituteCodeFixProviderTests.VerifyTests.csto verify correct rewriting of property get and set verifications, including both positive (Received) and negative (DidNotReceive) cases.