diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fdfb9187..860fa5b40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` fail due to inaccessible property accessors (@Mexe13, #845) ## 4.11.0 (2019-05-28) diff --git a/src/Moq/Interception/InterceptionAspects.cs b/src/Moq/Interception/InterceptionAspects.cs index e5fa1607b..c52f908fd 100644 --- a/src/Moq/Interception/InterceptionAspects.cs +++ b/src/Moq/Interception/InterceptionAspects.cs @@ -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; diff --git a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs index 3cfa67419..71ddb5caa 100644 --- a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs +++ b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs @@ -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(); + } + + [Fact] + public void Mock_Of_plus_property_access_can_deal_with_internal_setter() + { + var mocked = Mock.Of(); + _ = mocked.Bar; + } + + [Fact] + public void SetupAllProperties_can_deal_with_internal_setter() + { + var mock = new Mock(); + mock.SetupAllProperties(); + } + + [Fact] + public void SetupAllProperties_plus_property_access_can_deal_with_internal_setter() + { + var mock = new Mock(); + mock.SetupAllProperties(); + _ = mock.Object.Bar; + } + } + #endregion + // Old @ Google Code #region #47