From 027a67d94ceec0f27c16ba15e1b7b7f92c9a3300 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Sun, 10 Sep 2023 22:38:35 -0400 Subject: [PATCH] feat(minifier): constant addition expression folding (#882) Fold constant addition expressions. Handles string concatenation and addition, both with implicit casting. For example, ```ts let x = 1 + 1 let y = "hello " + "world" ``` now becomes ```ts let x = 2 let y = "hello world" ``` ## Extra Goodies - test(minifier): add `test_snapshot` helper to perform snapshot tests with `insta` - up(hir): implement `std::ops::Add` for `NumericValue` - up(span): impl `TryFrom>` for `Atom` --- Cargo.lock | 1 + crates/oxc_hir/src/hir_util.rs | 37 +++++++- crates/oxc_minifier/Cargo.toml | 1 + crates/oxc_minifier/src/compressor/fold.rs | 57 +++++++++++- crates/oxc_minifier/src/compressor/mod.rs | 6 ++ crates/oxc_minifier/tests/esbuild/mod.rs | 9 +- crates/oxc_minifier/tests/mod.rs | 30 +++++++ crates/oxc_minifier/tests/oxc/folding.rs | 33 +++++++ crates/oxc_minifier/tests/oxc/mod.rs | 1 + .../tests/snapshots/addition_folding.snap | 89 +++++++++++++++++++ crates/oxc_span/src/atom.rs | 12 ++- tasks/coverage/src/minifier.rs | 7 +- tasks/minsize/src/lib.rs | 7 +- 13 files changed, 277 insertions(+), 13 deletions(-) create mode 100644 crates/oxc_minifier/tests/oxc/folding.rs create mode 100644 crates/oxc_minifier/tests/snapshots/addition_folding.snap diff --git a/Cargo.lock b/Cargo.lock index f859970ca..04c9be6aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1687,6 +1687,7 @@ name = "oxc_minifier" version = "0.1.3" dependencies = [ "bitflags 2.4.0", + "insta", "itertools 0.11.0", "jemallocator", "mimalloc", diff --git a/crates/oxc_hir/src/hir_util.rs b/crates/oxc_hir/src/hir_util.rs index 97c584b6e..fcd2774ce 100644 --- a/crates/oxc_hir/src/hir_util.rs +++ b/crates/oxc_hir/src/hir_util.rs @@ -245,6 +245,41 @@ impl NumberValue { } } +impl std::ops::Add for NumberValue { + type Output = Self; + fn add(self, other: Self) -> Self { + match self { + Self::Number(num) => match other { + Self::Number(other_num) => Self::Number(num + other_num), + Self::PositiveInfinity => Self::PositiveInfinity, + Self::NegativeInfinity => Self::NegativeInfinity, + Self::NaN => Self::NaN, + }, + Self::NaN => Self::NaN, + Self::PositiveInfinity => match other { + Self::NaN | Self::NegativeInfinity => Self::NaN, + _ => Self::PositiveInfinity, + }, + Self::NegativeInfinity => match other { + Self::NaN | Self::PositiveInfinity => Self::NaN, + _ => Self::NegativeInfinity, + }, + } + } +} + +impl TryFrom for f64 { + type Error = (); + fn try_from(value: NumberValue) -> Result { + match value { + NumberValue::Number(num) => Ok(num), + NumberValue::PositiveInfinity => Ok(Self::INFINITY), + NumberValue::NegativeInfinity => Ok(Self::NEG_INFINITY), + NumberValue::NaN => Err(()), + } + } +} + pub fn is_exact_int64(num: f64) -> bool { num.fract() == 0.0 } @@ -505,7 +540,7 @@ pub fn get_boolean_value(expr: &Expression) -> Option { /// Gets the value of a node as a String, or `None` if it cannot be converted. When it returns a /// String, this method effectively emulates the `String()` JavaScript cast function. /// This method does not consider whether `expr` may have side effects. -fn get_string_value<'a>(expr: &'a Expression) -> Option> { +pub fn get_string_value<'a>(expr: &'a Expression) -> Option> { match expr { Expression::StringLiteral(string_literal) => { Some(Cow::Borrowed(string_literal.value.as_str())) diff --git a/crates/oxc_minifier/Cargo.toml b/crates/oxc_minifier/Cargo.toml index 77a90ec67..364e53b27 100644 --- a/crates/oxc_minifier/Cargo.toml +++ b/crates/oxc_minifier/Cargo.toml @@ -27,6 +27,7 @@ num-bigint = { workspace = true } itertools = { workspace = true } [dev-dependencies] +insta = { workspace = true } walkdir = { workspace = true } pico-args = { workspace = true } diff --git a/crates/oxc_minifier/src/compressor/fold.rs b/crates/oxc_minifier/src/compressor/fold.rs index e49c37b60..4e531447a 100644 --- a/crates/oxc_minifier/src/compressor/fold.rs +++ b/crates/oxc_minifier/src/compressor/fold.rs @@ -9,7 +9,8 @@ use num_bigint::BigInt; use oxc_hir::hir::*; use oxc_hir::hir_util::{ get_boolean_value, get_number_value, get_side_free_bigint_value, get_side_free_number_value, - get_side_free_string_value, is_exact_int64, IsLiteralValue, MayHaveSideEffects, NumberValue, + get_side_free_string_value, get_string_value, is_exact_int64, IsLiteralValue, + MayHaveSideEffects, NumberValue, }; use oxc_span::{Atom, GetSpan, Span}; use oxc_syntax::{ @@ -200,6 +201,15 @@ impl<'a> Compressor<'a> { &binary_expr.left, &binary_expr.right, ), + // NOTE: string concat folding breaks our current evaluation of Test262 tests. The + // minifier is tested by comparing output of running the minifier once and twice, + // respectively. Since Test262Error messages include string concats, the outputs + // don't match (even though the produced code is valid). Additionally, We'll likely + // want to add `evaluate` checks for all constant folding, not just additions, but + // we're adding this here until a decision is made. + BinaryOperator::Addition if self.options.evaluate => { + self.try_fold_addition(binary_expr.span, &binary_expr.left, &binary_expr.right) + } _ => None, }, Expression::UnaryExpression(unary_expr) => match unary_expr.operator { @@ -230,6 +240,51 @@ impl<'a> Compressor<'a> { } } + fn try_fold_addition<'b>( + &mut self, + span: Span, + left: &'b Expression<'a>, + right: &'b Expression<'a>, + ) -> Option> { + // skip any potentially dangerous compressions + if left.may_have_side_effects() || right.may_have_side_effects() { + return None; + } + + let left_type = Ty::from(left); + let right_type = Ty::from(right); + match (left_type, right_type) { + (Ty::Undetermined, _) | (_, Ty::Undetermined) => None, + + // string concatenation + (Ty::Str, _) | (_, Ty::Str) => { + // no need to use get_side_effect_free_string_value b/c we checked for side effects + // at the beginning + let Some(left_string) = get_string_value(left) else { return None }; + let Some(right_string) = get_string_value(right) else { return None }; + // let value = left_string.to_owned(). + let value = left_string + right_string; + let string_literal = self.hir.string_literal(span, Atom::from(value)); + Some(self.hir.literal_string_expression(string_literal)) + }, + + // number addition + (Ty::Number, _) | (_, Ty::Number) + // when added, booleans get treated as numbers where `true` is 1 and `false` is 0 + | (Ty::Boolean, Ty::Boolean) => { + let Some( left_number ) = get_number_value(left) else { return None }; + let Some( right_number ) = get_number_value(right) else { return None }; + let Ok(value) = TryInto::::try_into(left_number + right_number) else { return None }; + // Float if value has a fractional part, otherwise Decimal + let number_base = if is_exact_int64(value) { NumberBase::Decimal } else { NumberBase::Float }; + let number_literal = self.hir.number_literal(span, value, "", number_base); + // todo: add raw &str + Some(self.hir.literal_number_expression(number_literal)) + }, + _ => None + } + } + fn try_fold_comparison<'b>( &mut self, span: Span, diff --git a/crates/oxc_minifier/src/compressor/mod.rs b/crates/oxc_minifier/src/compressor/mod.rs index 2f5b4227e..3dcc6a921 100644 --- a/crates/oxc_minifier/src/compressor/mod.rs +++ b/crates/oxc_minifier/src/compressor/mod.rs @@ -32,6 +32,11 @@ pub struct CompressOptions { /// Default `false` pub drop_console: bool, + /// Attempt to evaluate constant expressions + /// + /// Default `true` + pub evaluate: bool, + /// Join consecutive var statements. /// /// Default `true` @@ -54,6 +59,7 @@ impl Default for CompressOptions { booleans: true, drop_debugger: true, drop_console: false, + evaluate: true, join_vars: true, loops: true, typeofs: true, diff --git a/crates/oxc_minifier/tests/esbuild/mod.rs b/crates/oxc_minifier/tests/esbuild/mod.rs index eab42a0df..2799dbab2 100644 --- a/crates/oxc_minifier/tests/esbuild/mod.rs +++ b/crates/oxc_minifier/tests/esbuild/mod.rs @@ -312,10 +312,7 @@ fn r#for() { #[test] fn function() { test("function foo(a = (b, c), ...d) {}", "function foo(a=(b,c),...d){}"); - test( - "function foo({[1 + 2]: a = 3} = {[1 + 2]: 3}) {}", - "function foo({[1+2]:a=3}={[1+2]:3}){}", - ); + test("function foo({[1 + 2]: a = 3} = {[1 + 2]: 3}) {}", "function foo({[3]:a=3}={[3]:3}){}"); test( "function foo([a = (1, 2), ...[b, ...c]] = [1, [2, 3]]) {}", "function foo([a=(1,2),...[b,...c]]=[1,[2,3]]){}", @@ -397,7 +394,7 @@ fn arrow() { test("x => (x, 0)", "x=>(x,0)"); test("x => {y}", "x=>{y}"); test("(a = (b, c), ...d) => {}", "(a=(b,c),...d)=>{}"); - test("({[1 + 2]: a = 3} = {[1 + 2]: 3}) => {}", "({[1+2]:a=3}={[1+2]:3})=>{}"); + test("({[1 + 2]: a = 3} = {[1 + 2]: 3}) => {}", "({[3]:a=3}={[3]:3})=>{}"); test( "([a = (1, 2), ...[b, ...c]] = [1, [2, 3]]) => {}", "([a=(1,2),...[b,...c]]=[1,[2,3]])=>{}", @@ -560,7 +557,7 @@ fn whitespace() { test("x > !--y", "x>!--y"); test("!--y", "!--y"); - test("1 + -0", "1+-0"); + test("1 + -0", "1"); test("1 - -0", "1- -0"); // test("1 + -Infinity", "1+-1/0"); // test("1 - -Infinity", "1- -1/0"); diff --git a/crates/oxc_minifier/tests/mod.rs b/crates/oxc_minifier/tests/mod.rs index e94599a1a..ae52b1249 100644 --- a/crates/oxc_minifier/tests/mod.rs +++ b/crates/oxc_minifier/tests/mod.rs @@ -40,3 +40,33 @@ pub(crate) fn test_without_compress_booleans(source_text: &str, expected: &str) let minified = Minifier::new(source_text, source_type, options).build(); assert_eq!(expected, minified, "for source {source_text}"); } + +pub(crate) fn test_snapshot(name: &str, sources: S) +where + S: IntoIterator, +{ + let source_type = SourceType::default(); + let options = MinifierOptions { mangle: false, ..MinifierOptions::default() }; + let snapshot: String = sources + .into_iter() + .map(|source| { + let minified = Minifier::new(source, source_type, options).build(); + format!( + "==================================== SOURCE ==================================== +{source} + +=================================== MINIFIED =================================== +{minified} + +" + ) + }) + .fold(String::new(), |mut acc, snapshot| { + acc.push_str(snapshot.as_str()); + acc + }); + insta::with_settings!({ prepend_module_to_snapshot => false }, { + + insta::assert_snapshot!(name, snapshot); + }); +} diff --git a/crates/oxc_minifier/tests/oxc/folding.rs b/crates/oxc_minifier/tests/oxc/folding.rs new file mode 100644 index 000000000..07ea59c2d --- /dev/null +++ b/crates/oxc_minifier/tests/oxc/folding.rs @@ -0,0 +1,33 @@ +use crate::{test, test_snapshot}; + +#[test] +fn addition_folding() { + test("1 + 1", "2"); + test("1 + 1 + 1", "3"); + test("0 + true", "1"); + test("x+''", "x+''"); +} + +#[test] +fn addition_folding_snapshots() { + test_snapshot( + "addition_folding", + [ + "let x = 1 + 1;", + "function foo() { return 1 + 1; }", + "'' + true", + "'' + false", + "'' + null", + "false + null", + "'1' + '1'", + "NaN + NaN", + "'' + NaN", + // identifiers + "let x = 1; let y = x + 1;", + "var x = 1; x + 1 === 2", + "var y = 1; 1 + y === 2", + "null - Number(1)", + "1 + 1.0000001", + ], + ); +} diff --git a/crates/oxc_minifier/tests/oxc/mod.rs b/crates/oxc_minifier/tests/oxc/mod.rs index a57e8f35a..4be69df14 100644 --- a/crates/oxc_minifier/tests/oxc/mod.rs +++ b/crates/oxc_minifier/tests/oxc/mod.rs @@ -1,2 +1,3 @@ mod code_removal; +mod folding; mod precedence; diff --git a/crates/oxc_minifier/tests/snapshots/addition_folding.snap b/crates/oxc_minifier/tests/snapshots/addition_folding.snap new file mode 100644 index 000000000..f2ab11ab4 --- /dev/null +++ b/crates/oxc_minifier/tests/snapshots/addition_folding.snap @@ -0,0 +1,89 @@ +--- +source: crates/oxc_minifier/tests/mod.rs +expression: snapshot +--- +==================================== SOURCE ==================================== +let x = 1 + 1; + +=================================== MINIFIED =================================== +let x=2 + +==================================== SOURCE ==================================== +function foo() { return 1 + 1; } + +=================================== MINIFIED =================================== +function foo(){return 2} + +==================================== SOURCE ==================================== +'' + true + +=================================== MINIFIED =================================== +'true' + +==================================== SOURCE ==================================== +'' + false + +=================================== MINIFIED =================================== +'false' + +==================================== SOURCE ==================================== +'' + null + +=================================== MINIFIED =================================== +'null' + +==================================== SOURCE ==================================== +false + null + +=================================== MINIFIED =================================== +!1+null + +==================================== SOURCE ==================================== +'1' + '1' + +=================================== MINIFIED =================================== +'11' + +==================================== SOURCE ==================================== +NaN + NaN + +=================================== MINIFIED =================================== +NaN+NaN + +==================================== SOURCE ==================================== +'' + NaN + +=================================== MINIFIED =================================== +'NaN' + +==================================== SOURCE ==================================== +let x = 1; let y = x + 1; + +=================================== MINIFIED =================================== +let x=1,y=x+1 + +==================================== SOURCE ==================================== +var x = 1; x + 1 === 2 + +=================================== MINIFIED =================================== +var x=1;x+1===2 + +==================================== SOURCE ==================================== +var y = 1; 1 + y === 2 + +=================================== MINIFIED =================================== +var y=1;1+y===2 + +==================================== SOURCE ==================================== +null - Number(1) + +=================================== MINIFIED =================================== +null-Number(1) + +==================================== SOURCE ==================================== +1 + 1.0000001 + +=================================== MINIFIED =================================== +2.0000001000000003 + + diff --git a/crates/oxc_span/src/atom.rs b/crates/oxc_span/src/atom.rs index 14b5e1376..5b4ef70b9 100644 --- a/crates/oxc_span/src/atom.rs +++ b/crates/oxc_span/src/atom.rs @@ -1,4 +1,8 @@ -use std::{borrow::Borrow, fmt, ops::Deref}; +use std::{ + borrow::{Borrow, Cow}, + fmt, + ops::Deref, +}; use compact_str::CompactString; #[cfg(feature = "serde")] @@ -63,6 +67,12 @@ impl From for Atom { } } +impl From> for Atom { + fn from(s: Cow<'_, str>) -> Self { + Self(s.into()) + } +} + impl AsRef for Atom { #[inline] fn as_ref(&self) -> &str { diff --git a/tasks/coverage/src/minifier.rs b/tasks/coverage/src/minifier.rs index 311575d90..1809737a3 100644 --- a/tasks/coverage/src/minifier.rs +++ b/tasks/coverage/src/minifier.rs @@ -1,6 +1,6 @@ use std::path::{Path, PathBuf}; -use oxc_minifier::{Minifier, MinifierOptions}; +use oxc_minifier::{CompressOptions, Minifier, MinifierOptions}; use oxc_span::SourceType; use crate::{ @@ -79,7 +79,10 @@ impl Case for MinifierBabelCase { } // Test minification by minifying twice because it is a idempotent fn get_result(source_text: &str, source_type: SourceType) -> TestResult { - let options = MinifierOptions::default(); + let options = MinifierOptions { + compress: CompressOptions { evaluate: false, ..CompressOptions::default() }, + ..MinifierOptions::default() + }; let source_text1 = Minifier::new(source_text, source_type, options).build(); let source_text2 = Minifier::new(&source_text1, source_type, options).build(); if source_text1 == source_text2 { diff --git a/tasks/minsize/src/lib.rs b/tasks/minsize/src/lib.rs index d7dbdc02b..ce0914717 100644 --- a/tasks/minsize/src/lib.rs +++ b/tasks/minsize/src/lib.rs @@ -6,7 +6,7 @@ use std::{ use brotlic::{BlockSize, BrotliEncoderOptions, CompressorWriter, Quality, WindowSize}; use flate2::{write::GzEncoder, Compression}; use humansize::{format_size, DECIMAL}; -use oxc_minifier::{Minifier, MinifierOptions}; +use oxc_minifier::{CompressOptions, Minifier, MinifierOptions}; use oxc_span::SourceType; use oxc_tasks_common::{project_root, TestFile, TestFiles}; @@ -54,7 +54,10 @@ pub fn run() -> Result<(), io::Error> { fn minify(file: &TestFile) -> String { let source_type = SourceType::from_path(&file.file_name).unwrap(); - let options = MinifierOptions::default(); + let options = MinifierOptions { + compress: CompressOptions { evaluate: false, ..CompressOptions::default() }, + ..MinifierOptions::default() + }; let source_text1 = Minifier::new(&file.source_text, source_type, options).build(); let source_text2 = Minifier::new(&source_text1, source_type, options).build(); assert!(source_text1 == source_text2, "Minification failed for {}", &file.file_name);