sourcemap: make ParseResult a Result alias (fixes clippy on main)

[dylan-conway]

What does this PR do?

cargo clippy has been red on main since #38263: that change shrank ParseResultFail to 8 bytes, so bun_sourcemap::ParseResult { Fail(ParseResultFail), Success(ParsedSourceMap) } (152-byte Success) now trips clippy::large_enum_variant, which the clippy workflow denies. Every PR checked against current main inherits the failure.

ParseResult was only ever built and immediately matched like a Result, so this defines it as pub type ParseResult = Result<ParsedSourceMap, ParseResultFail>; and switches the four call sites (mapping::parse, parse_json, the node:module SourceMap constructor, and the dev server's SourceMapStore) to Ok/Err. No boxing or #[allow]; parse_json gets to use ?. No behavior change intended.

How did you verify your code works?

• bun run rust:clippy (the CI command, with build/debug/codegen generated) → 0 warnings, 0 errors across the workspace.
• Debug build: new (require("node:module").SourceMap)(...) with valid mappings (findEntry result) and with malformed mappings (SyntaxError: Invalid source index delta at 1), plus an error thrown from a bun build --sourcemap=inline output file — all three produce output identical to bun 1.4.0.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e6d57b82-f42e-466d-974b-0442cb844fb3

📥 Commits

Reviewing files that changed from the base of the PR and between 18391f6 and 678bb33.

📒 Files selected for processing (5)

• src/runtime/bake/dev_server/source_map_store.rs
• src/sourcemap/Mapping.rs
• src/sourcemap/error.rs
• src/sourcemap/lib.rs
• src/sourcemap_jsc/JSSourceMap.rs

---

Walkthrough

The sourcemap parser now exposes a standard Result type. Parser failures return Err, successful parses return Ok, and downstream consumers use the updated variants.

Changes

Sourcemap Result migration

Layer / File(s)	Summary
Parser Result contract and production src/sourcemap/lib.rs, src/sourcemap/Mapping.rs, src/sourcemap/error.rs	ParseResult is now a Result alias. Mapping.parse returns Ok and Err values. Documentation uses ParseResultFail.
JSON parsing propagation src/sourcemap/lib.rs	parse_json propagates mapping parse failures with map_err.
Parser consumer updates src/runtime/bake/dev_server/source_map_store.rs, src/sourcemap_jsc/JSSourceMap.rs	Consumers match parsing results with Ok and Err. Existing warning and syntax-error handling remains unchanged.

Possibly related PRs

• oven-sh/bun#38263: Refactors ParseResultFail error messages in the same sourcemap parsing paths.

Suggested reviewers: alii, robobun

Mergeability Score: ⚪ Minimal · up to 678bb

This localized change replaces the sourcemap parser result with a standard Result while preserving reported success and error behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains the motivation, implementation, intended behavior, and verification results using the required template sections.
Title check	✅ Passed	The title clearly identifies the main change and its purpose: replacing ParseResult with a Result alias to fix Clippy on main.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical enum→Result alias swap to unblock clippy on main; the inline finding is a comment-only nit.

Extended reasoning...

Overview

Replaces the two-variant ParseResult enum with pub type ParseResult = Result<ParsedSourceMap, ParseResultFail> and mechanically renames Fail/Success to Err/Ok at the four sites that construct or match it: mapping::parse (13 return sites), parse_json in lib.rs (now uses .map_err(|fail| fail.err)?), JSSourceMap::constructor, and SourceMapStore::get_parsed_source_map. One doc-comment reference updated in error.rs.

Security risks

None. No input handling, allocation, or control-flow changes — purely a type-level rename. The .map_err(...)? rewrite in parse_json is behaviorally identical to the deleted match (returns Err(fail.err) on failure, binds the success value otherwise).

Level of scrutiny

Low. This is a lint-fix refactor with no intended behavior change; every hunk is a 1:1 variant rename. I checked that the crate-local Result<T, E = Error> alias re-exported into lib.rs still resolves correctly with an explicit second type parameter (it does — Result<ParsedSourceMap, ParseResultFail> overrides the default E). The dropped ParseResult import in JSSourceMap.rs is dead after the match arms switched to Ok/Err.

Other factors

The one finding (three stale ParseResult::Fail references remaining in a C++ comment and two test descriptions) is comment-only and doesn't affect behavior. Author verified bun run rust:clippy is clean and runtime behavior of node:module SourceMap (valid mappings, malformed mappings, inline-sourcemap error) matches bun 1.4.0. Given this unblocks CI for all other PRs and is a pure mechanical rename, approving.

[claude]

🟡 This doc-comment update from ParseResult::Fail → ParseResultFail missed three sibling references: src/jsc/bindings/highway_sourcemap.cpp:918 and test/js/node/module/sourcemap-simd.test.ts:447,462 still say ParseResult::Fail. Comment-only, no behavior impact — but per REVIEW.md "grep for every sibling site sharing the pattern", these should be updated in the same PR.

Extended reasoning...

What the bug is

This PR replaces enum ParseResult { Fail(ParseResultFail), Success(ParsedSourceMap) } with pub type ParseResult = Result<ParsedSourceMap, ParseResultFail>, so the ParseResult::Fail variant no longer exists. The PR correctly updated the doc comment at src/sourcemap/error.rs:40 from ParseResult::Fail to ParseResultFail to reflect this, but missed three other comments in the repo that still reference the deleted variant.

The specific stale sites

A repo-wide grep for ParseResult::Fail after this PR shows:

• src/jsc/bindings/highway_sourcemap.cpp:918 — // and reports the exact same ParseResult::Fail as before.
• test/js/node/module/sourcemap-simd.test.ts:447 — // Scalar caps at 8 bytes and returns no-progress -> ParseResult::Fail;
• test/js/node/module/sourcemap-simd.test.ts:462 — test("out-of-range source index: identical ParseResult::Fail", ...)

These are the only remaining hits; the PR updated all Rust match arms and the error.rs doc comment but did not sweep C++ comments or test descriptions.

Why existing checks don't catch this

These are all inside comments (or a test title string), so neither rustc, clang, nor clippy sees them. The PR's stated verification (bun run rust:clippy + runtime smoke tests) exercises none of them.

Why this is worth fixing

REVIEW.md, under Correctness: the bug class, not the bug → "Fix the whole class in the same PR — grep for every sibling site sharing the pattern" and "Signature changes and renames → grep the whole repo". The PR explicitly recognized this class by editing error.rs:40, so the three remaining sites are same-class stragglers. Left as-is, a future reader grepping for ParseResult::Fail (from these comments) will find no such symbol.

Step-by-step proof

1. Before this PR, ParseResult::Fail is a real enum variant (src/sourcemap/lib.rs:108 in the base).
2. This PR deletes it: -pub enum ParseResult { Fail(...), Success(...) } / +pub type ParseResult = Result<...>.
3. This PR updates one comment referencing it: error.rs:40 ParseResult::Fail → ParseResultFail.
4. rg 'ParseResult::Fail' on the post-PR tree still returns 3 hits (listed above) — none of them in Rust, so they were missed by the mechanical Fail( → Err( / Success( → Ok( sweep.

How to fix

Update the three comments to say ParseResultFail (or Err(ParseResultFail) where the phrasing describes the return value), matching the change already made at error.rs:40. Zero behavior impact.

[alii]

Ooops, sorry just also fixed in #38280