Finish implementation of command builders

src/Discord.Net.Commands/Builders/CommandBuilder.cs

@@ -22,7 +22,7 @@ namespace Discord.Commands.Builders
         public string Name { get; set; }
         public string Summary { get; set; }
         public string Remarks { get; set; }
-        public Func<CommandContext, object[], Task> Callback { get; set; }
+        public Func<CommandContext, object[], IDependencyMap, Task> Callback { get; set; }
         public ModuleBuilder Module { get; }
 
         public List<PreconditionAttribute> Preconditions => preconditions;
@@ -47,7 +47,7 @@ namespace Discord.Commands.Builders
             return this;
         }
 
-        public CommandBuilder SetCallback(Func<CommandContext, object[], Task> callback)
+        public CommandBuilder SetCallback(Func<CommandContext, object[], IDependencyMap, Task> callback)
         {
             Callback = callback;
             return this;

src/Discord.Net.Commands/Builders/ModuleBuilder.cs

@@ -12,16 +12,16 @@ namespace Discord.Commands.Builders
         private List<string> aliases;
 
         public ModuleBuilder()
-            : this(null)
-        { }
-
-        internal ModuleBuilder(ModuleBuilder parent)
         {
             commands = new List<CommandBuilder>();
             submodules = new List<ModuleBuilder>();
             preconditions = new List<PreconditionAttribute>();
             aliases = new List<string>();
-            
+        }
+
+        internal ModuleBuilder(ModuleBuilder parent)
+            : this()
+        {
             ParentModule = parent;
         }
 

src/Discord.Net.Commands/Builders/ParameterBuilder.cs

@@ -39,6 +39,7 @@ namespace Discord.Commands.Builders
 
         public ParameterBuilder SetDefault<T>(T defaultValue)
         {
+            Optional = true;
             DefaultValue = defaultValue;
             ParameterType = typeof(T);
 

src/Discord.Net.Commands/CommandService.cs

@@ -11,10 +11,8 @@ namespace Discord.Commands
 {
     public class CommandService
     {
-        private static readonly TypeInfo _moduleTypeInfo = typeof(ModuleBase).GetTypeInfo();
-
         private readonly SemaphoreSlim _moduleLock;
-        private readonly ConcurrentDictionary<Type, ModuleInfo> _moduleDefs;
+        private readonly ConcurrentDictionary<Type, ModuleInfo> _moduleDefs; 
         private readonly ConcurrentDictionary<Type, TypeReader> _typeReaders;
         private readonly CommandMap _map;
 
@@ -71,16 +69,18 @@ namespace Discord.Commands
             try
             {
                 var typeInfo = typeof(T).GetTypeInfo();
-                if (!_moduleTypeInfo.IsAssignableFrom(typeInfo))
-                    throw new ArgumentException($"Modules must inherit ModuleBase.");
-
-                if (typeInfo.IsAbstract)
-                    throw new InvalidOperationException("Modules must not be abstract.");
 
                 if (_moduleDefs.ContainsKey(typeof(T)))
                     throw new ArgumentException($"This module has already been added.");
 
-                return AddModuleInternal(typeInfo);
+                var module = ModuleClassBuilder.Build(this, typeInfo).First();
+
+                _moduleDefs[typeof(T)] = module;
+
+                foreach (var cmd in module.Commands)
+                    _map.AddCommand(cmd);
+
+                return module;
             }
             finally
             {
@@ -93,43 +93,25 @@ namespace Discord.Commands
             await _moduleLock.WaitAsync().ConfigureAwait(false);
             try
             {
-                foreach (var type in assembly.ExportedTypes)
+                var types = ModuleClassBuilder.Search(assembly);
-                {
+                return ModuleClassBuilder.Build(types, this).ToImmutableArray();
-                    if (!_moduleDefs.ContainsKey(type))
-                    {
-                        var typeInfo = type.GetTypeInfo();
-                        if (_moduleTypeInfo.IsAssignableFrom(typeInfo))
-                        {
-                            var dontAutoLoad = typeInfo.GetCustomAttribute<DontAutoLoadAttribute>();
-                            if (dontAutoLoad == null && !typeInfo.IsAbstract)
-                                moduleDefs.Add(AddModuleInternal(typeInfo));
-                        }
-                    }
-                }
-                return moduleDefs.ToImmutable();
             }
             finally
             {
                 _moduleLock.Release();
             }
         }
-        private ModuleInfo AddModuleInternal(TypeInfo typeInfo)
-        {
-            var moduleDef = new ModuleInfo(typeInfo, this);
-            _moduleDefs[typeInfo.AsType()] = moduleDef;
-
-            foreach (var cmd in moduleDef.Commands)
-                _map.AddCommand(cmd);
-
-            return moduleDef;
-        }
 
         public async Task<bool> RemoveModule(ModuleInfo module)
         {
             await _moduleLock.WaitAsync().ConfigureAwait(false);
             try
             {
-                return RemoveModuleInternal(module.Source.BaseType);
+                var type = _moduleDefs.FirstOrDefault(x => x.Value == module);
+                if (default(KeyValuePair<Type, ModuleInfo>).Key == type.Key)
+                    throw new KeyNotFoundException($"Could not find the key for the module {module?.Name ?? module?.Aliases?.FirstOrDefault()}");
+
+                return RemoveModuleInternal(type.Key);
             }
             finally
             {

src/Discord.Net.Commands/Info/CommandInfo.cs

@@ -15,7 +15,7 @@ namespace Discord.Commands
         private static readonly System.Reflection.MethodInfo _convertParamsMethod = typeof(CommandInfo).GetTypeInfo().GetDeclaredMethod(nameof(ConvertParamsList));
         private static readonly ConcurrentDictionary<Type, Func<IEnumerable<object>, object>> _arrayConverters = new ConcurrentDictionary<Type, Func<IEnumerable<object>, object>>();
 
-        private readonly Func<CommandContext, object[], Task> _action;
+        private readonly Func<CommandContext, object[], IDependencyMap, Task> _action;
 
         public ModuleInfo Module { get; }
         public string Name { get; }
@@ -89,7 +89,7 @@ namespace Discord.Commands
             return await CommandParser.ParseArgs(this, context, input, 0).ConfigureAwait(false);
         }
 
-        public Task<ExecuteResult> Execute(CommandContext context, ParseResult parseResult)
+        public Task<ExecuteResult> Execute(CommandContext context, ParseResult parseResult, IDependencyMap map)
         {
             if (!parseResult.IsSuccess)
                 return Task.FromResult(ExecuteResult.FromError(parseResult));
@@ -110,23 +110,26 @@ namespace Discord.Commands
                 paramList[i] = parseResult.ParamValues[i].Values.First().Value;
             }
 
-            return Execute(context, argList, paramList);
+            return Execute(context, argList, paramList, map);
         }
-        public async Task<ExecuteResult> Execute(CommandContext context, IEnumerable<object> argList, IEnumerable<object> paramList)
+        public async Task<ExecuteResult> Execute(CommandContext context, IEnumerable<object> argList, IEnumerable<object> paramList, IDependencyMap map)
         {
+            if (map == null)
+                map = DependencyMap.Empty;
+
             try
             {
                 var args = GenerateArgs(argList, paramList);
                 switch (RunMode)
                 {
                     case RunMode.Sync: //Always sync
-                        await _action(context, args).ConfigureAwait(false);
+                        await _action(context, args, map).ConfigureAwait(false);
                         break;
                     case RunMode.Mixed: //Sync until first await statement
-                        var t1 = _action(context, args);
+                        var t1 = _action(context, args, map);
                         break;
                     case RunMode.Async: //Always async
-                        var t2 = Task.Run(() => _action(context, args));
+                        var t2 = Task.Run(() => _action(context, args, map));
                         break;
                 }
                 return ExecuteResult.FromSuccess();

src/Discord.Net.Commands/Info/ModuleInfo.cs

@@ -31,14 +31,15 @@ namespace Discord.Commands
             Preconditions = BuildPreconditions(builder).ToImmutableArray();
         }
 
-        private static List<string> BuildAliases(ModuleBuilder builder)
+        private static IEnumerable<string> BuildAliases(ModuleBuilder builder)
         {
             IEnumerable<string> result = null;
 
             Stack<ModuleBuilder> builderStack = new Stack<ModuleBuilder>();
+            builderStack.Push(builder);
 
-            ModuleBuilder parent = builder;
+            ModuleBuilder parent = builder.ParentModule;
-            while (parent.ParentModule != null)
+            while (parent != null)
             {
                 builderStack.Push(parent);
                 parent = parent.ParentModule;
@@ -49,11 +50,13 @@ namespace Discord.Commands
                 ModuleBuilder level = builderStack.Pop(); // get the topmost builder
                 if (result == null)
                     result = level.Aliases.ToList(); // create a shallow copy so we don't overwrite the builder unexpectedly
-                else
+                else if (result.Count() > level.Aliases.Count)
                     result = result.Permutate(level.Aliases, (first, second) => first + " " + second);
+                else
+                    result = level.Aliases.Permutate(result, (second, first) => first + " " + second);
             }
 
-            return result.ToList();
+            return result;
         }
 
         private static List<PreconditionAttribute> BuildPreconditions(ModuleBuilder builder)

src/Discord.Net.Commands/Utilities/ModuleClassBuilder.cs

@@ -0,0 +1,213 @@
+using System;
+using System.Linq;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Threading.Tasks;
+
+using Discord.Commands.Builders;
+
+namespace Discord.Commands
+{
+    internal static class ModuleClassBuilder
+    {
+        private static readonly TypeInfo _moduleTypeInfo = typeof(ModuleBase).GetTypeInfo();
+
+        public static IEnumerable<TypeInfo> Search(Assembly assembly)
+        {
+            foreach (var type in assembly.ExportedTypes)
+            {
+                var typeInfo = type.GetTypeInfo();
+                if (IsValidModuleDefinition(typeInfo) &&
+                    !typeInfo.IsDefined(typeof(DontAutoLoadAttribute)))
+                {
+                    yield return typeInfo;
+                }
+            }
+        }
+
+        public static IEnumerable<ModuleInfo> Build(CommandService service, params TypeInfo[] validTypes) => Build(validTypes, service);
+        public static IEnumerable<ModuleInfo> Build(IEnumerable<TypeInfo> validTypes, CommandService service)
+        {
+            if (!validTypes.Any())
+                throw new InvalidOperationException("Could not find any valid modules from the given selection");
+            
+            var topLevelGroups = validTypes.Where(x => x.DeclaringType == null);
+            var subGroups = validTypes.Intersect(topLevelGroups);
+
+            var builtTypes = new List<TypeInfo>();
+
+            var result = new List<ModuleInfo>();
+
+            foreach (var typeInfo in topLevelGroups)
+            {
+                // this shouldn't be the case; may be safe to remove?
+                if (builtTypes.Contains(typeInfo))
+                    continue;
+
+                builtTypes.Add(typeInfo);
+
+                var module = new ModuleBuilder();
+
+                BuildModule(module, typeInfo, service);
+                BuildSubTypes(module, typeInfo.DeclaredNestedTypes, builtTypes, service);
+
+                result.Add(module.Build(service));
+            }
+
+            return result;
+        }
+
+        private static void BuildSubTypes(ModuleBuilder builder, IEnumerable<TypeInfo> subTypes, List<TypeInfo> builtTypes, CommandService service)
+        {
+            foreach (var typeInfo in subTypes)
+            {
+                if (builtTypes.Contains(typeInfo))
+                    continue;
+
+                builtTypes.Add(typeInfo);
+                
+                builder.AddSubmodule((module) => {
+                    BuildModule(module, typeInfo, service);
+                    BuildSubTypes(module, typeInfo.DeclaredNestedTypes, builtTypes, service);
+                });
+            }
+        }
+
+        private static void BuildModule(ModuleBuilder builder, TypeInfo typeInfo, CommandService service)
+        {
+            var attributes = typeInfo.GetCustomAttributes();
+
+            foreach (var attribute in attributes)
+            {
+                // TODO: C#7 type switch
+                if (attribute is NameAttribute)
+                    builder.Name = (attribute as NameAttribute).Text;
+                else if (attribute is SummaryAttribute)
+                    builder.Summary = (attribute as SummaryAttribute).Text;
+                else if (attribute is RemarksAttribute)
+                    builder.Remarks = (attribute as RemarksAttribute).Text;
+                else if (attribute is AliasAttribute)
+                    builder.AddAliases((attribute as AliasAttribute).Aliases);
+                else if (attribute is GroupAttribute)
+                    builder.AddAliases((attribute as GroupAttribute).Prefix);
+                else if (attribute is PreconditionAttribute)
+                    builder.AddPrecondition(attribute as PreconditionAttribute);
+            }
+
+            var validCommands = typeInfo.DeclaredMethods.Where(x => IsValidCommandDefinition(x));
+
+            foreach (var method in validCommands)
+            {
+                builder.AddCommand((command) => {
+                    BuildCommand(command, typeInfo, method, service);
+                });
+            }
+        }
+
+        private static void BuildCommand(CommandBuilder builder, TypeInfo typeInfo, MethodInfo method, CommandService service)
+        {
+            var attributes = method.GetCustomAttributes();
+
+            foreach (var attribute in attributes)
+            {
+                // TODO: C#7 type switch
+                if (attribute is NameAttribute)
+                    builder.Name = (attribute as NameAttribute).Text;
+                else if (attribute is SummaryAttribute)
+                    builder.Summary = (attribute as SummaryAttribute).Text;
+                else if (attribute is RemarksAttribute)
+                    builder.Remarks = (attribute as RemarksAttribute).Text;
+                else if (attribute is AliasAttribute)
+                    builder.AddAliases((attribute as AliasAttribute).Aliases);
+                else if (attribute is GroupAttribute)
+                    builder.AddAliases((attribute as GroupAttribute).Prefix);
+                else if (attribute is PreconditionAttribute)
+                    builder.AddPrecondition(attribute as PreconditionAttribute);
+            }
+
+            var parameters = method.GetParameters();
+            int pos = 0, count = parameters.Length;
+            foreach (var paramInfo in parameters)
+            {
+                builder.AddParameter((parameter) => {
+                    BuildParameter(parameter, paramInfo, pos++, count, service);
+                });
+            }
+
+            var createInstance = ReflectionUtils.CreateBuilder<ModuleBase>(typeInfo, service);
+
+            builder.Callback = (ctx, args, map) => {
+                var instance = createInstance(map);
+                instance.Context = ctx;
+                try
+                {
+                    return method.Invoke(instance, args) as Task ?? Task.CompletedTask;
+                }
+                finally{
+                    (instance as IDisposable)?.Dispose();
+                }
+            };
+        }
+
+        private static void BuildParameter(ParameterBuilder builder, System.Reflection.ParameterInfo paramInfo, int position, int count, CommandService service)
+        {
+            var attributes = paramInfo.GetCustomAttributes();
+            var paramType = paramInfo.ParameterType;
+
+            builder.Optional = paramInfo.IsOptional;
+            builder.DefaultValue = paramInfo.HasDefaultValue ? paramInfo.DefaultValue : null;
+
+            foreach (var attribute in attributes)
+            {
+                // TODO: C#7 type switch
+                if (attribute is NameAttribute)
+                    builder.Name = (attribute as NameAttribute).Text;
+                else if (attribute is SummaryAttribute)
+                    builder.Summary = (attribute as SummaryAttribute).Text;
+                else if (attribute is ParamArrayAttribute)
+                {
+                    builder.Multiple = true;
+                    paramType = paramType.GetElementType();
+                }
+                else if (attribute is RemainderAttribute)
+                {
+                    if (position != count-1)
+                        throw new InvalidOperationException("Remainder parameters must be the last parameter in a command.");
+                    
+                    builder.Remainder = true;
+                }
+            }
+
+            var reader = service.GetTypeReader(paramType);
+            if (reader == null)
+            {
+                var paramTypeInfo = paramType.GetTypeInfo();
+                if (paramTypeInfo.IsEnum)
+                {
+                    reader = EnumTypeReader.GetReader(paramType);
+                    service.AddTypeReader(paramType, reader);
+                }
+                else
+                {
+                    throw new InvalidOperationException($"{paramType.FullName} is not supported as a command parameter, are you missing a TypeReader?");
+                }
+            }
+
+            builder.TypeReader = reader;
+        }
+
+        private static bool IsValidModuleDefinition(TypeInfo typeInfo)
+        {
+            return _moduleTypeInfo.IsAssignableFrom(typeInfo) &&
+                   !typeInfo.IsAbstract;
+        }
+
+        private static bool IsValidCommandDefinition(MethodInfo methodInfo)
+        {
+            return methodInfo.IsDefined(typeof(CommandAttribute)) &&
+                   methodInfo.ReturnType != typeof(Task) &&
+                   !methodInfo.IsStatic &&
+                   !methodInfo.IsGenericMethod;
+        }
+    }
+}