-
-
Notifications
You must be signed in to change notification settings - Fork 803
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
AmbiguousMatchException when setting up the property, that hides another one #939
Conversation
Commit time branch was AmbiguousMatchException.
Commit time branch was AmbiguousMatchException.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the bug fix... Looks good to me! Apart from the request below, could you please also add an entry to the CHANGELOG.md
?
The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
+## Unreleased
+
+#### Fixed
+
+* <description of your bug fix> (@ishatalkin, #939)
+
+
## 4.13.0 (2019-08-31)
Thanks!
Ok! I'll do the stuff tomorrow.
ср, 16 окт. 2019 г., 20:23 stakx <notifications@github.com>:
… ***@***.**** requested changes on this pull request.
Thanks for the bug fix... Looks good to me! Apart from the request below,
could you please also add an entry to the CHANGELOG.md?
The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
+## Unreleased++#### Fixed++* <description of your bug fix> ***@***.***, #939)++
## 4.13.0 (2019-08-31)
Thanks!
------------------------------
In src/Moq/ExpressionExtensions.cs
<#939 (comment)>:
> @@ -320,7 +320,9 @@ internal static PropertyInfo GetReboundProperty(this MemberExpression expression
// we "upgrade" to the derived property.
if (property.DeclaringType != expression.Expression.Type)
{
- var derivedProperty = expression.Expression.Type.GetProperty(property.Name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
+ var derivedProperty = expression.Expression.Type
+ .GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
+ .SingleOrDefault(p => p.Name == property.Name && p.PropertyType == property.PropertyType);
Could you please rewrite this query as follows?:
var derivedProperty = expression.Expression.Type
.GetMember(property.Name, MemberTypes.Property, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
.Cast<PropertyInfo>()
.SingleOrDefault(p => p.PropertyType == property.PropertyType);
This should be more efficient as the filtering by name happens earlier
(inside System.Reflection).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#939>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC2TXXKUNILRHYZVO5CQZ7DQO5E2HANCNFSM4JBGX3EA>
.
|
Commit time branch was AmbiguousMatchException. Commit time branch was AmbiguousMatchException.
Commit time branch was AmbiguousMatchException.
@stakx, I've made the changes. Is it OK now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Btw. it appears that you are using a different email address in your commits than on GitHub, so your commits won't be linked to your profile when viewed on GitHub. If you want to fix that, now would be a good time.
Otherwise I'll merge your PR tonight (in approx. 8 hours).
Added secondary e-mail address to my profile. Feel free to merge the PR |
🚀 Thank you for contributing, @ishatalkin! |
mock.Setup(m => m.Prop) throws AmbiguousMatchException in case that:
You can repropduce the exception with the following code:
Stacktrace:
Here is pull request that fixes the bug.
This bug is somewhat similar with #930, but here is the problem of Moq.