From 464da6f7bd6000e99919b21636a1e60d9ba039c2 Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Wed, 4 Jul 2018 09:26:33 -0400 Subject: [PATCH 1/5] Add spec showing in parameter passed to fake interface cannot be mutated --- tests/FakeItEasy.Specs/InParametersSpecs.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/FakeItEasy.Specs/InParametersSpecs.cs b/tests/FakeItEasy.Specs/InParametersSpecs.cs index 765b50191..baf5c1d2f 100644 --- a/tests/FakeItEasy.Specs/InParametersSpecs.cs +++ b/tests/FakeItEasy.Specs/InParametersSpecs.cs @@ -35,6 +35,25 @@ public void FakingInParam(IInParam fake, int argument, int result) .x(() => result.Should().Be(42)); } + [Scenario] + public void SettingInParamInterface(IInParam fake, int argument) + { + "Given a faked interface with a method that takes an 'in' parameter" + .x(() => fake = A.Fake()); + + "And the 'in' argument has a known value" + .x(() => argument = 17); + + "And a call to this method is configured to set a new value for the parameter" + .x(() => A.CallTo(() => fake.Foo(A._)).AssignsOutAndRefParameters(19)); + + "When the method is called" + .x(() => fake.Foo(argument)); + + "Then the 'in' argument retains its original value" + .x(() => argument.Should().Be(17)); + } + // A characterization test, representing the current capabilities of the code, not the desired state. // If it start failing, update it and fix the "What can be faked" documentation page. // From 7b01a2c85dda072a00827fbcff616ac5b438625b Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Wed, 4 Jul 2018 09:27:21 -0400 Subject: [PATCH 2/5] Add spec showing in parameter passed to fake delegate cannot be mutated --- tests/FakeItEasy.Specs/InParametersSpecs.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/FakeItEasy.Specs/InParametersSpecs.cs b/tests/FakeItEasy.Specs/InParametersSpecs.cs index baf5c1d2f..6d0c5817f 100644 --- a/tests/FakeItEasy.Specs/InParametersSpecs.cs +++ b/tests/FakeItEasy.Specs/InParametersSpecs.cs @@ -19,6 +19,8 @@ public interface IGenericInParam int Foo(in int x); } + public delegate void DelegateWithInParam(in int i); + [Scenario] public void FakingInParam(IInParam fake, int argument, int result) { @@ -54,6 +56,25 @@ public void SettingInParamInterface(IInParam fake, int argument) .x(() => argument.Should().Be(17)); } + [Scenario] + public void SettingInParamDelegate(DelegateWithInParam fake, int argument) + { + "Given a faked delegate with a method that takes an 'in' parameter" + .x(() => fake = A.Fake()); + + "And the 'in' argument has a known value" + .x(() => argument = 17); + + "And a call to this method is configured to set a new value for the parameter" + .x(() => A.CallTo(() => fake.Invoke(A._)).AssignsOutAndRefParameters(19)); + + "When the method is called" + .x(() => fake.Invoke(argument)); + + "Then the 'in' argument retains its original value" + .x(() => argument.Should().Be(17)); + } + // A characterization test, representing the current capabilities of the code, not the desired state. // If it start failing, update it and fix the "What can be faked" documentation page. // From bc022d18343567b83841bc709fa49b7c91ec616f Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Wed, 4 Jul 2018 15:07:57 -0400 Subject: [PATCH 3/5] Update Castle.Core to 4.3.1 for .NET Framework 4.x --- src/FakeItEasy/FakeItEasy.csproj | 10 +++++++--- .../ApiApproval.ApproveApi40.approved.txt | 1 + .../ApiApproval.ApproveApi45.approved.txt | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/FakeItEasy/FakeItEasy.csproj b/src/FakeItEasy/FakeItEasy.csproj index a3706547e..88b9efeb6 100644 --- a/src/FakeItEasy/FakeItEasy.csproj +++ b/src/FakeItEasy/FakeItEasy.csproj @@ -15,6 +15,10 @@ TDD;unittesting;mocks;mocking;fakes;faking;stubs;stubbing;spy;spies;doubles;isolation;substitutes;substitution + + 4.3.1 + + @@ -39,7 +43,7 @@ - + @@ -53,7 +57,7 @@ - + @@ -68,7 +72,7 @@ - + diff --git a/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi40.approved.txt b/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi40.approved.txt index 4270005f2..d7318e9cd 100644 --- a/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi40.approved.txt +++ b/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi40.approved.txt @@ -79,6 +79,7 @@ namespace Castle.DynamicProxy.Internal public static bool IsFinalizer(this System.Reflection.MethodInfo methodInfo) { } public static bool IsGetType(this System.Reflection.MethodInfo methodInfo) { } public static bool IsMemberwiseClone(this System.Reflection.MethodInfo methodInfo) { } + public static bool IsNullableType(this System.Type type) { } public static void SetStaticField(this System.Type type, string fieldName, System.Reflection.BindingFlags additionalFlags, object value) { } public static System.Reflection.MemberInfo[] Sort(System.Reflection.MemberInfo[] members) { } } diff --git a/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi45.approved.txt b/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi45.approved.txt index 2e8f04839..2ba862979 100644 --- a/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi45.approved.txt +++ b/tests/FakeItEasy.Tests.Approval/ApiApproval.ApproveApi45.approved.txt @@ -79,6 +79,7 @@ namespace Castle.DynamicProxy.Internal public static bool IsFinalizer(this System.Reflection.MethodInfo methodInfo) { } public static bool IsGetType(this System.Reflection.MethodInfo methodInfo) { } public static bool IsMemberwiseClone(this System.Reflection.MethodInfo methodInfo) { } + public static bool IsNullableType(this System.Type type) { } public static void SetStaticField(this System.Type type, string fieldName, System.Reflection.BindingFlags additionalFlags, object value) { } public static System.Reflection.MemberInfo[] Sort(System.Reflection.MemberInfo[] members) { } } From 9ab9243e09f985140575cd04411611d8de986f44 Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Wed, 4 Jul 2018 15:21:33 -0400 Subject: [PATCH 4/5] Consolidate ILMerge version number --- src/FakeItEasy/FakeItEasy.csproj | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/FakeItEasy/FakeItEasy.csproj b/src/FakeItEasy/FakeItEasy.csproj index 88b9efeb6..2e2785657 100644 --- a/src/FakeItEasy/FakeItEasy.csproj +++ b/src/FakeItEasy/FakeItEasy.csproj @@ -17,6 +17,7 @@ 4.3.1 + 2.14.1208 @@ -56,12 +57,12 @@ - + - + @@ -71,12 +72,12 @@ - + - + From eebd68b36e3b795fa80d80042fe4bb433589ae4e Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Wed, 4 Jul 2018 22:31:41 -0400 Subject: [PATCH 5/5] Reject attempts to assign values to in parameters --- .../Configuration/BuildableCallRule.cs | 2 +- .../ExpressionArgumentConstraintFactory.cs | 9 ++---- src/FakeItEasy/FakeItEasy.csproj | 4 +-- src/FakeItEasy/ParameterInfoExtensions.cs | 31 +++++++++++++++++++ tests/FakeItEasy.Specs/InParametersSpecs.cs | 30 +++++++++--------- 5 files changed, 52 insertions(+), 24 deletions(-) create mode 100644 src/FakeItEasy/ParameterInfoExtensions.cs diff --git a/src/FakeItEasy/Configuration/BuildableCallRule.cs b/src/FakeItEasy/Configuration/BuildableCallRule.cs index 3f63c4c85..f31ea1b50 100644 --- a/src/FakeItEasy/Configuration/BuildableCallRule.cs +++ b/src/FakeItEasy/Configuration/BuildableCallRule.cs @@ -204,7 +204,7 @@ private static ICollection GetIndexesOfOutAndRefParameters(IInterceptedFake var arguments = fakeObjectCall.Method.GetParameters(); for (var i = 0; i < arguments.Length; i++) { - if (arguments[i].ParameterType.IsByRef) + if (arguments[i].IsOutOrRef()) { indexes.Add(i); } diff --git a/src/FakeItEasy/Expressions/ExpressionArgumentConstraintFactory.cs b/src/FakeItEasy/Expressions/ExpressionArgumentConstraintFactory.cs index 6330861df..cf9ea0681 100644 --- a/src/FakeItEasy/Expressions/ExpressionArgumentConstraintFactory.cs +++ b/src/FakeItEasy/Expressions/ExpressionArgumentConstraintFactory.cs @@ -28,10 +28,10 @@ public virtual IArgumentConstraint GetArgumentConstraint(ParsedArgumentExpressio return this.CreateParamArrayConstraint((NewArrayExpression)argument.Expression, parameterType); } - var isByRefArgument = IsByRefArgument(argument); + var isOutOrRefArgument = argument.ArgumentInformation.IsOutOrRef(); var constraint = this.GetArgumentConstraintFromExpression(argument.Expression, parameterType, out var argumentValue); - if (isByRefArgument) + if (isOutOrRefArgument) { if (IsOutArgument(argument)) { @@ -66,11 +66,6 @@ private static bool IsOutArgument(ParsedArgumentExpression argument) return argument.ArgumentInformation.IsOut; } - private static bool IsByRefArgument(ParsedArgumentExpression argument) - { - return argument.ArgumentInformation.ParameterType.IsByRef; - } - private static IArgumentConstraint CreateEqualityConstraint(object expressionValue) { return new EqualityArgumentConstraint(expressionValue); diff --git a/src/FakeItEasy/FakeItEasy.csproj b/src/FakeItEasy/FakeItEasy.csproj index 2e2785657..5a8b951a9 100644 --- a/src/FakeItEasy/FakeItEasy.csproj +++ b/src/FakeItEasy/FakeItEasy.csproj @@ -40,7 +40,7 @@ - $(DefineConstants);FEATURE_NETCORE_REFLECTION;FEATURE_EXCEPTION_DISPATCH_INFO;FEATURE_ARRAY_EMPTY + $(DefineConstants);FEATURE_NETCORE_REFLECTION;FEATURE_EXCEPTION_DISPATCH_INFO;FEATURE_ARRAY_EMPTY;FEATURE_PARAMETERINFO_CUSTOMATTRIBUTES_PROPERTY @@ -53,7 +53,7 @@ - $(DefineConstants);FEATURE_BINARY_SERIALIZATION;FEATURE_REFLECTION_GETASSEMBLIES + $(DefineConstants);FEATURE_BINARY_SERIALIZATION;FEATURE_REFLECTION_GETASSEMBLIES;FEATURE_PARAMETERINFO_CUSTOMATTRIBUTES_PROPERTY diff --git a/src/FakeItEasy/ParameterInfoExtensions.cs b/src/FakeItEasy/ParameterInfoExtensions.cs new file mode 100644 index 000000000..15934f5b5 --- /dev/null +++ b/src/FakeItEasy/ParameterInfoExtensions.cs @@ -0,0 +1,31 @@ +namespace FakeItEasy +{ + using System.Linq; + using System.Reflection; + + /// + /// Provides extension methods for . + /// + internal static class ParameterInfoExtensions + { + private const string IsReadOnlyAttributeFullName = "System.Runtime.CompilerServices.IsReadOnlyAttribute"; + + public static bool IsOutOrRef(this ParameterInfo parameterInfo) + { + if (!parameterInfo.ParameterType.IsByRef) + { + return false; + } + +#if FEATURE_PARAMETERINFO_CUSTOMATTRIBUTES_PROPERTY + var parameterAttributes = parameterInfo.CustomAttributes; + return parameterAttributes == null || + parameterAttributes.All(customAttributeData => customAttributeData.AttributeType.FullName != IsReadOnlyAttributeFullName); +#else + var parameterAttributes = parameterInfo.GetCustomAttributesData(); + return parameterAttributes == null || + parameterAttributes.All(customAttributeData => customAttributeData.Constructor.DeclaringType?.FullName != IsReadOnlyAttributeFullName); +#endif + } + } +} diff --git a/tests/FakeItEasy.Specs/InParametersSpecs.cs b/tests/FakeItEasy.Specs/InParametersSpecs.cs index 6d0c5817f..9fe6ff9a0 100644 --- a/tests/FakeItEasy.Specs/InParametersSpecs.cs +++ b/tests/FakeItEasy.Specs/InParametersSpecs.cs @@ -38,41 +38,43 @@ public void FakingInParam(IInParam fake, int argument, int result) } [Scenario] - public void SettingInParamInterface(IInParam fake, int argument) + public void SettingInParamInterface(IInParam fake, Exception exception) { "Given a faked interface with a method that takes an 'in' parameter" .x(() => fake = A.Fake()); - "And the 'in' argument has a known value" - .x(() => argument = 17); - "And a call to this method is configured to set a new value for the parameter" .x(() => A.CallTo(() => fake.Foo(A._)).AssignsOutAndRefParameters(19)); "When the method is called" - .x(() => fake.Foo(argument)); + .x(() => exception = Record.Exception(() => fake.Foo(A.Dummy()))); + + "Then it throws an exception" + .x(() => exception.Should().BeAnExceptionOfType()); - "Then the 'in' argument retains its original value" - .x(() => argument.Should().Be(17)); + "And the exception message indicates that the out and ref parameter counts don't agree" + .x(() => exception.Message.Should() + .Be("The number of values for out and ref parameters specified does not match the number of out and ref parameters in the call.")); } [Scenario] - public void SettingInParamDelegate(DelegateWithInParam fake, int argument) + public void SettingInParamDelegate(DelegateWithInParam fake, Exception exception) { "Given a faked delegate with a method that takes an 'in' parameter" .x(() => fake = A.Fake()); - "And the 'in' argument has a known value" - .x(() => argument = 17); - "And a call to this method is configured to set a new value for the parameter" .x(() => A.CallTo(() => fake.Invoke(A._)).AssignsOutAndRefParameters(19)); "When the method is called" - .x(() => fake.Invoke(argument)); + .x(() => exception = Record.Exception(() => fake.Invoke(A.Dummy()))); + + "Then it throws an exception" + .x(() => exception.Should().BeAnExceptionOfType()); - "Then the 'in' argument retains its original value" - .x(() => argument.Should().Be(17)); + "And the exception message indicates that the out and ref parameter counts don't agree" + .x(() => exception.Message.Should() + .Be("The number of values for out and ref parameters specified does not match the number of out and ref parameters in the call.")); } // A characterization test, representing the current capabilities of the code, not the desired state.