api: mark Format as #[non_exhaustive] - #154
Open
marcellmanfrin wants to merge 2 commits into
Open
Conversation
New formats are added over time: standalone HTML and MHTML support are in flight in firecrawl#147 and firecrawl#149. Adding a variant to the public Format enum breaks downstream users that match it exhaustively. Mark the enum #[non_exhaustive] so future variants are forward-compatible: crates outside this workspace must include a wildcard arm when matching. The in-repo Node and WASM bindings match anydoc::Format exhaustively, so add an explicit unreachable arm to each From<anydoc::Format> implementation; the bindings are released in lockstep with the core crate, so the arm is defensive only. The Python bindings resolve format names through a lookup table and need no change. Motivated by the Cubic finding on firecrawl#149 (src/lib.rs:50).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Cubic finding (PR 154, src/lib.rs): the doc note wrongly implied that downstream exhaustive matches keep compiling. #[non_exhaustive] requires downstream callers to match with a wildcard arm; only wildcard-bearing matches remain compatible with future variants. Full gate re-run on this exact tree: Rust fmt/clippy/tests clean, Node 19/19 with deterministic bindings, WASM 8/8, Python 11/11.
Author
|
@cubic-dev-ai ultrareview: focus on the #[non_exhaustive] API change and the wildcard arms added to the Node/WASM bindings. |
@marcellmanfrin Couldn't start the ultrareview: PR author |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope
Marks the public
Formatenum#[non_exhaustive]and adds defensive wildcard arms to the in-repo Node and WASM bindings. No conversion behavior changes.Motivation
#147 and #149 add
HtmlandMhtmlvariants toFormat. Adding a variant to a public enum breaks downstream users with exhaustivematchexpressions (flagged by Cubic on #149,src/lib.rs:50).#[non_exhaustive]makes future variant additions forward-compatible: crates outside this workspace must include a wildcard arm, while references likeFormat::Pdfkeep working unchanged.Changes
src/lib.rs:#[non_exhaustive]onFormat+ doc note.node/src/lib.rsandwasm/src/lib.rs: the bindings matchanydoc::Formatexhaustively inFrom<anydoc::Format>, so each gains an explicitother => unreachable!(...)arm. The bindings are released in lockstep with the core crate; the arm is defensive only.Validation (full gate on the exact candidate SHA
73f851b66c8f1926221bb4e4028e833e88685ef2, Windows, cargo 1.98.0)cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo test --locked— all clean, all suites pass.npm ci/npm run build/npm test— 19 passed, 0 failed; committednode/index.js/node/index.d.tsare deterministic (zero diff after rebuild).wasm-pack build wasm --release --target web --scope firecrawl;node --test wasm/test.mjs— 8 passed, 0 failed.python -m unittest discover -s python/testsfrom the repo root — 11 tests OK.Fixture note: this candidate is based on
main, which intentionally does not include HTML/MHTML support, so the private real-fixture conversion matrix from the repository policy does not apply here (this PR changes no conversion logic).Summary by cubic
Marks the
Formatenum#[non_exhaustive]so future variant additions (like the in-flightHtmlandMhtml) won't break downstream matches. Downstream callers must now include a wildcard arm when matching. Adds defensiveunreachable!()arms to the Node and WASM bindings'From<anydoc::Format>implementations; the Python bindings are unchanged since they resolve format names via a lookup table. No conversion behavior changes.Written for commit 7a5c8ba. Summary will update on new commits.