Skip to content

chore: bump System.CommandLine from 2.0.0+beta.5 to 2.0.0 #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Seddryck wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: bump System.CommandLine from 2.0.0+beta.5 to 2.0.0 #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Didot.Cli/Didot.Cli.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
Expand Down Expand Up @@ -26,7 +26,7 @@
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="9.0.10" />
<PackageReference Include="Microsoft.Extensions.Hosting" Version="9.0.10" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.10" />
<PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1"/>
<PackageReference Include="System.CommandLine" Version="2.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Didot.Cli/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ public static async Task<int> Main(string[] args)
logger.LogInformation($"Didot Command Line Interface: version {Assembly.GetExecutingAssembly().GetName().Version}");

var renderCommand = host.Services.GetRequiredService<RootCommand>();
return await renderCommand.InvokeAsync(args);
return await renderCommand.Parse(args).InvokeAsync();
}
}
80 changes: 42 additions & 38 deletions src/Didot.Cli/RenderCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,65 +15,69 @@ public class RenderCommand : RootCommand
public RenderCommand(RenderOptions options, ILogger<RenderCommand>? logger = null)
: base("Didot Command Line Interface")
{
options.EngineExtensions.SetDefaultValue(new Dictionary<string, string>());
options.ParserExtensions.SetDefaultValue(new Dictionary<string, string>());
options.ParserParams.SetDefaultValue(new Dictionary<string, string>());
options.Sources.SetDefaultValue(new Dictionary<string, string>());
options.EngineExtensions.DefaultValueFactory = _ => [];
options.ParserExtensions.DefaultValueFactory = _ => [];
options.ParserParams.DefaultValueFactory = _ => [];
options.Sources.DefaultValueFactory = _ => [];

AddOption(options.Template);
AddOption(options.Engine);
AddOption(options.EngineExtensions);
AddOption(options.Sources);
AddOption(options.StdIn);
AddOption(options.Parser);
AddOption(options.ParserExtensions);
AddOption(options.ParserParams);
AddOption(options.Output);
Options.Add(options.Template);
Options.Add(options.Engine);
Options.Add(options.EngineExtensions);
Options.Add(options.Sources);
Options.Add(options.StdIn);
Options.Add(options.Parser);
Options.Add(options.ParserExtensions);
Options.Add(options.ParserParams);
Options.Add(options.Output);

AddValidator(result =>
Validators.Add(result =>
{
var stdInProvided = result.GetValueForOption(options.StdIn);
var stdInProvided = result.GetValue(options.StdIn);
var sourcesProvided = false;
try
{
sourcesProvided = result.GetValueForOption(options.Sources) is not null
&& result.GetValueForOption(options.Sources)!.Any();
sourcesProvided = (result.GetValue(options.Sources)?.Any() ?? false);
}
catch { }
catch
{ }
Comment on lines 37 to +42
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Review empty catch blocks in validators.

The validators use empty catch blocks around GetValue(options.Sources). While this appears defensive, silently swallowing exceptions could hide unexpected issues during parsing.

Consider whether GetValue can actually throw for the Sources option in System.CommandLine 2.0.0, or if there's a safer pattern (e.g., checking if the option was provided before calling GetValue). If exceptions are expected here, consider logging them or at least adding a comment explaining why they're caught and ignored.

Also applies to: 62-67

🏁 Script executed:

#!/bin/bash
# Check if there are other patterns in the codebase for handling optional GetValue calls
rg -n -A2 -B2 'GetValue.*Sources' --type cs

Length of output: 2043

I'll verify whether GetValue actually throws exceptions in System.CommandLine 2.0.0 and check for other patterns in the codebase.

Let me search for more context about how the Sources option is defined in the RenderCommand:

Now let me examine the RenderCommand code to understand the context better:

Based on my verification, the review comment is accurate. The empty catch blocks around GetValue() in RenderCommand.cs are problematic for several reasons:

  1. GetValue can throw exceptions for parsing errors, when no symbol is defined, or when the parsed result cannot be cast to the expected type.

  2. Empty catch blocks defeat the purpose of exceptions—when an exception occurs, nothing happens and the program fails for an unknown reason, leaving the application in an unknown state.

  3. All official System.CommandLine documentation examples show GetValue() called without try-catch blocks, suggesting this defensive pattern is unnecessary and non-idiomatic.

  4. Empty catch blocks hide errors and make debugging difficult, and exceptions should always be logged or handled appropriately.

Remove empty catch blocks and handle exceptions appropriately.

The try-catch blocks at lines 37-42 and 62-67 silently swallow exceptions without any logging or handling. Either:

  • Verify that GetValue() won't throw given the context (e.g., by checking result.Errors beforehand) and remove the try-catch, or
  • If exceptions are genuinely expected, catch them with specific exception types, log the issue, and add a comment explaining why this is needed.

Using bare catch { } masks errors and violates C# best practices.

🤖 Prompt for AI Agents 
In src/Didot.Cli/RenderCommand.cs around lines 37-42 (and similarly 62-67),
remove the empty catch blocks that silently swallow exceptions from
result.GetValue(); instead either (a) eliminate the try/catch and rely on
pre-checking result.Errors and the presence of the option/symbol before calling
GetValue(), or (b) if an exception can legitimately occur, catch specific
exception types only, log the error with context and add a comment explaining
why the exception is expected; do not use bare catch { } blocks.


if (stdInProvided && sourcesProvided)
result.ErrorMessage = "The --stdin option cannot be provided together with the --source option.";
result.AddError("The --stdin option cannot be provided together with the --source option.");
});

AddValidator(result =>
Validators.Add(result =>
{
var stdInProvided = result.GetValueForOption(options.StdIn);
var parserProvided = !string.IsNullOrEmpty(result.GetValueForOption(options.Parser));
var stdInProvided = result.GetValue(options.StdIn);
var parserProvided = !string.IsNullOrEmpty(result.GetValue(options.Parser));

if (stdInProvided && !parserProvided)
result.ErrorMessage = "The --parser option is required when using --stdin to specify the input source.";
result.AddError("The --parser option is required when using --stdin to specify the input source.");
});

AddValidator(result =>
Validators.Add(result =>
{
var stdInProvided = result.GetValueForOption(options.StdIn);
var stdInProvided = result.GetValue(options.StdIn);

var sourcesProvided = false;
try
{
var sourceProvided = result.GetValueForOption(options.Sources) is null || result.GetValueForOption(options.Sources)!.Any();
if (!stdInProvided && !sourceProvided)
result.ErrorMessage = "The --stdin option is required when not using --source to specify the input file(s).";
sourcesProvided = (result.GetValue(options.Sources)?.Any() ?? false);
}
catch { }
catch
{ }

if (!stdInProvided && !sourcesProvided)
result.AddError("The --stdin option is required when not using --source to specify the input file(s).");
});

this.SetHandler(new RenderCommandHandler(logger).Execute
, options.Template
, options.Engine
, options.EngineExtensions
, options.Sources
, options.Parser
, options.ParserExtensions
, options.ParserParams
, options.Output);
SetAction(parsed => new RenderCommandHandler(logger).Execute(
parsed.GetRequiredValue(options.Template)
, parsed.GetValue(options.Engine) ?? string.Empty
, parsed.GetRequiredValue(options.EngineExtensions)
, parsed.GetRequiredValue(options.Sources)
, parsed.GetValue(options.Parser) ?? string.Empty
, parsed.GetRequiredValue(options.ParserExtensions)
, parsed.GetRequiredValue(options.ParserParams)
, parsed.GetValue(options.Output) ?? string.Empty));
}
}
64 changes: 30 additions & 34 deletions src/Didot.Cli/RenderOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,87 +11,83 @@
namespace Didot.Cli;
public class RenderOptions
{
public Option<string> Template { get; } = new Option<string>(
new[] { "-t", "--template" },
description: "Path to the template file."
)
public Option<string> Template { get; } = new Option<string>("--template", ["-t"])
{
IsRequired = true,
Description = "Path to the template file.",
Required = true,
Arity = ArgumentArity.ExactlyOne
};

public Option<string> Engine { get; } = new Option<string>(
new[] { "-e", "--engine" },
description: "The template engine to use or to enforce when rendering the template.",
parseArgument: result => result.Tokens.Any() ? result.Tokens[0].Value.Trim() : string.Empty
)
public Option<string> Engine { get; } = new Option<string>("--engine", ["-e"])
{
CustomParser = result => result.Tokens.Any() ? result.Tokens[0].Value.Trim() : string.Empty,
Description = "The template engine to use or to enforce when rendering the template.",
Arity = ArgumentArity.ZeroOrOne
};

public Option<Dictionary<string, string>> EngineExtensions { get; } = new Option<Dictionary<string, string>>(
new[] { "-x", "--engine-extension" },
description: "Associate a file's extension to a specific template engine.",
parseArgument: result => ParseKeyValuePairs(result, ':', ';', NormalizeExtension)
"--engine-extension",["-x"]
)
{
CustomParser = result => ParseKeyValuePairs(result, ':', ';', NormalizeExtension),
Description = "Associate a file's extension to a specific template engine.",
Arity = ArgumentArity.ZeroOrMore,
AllowMultipleArgumentsPerToken = false
};

public Option<Dictionary<string, string>> Sources { get; } = new Option<Dictionary<string, string>>(
new[] { "-s", "--source" },
description: "Path to the source file.",
parseArgument: result => ParseKeyValuePairs(result, ':', ';', null, true)
"--source", ["-s"]
)
{
Description = "Path to the source file.",
CustomParser = result => ParseKeyValuePairs(result, ':', ';', null, true),
Arity = ArgumentArity.ZeroOrMore,
AllowMultipleArgumentsPerToken = true
};

public Option<bool> StdIn { get; } = new Option<bool>(
new[] { "-i", "--stdin" },
description: "Indicates that the input will come from stdin."
"--stdin", ["-i"]
)
{
Description = "Indicates that the input will come from stdin.",
Arity = ArgumentArity.ZeroOrOne
};

public Option<string> Parser { get; } = new Option<string>(
new[] { "-r", "--parser" },
description: "The parser to use when reading from StdIn or to enforce when reading from files.",
parseArgument: result => result.Tokens.Any() ? result.Tokens[0].Value.Trim() : string.Empty
"--parser", ["-r"]
)
{
CustomParser = result => result.Tokens.Any() ? result.Tokens[0].Value.Trim() : string.Empty,
Description = "The parser to use when reading from StdIn or to enforce when reading from files.",
Arity = ArgumentArity.ZeroOrOne
};

public Option<Dictionary<string, string>> ParserExtensions { get; } = new Option<Dictionary<string, string>>(
new[] { "-X", "--parser-extension" },
description: "Associate a file's extension to a specific parser.",
parseArgument: result => ParseKeyValuePairs(result, ':', ';', NormalizeExtension)
"--parser-extension", ["-X"]
)
{
Arity = ArgumentArity.ZeroOrMore,
AllowMultipleArgumentsPerToken = true
AllowMultipleArgumentsPerToken = true,
CustomParser = result => ParseKeyValuePairs(result, ':', ';', NormalizeExtension),
Description = "Associate a file's extension to a specific parser.",
};

public Option<Dictionary<string, string>> ParserParams { get; } = new Option<Dictionary<string, string>>(
new[] { "-P", "--parser-parameter" },
description: "Provide key-value parameters for parsers, such as configuration or dialect settings.",
parseArgument: result => ParseKeyValuePairs(result, ':', ';')
"--parser-parameter", ["-P"]
)
{
Arity = ArgumentArity.ZeroOrMore,
AllowMultipleArgumentsPerToken = true
AllowMultipleArgumentsPerToken = true,
CustomParser = result => ParseKeyValuePairs(result, ':', ';'),
Description = "Provide key-value parameters for parsers, such as configuration or dialect settings.",
};

public Option<string> Output { get; } = new Option<string>(
new[] { "-o", "--output" },
description: "Path to the generated file."
"--output", ["-o"]
)
{
Arity = ArgumentArity.ZeroOrOne
Arity = ArgumentArity.ZeroOrOne,
Description = "Path to the generated file."
};

private static Dictionary<string, string> ParseKeyValuePairs(ArgumentResult result, char keyValueSeparator, char pairSeparator, Func<string, string>? normalizeKey = null, bool allowEmptyKey = false)
Expand All @@ -110,7 +106,7 @@ private static Dictionary<string, string> ParseKeyValuePairs(ArgumentResult resu
dictionary[string.Empty] = keyValue[0].Trim();
else
{
result.ErrorMessage = $"The key is missing for the key-value pair: {pair}. A key is required when multiple key-value pairs are provided.";
result.AddError($"The key is missing for the key-value pair: {pair}. A key is required when multiple key-value pairs are provided.");
return dictionary;
}
}
Expand All @@ -121,7 +117,7 @@ private static Dictionary<string, string> ParseKeyValuePairs(ArgumentResult resu
}
else
{
result.ErrorMessage = $"Invalid key-value pair: {pair}";
result.AddError($"Invalid key-value pair: {pair}");
return dictionary;
}
}
Expand Down
2 changes: 1 addition & 1 deletion testing/Didot.Cli.Testing/ProgramTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public async Task Main_MissingTemplate_Failure()
};
var exitCode = await Program.Main(args);
Assert.That(exitCode, Is.Not.EqualTo(0));
Assert.That(ReadErrorStream(), Does.StartWith("Option '-t' is required."));
Assert.That(ReadErrorStream(), Does.Contain("Option '--template' is required."));
}

