DefaultValue overrides command line values

[orjan]

Description

When adding a DefaultValue it overrides the value being passed to the command line, see Should_Override_Default_Value_For_CommandOption.

Debug

I've tried to debug the code but I don't have full picture, but I've noticed two things.

First we're setting the value 1567 from the command line, later in the execution path we override that value with the default value 10.

There's some kind guard trying to make sure that we're not adding adding the parameter as unmapped but Mapped is empty during the second phase of the execution.

I've tried the test below with 0.46.0 but it's not working so it's not a new regression at least.

This might be a potential bug or it might be a miss-configuration from my side?

Workaround

    [CommandOption("-l|--limit")]
    // [DefaultValue(10)]
    public int Limit { get; set; } = 10;

Failing test

namespace Spectre.Console.Tests.Unit.Cli;

public sealed class DefaultBranchSettingTests
{
    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void Should_Get_Default_Value_For_CommandOption(bool strictParsing)
    {
        // Given
        var app = new CommandAppTester();
        app.Configure(config =>
        {
            config.Settings.StrictParsing = strictParsing;
            config.PropagateExceptions();
            config.AddBranch<BranchSettings>("branch", animal =>
            {
                animal.AddCommand<BranchCommand>("cut");
            });
        });

        // When
        var result = app.Run(new[]
        {
            "branch", "cut",
        });

        // Then
        result.ExitCode.ShouldBe(0);
        var settings = result.Settings.ShouldBeOfType<BranchSettings>();
        settings.Limit.ShouldBe(10);
    }

    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void Should_Override_Default_Value_For_CommandOption(bool strictParsing)
    {
        // Given
        var app = new CommandAppTester();
        app.Configure(config =>
        {
            config.Settings.StrictParsing = strictParsing;
            config.PropagateExceptions();
            config.AddBranch<BranchSettings>("branch", branch =>
            {
                branch.AddCommand<BranchCommand>("cut");
            });
        });

        // When
        var result = app.Run(new[]
        {
            "branch", "--limit", "1567", "cut",
        });

        // Then
        result.ExitCode.ShouldBe(0);
        var settings = result.Settings.ShouldBeOfType<BranchSettings>();
        settings.Limit.ShouldBe(1567);
    }

    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void Should_Get__Value_For_CommandOption(bool strictParsing)
    {
        // Given
        var app = new CommandAppTester();
        app.Configure(config =>
        {
            config.Settings.StrictParsing = strictParsing;
            config.PropagateExceptions();
            config.AddBranch<BranchSettings>("branch", branch =>
            {
                branch.AddCommand<BranchCommand>("cut");
            });
        });

        // When
        var result = app.Run(new[]
        {
            "branch", "--age", "47", "cut",
        });

        // Then
        result.ExitCode.ShouldBe(0);
        var settings = result.Settings.ShouldBeOfType<BranchSettings>();
        settings.Age.ShouldBe(47);
    }
}

file sealed class BranchCommand : Command<BranchSettings>
{
    public override int Execute(CommandContext context, BranchSettings settings)
    {
        return 0;
    }
}

file sealed class BranchSettings : CommandSettings
{
    [CommandOption("-l|--limit")]
    [DefaultValue(10)]
    public int Limit { get; set; }

    [CommandOption("--age")]
    public int Age { get; set; }
}

[orjan]

Another workaround is to flip the arguments e.g.

"branch", "cut", "--limit", "1567"

This test is passing

    [Theory]
    [InlineData(true)]
    [InlineData(false)]
    public void Should_Override_Default_Value_For_CommandOptionOrdering(bool strictParsing)
    {
        // Given
        var app = new CommandAppTester();
        app.Configure(config =>
        {
            config.Settings.StrictParsing = strictParsing;
            config.PropagateExceptions();
            config.AddBranch<BranchSettings>("branch", branch =>
            {
                branch.AddCommand<BranchCommand>("cut");
            });
        });

        // When
        var result = app.Run(new[]
        {
            "branch", "cut", "--limit", "1567",
        });

        // Then
        result.ExitCode.ShouldBe(0);
        var settings = result.Settings.ShouldBeOfType<BranchSettings>();
        settings.Limit.ShouldBe(1567);
    }

[nils-a]

Looks like this could be related to #401 and/or #1612

[orjan]

Yes they are probably related.

I've seen a guard comparing the types for the branch and the command settings, so another work around is to

file sealed class BranchCommand : Command<BranchCutterSettings>
{
    public override int Execute(CommandContext context, BranchCutterSettings settings)
    {
        return 0;
    }
}

file class BranchSettings : CommandSettings
{
    [CommandOption("-l|--limit")]
    [DefaultValue(10)]
    public int Limit { get; set; }

    [CommandOption("--age")]
    public int Age { get; set; }
}

file sealed class BranchCutterSettings : BranchSettings;

We we doesn't support composition e.g. new MyCommandSetting<BranchSetting>(branchSetting) there's little use of the ordering for things? The only the command cares about is the fully populated settings object?