Avoid IndexOutOfRangeException in command-line parsing

[Youssef1313]

Fixes #7646

[Copilot]

Pull request overview

Adds a regression test and hardens command-line parsing to prevent IndexOutOfRangeException when an empty argument is encountered (fixing #7646).

Changes:

• Add a unit test covering parsing/validation with an empty argument.
• Guard access to currentArg[0] by checking currentArg.Length > 0 in the parser.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/ArgumentArityTests.cs	Adds a regression test ensuring empty arguments don’t crash parsing/validation and produce an invalid result with an error message.
src/Platform/Microsoft.Testing.Platform/CommandLine/Parser.cs	Prevents out-of-range indexing when the first argument is an empty string.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[AraHaan]

I feel like the code that splits the args up by " " could also use the overload with the option to remove empty the strings from the resulting array as well.

[Youssef1313]

We don't split anything. We receive string[] from the entry point. If it contains an empty string, it looks like it shouldn't be ignored.

[Evangelink]

The production fix is correct, minimal, and well-targeted — nice work. The only unguarded currentArg[0] access in the parser is properly fixed.

A couple of suggestions on the test:

1. Assertion style: Consider using FluentAssertions (Should().BeFalse(), Should().Contain(...)) per repo conventions.
2. Brittleness: An exact multi-line match (with pragma suppression for the embedded tab) is fragile. A Contain("Unexpected argument") check validates the regression equally well and is more maintainable.

Both are minor — the fix itself is solid.

[Evangelink]

The production fix is correct and minimal — this was the only unguarded indexing on currentArg in the method. The StartsWith('@') above is safe on empty strings, and the length checks on lines 68-69 (Length > 1, Length > 2) already had proper guards. 👍

[Evangelink]

Per repo conventions in .github/copilot-instructions.md:

"All assertions must be written using FluentAssertions style of assertion."

The new test uses Assert.IsFalse and Assert.AreEqual. Consider switching to:

result.IsValid.Should().BeFalse();
result.ErrorMessage.Should().Contain("Unexpected argument");

Using Should().Contain(...) instead of exact multi-line match would also avoid the #pragma disable SA1027 and make the test more resilient to harmless formatting changes in the error message (indentation, newlines, trailing space).

(The existing tests in the same file also use Assert.* — that's a pre-existing inconsistency, but a new test is a good opportunity to follow the stated convention.)

[Evangelink]

Disabling SA1027 to embed a literal tab in the raw string is a bit of code smell in a test. If you do keep the exact-match assertion, you can avoid the pragma by constructing the expected string with "\t":

var expected = string.Join(
    Environment.NewLine,
    "Invalid command line arguments:",
    "\t- Unexpected argument ",
    string.Empty);
result.ErrorMessage.Should().Be(expected);

Though as noted above, a Should().Contain("Unexpected argument") assertion would sidestep this entirely and still validate the regression.