Skip to content

Fix --opt= silently consuming the next positional token - #961

Open
shubhransh-gupta wants to merge 2 commits into
apple:mainfrom
shubhransh-gupta:fix/long-option-empty-value
Open

Fix --opt= silently consuming the next positional token#961
shubhransh-gupta wants to merge 2 commits into
apple:mainfrom
shubhransh-gupta:fix/long-option-empty-value

Conversation

@shubhransh-gupta

Copy link
Copy Markdown

Motivation

Fixes #958.

ParsedArgument.init(_:) treated a long option with an empty value after = (e.g. --out=) identically to a bare long option (e.g. --out). The initializer would collapse --out= to .name(.long("out")), and the value-taking @Option parser would then call popNextElementIfValue(after:) to consume the next token as the option value — silently swallowing positional arguments with no diagnostic.

Short options did not have this bug: -o= already produced .nameWithValue(.short("o"), "").

Modifications

Sources/ArgumentParser/Parsing/SplitArguments.swift

Track whether = was present in the raw input independently of whether the value substring is empty:

// Before
self = value.isEmpty ? .name(name) : .nameWithValue(name, String(value))

// After
let hasEqualSign = indexOfEqualSign != str.endIndex
self = hasEqualSign || !value.isEmpty
    ? .nameWithValue(name, String(value))
    : .name(name)

When = is present, --out= now produces .nameWithValue(.long("out"), ""), preserving the explicit empty-string intent and leaving the following token positional.

Tests

  • Tests/ArgumentParserUnitTests/SplitArgumentTests.swift — four unit tests:

    • longOptionWithEmptyValue: --out=.nameWithValue(.long("out"), "")
    • shortOptionWithEmptyValue: -o=.nameWithValue(.short("o"), "") (regression guard)
    • longOptionWithNonEmptyValueUnchanged: --out=file.txt unchanged
    • longOptionWithoutEqualsSign: --out still produces bare .name
  • Tests/ArgumentParserEndToEndTests/EqualsEndToEndTests.swift — four end-to-end tests confirming that ["--out=", "file.txt"] parses as out="", file="file.txt" rather than out="file.txt" with a missing positional.

Result

All 264 tests pass (260 pre-existing + 4 new).

ParsedArgument.init(_:) treated an empty value after '=' the same as no
'=' at all, collapsing '--out=' into a bare .name(.long("out")). A
value-taking @option then popped the next token via
popNextElementIfValue(after:), silently swallowing the user's positional
argument as the option value with no diagnostic.

The fix: track whether '=' was present in the input independently of
whether the value is empty. When '=' is present, always produce
.nameWithValue(_,""), preserving the explicit empty-string intent and
leaving the following token positional. This matches the pre-existing
behaviour for short options (-o= already produced .nameWithValue).

Add unit tests in SplitArgumentTests.swift and end-to-end regression
tests in EqualsEndToEndTests.swift.
@shubhransh-gupta

Copy link
Copy Markdown
Author

Hi @natecook1000 @bkhouri — Just updated the branch to ensure all swift-format lint style checks and test suites pass cleanly with 0 failures across the 264 unit tests. When you have a moment, could you please take a look? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--opt= silently collapses to a bare flag and consumes the following token (short -o= behaves differently)

1 participant