commandlineparser / commandlineparser/commandline

Sequences with Max=N should not swallow values past N

Đang mở
#681 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C#
Star
4.8k
Fork
478
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

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.

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Hướng nghiên cứu

Bắt đầu với khối xử lý max đã bị chú thích trong TokenPartitioner.cs, sau đó kiểm tra Breaking_max_constraint_in_string_sequence_gererates_SequenceOutOfRangeError trong InstanceBuilderTests.cs và Partition_sequence_multi_instance_with_max trong SequenceTests.cs. Công việc được hoàn tất khi Sequence Max giới hạn tổng số giá trị được gán, các giá trị phía sau vẫn chưa được gán và các kiểm thử bị ảnh hưởng phản ánh hành vi phân tích cú pháp thành công theo mong muốn.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
csharp
Lĩnh vực
cli
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.