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

Prevent auto-stubbing failure due to inaccessible property accessors #847

Merged
merged 3 commits into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
#### Fixed

* `mock.SetupAllProperties()` now setups write-only properties for strict mocks, so that accessing such properties will not throw anymore. (@vanashimko, #836)
* Regression: `mock.SetupAllProperties()` and `Mock.Of<T>` fail due to inaccessible property accessors (@Mexe13, #845)


## 4.11.0 (2019-05-28)
Expand Down
30 changes: 23 additions & 7 deletions src/Moq/Interception/InterceptionAspects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,20 +359,36 @@ public static bool Handle(Invocation invocation, Mock mock)
if (property.CanRead)
{
var getter = property.GetGetMethod(true);
propertyValue = CreateInitialPropertyValue(mock, getter);
getterSetup = new AutoImplementedPropertyGetterSetup(expression, getter, () => propertyValue);
mock.Setups.Add(getterSetup);
if (ProxyFactory.Instance.IsMethodVisible(getter, out _))
{
propertyValue = CreateInitialPropertyValue(mock, getter);
getterSetup = new AutoImplementedPropertyGetterSetup(expression, getter, () => propertyValue);
mock.Setups.Add(getterSetup);
}

// If we wanted to optimise for speed, we'd probably be forgiven
// for removing the above `IsMethodVisible` guard, as it's rather
// unlikely to encounter non-public getters such as the following
// in real-world code:
//
// public T Property { internal get; set; }
//
// Usually, it's only the setters that are made non-public. For
// now however, we prefer correctness.
}

Setup setterSetup = null;
if (property.CanWrite)
{
MethodInfo setter = property.GetSetMethod(nonPublic: true);
setterSetup = new AutoImplementedPropertySetterSetup(expression, setter, (newValue) =>
if (ProxyFactory.Instance.IsMethodVisible(setter, out _))
{
propertyValue = newValue;
});
mock.Setups.Add(setterSetup);
setterSetup = new AutoImplementedPropertySetterSetup(expression, setter, (newValue) =>
{
propertyValue = newValue;
});
mock.Setups.Add(setterSetup);
}
}

Setup setupToExecute = invocationMethod.IsPropertyGetter() ? getterSetup : setterSetup;
Expand Down
39 changes: 39 additions & 0 deletions tests/Moq.Tests/Regressions/IssueReportsFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2571,6 +2571,45 @@ public void Setting_up_indexer_with_SetupProperty_throws_with_error_message_poin

#endregion

#region 845

public class Issue845
{
public class Foo
{
public virtual object Bar { get; internal set; }
}

[Fact]
public void Mock_Of_can_deal_with_internal_setter()
{
_ = Mock.Of<Foo>();
}

[Fact]
public void Mock_Of_plus_property_access_can_deal_with_internal_setter()
{
var mocked = Mock.Of<Foo>();
_ = mocked.Bar;
}

[Fact]
public void SetupAllProperties_can_deal_with_internal_setter()
{
var mock = new Mock<Foo>();
mock.SetupAllProperties();
}

[Fact]
public void SetupAllProperties_plus_property_access_can_deal_with_internal_setter()
{
var mock = new Mock<Foo>();
mock.SetupAllProperties();
_ = mock.Object.Bar;
}
}
#endregion

// Old @ Google Code

#region #47
Expand Down