diff --git a/crates/oxc_minifier/src/ast_passes/collapse_variable_declarations.rs b/crates/oxc_minifier/src/ast_passes/collapse_variable_declarations.rs index 92dab5674dcb8..ec7374dab60d1 100644 --- a/crates/oxc_minifier/src/ast_passes/collapse_variable_declarations.rs +++ b/crates/oxc_minifier/src/ast_passes/collapse_variable_declarations.rs @@ -4,10 +4,9 @@ use oxc_traverse::{Traverse, TraverseCtx}; use crate::{CompressOptions, CompressorPass}; -/// Collapse variable declarations (TODO: and assignments). +/// Collapse variable declarations. /// /// `var a; var b = 1; var c = 2` => `var a, b = 1; c = 2` -/// TODO: `a = null; b = null;` => `a = b = null` pub struct CollapseVariableDeclarations { options: CompressOptions, } diff --git a/crates/oxc_minifier/src/ast_passes/exploit_assigns.rs b/crates/oxc_minifier/src/ast_passes/exploit_assigns.rs new file mode 100644 index 0000000000000..d70ccb0bb6481 --- /dev/null +++ b/crates/oxc_minifier/src/ast_passes/exploit_assigns.rs @@ -0,0 +1,295 @@ +use oxc_traverse::Traverse; + +use crate::CompressorPass; + +/// Tries to chain assignments together. +/// +/// +pub struct ExploitAssigns; + +impl<'a> CompressorPass<'a> for ExploitAssigns {} + +impl<'a> Traverse<'a> for ExploitAssigns {} + +impl ExploitAssigns { + pub fn new() -> Self { + Self {} + } +} + +#[cfg(test)] +mod test { + use oxc_allocator::Allocator; + + use crate::tester; + + fn test(source_text: &str, expected: &str) { + let allocator = Allocator::default(); + let mut pass = super::ExploitAssigns::new(); + tester::test(&allocator, source_text, expected, &mut pass); + } + + fn test_same(source_text: &str) { + test(source_text, source_text); + } + + #[test] + #[ignore] + fn test_expr_exploitation_types() { + test("a = true; b = true", "b = a = true"); + test("a = !0; b = !0", "b = a = !0"); + test("a = !1; b = !1", "b = a = !1"); + test("a = void 0; b = void 0", "b = a = void 0"); + test("a = -Infinity; b = -Infinity", "b = a = -Infinity"); + } + + #[test] + #[ignore] + fn test_expr_exploitation_types2() { + test("a = !0; b = !0", "b = a = !0"); + } + + #[test] + #[ignore] + fn nullish_coalesce() { + test("a = null; a ?? b;", "(a = null)??b"); + test("a = true; if (a ?? a) { foo(); }", "if ((a = true) ?? a) { foo() }"); + test("a = !0; if (a ?? a) { foo(); }", "if ((a = !0) ?? a) { foo() }"); + } + + #[test] + #[ignore] + fn test_expr_exploitation() { + test("a = null; b = null; var c = b", "var c = b = a = null"); + test("a = null; b = null", "b = a = null"); + test("a = undefined; b = undefined", "b = a = undefined"); + test("a = 0; b = 0", "b=a=0"); + test("a = 'foo'; b = 'foo'", "b = a = \"foo\""); + test("a = c; b = c", "b=a=c"); + + test_same("a = 0; b = 1"); + test_same("a = \"foo\"; b = \"foox\""); + + test("a = null; a && b;", "(a = null)&&b"); + test("a = null; a || b;", "(a = null)||b"); + + test("a = null; a ? b : c;", "(a = null) ? b : c"); + + test("a = null; this.foo = null;", "this.foo = a = null"); + test("function f(){ a = null; return null; }", "function f(){return a = null}"); + + test("a = true; if (a) { foo(); }", "if (a = true) { foo() }"); + test("a = true; if (a && a) { foo(); }", "if ((a = true) && a) { foo() }"); + test("a = false; if (a) { foo(); }", "if (a = false) { foo() }"); + + test("a = !0; if (a) { foo(); }", "if (a = !0) { foo() }"); + test("a = !0; if (a && a) { foo(); }", "if ((a = !0) && a) { foo() }"); + test("a = !1; if (a) { foo(); }", "if (a = !1) { foo() }"); + + test_same("a = this.foo; a();"); + test("a = b; b = a;", "b = a = b"); + test_same("a = b; a.c = a"); + test("this.foo = null; this.bar = null;", "this.bar = this.foo = null"); + test( + "this.foo = null; this.bar = null; this.baz = this.bar", + "this.baz = this.bar = this.foo = null", + ); + test( + "this.foo = null; this.bar = null; this.baz = this?.bar", + "this.bar = this.foo = null; this.baz = this?.bar;", + ); + test("this.foo = null; a = null;", "a = this.foo = null"); + test("this.foo = null; a = this.foo;", "a = this.foo = null"); + test_same("this.foo = null; a = this?.foo;"); + test("a.b.c=null; a=null;", "a = a.b.c = null"); + test_same("a = null; a.b.c = null"); + test("(a=b).c = null; this.b = null;", "this.b = (a=b).c = null"); + test_same("if(x) a = null; else b = a"); + } + + #[test] + #[ignore] + fn test_let_const_assignment() { + test("a = null; b = null; let c = b", "let c = b = a = null"); + } + + #[test] + #[ignore] + fn test_block_scope() { + test("{ a = null; b = null; c = b }", "{ c = b = a = null }"); + + // TODO (simranarora) What should we have as the intended behavior with block scoping? + test( + "a = null; b = null; { c = b; }", + // "{ c = b = a = null; } + "b = a = null; { c = b; }", + ); + } + + #[test] + #[ignore] + fn test_exploit_in_arrow_function() { + test("() => { a = null; return null; }", "() => { return a = null }"); + } + + #[test] + #[ignore] + fn test_nested_expr_exploitation() { + test( + "this.foo = null; this.bar = null; this.baz = null;", + "this.baz = this.bar = this.foo = null", + ); + + test( + "a = 3; this.foo = a; this.bar = a; this.baz = 3;", + "this.baz = this.bar = this.foo = a = 3", + ); + test( + "a = 3; this.foo = a; this.bar = this.foo; this.baz = a;", + "this.baz = this.bar = this.foo = a = 3", + ); + // recursively optimize assigns until optional chaining on RHS + test( + "a = 3; this.foo = a; this.bar = this?.foo; this.baz = a;", + "this.foo = a = 3; this.bar = this?.foo; this.baz = a;", + ); + test( + "a = 3; this.foo = a; this.bar = 3; this.baz = this.foo;", + "this.baz = this.bar = this.foo = a = 3", + ); + // recursively optimize assigns until optional chaining on RHS + test( + "a = 3; this.foo = a; this.bar = 3; this.baz = this?.foo;", + "this.bar = this.foo = a = 3; this.baz = this?.foo;", + ); + // test( + // "a = 3; this.foo = a; a = 3; this.bar = 3; " + "a = 3; this.baz = this.foo;", + // "this.baz = a = this.bar = a = this.foo = a = 3", + // ); + // recursively optimize assigns until optional chaining on RHS + // test( + // lines("a = 3; this.foo = a; a = 3; this.bar = 3; a = 3; this.baz = this?.foo;"), + // lines("a = this.bar = a = this.foo = a = 3; this.baz = this?.foo;"), + // ); + + // test( + // "a = 4; this.foo = a; a = 3; this.bar = 3; " + "a = 3; this.baz = this.foo;", + // "this.foo = a = 4; a = this.bar = a = 3; this.baz = this.foo", + // ); + // recursively optimize assigns until optional chaining on RHS + // test( + // lines("a = 4; this.foo = a; a = 3; this.bar = 3; a = 3; this.baz = this?.foo;"), + // lines("this.foo = a = 4;", "a = this.bar = a = 3;", "this.baz = this?.foo;"), + // ); + + // test( + // "a = 3; this.foo = a; a = 4; this.bar = 3; " + "a = 3; this.baz = this.foo;", + // "this.foo = a = 3; a = 4; a = this.bar = 3; this.baz = this.foo", + // ); + // test( + // lines("a = 3; this.foo = a; a = 4; this.bar = 3; ", "a = 3; this.baz = this?.foo;"), + // lines("this.foo = a = 3;", "a = 4;", "a = this.bar = 3;", "this.baz = this?.foo;"), + // ); + // test( + // "a = 3; this.foo = a; a = 3; this.bar = 3; " + "a = 4; this.baz = this.foo;", + // "this.bar = a = this.foo = a = 3; a = 4; this.baz = this.foo", + // ); + // test( + // lines("a = 3; this.foo = a; a = 3; this.bar = 3; a = 4; this.baz = this?.foo;"), + // lines("this.bar = a = this.foo = a = 3;", "a = 4;", "this.baz = this?.foo;"), + // ); + } + + #[test] + #[ignore] + fn test_bug1840071() { + // Some external properties are implemented as setters. Let's + // make sure that we don't collapse them inappropriately. + test("a.b = a.x; if (a.x) {}", "if (a.b = a.x) {}"); + test_same("a.b = a?.x; if (a?.x) {}"); + test_same("a.b = a.x; if (a.b) {}"); + test("a.b = a.c = a.x; if (a.x) {}", "if (a.b = a.c = a.x) {}"); + test_same("a.b = a.c = a?.x; if (a?.x) {}"); + + test_same("a.b = a.c = a.x; if (a.c) {}"); + test_same("a.b = a.c = a.x; if (a.b) {}"); + } + + #[test] + #[ignore] + fn test_bug2072343() { + test_same("a = a.x;a = a.x"); + test_same("a = a.x;b = a.x"); + test("b = a.x;a = a.x", "a = b = a.x"); + test_same("b = a?.x;a = a?.x"); + test_same("a.x = a;a = a.x"); + test_same("a.b = a.b.x;a.b = a.b.x"); + test_same("a.y = a.y.x;b = a.y;c = a.y.x"); + test("a = a.x;b = a;c = a.x", "b = a = a.x;c = a.x"); + test("b = a.x;a = b;c = a.x", "a = b = a.x;c = a.x"); + } + + #[test] + #[ignore] + fn test_bad_collapse_into_call() { + // Can't collapse this, because if we did, 'foo' would be called + // in the wrong 'this' context. + test_same("this.foo = function() {}; this.foo();"); + } + + #[test] + #[ignore] + fn test_bad_collapse() { + test_same("this.$e$ = []; this.$b$ = null;"); + } + + #[test] + #[ignore] + fn test_issue1017() { + test_same("x = x.parentNode.parentNode; x = x.parentNode.parentNode;"); + } + + #[test] + #[ignore] + fn test_destructuring_lhs_array_ideal_behaviours() { + test_same("a => { ([a] = a); return a; }"); // `a` is being reassigned. + test_same("a => { ([b] = a); return a; }"); // Evaluating `b` could side-effect `a`. + test_same("a => { ([a = foo()] = a); return a; }"); // `foo` may be invoked. + test_same("(a, b) => { (a = [a] = b); return b; }"); // Evaluating `a` could side-effect `b`. + } + + #[test] + #[ignore] + fn test_destructuring_lhs_array_backoff_behaviours() { + // TODO(b/123102446): We really like to collapse some of these chained assignments. + + test_same("(a, b) => { ([a] = a = b); return b; }"); // The middle `a` is redundant. + test_same("(a, b) => { ([a] = a = b); return a; }"); // The middle `a` is redundant. + test( + "(a, b) => { (a = [a] = b); return a; }", // The final `a` is redundant. + "(a, b) => { return (a = [a] = b); }", + ); + } + + #[test] + #[ignore] + fn test_destructuring_lhs_object_ideal_behaviours() { + test_same("a => { ({a} = a); return a; }"); // `a` is being reassigned. + test_same("a => { ({b} = a); return a; }"); // Evaluating `b` could side-effect `a`. + test_same("a => { ({a = foo()} = a); return a; }"); // `foo` may be invoked. + test_same("(a, b) => { (a = {a} = b); return b; }"); // Evaluating `a` could side-effect `b`. + } + + #[test] + #[ignore] + fn test_destructuring_lhs_object_backoff_behaviours() { + // TODO(b/123102446): We really like to collapse some of these chained assignments. + + test_same("(a, b) => { ({a} = a = b); return b; }"); // The middle `a` is redundant. + test_same("(a, b) => { ({a} = a = b); return a; }"); // The middle `a` is redundant. + test( + "(a, b) => { (a = {a} = b); return a; }", // The final `a` is redundant. + "(a, b) => { return (a = {a} = b); }", + ); + } +} diff --git a/crates/oxc_minifier/src/ast_passes/mod.rs b/crates/oxc_minifier/src/ast_passes/mod.rs index e7d4c265a5fb6..13ebd3ec8c1f4 100644 --- a/crates/oxc_minifier/src/ast_passes/mod.rs +++ b/crates/oxc_minifier/src/ast_passes/mod.rs @@ -1,16 +1,20 @@ mod collapse_variable_declarations; +mod exploit_assigns; mod peephole_fold_constants; mod peephole_minimize_conditions; mod peephole_remove_dead_code; mod peephole_substitute_alternate_syntax; mod remove_syntax; +mod statement_fusion; pub use collapse_variable_declarations::CollapseVariableDeclarations; +pub use exploit_assigns::ExploitAssigns; pub use peephole_fold_constants::PeepholeFoldConstants; pub use peephole_minimize_conditions::PeepholeMinimizeConditions; pub use peephole_remove_dead_code::PeepholeRemoveDeadCode; pub use peephole_substitute_alternate_syntax::PeepholeSubstituteAlternateSyntax; pub use remove_syntax::RemoveSyntax; +pub use statement_fusion::StatementFusion; use oxc_ast::ast::Program; use oxc_semantic::{ScopeTree, SymbolTable}; diff --git a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs new file mode 100644 index 0000000000000..f7f8d57c38500 --- /dev/null +++ b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs @@ -0,0 +1,204 @@ +use oxc_traverse::Traverse; + +use crate::CompressorPass; + +/// Statement Fusion +/// +/// Tries to fuse all the statements in a block into a one statement by using COMMAs or statements. +/// +/// +pub struct StatementFusion; + +impl<'a> CompressorPass<'a> for StatementFusion {} + +impl<'a> Traverse<'a> for StatementFusion {} + +impl StatementFusion { + pub fn new() -> Self { + Self {} + } +} + +#[cfg(test)] +mod test { + use oxc_allocator::Allocator; + + use crate::tester; + + fn test(source_text: &str, expected: &str) { + let allocator = Allocator::default(); + let mut pass = super::StatementFusion::new(); + tester::test(&allocator, source_text, expected, &mut pass); + } + + fn test_same(source_text: &str) { + test(source_text, source_text); + } + + fn fuse(before: &str, after: &str) { + test( + &("function F(){if(CONDITION){".to_string() + before + "}}"), + &("function F(){if(CONDITION){".to_string() + after + "}}"), + ); + } + + fn fuse_same(code: &str) { + test_same(&("function F(){if(CONDITION){".to_string() + code + "}}")); + } + + #[test] + #[ignore] + fn nothing_to_do() { + fuse_same(""); + fuse_same("a"); + fuse_same("a()"); + fuse_same("if(a()){}"); + } + + #[test] + #[ignore] + fn fold_block_with_statements() { + fuse("a;b;c", "a,b,c"); + fuse("a();b();c();", "a(),b(),c()"); + fuse("a(),b();c(),d()", "a(),b(),c(),d()"); + fuse("a();b(),c(),d()", "a(),b(),c(),d()"); + fuse("a(),b(),c();d()", "a(),b(),c(),d()"); + } + + #[test] + #[ignore] + fn fold_block_into_if() { + fuse("a;b;c;if(x){}", "if(a,b,c,x){}"); + fuse("a;b;c;if(x,y){}else{}", "if(a,b,c,x,y){}else{}"); + fuse("a;b;c;if(x,y){}", "if(a,b,c,x,y){}"); + fuse("a;b;c;if(x,y,z){}", "if(a,b,c,x,y,z){}"); + + // Can't fuse if there are statements after the IF. + fuse_same("a();if(a()){}a()"); + } + + #[test] + #[ignore] + fn fold_block_return() { + fuse("a;b;c;return x", "return a,b,c,x"); + fuse("a;b;c;return x+y", "return a,b,c,x+y"); + + // DeadAssignmentElimination would have cleaned it up anyways. + fuse_same("a;b;c;return x;a;b;c"); + } + + #[test] + #[ignore] + fn fold_block_throw() { + fuse("a;b;c;throw x", "throw a,b,c,x"); + fuse("a;b;c;throw x+y", "throw a,b,c,x+y"); + fuse_same("a;b;c;throw x;a;b;c"); + } + + #[test] + #[ignore] + fn fold_switch() { + fuse("a;b;c;switch(x){}", "switch(a,b,c,x){}"); + } + + #[test] + #[ignore] + fn fuse_into_for_in1() { + fuse("a;b;c;for(x in y){}", "for(x in a,b,c,y){}"); + } + + #[test] + #[ignore] + fn fuse_into_for_in2() { + // This test case causes a parse warning in ES5 strict out, but is a parse error in ES6+ out. + // setAcceptedLanguage(CompilerOptions.LanguageMode.ECMASCRIPT5_STRICT); + // set_expect_parse_warnings_in_this_test(); + fuse_same("a();for(var x = b() in y){}"); + } + + #[test] + #[ignore] + fn fuse_into_vanilla_for1() { + fuse("a;b;c;for(;g;){}", "for(a,b,c;g;){}"); + fuse("a;b;c;for(d;g;){}", "for(a,b,c,d;g;){}"); + fuse("a;b;c;for(d,e;g;){}", "for(a,b,c,d,e;g;){}"); + fuse_same("a();for(var x;g;){}"); + } + + #[test] + #[ignore] + fn fuse_into_vanilla_for2() { + fuse_same("a;b;c;for(var d;g;){}"); + fuse_same("a;b;c;for(let d;g;){}"); + fuse_same("a;b;c;for(const d = 5;g;){}"); + } + + #[test] + #[ignore] + fn fuse_into_label() { + fuse("a;b;c;label:for(x in y){}", "label:for(x in a,b,c,y){}"); + fuse("a;b;c;label:for(;g;){}", "label:for(a,b,c;g;){}"); + fuse("a;b;c;l1:l2:l3:for(;g;){}", "l1:l2:l3:for(a,b,c;g;){}"); + fuse_same("a;b;c;label:while(true){}"); + } + + #[test] + #[ignore] + fn fuse_into_block() { + fuse("a;b;c;{d;e;f}", "{a,b,c,d,e,f}"); + fuse( + "a;b; label: { if(q) break label; bar(); }", + "label: { if(a,b,q) break label; bar(); }", + ); + fuse_same("a;b;c;{var x;d;e;}"); + fuse_same("a;b;c;label:{break label;d;e;}"); + } + + #[test] + #[ignore] + fn no_fuse_into_while() { + fuse_same("a;b;c;while(x){}"); + } + + #[test] + #[ignore] + fn no_fuse_into_do() { + fuse_same("a;b;c;do{}while(x)"); + } + + #[test] + #[ignore] + fn no_fuse_into_block() { + // Never fuse a statement into a block that contains let/const/class declarations, or you risk + // colliding variable names. (unless the AST is normalized). + fuse("a; {b;}", "{a,b;}"); + fuse("a; {b; var a = 1;}", "{a,b; var a = 1;}"); + fuse_same("a; { b; let a = 1; }"); + fuse_same("a; { b; const a = 1; }"); + fuse_same("a; { b; class a {} }"); + fuse_same("a; { b; function a() {} }"); + fuse_same("a; { b; const otherVariable = 1; }"); + + // enable_normalize(); + test( + "function f(a) { if (COND) { a; { b; let a = 1; } } }", + "function f(a) { if (COND) { { a,b; let a$jscomp$1 = 1; } } }", + ); + test( + "function f(a) { if (COND) { a; { b; let otherVariable = 1; } } }", + "function f(a) { if (COND) { { a,b; let otherVariable = 1; } } }", + ); + } + + #[test] + #[ignore] + fn no_global_scope_changes() { + test_same("a,b,c"); + } + + #[test] + #[ignore] + fn no_function_block_changes() { + test_same("function foo() { a,b,c }"); + } +} diff --git a/crates/oxc_minifier/src/compressor.rs b/crates/oxc_minifier/src/compressor.rs index 7245f65ee415a..c7b3be6e10a30 100644 --- a/crates/oxc_minifier/src/compressor.rs +++ b/crates/oxc_minifier/src/compressor.rs @@ -5,8 +5,9 @@ use oxc_traverse::TraverseCtx; use crate::{ ast_passes::{ - CollapseVariableDeclarations, PeepholeFoldConstants, PeepholeMinimizeConditions, - PeepholeRemoveDeadCode, PeepholeSubstituteAlternateSyntax, RemoveSyntax, + CollapseVariableDeclarations, ExploitAssigns, PeepholeFoldConstants, + PeepholeMinimizeConditions, PeepholeRemoveDeadCode, PeepholeSubstituteAlternateSyntax, + RemoveSyntax, StatementFusion, }, CompressOptions, CompressorPass, }; @@ -41,13 +42,13 @@ impl<'a> Compressor<'a> { return; } - // TODO: inline variables self.fold_constants(program, &mut ctx); self.minimize_conditions(program, &mut ctx); self.remove_dead_code(program, &mut ctx); - // TODO: StatementFusion + self.statement_fusion(program, &mut ctx); self.substitute_alternate_syntax(program, &mut ctx); - self.collapse(program, &mut ctx); + self.collapse_variable_declarations(program, &mut ctx); + self.exploit_assigns(program, &mut ctx); } fn dead_code_elimination(self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { @@ -76,7 +77,15 @@ impl<'a> Compressor<'a> { PeepholeRemoveDeadCode::new().build(program, ctx); } - fn collapse(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { + fn statement_fusion(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { + StatementFusion::new().build(program, ctx); + } + + fn collapse_variable_declarations(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { CollapseVariableDeclarations::new(self.options).build(program, ctx); } + + fn exploit_assigns(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { + ExploitAssigns::new().build(program, ctx); + } }