JS: clean up var decls that were hoisted, but after merging no longer…

… need hoisting
tdewolff · Apr 5, 2022 · 8c98c9e · 8c98c9e
1 parent 0b7413d
commit 8c98c9e
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 24 deletions.
diff --git a/js/js.go b/js/js.go
@@ -535,31 +535,33 @@ func (m *jsMinifier) minifyVarDecl(decl *js.VarDecl, onlyDefines bool) {
 			}
 		}
 	} else {
-		m.write(decl.TokenType.Bytes())
-		m.writeSpaceBeforeIdent()
-
-		// move single var decls forward and order for GZIP optimization
-		start := 0
-		if _, ok := decl.List[0].Binding.(*js.Var); !ok {
-			start++
-		}
-		for i, item := range decl.List {
-			if v, ok := item.Binding.(*js.Var); ok && item.Default == nil && len(v.Data) == 1 {
-				for j := start; j < len(decl.List); j++ {
-					if v2, ok := decl.List[j].Binding.(*js.Var); ok && decl.List[j].Default == nil && len(v2.Data) == 1 {
-						if m.renamer.identOrder[v2.Data[0]] < m.renamer.identOrder[v.Data[0]] {
-							continue
-						} else if m.renamer.identOrder[v2.Data[0]] == m.renamer.identOrder[v.Data[0]] {
-							break
+		if decl.TokenType == js.VarToken {
+			// move single var decls forward and order for GZIP optimization
+			start := 0
+			if _, ok := decl.List[0].Binding.(*js.Var); !ok {
+				start++
+			}
+			for i := 0; i < len(decl.List); i++ {
+				item := decl.List[i]
+				if v, ok := item.Binding.(*js.Var); ok && item.Default == nil && len(v.Data) == 1 {
+					for j := start; j < len(decl.List); j++ {
+						if v2, ok := decl.List[j].Binding.(*js.Var); ok && decl.List[j].Default == nil && len(v2.Data) == 1 {
+							if m.renamer.identOrder[v2.Data[0]] < m.renamer.identOrder[v.Data[0]] {
+								continue
+							} else if m.renamer.identOrder[v2.Data[0]] == m.renamer.identOrder[v.Data[0]] {
+								break
+							}
 						}
+						decl.List = append(decl.List[:i], decl.List[i+1:]...)
+						decl.List = append(decl.List[:j], append([]js.BindingElement{item}, decl.List[j:]...)...)
+						break
 					}
-					decl.List = append(decl.List[:i], decl.List[i+1:]...)
-					decl.List = append(decl.List[:j], append([]js.BindingElement{item}, decl.List[j:]...)...)
-					break
 				}
 			}
 		}
 
+		m.write(decl.TokenType.Bytes())
+		m.writeSpaceBeforeIdent()
 		for i, item := range decl.List {
 			if i != 0 {
 				m.write(commaBytes)

diff --git a/js/js_test.go b/js/js_test.go
@@ -54,7 +54,7 @@ func TestJS(t *testing.T) {
 		{`/a/ instanceof b`, `/a/ instanceof b`},
 		{`[a] instanceof b`, `[a]instanceof b`},
 		{`let a = 5;a`, `let a=5;a`},
-		{`let a = 5,b;a,b`, `let b,a=5;a,b`},
+		{`let a = 5,b;a,b`, `let a=5,b;a,b`},
 		{`let a,b = 5;a,b`, `let a,b=5;a,b`},
 		{`function a(){}`, `function a(){}`},
 		{`function a(b){b}`, `function a(b){b}`},
@@ -689,8 +689,8 @@ func TestJS(t *testing.T) {
 		{`a<</script>/`, `a<< /script>/`},
 		{`function f(a,b){a();for(const c of b){const b=0}}`, `function f(a,b){a();for(const c of b){const b=0}}`},
 		{`return a,b,void 0`, `return a,b`},
-		//{`var arr=[];var slice=arr.slice;var concat=arr.concat;var push=arr.push;var indexOf=arr.indexOf;var class2type={};`, `var arr=[],slice=arr.slice,concat=arr.concat,push=arr.push,indexOf=arr.indexOf,class2type={}`},
-		//{`var arr=[];var class2type={};a=5;var rlocalProtocol=0`, `var arr=[],class2type={};a=5;var rlocalProtocol=0`},
+		{`var arr=[];var slice=arr.slice;var concat=arr.concat;var push=arr.push;var indexOf=arr.indexOf;var class2type={};`, `var arr=[],slice=arr.slice,concat=arr.concat,push=arr.push,indexOf=arr.indexOf,class2type={}`},
+		{`var arr=[];var class2type={};a=5;var rlocalProtocol=0`, `var arr=[],class2type={};a=5;var rlocalProtocol=0`},
 
 		// bugs
 		{`({"":a})`, `({"":a})`},                 // go-fuzz
@@ -769,7 +769,7 @@ func TestJSVarRenaming(t *testing.T) {
 		{`!function(a){a;for(var b=0;;);};var c;var d;`, `!function(a){a;for(var b=0;;);};var c,d`},
 		{`!function(){var b;b;{(T=x),T}{var T}}`, `!function(){var a,b;b,a=x,a}`},
 		{`var T;T;!function(){var b;b;{(T=x),T}{var T}}`, `var T;T,!function(){var a,b;b,a=x,a}`},
-		{`!function(){let a=b,b=c,c=d,d=e,e=f,f=g,g=h,h=a,j;for(let i=0;;)j=4}`, `!function(){let i,a=b,b=c,c=d,d=e,e=f,f=g,g=h,h=a;for(let a=0;;)i=4}`},
+		{`!function(){let a=b,b=c,c=d,d=e,e=f,f=g,g=h,h=a,j;for(let i=0;;)j=4}`, `!function(){let a=b,b=c,c=d,d=e,e=f,f=g,g=h,h=a,i;for(let a=0;;)i=4}`},
 		{`function a(){var name;with(z){name}} function b(){var name;name}`, `function a(){var name;with(z)name}function b(){var a;a}`},
 		{`!function(){var name;{name;!function(){name;var other;other}}}`, `!function(){var a;a,!function(){a;var b;b}}`},
 		{`name=function(){var a001,a002,a003,a004,a005,a006,a007,a008,a009,a010,a011,a012,a013,a014,a015,a016,a017,a018,a019,a020,a021,a022,a023,a024,a025,a026,a027,a028,a029,a030,a031,a032,a033,a034,a035,a036,a037,a038,a039,a040,a041,a042,a043,a044,a045,a046,a047,a048,a049,a050,a051,a052,a053,a054,a055,a056,a057,a058,a059,a060,a061,a062,a063,a064,a065,a066,a067,a068,a069,a070,a071,a072,a073,a074,a075,a076,a077,a078,a079,a080,a081,a082,a083,a084,a085,a086,a087,a088,a089,a090,a091,a092,a093,a094,a095,a096,a097,a098,a099,a100,a101,a102,a103,a104,a105,a106,a107,a108,a109,a110,a111,a112,a113,a114,a115,a116,a117,a118,a119;a001,a002,a003,a004,a005,a006,a007,a008,a009,a010,a011,a012,a013,a014,a015,a016,a017,a018,a019,a020,a021,a022,a023,a024,a025,a026,a027,a028,a029,a030,a031,a032,a033,a034,a035,a036,a037,a038,a039,a040,a041,a042,a043,a044,a045,a046,a047,a048,a049,a050,a051,a052,a053,a054,a055,a056,a057,a058,a059,a060,a061,a062,a063,a064,a065,a066,a067,a068,a069,a070,a071,a072,a073,a074,a075,a076,a077,a078,a079,a080,a081,a082,a083,a084,a085,a086,a087,a088,a089,a090,a091,a092,a093,a094,a095,a096,a097,a098,a099,a100,a101,a102,a103,a104,a105,a106,a107,a108,a109,a110,a111,a112,a113,a114,a115,a116,a117,a118,a119}`,

diff --git a/js/stmtlist.go b/js/stmtlist.go
@@ -181,6 +181,15 @@ func (m *jsMinifier) optimizeStmtList(list []js.IStmt, blockType blockType) []js
 					// merge const and let declarations, or non-hoisted var declarations
 					right.List = append(left.List, right.List...)
 					j--
+
+					// remove from vardecls list of scope
+					scope := left.Scope.Func
+					for i, decl := range scope.VarDecls {
+						if left == decl {
+							scope.VarDecls = append(scope.VarDecls[:i], scope.VarDecls[i+1:]...)
+							break
+						}
+					}
 				} else if left.TokenType == js.VarToken {
 					if exprStmt, ok := list[i].(*js.ExprStmt); ok {
 						// pull in assignments to variables into the declaration, e.g. var a;a=5  =>  var a=5

diff --git a/js/vars.go b/js/vars.go
@@ -197,11 +197,23 @@ func addDefinition(decl *js.VarDecl, binding js.IBinding, value js.IExpr, forwar
 	}
 
 	// remove variables in destination
+RemoveVarsLoop:
 	for _, vbind := range vars {
 		for i, item := range decl.List {
 			if v, ok := item.Binding.(*js.Var); ok && v == vbind {
+				// item.Default == nil
 				decl.List = append(decl.List[:i], decl.List[i+1:]...)
-				break
+				continue RemoveVarsLoop
+			}
+		}
+
+		// variable declaration must be somewhere else, find and remove it
+		for _, decl2 := range decl.Scope.Func.VarDecls {
+			for i, item := range decl2.List {
+				if v, ok := item.Binding.(*js.Var); ok && item.Default == nil && v == vbind {
+					decl2.List = append(decl2.List[:i], decl2.List[i+1:]...)
+					continue RemoveVarsLoop
+				}
 			}
 		}
 	}
@@ -232,6 +244,16 @@ func mergeVarDecls(dst, src *js.VarDecl, forward bool) bool {
 			j--
 		}
 	}
+	if merge {
+		// remove from vardecls list of scope
+		scope := src.Scope.Func
+		for i, decl := range scope.VarDecls {
+			if src == decl {
+				scope.VarDecls = append(scope.VarDecls[:i], scope.VarDecls[i+1:]...)
+				break
+			}
+		}
+	}
 	return merge
 }