commandlineparser / commandlineparser/commandline
Sequences with Max=N should not swallow values past N
- 主要语言
- C#
- 星标
- 4.8k
- 派生
- 478
- PR 合并指标
- 30 天内没有已合并 PR
描述
While implementing #678, I discovered a bug in how CommandLineParser handles sequences. Currently, the following test will fail:
```csharp
[Fact]
public void Default_parser_stops_at_sequence_max()
{
// Arrange
var sut = Parser.Default;
var args = new string [] {"--stringvalue=foo", "-i1", "2", "3", "4", "256", "-x" };
var expected = new Simple_Options {
StringValue = "foo",
ShortAndLong = null,
IntSequence = new[] { 1, 2, 3, 4 },
BoolValue = true,
LongValue = 256,
};
// Act
var result = sut.ParseArguments(args);
// Assert
result.Should().BeOfType>();
result.As>().Value.Should().BeEquivalentTo(expected);
}
```
In the Simple_Options class, IntSequence is defined as an Option of type `IEnumerable` with Min=3 and Max=4, and LongValue is defined as a Value with index 0. I believe what should happen is that the Max=4 on IntSequence should be taken into account by CommandLineParser, and it will stop assigning values to IntSequence once it has assigned 4 values. Then the next value (256) will be a value parameter, and it will assigned to LongValue since LongValue has index 0. However, what *actually* happens is that the number 256 gets assigned to IntSequence, and then ParseArguments returns a NotParsed object with a SequenceOutOfRangeError because IntSequence got assigned 5 values and its max is 4.
There's currently a commented-out code block in TokenPartitioner.cs that would check the Max attribute (if there is one) of a sequence, and stop assigning Value tokens to that sequence once the max is reached. I added it while I was implementing #678, then commented it out when I realized that it was making a different test fail: the Breaking_max_constraint_in_string_sequence_gererates_SequenceOutOfRangeError test in InstanceBuilderTests.cs (which is slightly misspelled, as "gererates" should be "generates" in its title) actually expects the current behavior, where the tokenizer swallows up too many tokens and then throws an error. I believe this is wrong, and that test should expect the parser to *succeed* with one extra leftover arg that didn't get inserted into the Parsed instance (and that extra arg wouldn't be accessible to the user anywhere, but that's what #680 is about).
Another test that would also fail if I turn on the "stop when you've reached Max number of items" feature is the Partition_sequence_multi_instance_with_max test in SequenceTests.cs. It takes a command line like `--str=strvalue freevalue --seq seqval0 seqval1 -x freevalue2 --seq seqval2 seqval3 --seq seqval4 seqval5` where `seq` is a sequence with Max=3. The test expects `seq` to end up with *six* values assigned to it (because `--seq` is specified three times with 2 values each, so each time it's specified it doesn't go over its max), but I believe it should only have *three* values assigned to it in total. The question here is, what would library users expect when they write the following?
```csharp
[Option('s', "string-seq", Max=3)]
public IEnumerable StringSequence { get; set; }
```
Would they expect StringSequence to be able to have more than 3 items it in? My answer is no: if library users assign Max=3 to StringSequence and it ends up with 6 items in it, they would rightfully consider that to be a bug. The Max attribute should describe the maximum *total* number of items that can be assigned to StringSequence, not the maximum that can be assigned in any one single use of the `-s` parameter on the command line. So when the command line `-s 1 2 -s 3 4 -s 5 6` is parsed, I believe the value of StringSequence should be `{ "1", "2", "3" }` and the three extra args should be ignored.
I'll be submitting a PR shortly to fix this bug, which was only just introduced when #678 was merged.
贡献指南
这个仓库没有索引到贡献指南
调研方向
从 TokenPartitioner.cs 中被注释掉的 max 处理代码块开始,然后检查 InstanceBuilderTests.cs 中的 Breaking_max_constraint_in_string_sequence_gererates_SequenceOutOfRangeError 和 SequenceTests.cs 中的 Partition_sequence_multi_instance_with_max。当 Sequence Max 限制已分配值的总数、后续值保持未分配状态,并且受影响的测试反映预期的成功解析行为时,即视为完成。
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- csharp
- 领域
- cli
- Issue 类型
- 缺陷
- 难度
- 4/5
- 预计耗时
- 3-5 天
- 活跃度
- 停滞
- 描述清晰度
- 描述清楚
- 新手友好度
- 35/100