fix(oxc_ecmascript): +[false] and +[true] should evaluate to NaN#19174
fix(oxc_ecmascript): +[false] and +[true] should evaluate to NaN#19174graphite-app[bot] merged 1 commit intomainfrom
+[false] and +[true] should evaluate to NaN#19174Conversation
+[false] and +[true] should evaluate to NaN
+[false] and +[true] should evaluate to NaN+[false] and +[true] should evaluate to NaN
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
Merge activity
|
#19174) ## Fix minifier incorrectly handling `+[false]` and `+[true]` - [x] Explore repository structure and locate the bug - [x] Identify the root cause in `to_number` implementation for arrays - [x] Add test case for the bug in minifier fold_constants tests - [x] Add test case in to_number unit tests - [x] Fix the bug in `to_number.rs` to properly convert arrays to strings first - [x] Run tests to verify the fix - all tests passing - [x] Manual verification with minifier example - outputs NaN correctly - [x] Run code review - no issues found - [x] Run security check - no vulnerabilities found - [x] Comprehensive testing of all array conversion cases - all correct - [x] Apply cargo fmt to fix formatting - [x] Address PR feedback: hoist use statements, simplify comments and code **Summary:** Fixed a bug where `+[false]` and `+[true]` were incorrectly minified to `0` and `1` instead of `NaN`. The issue was in the `to_number` implementation for arrays in `oxc_ecmascript`, which was bypassing the proper ECMAScript conversion chain (array → string → number). Now correctly converts arrays to strings first via `to_js_string`, then converts the string to a number. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>minifier: `+[false]` and `+[true]` are incorrectly minified</issue_title> > <issue_description><!-- >⚠️ IMPORTANT⚠️ > Please use the appropriate template for each! > > - If you are reporting a bug for Oxlint or the Oxc VS Code extension: > - https://github.com/oxc-project/oxc/issues/new?template=linter_bug_report.yaml > - If you are reporting a bug for Oxfmt, especially difference with Prettier: > - https://github.com/oxc-project/oxc/issues/new?template=formatter_diff_report.yaml > > Other than that, feel free to enter whatever information you like. > > Please include as much detail as possible, such as version information. > Also, if you have steps to reproduce the issue, that would be very helpful. > --> > I was messing around with [jsfuck](https://jsfuck.com/) and found out that oxc_minifier doesn't minify `+[false]` or `+[true]` correctly. It is supposed to evalutate to `NaN`, but Oxc just grabs the boolean and uses that. This makes the 'minified' jsfuck code invalid (not that I'm using jsfuck on a day to day basis) > > Original: > ```ts > const a = +[false] > const b = +[true] > ``` > > Oxc minified: > ```ts > const a = 0 > const b = 1; > ``` > > Expected: > ```ts > const a = NaN > const b = NaN; > ``` > > [Playground](https://playground.oxc.rs/?code=const+a+%3D+%2B%5Bfalse%5D%0Aconst+b+%3D+%2B%5Btrue%5D) (enable minify syntax) > > </issue_description> > > <agent_instructions>probably there a bug in the constant evaluator in oxc_ecmascript that handles `UnaryExpression`</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #19084 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
67f3a9e to
68c7dd2
Compare
There was a problem hiding this comment.
Pull request overview
Fixes constant folding / ToNumber semantics for array literals so expressions like +[false] and +[true] correctly evaluate to NaN (via array → string → number conversion), aligning minifier output with ECMAScript behavior.
Changes:
- Update
oxc_ecmascript::ToNumberforArrayExpressionto convert viato_js_stringthenstring_to_number, with a fast-path for multi-element arrays. - Add minifier peephole regression tests ensuring
+[false]/+[true]fold toNaN. - Add
to_numberunit tests for arrays containing boolean elements.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/oxc_minifier/tests/peephole/fold_constants.rs | Adds regression folding assertions for +[false] / +[true] to ensure minifier outputs NaN. |
| crates/oxc_minifier/tests/ecmascript/to_number.rs | Extends ToNumber unit tests to cover array literals containing booleans. |
| crates/oxc_ecmascript/src/to_number.rs | Fixes array ToNumber implementation to follow array→string→number conversion semantics. |
#19174) ## Fix minifier incorrectly handling `+[false]` and `+[true]` - [x] Explore repository structure and locate the bug - [x] Identify the root cause in `to_number` implementation for arrays - [x] Add test case for the bug in minifier fold_constants tests - [x] Add test case in to_number unit tests - [x] Fix the bug in `to_number.rs` to properly convert arrays to strings first - [x] Run tests to verify the fix - all tests passing - [x] Manual verification with minifier example - outputs NaN correctly - [x] Run code review - no issues found - [x] Run security check - no vulnerabilities found - [x] Comprehensive testing of all array conversion cases - all correct - [x] Apply cargo fmt to fix formatting - [x] Address PR feedback: hoist use statements, simplify comments and code - [x] Address bot feedback: optimize count, remove redundant import, add test coverage **Summary:** Fixed a bug where `+[false]` and `+[true]` were incorrectly minified to `0` and `1` instead of `NaN`. The issue was in the `to_number` implementation for arrays in `oxc_ecmascript`, which was bypassing the proper ECMAScript conversion chain (array → string → number). Now correctly converts arrays to strings first via `to_js_string`, then converts the string to a number. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>minifier: `+[false]` and `+[true]` are incorrectly minified</issue_title> > <issue_description><!-- >⚠️ IMPORTANT⚠️ > Please use the appropriate template for each! > > - If you are reporting a bug for Oxlint or the Oxc VS Code extension: > - https://github.com/oxc-project/oxc/issues/new?template=linter_bug_report.yaml > - If you are reporting a bug for Oxfmt, especially difference with Prettier: > - https://github.com/oxc-project/oxc/issues/new?template=formatter_diff_report.yaml > > Other than that, feel free to enter whatever information you like. > > Please include as much detail as possible, such as version information. > Also, if you have steps to reproduce the issue, that would be very helpful. > --> > I was messing around with [jsfuck](https://jsfuck.com/) and found out that oxc_minifier doesn't minify `+[false]` or `+[true]` correctly. It is supposed to evalutate to `NaN`, but Oxc just grabs the boolean and uses that. This makes the 'minified' jsfuck code invalid (not that I'm using jsfuck on a day to day basis) > > Original: > ```ts > const a = +[false] > const b = +[true] > ``` > > Oxc minified: > ```ts > const a = 0 > const b = 1; > ``` > > Expected: > ```ts > const a = NaN > const b = NaN; > ``` > > [Playground](https://playground.oxc.rs/?code=const+a+%3D+%2B%5Bfalse%5D%0Aconst+b+%3D+%2B%5Btrue%5D) (enable minify syntax) > > </issue_description> > > <agent_instructions>probably there a bug in the constant evaluator in oxc_ecmascript that handles `UnaryExpression`</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #19084 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
04e6bc3 to
110c300
Compare
### 💥 BREAKING CHANGES - 2bf7293 mangler: [**BREAKING**] Enable `top_level` by default for modules and commonjs (#18278) (sapphi-red) - 48b0542 span: [**BREAKING**] SourceType::ts should set module to unambigious (#18873) (Boshen) ### 🚀 Features - 500d071 minifier: Local traverse ctx and generated minifier traverse (#19106) (Boshen) - 142a1be parser: Detect binary files with TS1490 error (#19047) (Boshen) - e316857 allocator/bitset: Add `Ones` iterator to `BitSet` (#19027) (sapphi-red) - 742ad3f minifier: Default `invalid_import_side_effects` to `false` (#18916) (sapphi-red) - 0eff6be parser: Error JSX-like type assertions and generics in `.mts`/`.cts` (#18910) (Boshen) - 18320c6 span: Store file extension in `SourceType` (#18893) (Boshen) ### 🐛 Bug Fixes - a7514e4 isolated-declarations: Preserve const context in literal type inference (#19178) (camc314) - 312e756 isolated-declarations: Preserve readonly literal initializers (#19177) (camc314) - d0ca8d0 isolated-declarations: Skip parenthesis when inferring type (#19176) (camc314) - 110c300 oxc_ecmascript: `+[false]` and `+[true]` should evaluate to `NaN` (#19174) (copilot-swe-agent) - f32ea19 semantic: Report redeclaration error for import bindings conflicting with value declarations (#19068) (Boshen) - 3aeba7a semantic: Report redeclaration error for `function a() {} var a` in module mode (#19041) (Boshen) - 35e32c6 coverage: Match Babel's options.json inheritance for test fixtures (#19002) (Boshen) - 463d60d semantic: Skip TS2391 for standalone computed-name class methods (#19025) (Boshen) - 56c086b parser: Add modifier ordering validation (TS1029) (#19024) (Boshen) - 6067a49 linter/jsdoc: False positive in `check-tag-names` for `@` in email addresses and npm scopes (#19021) (Boshen) - b13bb70 semantic/jsdoc: Inline tags like `{@link}` break jsdoc parsing (#19019) (Boshen) - e3609e3 regular_expression: Preserve UnicodeEscape CharacterKind in string literals (#18998) (Boshen) - 57917ee parser: Parse decorators on rest parameters (#18938) (Boshen) - 487601b napi: Disable mimalloc on Windows to fix worker_threads crash (#18923) (Boshen) - 1f6b193 parser: Validate TypeScript import type options (#18889) (Boshen) - 1663184 parser: Allow conditional types in function type parameters (#18886) (Boshen) - 5758046 parser: Error on property access after instantiation expression (#18887) (Boshen) - 5eb4a94 parser: Handle `<<` as two `<` tokens in type argument contexts (#18885) (Boshen) ### ⚡ Performance - ed8c054 oxc_str: Add precomputed hash to Ident for fast HashMap lookups (#19143) (Boshen) - d4a0867 transformer_plugins: Switch ReplaceGlobalDefines from Traverse to VisitMut (#19146) (Boshen) - 9eb16b3 syntax: Pack ASCII identifier tables into single bitflag table (#19088) (Boshen) - e7595d1 mangler: Use BitSet for exported symbols set (#19023) (sapphi-red) - 2537924 semantic: Optimize scope resolution with fast paths and inlining (#19029) (Boshen) - 69a8d85 mangler: Use BitSet for keep_names symbols set (#19028) (sapphi-red) - f78c525 parser: Try hybrid parsing for jsx children and closing element/fragments (#18789) (camchenry) Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
### 💥 BREAKING CHANGES - 2bf7293 mangler: [**BREAKING**] Enable `top_level` by default for modules and commonjs (oxc-project#18278) (sapphi-red) - 48b0542 span: [**BREAKING**] SourceType::ts should set module to unambigious (oxc-project#18873) (Boshen) ### 🚀 Features - 500d071 minifier: Local traverse ctx and generated minifier traverse (oxc-project#19106) (Boshen) - 142a1be parser: Detect binary files with TS1490 error (oxc-project#19047) (Boshen) - e316857 allocator/bitset: Add `Ones` iterator to `BitSet` (oxc-project#19027) (sapphi-red) - 742ad3f minifier: Default `invalid_import_side_effects` to `false` (oxc-project#18916) (sapphi-red) - 0eff6be parser: Error JSX-like type assertions and generics in `.mts`/`.cts` (oxc-project#18910) (Boshen) - 18320c6 span: Store file extension in `SourceType` (oxc-project#18893) (Boshen) ### 🐛 Bug Fixes - a7514e4 isolated-declarations: Preserve const context in literal type inference (oxc-project#19178) (camc314) - 312e756 isolated-declarations: Preserve readonly literal initializers (oxc-project#19177) (camc314) - d0ca8d0 isolated-declarations: Skip parenthesis when inferring type (oxc-project#19176) (camc314) - 110c300 oxc_ecmascript: `+[false]` and `+[true]` should evaluate to `NaN` (oxc-project#19174) (copilot-swe-agent) - f32ea19 semantic: Report redeclaration error for import bindings conflicting with value declarations (oxc-project#19068) (Boshen) - 3aeba7a semantic: Report redeclaration error for `function a() {} var a` in module mode (oxc-project#19041) (Boshen) - 35e32c6 coverage: Match Babel's options.json inheritance for test fixtures (oxc-project#19002) (Boshen) - 463d60d semantic: Skip TS2391 for standalone computed-name class methods (oxc-project#19025) (Boshen) - 56c086b parser: Add modifier ordering validation (TS1029) (oxc-project#19024) (Boshen) - 6067a49 linter/jsdoc: False positive in `check-tag-names` for `@` in email addresses and npm scopes (oxc-project#19021) (Boshen) - b13bb70 semantic/jsdoc: Inline tags like `{@link}` break jsdoc parsing (oxc-project#19019) (Boshen) - e3609e3 regular_expression: Preserve UnicodeEscape CharacterKind in string literals (oxc-project#18998) (Boshen) - 57917ee parser: Parse decorators on rest parameters (oxc-project#18938) (Boshen) - 487601b napi: Disable mimalloc on Windows to fix worker_threads crash (oxc-project#18923) (Boshen) - 1f6b193 parser: Validate TypeScript import type options (oxc-project#18889) (Boshen) - 1663184 parser: Allow conditional types in function type parameters (oxc-project#18886) (Boshen) - 5758046 parser: Error on property access after instantiation expression (oxc-project#18887) (Boshen) - 5eb4a94 parser: Handle `<<` as two `<` tokens in type argument contexts (oxc-project#18885) (Boshen) ### ⚡ Performance - ed8c054 oxc_str: Add precomputed hash to Ident for fast HashMap lookups (oxc-project#19143) (Boshen) - d4a0867 transformer_plugins: Switch ReplaceGlobalDefines from Traverse to VisitMut (oxc-project#19146) (Boshen) - 9eb16b3 syntax: Pack ASCII identifier tables into single bitflag table (oxc-project#19088) (Boshen) - e7595d1 mangler: Use BitSet for exported symbols set (oxc-project#19023) (sapphi-red) - 2537924 semantic: Optimize scope resolution with fast paths and inlining (oxc-project#19029) (Boshen) - 69a8d85 mangler: Use BitSet for keep_names symbols set (oxc-project#19028) (sapphi-red) - f78c525 parser: Try hybrid parsing for jsx children and closing element/fragments (oxc-project#18789) (camchenry) Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
Fix minifier incorrectly handling
+[false]and+[true]to_numberimplementation for arraysto_number.rsto properly convert arrays to strings firstSummary:
Fixed a bug where
+[false]and+[true]were incorrectly minified to0and1instead ofNaN. The issue was in theto_numberimplementation for arrays inoxc_ecmascript, which was bypassing the proper ECMAScript conversion chain (array → string → number). Now correctly converts arrays to strings first viato_js_string, then converts the string to a number.Original prompt
+[false]and+[true]are incorrectly minified #19084💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.