[Test]
Expand Down
22 changes: 8 additions & 14 deletions testing/Didot.Cli.Testing/RenderOptions/EngineExtensionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ public class EngineExtensionTests
public void EngineExtension_Empty_Valid()
{
var options = new Cli.RenderOptions();
var parser = new Parser(new RenderCommand(options));
var args = new List<string>() { "--template=file1.txt", "--stdin", "--parser=json" };

var result = parser.Parse(args);
var context = new InvocationContext(result);
var result = new RenderCommand(options).Parse(args);

Assert.That(context.ParseResult.GetValueForOption(options.EngineExtensions), Is.Empty);
Assert.That(result.GetValue(options.EngineExtensions), Is.Empty);
}

[Test]
Expand All @@ -30,14 +28,12 @@ public void EngineExtension_Empty_Valid()
public void EngineExtension_One_Valid(string additionalArgs)
{
var options = new Cli.RenderOptions();
var parser = new Parser(new RenderCommand(options));
var args = new List<string>() { "--template=file1.txt", "--stdin", "--parser=json", additionalArgs };

var result = parser.Parse(args);
var context = new InvocationContext(result);
var result = new RenderCommand(options).Parse(args);

Assert.That(context.ParseResult.Errors, Is.Null.Or.Empty);
var value = context.ParseResult.GetValueForOption(options.EngineExtensions);
Assert.That(result.Errors, Is.Null.Or.Empty);
var value = result.GetValue(options.EngineExtensions);
Assert.That(value, Is.Not.Null);
Assert.That(value, Has.Count.EqualTo(1));
Assert.That(value, Does.ContainKey(".liquid"));
Expand All @@ -51,14 +47,12 @@ public void EngineExtension_One_Valid(string additionalArgs)
public void EngineExtension_Many_Valid(string additionalArgs)
{
var options = new Cli.RenderOptions();
var parser = new Parser(new RenderCommand(options));
var args = new List<string>() { "--template=file1.txt", "--stdin", "--parser=json", additionalArgs };

var result = parser.Parse(args);
var context = new InvocationContext(result);
var result = new RenderCommand(options).Parse(args);

Assert.That(context.ParseResult.Errors, Is.Null.Or.Empty);
var value = context.ParseResult.GetValueForOption(options.EngineExtensions);
Assert.That(result.Errors, Is.Null.Or.Empty);
var value = result.GetValue(options.EngineExtensions);
Assert.That(value, Is.Not.Null);
Assert.That(value, Has.Count.EqualTo(2));
Assert.That(value, Does.ContainKey(".liquid"));
Expand Down
16 changes: 6 additions & 10 deletions testing/Didot.Cli.Testing/RenderOptions/OutputTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,24 @@ public class OutputTests
public void Output_Provided_Valid(params string[] optionArray)
{
var options = new Cli.RenderOptions();
var parser = new Parser(new RenderCommand(options));
var args = new List<string>() { "--template=file1.txt", "--stdin", "--parser=json" };
args.AddRange(optionArray);

var result = parser.Parse(args);
var context = new InvocationContext(result);
var result = new RenderCommand(options).Parse(args);

Assert.That(context.ParseResult.Errors, Is.Null.Or.Empty);
Assert.That(context.ParseResult.GetValueForOption(options.Output), Is.EqualTo("file1.txt"));
Assert.That(result.Errors, Is.Null.Or.Empty);
Assert.That(result.GetValue(options.Output), Is.EqualTo("file1.txt"));
}

[Test]
public void Output_NotProvided_Valid()
{
var options = new Cli.RenderOptions();
var parser = new Parser(new RenderCommand(options));
var args = new[] { "--template=file1.txt", "--stdin", "--parser=json" };

var result = parser.Parse(args);
var context = new InvocationContext(result);
var result = new RenderCommand(options).Parse(args);

Assert.That(context.ParseResult.Errors, Is.Null.Or.Empty);
Assert.That(context.ParseResult.GetValueForOption(options.Output), Is.Null);
Assert.That(result.Errors, Is.Null.Or.Empty);
Assert.That(result.GetValue(options.Output), Is.Null);
}
}
Loading