JS: variable usage, indexing and dot expressions have side-effects, f…

…ixes #725
tdewolff · Jul 10, 2024 · 767148c · 767148c
1 parent 9fab517
commit 767148c
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 21 deletions.
diff --git a/js/js_test.go b/js/js_test.go
@@ -428,6 +428,10 @@ func TestJS(t *testing.T) {
 		//{`function g(){return 5;function f(){}}`, `function g(){return 5;function f(){}}`},
 		//{`function g(){if(a)return a;else return b;var c;c=d}`, `function g(){var c;return a||b}`},
 		//{`()=>a()`, ``},
+		{`if(true){let b}`, ``},
+		{`if(true){let b=5}`, ``},
+		{`if(true){const b=c}`, `!0&&c`},
+		{`if(true){const b=c.d}`, `!0&&c.d`},
 
 		// arrow functions
 		{`() => {}`, `()=>{}`},
@@ -798,7 +802,7 @@ func TestJS(t *testing.T) {
 		{`if(!a)debugger;`, `if(!a)debugger`},                                                           // #370
 		{`export function a(b){b}`, `export function a(b){b}`},                                          // #375
 		{`switch(a){case 0:b=c;d=e}`, `switch(a){case 0:b=c,d=e}`},                                      // #426
-		{`if(a){const b=0}`, ``},                                                                        // #428
+		{`if(a){const b=0}`, `a`},                                                                       // #428
 		{`()=>({a(){b=!b}})`, `()=>({a(){b=!b}})`},                                                      // #429
 		{`var a=1;function f(){return 1}var{min,max}=Math;function g(){return 2}`, `a=1;function f(){return 1}var{min,max}=Math,a;function g(){return 2}`}, // #445
 		{`const f=x=>void console.log(x)`, `const f=x=>void console.log(x)`},                                                                               // #463

diff --git a/js/stmtlist.go b/js/stmtlist.go
@@ -7,6 +7,12 @@ import (
 func optimizeStmt(i js.IStmt) js.IStmt {
 	// convert if/else into expression statement, and optimize blocks
 	if ifStmt, ok := i.(*js.IfStmt); ok {
+		if ifStmt.Body != nil {
+			ifStmt.Body = optimizeStmt(ifStmt.Body)
+		}
+		if ifStmt.Else != nil {
+			ifStmt.Else = optimizeStmt(ifStmt.Else)
+		}
 		hasIf := !isEmptyStmt(ifStmt.Body)
 		hasElse := !isEmptyStmt(ifStmt.Else)
 		if unaryExpr, ok := ifStmt.Cond.(*js.UnaryExpr); ok && unaryExpr.Op == js.NotToken && hasElse {
@@ -15,9 +21,11 @@ func optimizeStmt(i js.IStmt) js.IStmt {
 			hasIf, hasElse = hasElse, hasIf
 		}
 		if !hasIf && !hasElse {
-			return &js.ExprStmt{Value: ifStmt.Cond}
+			if hasSideEffects(ifStmt.Cond) {
+				return &js.ExprStmt{Value: ifStmt.Cond}
+			}
+			return &js.EmptyStmt{}
 		} else if hasIf && !hasElse {
-			ifStmt.Body = optimizeStmt(ifStmt.Body)
 			if X, isExprBody := ifStmt.Body.(*js.ExprStmt); isExprBody {
 				if unaryExpr, ok := ifStmt.Cond.(*js.UnaryExpr); ok && unaryExpr.Op == js.NotToken {
 					left := groupExpr(unaryExpr.X, binaryLeftPrecMap[js.OrToken])
@@ -35,15 +43,12 @@ func optimizeStmt(i js.IStmt) js.IStmt {
 				return ifStmt
 			}
 		} else if !hasIf && hasElse {
-			ifStmt.Else = optimizeStmt(ifStmt.Else)
 			if X, isExprElse := ifStmt.Else.(*js.ExprStmt); isExprElse {
 				left := groupExpr(ifStmt.Cond, binaryLeftPrecMap[js.OrToken])
 				right := groupExpr(X.Value, binaryRightPrecMap[js.OrToken])
 				return &js.ExprStmt{&js.BinaryExpr{js.OrToken, left, right}}
 			}
 		} else if hasIf && hasElse {
-			ifStmt.Body = optimizeStmt(ifStmt.Body)
-			ifStmt.Else = optimizeStmt(ifStmt.Else)
 			XExpr, isExprBody := ifStmt.Body.(*js.ExprStmt)
 			YExpr, isExprElse := ifStmt.Else.(*js.ExprStmt)
 			if isExprBody && isExprElse {

diff --git a/js/util.go b/js/util.go
@@ -90,13 +90,6 @@ func isEmptyStmt(stmt js.IStmt) bool {
 		return true
 	} else if _, ok := stmt.(*js.EmptyStmt); ok {
 		return true
-	} else if decl, ok := stmt.(*js.VarDecl); ok && decl.TokenType == js.ErrorToken {
-		for _, item := range decl.List {
-			if item.Default != nil {
-				return false
-			}
-		}
-		return true
 	} else if block, ok := stmt.(*js.BlockStmt); ok {
 		for _, item := range block.List {
 			if ok := isEmptyStmt(item); !ok {
@@ -349,16 +342,18 @@ func exprPrec(i js.IExpr) js.OpPrec {
 func hasSideEffects(i js.IExpr) bool {
 	// assume that variable usage and that the index operator themselves have no side effects
 	switch expr := i.(type) {
-	case *js.Var, *js.LiteralExpr, *js.FuncDecl, *js.ClassDecl, *js.ArrowFunc, *js.NewTargetExpr, *js.ImportMetaExpr:
+	case *js.Var:
+		return true
+	case *js.LiteralExpr, *js.FuncDecl, *js.ClassDecl, *js.ArrowFunc, *js.NewTargetExpr, *js.ImportMetaExpr:
 		return false
 	case *js.NewExpr, *js.CallExpr, *js.YieldExpr:
 		return true
 	case *js.GroupExpr:
 		return hasSideEffects(expr.X)
 	case *js.DotExpr:
-		return hasSideEffects(expr.X)
+		return true
 	case *js.IndexExpr:
-		return hasSideEffects(expr.X) || hasSideEffects(expr.Y)
+		return true
 	case *js.CondExpr:
 		return hasSideEffects(expr.Cond) || hasSideEffects(expr.X) || hasSideEffects(expr.Y)
 	case *js.CommaExpr:

diff --git a/js/util_test.go b/js/util_test.go
@@ -107,24 +107,24 @@ func TestHasSideEffects(t *testing.T) {
 		has bool
 	}{
 		{"1", false},
-		{"a", false},
+		{"a", true},
 		{"a++", true},
 		{"a--", true},
 		{"++a", true},
 		{"--a", true},
 		{"delete a", true},
-		{"!a", false},
+		{"!0", false},
 		{"a=5", true},
 		{"a+=5", true},
 		{"a+5", false},
 		{"a()", true},
-		{"a.b", false},
+		{"a.b", true},
 		{"a.b()", true},
 		{"a().b", true},
-		{"a[b]", false},
+		{"a[b]", true},
 		{"a[b()]", true},
 		{"a()[b]", true},
-		{"a?.b", false},
+		{"a?.b", true},
 		{"a()?.b", true},
 		{"a?.b()", true},
 		{"new a", true},