pep508: bound the URL token at horizontal whitespace

[jonyoder]

Bounds the URL token to horizontal whitespace, matching upstream's [^ \t]+ exactly, and deletes the hand-rolled line-break loop that patched over the old \A\S+ rule.

---

What changed

• internal/pep508/tokenizer.go: URL regexp \A\S+ → \A[^ \t]+ (stops at space/tab only, like upstream). Rewrote the comment above it — the old text justified the rule by reference to WS = \s+, which #18640 already changed to [ \t]+; the new comment states current reality and keeps the reason a URL is allowed to contain ; (mandatory whitespace, not a semicolon ban, separates it from a following marker clause).
• internal/pep508/requirement.go: deleted the manual \n/\r consumption loop in parseRequirementDetails. It's dead once the regexp change makes the tokenizer itself the single source of truth for where a URL ends.
• internal/pep508/requirement_test.go: replaced TestParseRequirement_URLThenNewlineThenMarker_Splits, which pinned the old (wrong) behavior, with token-level tests for the corrected behavior — see below.

A brief acceptance criterion turned out to be wrong — verified against real upstream

The working assumption going in was that name @ url followed by a bare \n/\r/\r\n should be rejected, by analogy with name>=1\n being rejected. That's false. [^ \t]+ is greedy, and \n/\r aren't space/tab, so a trailing line break with nothing after it gets absorbed into the URL token itself — the parse succeeds, with the break literally in the URL string. I confirmed this by installing pypa/packaging at the exact pinned commit (4eb0753) and running it directly:

'name @ https://example.com\n'   -> OK url='https://example.com\n'
'name @ https://example.com\r\n' -> OK url='https://example.com\r\n'

name @ url\n; marker (more content after the break) is rejected upstream — that matches this repo's behavior and is what the deleted test actually exercised, just with the wrong URL value asserted.

Tests now assert the real, upstream-verified behavior at the token level (req.URL equals the URL plus the embedded break), not just parse success/failure, so a regression can't hide behind a coincidentally-correct pass/fail.

Verification (module root, both individually)

Regexp change, reverted to \A\S+ with the loop already gone:

--- FAIL: TestParseRequirement_URLForm_BareLineBreak_EmbeddedInURL
    Error: Expected whitespace after URL

Restored → PASS, all three suffixes (\n, \r, \r\n).

Loop deletion, isolated against the pre-change regexp (both files at origin/main, loop removed only):

--- FAIL: TestScratch_LoopIsolation
    Error: Expected whitespace after URL

With the loop present (true baseline) → PASS. Confirms the loop was load-bearing under the old regexp and is safely dead once the regexp changes.

Mutation proof (final state): reverted URL to \A\S+ → TestParseRequirement_URLForm_BareLineBreak_EmbeddedInURL fails. Restored → full suite green. Mutation not committed.

Full suite: go test ./... -count=1 -timeout 900s and go test ./... -race both green across all packages (module-wide, not just internal/pep508, since the tokenizer is shared with marker/, requirement/, reqtxt/). golangci-lint run ./... at the CI-pinned v2.11.2 (confirmed against .github/workflows/ci.yml) → 0 issues. gofmt -l on changed files → empty.

Independently re-reviewed and re-run by a second pass (not authored by the same session as the implementation): confirms the regex semantics, the token-level assertions, that the ;-in-URL regression guard (TestParseRequirement_URLContainingSemicolon_With/NoMarker) is untouched, and that no caller outside internal/pep508/ depends on the old behavior — reqtxt/preprocess.go already TrimSpaces each line before it reaches requirement.Parse, so this behavior change isn't reachable through that path anyway.

Product impact — corrected framing

The issue states "Product impact today: none — PPM consumes only license/." That's false: PPM imports requirement (2 sites: src/pyresolve/resolve.go, src/pyresolve/adapt.go) and marker (1 site: src/pyresolve/target.go, for environment data only, never parsing user-supplied marker text). However, name @ url specifically cannot reach requirement.Parse from PPM today: src/pyresolve/adapt.go's AdaptRequirements builds the PEP 508 string from name/extras/specs only (no URL field), and upstream of that, src/requirements/parser.go's isUnsupportedLine filters out any line containing @ before a requirements.Requirement is built, on both the online and offline-downloader paths. Correct statement: not reachable from PPM's current line filtering today, not "no product impact."

Scope

Touches only internal/pep508/tokenizer.go, internal/pep508/requirement.go, internal/pep508/requirement_test.go. Does not touch WS, Token.Unquoted, validateQuotedStringContents, or marker.go — that's a sibling lane (#19401). This branch went first since #19401 hadn't opened a PR yet; happy to rebase if it lands first and conflicts.

NEWS

None added, deliberately — this repo has no NEWS.md/CHANGELOG (uses chore(release): commits), and the change is library-internal, pre-GA.

Relates to rstudio/package-manager#19402 (do not auto-close: PPM pins an older version and won't pick this up until someone cuts a release and bumps the dependency).

🤖 Generated with Claude Code