Retire FluentMockVisitor
#978
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The pattern used for setting properties--that is, using a non-generic helper method (`Mocks.SetProperty`) instead of Moq's generic public API methods--can be applied again to get rid of more reflection code.
For this to work, `Mocks.SetupReturns` needs to be extended so it will allow setting non-interceptable properties. (Accessing such a property cannot be represented using `InvocationShape`, which guards against non-interceptable members, so they must be removed from setup expr's and set manually.) These changes conclude the conversion of LINQ to Mocks from `Fluent- MockVisitor` to the infrastructure provided by `Mock.SetupRecursive` and `expression.Split()`, which throws different exceptions when en- countering unsupported members. Adjust some tests to reflect this.
stakx
commented
Mar 22, 2020
| // in this parameter... | ||
| // | ||
| // Therefore, for the nested (quoted) expression, we short-circuit: | ||
| return node.NodeType == ExpressionType.Quote ? node : base.VisitUnary(node); |
There was a problem hiding this comment.
It may be safer to replace this guard with a different algorithm:
-
Implement a new visitor that travels along the "left spine" of an expression until it arrives at the root
ParameterExpression, then it stores a reference to it. -
Run that visitor on the
leftexpression and make the identifiedParameterExpressionavailable to the present visitor. -
Add checks in the present visitor so it only transforms expressions involving the same
ParameterExpressionthat was identified in the earlier steps.
stakx
commented
Mar 22, 2020
| @@ -116,33 +116,10 @@ private static Expression ConvertToSetup(Expression left, Expression right) | |||
| { | |||
| switch (left.NodeType) | |||
| { | |||
| case ExpressionType.MemberAccess: | |||
| var member = (MemberExpression)left; | |||
| Guard.NotField(member); | |||
There was a problem hiding this comment.
This check (and the few ones further down) are removed because expression.Split and InvocationShape should already perform their own checks.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FluentMockVisitorwas the original component used to automatically set up inner mocks for multi-dot setup/verification expressions. For the regularnew Mock<T>,mock.Setup,mock.VerifyAPIs, we began replacing this component with new infrastructure based onMock.SetupRecursiveandexpression.Split()— see approx. #765. However,FluentMockVisitorhas continued to be used for LINQ to Mocks.This PR converts LINQ to Mocks to use the new infrastructure so we don't have to maintain two parallel mechanisms for dealing with multi-dot expressions.
(This will also simplify things over in #976.)