8000 Drop `System.ValueTuple` NuGet dependency by stakx · Pull Request #721 · devlooped/moq · GitHub
[go: up one dir, main page]

Skip to content

Drop System.ValueTuple NuGet dependency - #721

Merged
stakx merged 2 commits into
devlooped:masterfrom
stakx:no-valuetuple-nuget-ref
Nov 8, 2018
Merged

stakx merged 2 commits into
devlooped:masterfrom
stakx:no-valuetuple-nuget-ref

Conversation

@stakx
@stakx stakx commented Nov 8, 2018 •
Copy link
Copy Markdown
Contributor

People are still experiencing assembly versioning problems on the .NET Framework because of the NuGet packages System.Threading.Tasks.Extensions and System.ValueTuple referenced by Moq. (See e.g. the recent #719.)

It is fairly trivial to get rid of the latter dependency... so let's get rid of it! (It means we can no longer use tuple syntax everywhere in Moq's code base, but as we haven't made excessive use of it so far, that's perhaps a small price to pay.)

stakx added 2 commits November 9, 2018 00:29
People are still experiencing assembly versioning problems on the .NET
Framework because of the NuGet packages `System.Threading.Tasks.Exten-
sions` and `System.ValueTuple` referenced by Moq.

It is fairly trivial to get rid of the latter dependency... so let's
get rid of it! (It means we can no longer use tuple syntax everywhere
in Moq's code base, but as we haven't made excessive use of it so far,
that's perhaps a small price to pay.)
["System.ValueTuple`5"] = CreateValueTupleOf,
["System.ValueTuple`6"] = CreateValueTupleOf,
["System.ValueTuple`7"] = CreateValueTupleOf,
["System.ValueTuple`8"] = CreateValueTupleOf,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this means is that Moq now recognises System.ValueTuple<…> types by name instead of by strict type identity. This mimicks what the C# compiler already does.

(Unlike the C# compiler, Moq however won't validate the structural makeup of the matched types, so Moq's default value providers will not work correctly with such user types that don't have the expected constructor signature.)

Func<Type, Mock, object> factory;
return this.factories.TryGetValue(handlerKey , out factory) ? factory.Invoke(type, mock)
: this.factories.TryGetValue(handlerKey.FullName, out factory) ? factory.Invoke(type, mock)
: this.GetFallbackDefaultValue(type, mock);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value providers will now do two dictionary lookups instead of one in many cases. In theory, that's a slight performance degradation, but it would be surprising if this mattered much in practice. (A dictionary lookup is likely nothing when compared to all the reflection and code emitting going on elsewhere.)

}
}

internal readonly struct CallInfo
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could declare a few generic tuple types instead of this and other concrete readonly structs. For now, that degree of reusability isn't needed, and we spare the JITter some additional work by avoiding generics. (And, perhaps more importantly, declaring concrete, non-generic types gives us a means of giving the constituent parts names more meaningful than just Item1, Item2, etc.)

this.arguments = arguments;
}

public void Deconstruct(out Expression expression, out MethodInfo method, out IReadOnlyList<Expression> arguments)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a Deconstruct instead of just making the fields public means that we can still use tuple syntax / deconstruction in assignments.

@stakx
stakx merged commit 6e58d91 into devlooped:master Nov 8, 2018
@stakx
stakx deleted the no-valuetuple-nuget-ref branch November 8, 2018 23:54
@stakx stakx added this to the 4.10.1 milestone Nov 8, 2018
@devlooped devlooped locked and limited conversation to collaborators Sep 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

0