Skip to content

fix(ecmascript): do not fold Number.isInteger / isSafeInteger of tiny non-integers to true - #27622

Merged
Dunqing merged 1 commit into
oxc-project:mainfrom
baltasarblanco:fix/number-is-integer-tiny
Oct 14, 2026
Merged

Dunqing merged 1 commit into
oxc-project:mainfrom
baltasarblanco:fix/number-is-integer-tiny

Conversation

@baltasarblanco

Copy link
Copy Markdown
Contributor

Bug

console.log(Number.isInteger(5e-324), Number.isSafeInteger(5e-324), Number.isInteger(1e-300), Number.isSafeInteger(-1e-20));
// before: console.log(!0,!0,!0,!0)   -> true true true true
// after:  console.log(!1,!1,!1,!1)   -> false false false false

(oxc-minify 0.153.0, default options; same output on main at 9a9eb83.) Also through a constant (const x = 5e-324; Number.isInteger(x) → !0).

It also changes control flow:

var i = setImmediate(function () { console.log("ran"); });
if (Number.isInteger(5e-324)) clearImmediate(i);

minifies to var e=setImmediate(function(){console.log(`ran`)});clearImmediate(e);: the original prints ran, the minified prints nothing.

try_fold_number_methods (crates/oxc_ecmascript/src/constant_evaluation/call_expr.rs) tests extracted.fract().abs() < f64::EPSILON. f64::EPSILON is about 2.2e-16, so every number with a smaller fractional part counts as an integer. Number.isInteger and Number.isSafeInteger only accept an integral Number: "a finite Number value whose mathematical value is in the set of integers". That is an exact check.

Fix

is_integral = n.is_finite() && n.trunc() == n, used by both methods.

Tests

In crates/oxc_minifier/tests/peephole/replace_known_methods.rs: test_fold_number_functions_is_safe_integer gets 5e-324 and -1e-20 → false, and a new test_fold_number_functions_is_integer covers 1, 1e300 → true and 1.5, 5e-324, 1e-300, Infinity, NaN → false. Both functions fail without the change.

Checks: cargo test --all-features -p oxc_ecmascript -p oxc_minifier, cargo clippy --all-targets --all-features (warnings denied) on both crates, cargo fmt --check, cargo doc (warnings denied) on both crates, typos, cargo shear, napi/minify build-test + test; cargo minsize, cargo allocs and cargo coverage leave every snapshot unchanged.

Found with a differential fuzzer I wrote (it runs the original and the minified program under Node and compares the output). The fix, the tests and this description were written with AI assistance, and I reviewed them. The checks above and the before/after runs of the inputs were done by the AI agent in my local checkout; I did not re-run them by hand.

@Dunqing
Dunqing requested a balanced review from Copilot October 14, 2026 05:45
@Dunqing

Dunqing commented Oct 14, 2026

Copy link
Copy Markdown
Member

/monitor-oxc run

@oxc-guard

oxc-guard Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation matches JavaScript number semantics and is covered by relevant regression tests.

0 open findings

What changed in this PR

Corrects constant folding for Number.isInteger and Number.isSafeInteger to require exact integral values.

Changes:

  • Replaces epsilon-based checks with finite, exact truncation comparison.
  • Adds regression coverage for tiny fractions and special values.
File Description
crates/​oxc_ecmascript/​src/​constant_evaluation/​call_expr.rs Implements exact integral-number detection.
crates/​oxc_minifier/​tests/​peephole/​replace_known_methods.rs Adds integer-folding regression tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@github-actions github-actions Bot added A-minifier Area - Minifier A-ecmascript Area - ECMAScript labels Oct 14, 2026
@codspeed

codspeed Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 87 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing baltasarblanco:fix/number-is-integer-tiny (d83827d) with main (6987a61)2

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (666938c) during the generation of this report, so 6987a61 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@Dunqing
Dunqing merged commit 3543672 into oxc-project:main Oct 14, 2026
42 checks passed
graphite-app Bot pushed a commit that referenced this pull request Oct 19, 2026
### 💥 BREAKING CHANGES

- c7d229b transformer: [**BREAKING**] Decode JSX entities to lone surrogates (#26712) (Dunqing)
- e3c1b11 ast: [**BREAKING**] Preserve lone surrogates in static property names (#27095) (Dunqing)
- a0a987c ast: [**BREAKING**] Preserve lone surrogates in static member names (#26677) (Dunqing)

### 🚀 Features

- 7bb2697 minifier: Minimize array join expression (#27905) (Armano)
- 5d91db8 ast: Generate node ID setters for enums (#27500) (camc314)
- 6b5057b minifier: Add de morgan optimization for void expressions (#27375) (Armano)

### 🐛 Bug Fixes

- ce6a088 minifier: Fold string indices by UTF-16 code unit (#27096) (Dunqing)
- 7c47246 linter: Check module specifiers and string values that contain lone surrogates (#26717) (Dunqing)
- 782ab6e linter: Search string values without UTF-8 conversion (#26716) (Dunqing)
- a5d8184 jsdoc: Parse code spans after quotes in descriptions (#27955) (leaysgur)
- bd37e78 ecmascript: Look through sequence expressions in `to_primitive` (#27837) (baltasarblanco)
- d92789d minifier: Drop debugger statements in unbraced bodies (#27867) (camc314)
- 863b359 ecmascript: Correct parseFloat decimal prefix scanning (#27695) (camc314)
- a2534c6 minifier: Preserve lexical initializers in dead for loops (#27676) (camc314)
- 3543672 ecmascript: Do not fold Number.isInteger / isSafeInteger of tiny non-integers to true (#27622) (Baltasar Blanco)
- 6987a61 ecmascript: BigInt loose equality folding (unknown operands and `StringToBigInt`) (#27656) (Baltasar Blanco)
- 40e5644 minifier: Preserve indirect access when folding into call targets (#27082) (waltu)
- 022306f ecmascript: Do not fold `Math.cbrt` of a negative number to `NaN` (#27621) (Baltasar Blanco)

### ⚡ Performance

- faa75d8 transformer: Remove redundant scratch vectors (#27922) (camc314)
- 88b5c35 minifier: Reduce code branches when evaluating roots (#27906) (Armano)
- fe1d367 codegen: Remove branches from loops printing arrays (#27840) (overlookmotel)
- 7bb6b62 codegen: Reduce hashmap lookups in `print_annotation_comment` (#27845) (overlookmotel)
- 31ffd08 codegen: Avoid unnecessary comment clone (#27843) (overlookmotel)
- a6456b7 codegen: Remove branch from `print_space_before_identifier` (#27842) (overlookmotel)
- eeff416 codegen: Simplify `print_space_before_identifier` (#27841) (overlookmotel)
- 5d877b8 diagnostics: Speed up zero-context span lookups (#27770) (camc314)
- 2ab6b63 packages/codegen: Remove redundant `printSpaceBeforeIdentifier` and `last` updates in `printParams` (#27652) (overlookmotel)
- e533278 codegen: Reduce writes in `printDecorators` (#27651) (overlookmotel)

### 📚 Documentation

- 29caafd codegen: Format table in doc comment (#27839) (overlookmotel)
- 3de03c1 packages/codegen: Correct outdated comments (#27838) (overlookmotel)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ecmascript Area - ECMAScript A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants