Refactor translator APIs

[dsplaisted]

In order to support .NET SDK Workloads, we will be updating the APIs for MSBuild SDK Resolvers. This is going to mean adding a few types to Microsoft.Build.Framework, which will need to be serialized / translated across node boundaries. Since ITranslatable isn't in the Microsoft.Build.Framework API, they can't implement that interface.

To make it easier to translate these objects, I propose the refactoring in this PR. ITranslator methods now take an ObjectTranslator delegate which can translate an object in both directions, as opposed to requiring that objects implement ITranslatable and that a factory be passed in. The TranslatorHelpers class includes extension methods which support the old APIs, so call sites that use ITranslatable objects and NodePacketValueFactory still work.

I also renamed / replaced the Translator<T> delegate (which wasn't used much) with an ObjectTranslator<T> delegate. I renamed it because it felt a bit confusing to have something called a "Translator" not implement the ITranslator interface. Calling it "ObjectTranslator" is a bit better, but maybe there's still a better name. I also swapped the order of the arguments to the delegate, so that you can write an ITranslator extension method that can either be called directly when translating a member field of an object, or passed to the ITranslator interface methods to help translate collection items.

[Forgind]

I like delegates, but is there any reason ITranslatable can't move to Microsoft.Build.Framework?

[dsplaisted]

I like delegates, but is there any reason ITranslatable can't move to Microsoft.Build.Framework?

Microsoft.Build.Framework is our public API and we can't make breaking changes to it. We'd have to pull ITranslator down together with it, and then we could no longer make API breaking changes to it.

[cdmihai]

Why not delete NodePacketValueFactory and replace it with ObjectTranslator?

[dsplaisted]

There's lots of code which already uses NodePacketValueFactory. A type that implements ITranslatable either needs to have a default constructor or have something like NodePacketValueFactory. I wasn't sure I wanted to commit to saying that all those types need to support default constructors.

[cdmihai]

But they can just as easily provide an ObjectTranslator, instead of NodePacketValueFactory, right? ObjectTranslator seems to me to be able to completely replace NodePacketValueFactory. Why keep two around when one can do the job?

[cdmihai]

I guess I'm a little bit confused how this enables types in Microsoft.Build.Framework to be translated without having them know about ITranslator or ITranslatable. Those new types wouldn't be able to provide ObjectTranslator delegates, because the delegate signature takes in an ITranslatable. Nor can they use the new TranslatorHelpers since it also depends on ITranslator. Can you please provide an example on how this would work on a new type in Microsoft.Build.Framework that needs translation?

If the translation is done by other types in Microsoft.Build, can't they just provide NodePacketValueFactory delegates for the types from Framework?

[dsplaisted]

@cdmihai the delegate takes an ITranslator, not an ITranslatable:

internal delegate void ObjectTranslator<T>(ITranslator translator, ref T objectToTranslate);

Here's an example of a helper method that can be implemented in the engine to translate a "POCO" from Microsoft.Build.Framework:

        public static void Translate(this ITranslator t, ref SdkReference sdkReference)
        {
            string name = null;
            string version = null;
            string minimumVersion = null;

            if (t.Mode == TranslationDirection.WriteToStream)
            {
                name = sdkReference.Name;
                version = sdkReference.Version;
                minimumVersion = sdkReference.MinimumVersion;
            }

            t.Translate(ref name);
            t.Translate(ref version);
            t.Translate(ref minimumVersion);

            if (t.Mode == TranslationDirection.ReadFromStream)
            {
                sdkReference = new SdkReference(name, version, minimumVersion);
            }
        }

[cdmihai]

Ok, so translation logic will be in Microsoft.Build, not Microsoft.Build.Framework. And ObjectTranslator is a convenient new delegate, equivalent to NodePacketValueFactory. The benefits over NodePacketValueFactory are that it avoids having to define an extra class / closure to hold the reference for the instance that needs translating, and it has a better name.

[Forgind]

Heads-up @dsplaisted:
I resolved a merge conflict with #5270. It wasn't too complicated, but let me know if I messed anything up.

[dsplaisted]

These changes are also part of #5269, so we will probably just use that PR. I also resolved the conflict there.

[rainersigwald]

Closing in favor of #5269; if you want to go in parts I'd be fine with that too.