AddDelegate produces a MissingMethodException

[JacobCZ]

Information

• OS: macOS 14.3
• Version: 0.48.0
• .NET Version: 8.0.100

Describe the bug
Using the AddDelegate method on a command branch produces a MissingMethodException.

To Reproduce

1. Create a simple CLI app
2. Add a branch
3. Use AddDelegate
4. Run the app

...

var app = new CommandApp();
app.Configure(cfg => {
  cfg.AddBranch("demo", d => {
    d.AddDelegate("delegate", ctx => {
      AnsiConsole.MarkupLine("[red]This doesn't work[/]");
      return 0;
    });
  });
});

...

app.Run(args);

This produces a MissingMethodException.

Spectre.Console.Cli.CommandRuntimeException: Could not resolve type 'Spectre.Console.Cli.CommandSettings'.
     System.MissingMethodException: Cannot dynamically create an instance of type 'Spectre.Console.Cli.CommandSettings'. Reason: Cannot create an abstract class.
       at System.RuntimeType.ActivatorCache..ctor(RuntimeType rt)
       at object System.RuntimeType.CreateInstanceDefaultCtor(bool publicOnly, bool wrapExceptions)
       at object Spectre.Console.Cli.TypeResolverAdapter.Resolve(Type type) in /_/src/Spectre.Console.Cli/Internal/TypeResolverAdapter.cs:28
  at object Spectre.Console.Cli.TypeResolverAdapter.Resolve(Type type) in /_/src/Spectre.Console.Cli/Internal/TypeResolverAdapter.cs:36
  at CommandSettings Spectre.Console.Cli.CommandPropertyBinder.CreateSettings(ITypeResolver resolver, Type settingsType) in /_/src/Spectre.Console.Cli/Internal/Binding/
     CommandPropertyBinder.cs:29
  at CommandSettings Spectre.Console.Cli.CommandPropertyBinder.CreateSettings(CommandValueLookup lookup, Type settingsType, ITypeResolver resolver) in
     /_/src/Spectre.Console.Cli/Internal/Binding/CommandPropertyBinder.cs:7
  at CommandSettings Spectre.Console.Cli.CommandBinder.Bind(CommandTree tree, Type settingsType, ITypeResolver resolver) in /_/src/Spectre.Console.Cli/Internal/CommandBinder.cs:26
  at Task<int> Spectre.Console.Cli.CommandExecutor.Execute(CommandTree leaf, CommandTree tree, CommandContext context, ITypeResolver resolver, IConfiguration configuration) in
     /_/src/Spectre.Console.Cli/Internal/CommandExecutor.cs:132
  at async Task<int> Spectre.Console.Cli.CommandExecutor.Execute(IConfiguration configuration, IEnumerable<string> args) in /_/src/Spectre.Console.Cli/Internal/CommandExecutor.cs:83
  at async Task<int> Spectre.Console.Cli.CommandApp.RunAsync(IEnumerable<string> args) in /_/src/Spectre.Console.Cli/CommandApp.cs:84
  at int Spectre.Console.Cli.CommandApp.Run(IEnumerable<string> args) in /_/src/Spectre.Console.Cli/CommandApp.cs:58
  at int Program.Main(string[] args) in /.../Program.cs:90

Expected behavior
The delegate function runs and prints "This doesn't work" in red

[patriksvensson]

@FrankRay78 Do you know if any of the work we've done lately might have affected this?

[FrankRay78]

It's be a long time since any core CLI changes have been made, so without stepping through the code/git history, I suspect it's been latent. Also, I've suspected for a while now, that branch behaviour, particularly branch of branch of branch, is not as thoroughly used/test covered, compared to default command, first level commands. I would guess there are other 'edge cases' like this, lying latent.

[gecko-byte]

@FrankRay78 @patriksvensson

Looking at the Empty Configurator in the case you call .AddDelegate in this case

 app.Configure(cfg =>
        {
            cfg.AddDelegate("a", _ =>
            {
                AnsiConsole.MarkupLine("[red]Complete[/]");
                return 0;
            });
        });

It will default to using configurator.AddDelegate<EmptyCommandSettings>
but when called in this case

app.Configure(cfg =>
        {
            cfg.AddBranch("a", d =>
            {
                d.AddDelegate("b", _ =>
                {
                    AnsiConsole.MarkupLine("[red]Complete[/]");
                    return 0;
                });
            });
        });

It defaulted to using TSettings even when no settings are supplied.
One possible solution is what I have attached in the diff below where we will check if the settings are abstract and if so fallback to EmptyCommandSettings. I have added tests for these two use cases and can make a PR if this looks correct!

@@ -324,11 +324,16 @@
     /// <param name="func">The delegate to execute as part of command execution.</param>
     /// <returns>A command configurator that can be used to configure the command further.</returns>
     public static ICommandConfigurator AddDelegate<TSettings>(
-        this IConfigurator<TSettings> configurator,
+        this IConfigurator<TSettings>? configurator,
         string name,
         Func<CommandContext, int> func)
-            where TSettings : CommandSettings
+        where TSettings : CommandSettings
     {
+        if (typeof(TSettings).IsAbstract)
+        {
+            AddDelegate(configurator as IConfigurator<EmptyCommandSettings>, name, func);
+        }
+
         if (configurator == null)
         {
             throw new ArgumentNullException(nameof(configurator));

[gecko-byte]

@JacobCZ adding this should fix it in the meantime!

...

var app = new CommandApp();
app.Configure(cfg => {
  cfg.AddBranch("demo", d => {
    d.AddDelegate<EmptyCommandSettings>("delegate", ctx => {
      AnsiConsole.MarkupLine("[red]This doesn't work[/]");
      return 0;
    });
  });
});

...

app.Run(args);

[FrankRay78]

I'd welcome a PR for this @BlazeFace, and I would prioritise its review with a view to merging.