Skip to content

fix(jsregexp): convert panics to EngineError::Panic using js_expect#4846

Merged
nekevss merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jsregexp
Mar 5, 2026
Merged

fix(jsregexp): convert panics to EngineError::Panic using js_expect#4846
nekevss merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jsregexp

Conversation

@KaustubhOG
Copy link
Copy Markdown
Contributor

Part of #3241

Converts all 17 panics in jsregexp.rs to EngineError::Panic using
js_expect introduced in #4828.

No public API signatures were changed — only 1 file modified.

Patterns converted:

  • .as_object().expect().as_object().js_expect()?
  • .map(|v| v.as_boolean().expect()).and_then(|v| v.as_boolean().js_expect().map_err(Into::into))
  • .map(|v| v.as_string().expect()...to_std_string().expect()).and_then(|v| v.as_string().js_expect()?.to_std_string().map_err(...))
  • nested .expect() calls in exec() converted using ? inside .and_then()

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 3, 2026 21:31
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 3, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,862 52,862 0
Passed 49,627 49,627 0
Ignored 2,263 2,263 0
Failed 972 972 0
Panics 0 0 0
Conformance 93.88% 93.88% 0.00%

Tested PR commit: a4ea5e7c1633d82ae92a9ab336c67b14e01da397

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 12.76596% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.13%. Comparing base (6ddc2b4) to head (a4ea5e7).
⚠️ Report is 756 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/builtins/jsregexp.rs 12.76% 41 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4846      +/-   ##
==========================================
+ Coverage   47.24%   57.13%   +9.89%     
==========================================
  Files         476      554      +78     
  Lines       46892    60484   +13592     
==========================================
+ Hits        22154    34558   +12404     
- Misses      24738    25926    +1188     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread core/engine/src/object/builtins/jsregexp.rs Outdated
Comment thread core/engine/src/object/builtins/jsregexp.rs Outdated
Comment thread core/engine/src/object/builtins/jsregexp.rs Outdated
Comment thread core/engine/src/object/builtins/jsregexp.rs Outdated
@nekevss nekevss added A-API Changes related to public APIs Waiting On Author Waiting on PR changes from the author labels Mar 4, 2026
@KaustubhOG KaustubhOG force-pushed the fix/convert-panics-jsregexp branch from 0224072 to 209b22b Compare March 4, 2026 09:27
@nekevss nekevss removed the Waiting On Author Waiting on PR changes from the author label Mar 4, 2026
Comment thread core/engine/src/object/builtins/jsregexp.rs
@KaustubhOG KaustubhOG force-pushed the fix/convert-panics-jsregexp branch from 209b22b to a4ea5e7 Compare March 4, 2026 15:46
@nekevss nekevss added this pull request to the merge queue Mar 5, 2026
Merged via the queue into boa-dev:main with commit 32bb2cf Mar 5, 2026
19 checks passed
@jedel1043 jedel1043 added A-Technical Debt Changes related to technical debt A-Internal Changes that don't modify execution behaviour and removed A-API Changes related to public APIs labels Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Internal Changes that don't modify execution behaviour A-Technical Debt Changes related to technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants