Provide a way for modules to add default assemblies and namespaces for the script

[jnm2]

I'd like to be able to have the module add itself to IScriptConventions.GetDefaultAssemblies. You can't do that without registering as the only IScriptConventions instance, and you can't register as the only IScriptConventions instance without losing access to the previous instance of IScriptConventions that you need to delegate to. If two modules need to do this, one will cancel out the other.

That's a general frustration with the registration model, no way to just decorate instances, but what would be nice is some way to supplement the default assemblies and namespaces.

[bjorkstromm]

Hmm, if we only could pass the container to the modules when registering. Then you could first resolve the current registered IScriptConventions and just wrap it inside your IScriptConventions implementation.

@patriksvensson what do you think about that idea? But in order to do that, we should create our own container abstraction, or else modules needs to take on Autofac dependency.

[jnm2]

Taking on an AutoFac dependency means locking it to 3.5.2, Cake's referenced version, which means making it incredibly difficult for my module to target netstandard1.6.

Container abstraction just needs Resolve and Release, right?

[patriksvensson]

@mholo65 Why not allow registering a IScriptConvention (singular) with the module that ScriptConventions can use.

We could change ScriptConventions constructor to something like:

public ScriptConventions(... IEnumerable<IScriptConvention> conventions)
{
  ...
}

[jnm2]

That's a nicer solution for this particular issue. But, next I will ask to be able to decorate the previous ICakeEngine instance, so @mholo65's solution kills more birds with one stone. Having both would be great too!

[patriksvensson]

IModule implementations already supports constructor injection of registered dependencies so I don't think this is necessary.

To replace a service, register another one of the same service type. The last registered one will always be the one that's used.

[jnm2]

@patriksvensson Ah, I didn't try taking the previous IScriptConventions as a constructor dependency in my ICakeModule implementation! You're right, that should work.

[bjorkstromm]

@jnm2 could you verify that suggested spproach work for you, and if it does please close this issue. Thanks!

[jnm2]

@mholo65 Yes. I'll hopefully get to that tomorrow.

[jnm2]

Confirmed working:

internal sealed class SomeCakeModule : ICakeModule
{
    private readonly ICakeEngine previousEngine;
    private readonly IScriptConventions previousScriptConventions;

    public BeforeDependenciesCakeModule(ICakeEngine previousEngine, IScriptConventions previousScriptConventions)
    {
        this.previousEngine = previousEngine;
        this.previousScriptConventions = previousScriptConventions;
    }

    public void Register(ICakeContainerRegistrar registrar)
    {
        registrar
            .RegisterInstance(new ExtendedCakeEngine(previousEngine))
            .As<ICakeEngine>();

        registrar
            .RegisterInstance(previousScriptConventions.CreateWithAppended(
                defaultNamespaces: new[] { typeof(Extensions).Namespace },
                defaultAssemblies: new[] { typeof(SomeCakeModule).GetTypeInfo().Assembly }))
            .As<IScriptConventions>();
    }
}

Where I define the extension method ScriptConventionsUtils.CreateWithAppended:

internal static class ScriptConventionsUtils
{
    public static IScriptConventions CreateWithAppended(this IScriptConventions baseConventions, IReadOnlyList<string> defaultNamespaces, IReadOnlyList<Assembly> defaultAssemblies)
    {
        return new AppendedScriptConventions(baseConventions, defaultNamespaces, defaultAssemblies);
    }

    private sealed class AppendedScriptConventions : IScriptConventions
    {
        private readonly IScriptConventions baseConventions;
        private readonly IReadOnlyList<string> defaultNamespaces;
        private readonly IReadOnlyList<Assembly> defaultAssemblies;

        public AppendedScriptConventions(IScriptConventions baseConventions, IReadOnlyList<string> defaultNamespaces, IReadOnlyList<Assembly> defaultAssemblies)
        {
            this.baseConventions = baseConventions ?? throw new ArgumentNullException(nameof(baseConventions));
            this.defaultNamespaces = defaultNamespaces ?? throw new ArgumentNullException(nameof(defaultNamespaces));
            this.defaultAssemblies = defaultAssemblies ?? throw new ArgumentNullException(nameof(defaultAssemblies));
        }

        public IReadOnlyList<string> GetDefaultNamespaces()
        {
            return Concat(baseConventions.GetDefaultNamespaces(), defaultNamespaces);
        }

        public IReadOnlyList<Assembly> GetDefaultAssemblies(DirectoryPath root)
        {
            return Concat(baseConventions.GetDefaultAssemblies(root), defaultAssemblies);
        }

        private static IReadOnlyList<T> Concat<T>(IReadOnlyList<T> first, IReadOnlyList<T> second)
        {
            var r = new T[first.Count + second.Count];
            first.CopyTo(r, 0);
            second.CopyTo(r, first.Count);
            return r;
        }
    }
}

Plus

internal static void CopyTo<T>(this IReadOnlyList<T> list, T[] array, int arrayIndex)
{
    if (list is IList<T> mutable)
    {
        mutable.CopyTo(array, arrayIndex);
    }
    else
    {
        var count = list.Count;
        for (var i = 0; i < count; i++)
            array[i + arrayIndex] = list[i];
    }
}

---

To Patrik's point, do you guys want to investigate making an easier API so that you don't have to define this extension method and replace IScriptConventions implementations?

[patriksvensson]

@jnm2 Perhaps another alternative would be to add a new attribute that could be used to tell cake about stuff a module needs?

[assembly:ImportNamespace("MyThing.Other")]
[assembly:ImportNamespace(typeof(SomeCakeModule))]
[assembly:ImportAssembly(typeof(SomeCakeModule))]

[jnm2]

@patriksvensson I like the syntax. Let's say someone doesn't want to be limited to compile-time constants- would they just do the IScriptConventions replacement? I can't think of any scenarios off the top of my head. Or could we have a CakeModuleBase implementing ICakeModule where you could override ImportedNamespaces and ImportedAssemblies properties?

[bjorkstromm]

I think I mentioned on some issue we should also honor FrameworkAssemblies specified in nuspec for a addin.

[patriksvensson]

@mholo65 Yes

@jnm2 We already use attributes to tell Cake what namespaces to be imported for a Cake alias, so doing the same with modules would keep things consistent.

[jnm2]

@patriksvensson Cool, I like it. Want a PR?

[patriksvensson]

@jnm2 Sure! Awesome! 👍

[jnm2]

@patriksvensson I'd like to stay symmetrical with addins and use CakeNamespaceImportAttribute and add a CakeAssemblyReferenceAttribute to match. Sound good?

[jnm2]

Things are never simple. Currently stuck at #1783 if you would be so kind as to review. =)