mock.Protected().Setup<int>("Foo") fails base implementation of Foo is hidden in the derived class#1342
mock.Protected().Setup<int>("Foo") fails base implementation of Foo is hidden in the derived class#1342VladimirKhvostov wants to merge 1 commit intodevlooped:mainfrom
Conversation
|
|
…s hidden in the derived class (devlooped#1341)
|
Thanks for the PR! Could you rebase on top of main, since files moved around? |
stakx
left a comment
There was a problem hiding this comment.
I haven't reviewed & tested this in detail TBH, but it generally looks good to me.
The changelog should be first updated with the latest few missing releases though, otherwise the changelog entry will end up with an incorrect (already published) version.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #1341 by improving how ProtectedMock selects hidden virtual methods and adds tests to cover this scenario.
- Adjusted
GetMethodlogic to prefer methods declared in derived classes when multiple matches exist. - Added a new test (
SetupResultAllowsHiddenVirtualMethods) to verify setup of hidden protected methods. - Updated changelog to document the fix.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Moq.Tests/ProtectedMockFixture.cs | Added a test verifying setup of hidden protected methods. |
| src/Moq/Protected/ProtectedMock.cs | Enhanced method resolution logic for hidden virtual methods. |
| CHANGELOG.md | Documented the fix in the Unreleased fixed section. |
|
|
||
| for (Type type = typeof(T); type != typeof(object); type = type.BaseType) | ||
| { | ||
| var method = methods.SingleOrDefault(m => m.DeclaringType == typeof(T)); |
There was a problem hiding this comment.
The loop variable type is never used: the predicate always checks DeclaringType == typeof(T). It should use the loop variable type (m.DeclaringType == type) to correctly select the most derived declaring type.
| var method = methods.SingleOrDefault(m => m.DeclaringType == typeof(T)); | |
| var method = methods.SingleOrDefault(m => m.DeclaringType == type); |
| methods = methods.Where(m => m.GetParameterTypes().CompareTo(argTypes, exact, considerTypeMatchers: false)); | ||
|
|
||
| if (methods.Count() < 2) |
There was a problem hiding this comment.
Calling Count() on an IEnumerable will enumerate it; since it's used again below, consider materializing methods into a collection (e.g., var list = methods.ToList()) to avoid multiple enumerations.
| methods = methods.Where(m => m.GetParameterTypes().CompareTo(argTypes, exact, considerTypeMatchers: false)); | |
| if (methods.Count() < 2) | |
| methods = methods.Where(m => m.GetParameterTypes().CompareTo(argTypes, exact, considerTypeMatchers: false)).ToList(); | |
| if (methods.Count < 2) |
This PR fixes an issue described in the #1341