Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-deprecate [Matcher] attribute? #770

Closed
stakx opened this issue Mar 4, 2019 · 1 comment · Fixed by #788
Closed

Re-deprecate [Matcher] attribute? #770

stakx opened this issue Mar 4, 2019 · 1 comment · Fixed by #788
Assignees
Milestone

Comments

@stakx
Copy link
Contributor

stakx commented Mar 4, 2019

More a note to myself than an actual issue:

In #732 I brought back the requirement that [Matcher] attributes must be placed on all matchers, including custom ones. This was done to fix a bug and to improve runtime efficiency (Moq being able to statically analyse argument expressions instead of having to compile & execute them.)

This was a good thought, but it also makes it impossible to put argument matchers in local variables to abbreviate a long query expression with repeated matchers; e.g.:

-Func<int> any = It.IsAny<int>;
-mock.Setup(m => m.Method(any(), any(), any(), ...);
-//                       ^^^^^  ^^^^^  ^^^^^
-// Those will not get recognized as being equivalent to `It.IsAny<int>()` because `any`
-// doesn't carry a `[Matcher]` attribute (how could it?!) and therefore won't be executed.
-// Instead, Moq will interpret those as the literal value returned by `It.IsAny<int>()`.

Edit: I was mistaken, the use case shown above actually doesn't work in previous versions, either, so it is by itself no reason to bring MatcherAttribute back.

With all the current PRs, it might become possible to fix the bug from #725 in a different way than was done in #732. If so, runtime performance alone perhaps isn't a good enough reason to reintroduce the [Matcher] attribute as mandatory; in that case, #732 should be reverted and #725 reopened to be solved in a different way.

@stakx stakx added this to the 4.11.0 milestone Mar 4, 2019
@stakx stakx self-assigned this Mar 8, 2019
@stakx
Copy link
Contributor Author

stakx commented Mar 10, 2019

MatcherAttribute re-deprecated for the sole reason of not causing unnecessary breaking changes in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant