fix constant folding bug with nested operands

evanw · Jan 27, 2023 · 0c3642e · 0c3642e
1 parent be94d37
commit 0c3642e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 10 deletions.
diff --git a/internal/js_ast/js_ast_helpers.go b/internal/js_ast/js_ast_helpers.go
@@ -1312,9 +1312,16 @@ func tryToStringOnNumberSafely(n float64) (string, bool) {
 	return "", false
 }
 
+type StringAdditionKind uint8
+
+const (
+	StringAdditionNormal StringAdditionKind = iota
+	StringAdditionWithNestedLeft
+)
+
 // This function intentionally avoids mutating the input AST so it can be
 // called after the AST has been frozen (i.e. after parsing ends).
-func FoldStringAddition(left Expr, right Expr) Expr {
+func FoldStringAddition(left Expr, right Expr, kind StringAdditionKind) Expr {
 	// "See through" inline enum constants
 	if l, ok := left.Data.(*EInlinedEnum); ok {
 		left = l.Value
@@ -1323,13 +1330,21 @@ func FoldStringAddition(left Expr, right Expr) Expr {
 		right = r.Value
 	}
 
-	if l, ok := left.Data.(*ENumber); ok {
-		switch right.Data.(type) {
-		case *EString, *ETemplate:
-			// "0 + 'x'" => "0 + 'x'"
-			// "0 + `${x}`" => "0 + `${x}`"
-			if str, ok := tryToStringOnNumberSafely(l.Value); ok {
-				left.Data = &EString{Value: helpers.StringToUTF16(str)}
+	// Transforming the left operand into a string is not safe if it comes from
+	// a nested AST node. The following transforms are invalid:
+	//
+	//   "0 + 1 + 'x'" => "0 + '1x'"
+	//   "0 + 1 + `${x}`" => "0 + `1${x}`"
+	//
+	if kind != StringAdditionWithNestedLeft {
+		if l, ok := left.Data.(*ENumber); ok {
+			switch right.Data.(type) {
+			case *EString, *ETemplate:
+				// "0 + 'x'" => "0 + 'x'"
+				// "0 + `${x}`" => "0 + `${x}`"
+				if str, ok := tryToStringOnNumberSafely(l.Value); ok {
+					left.Data = &EString{Value: helpers.StringToUTF16(str)}
+				}
 			}
 		}
 	}

diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -12756,13 +12756,13 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 
 		case js_ast.BinOpAdd:
 			// "'abc' + 'xyz'" => "'abcxyz'"
-			if result := js_ast.FoldStringAddition(e.Left, e.Right); result.Data != nil {
+			if result := js_ast.FoldStringAddition(e.Left, e.Right, js_ast.StringAdditionNormal); result.Data != nil {
 				return result, exprOut{}
 			}
 
 			if left, ok := e.Left.Data.(*js_ast.EBinary); ok && left.Op == js_ast.BinOpAdd {
 				// "x + 'abc' + 'xyz'" => "x + 'abcxyz'"
-				if result := js_ast.FoldStringAddition(left.Right, e.Right); result.Data != nil {
+				if result := js_ast.FoldStringAddition(left.Right, e.Right, js_ast.StringAdditionWithNestedLeft); result.Data != nil {
 					return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EBinary{Op: left.Op, Left: left.Left, Right: result}}, exprOut{}
 				}
 			}

diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -2621,6 +2621,17 @@ func TestConstantFolding(t *testing.T) {
 	expectPrinted(t, "x = Infinity === -Infinity", "x = false;\n")
 
 	expectPrinted(t, "x = 123n === 1_2_3n", "x = true;\n")
+
+	// We support folding strings from sibling AST nodes since that ends up being
+	// equivalent with string addition. For example, "(x + 'a') + 'b'" is the
+	// same as "x + 'ab'". However, this is not true for numbers. We can't turn
+	// "(x + 1) + '2'" into "x + '12'". These tests check for this edge case.
+	expectPrinted(t, "x = 'a' + 'b' + y", "x = \"ab\" + y;\n")
+	expectPrinted(t, "x = y + 'a' + 'b'", "x = y + \"ab\";\n")
+	expectPrinted(t, "x = '3' + 4 + y", "x = \"34\" + y;\n")
+	expectPrinted(t, "x = y + 4 + '5'", "x = y + 4 + \"5\";\n")
+	expectPrinted(t, "x = '3' + 4 + 5", "x = \"345\";\n")
+	expectPrinted(t, "x = 3 + 4 + '5'", "x = 3 + 4 + \"5\";\n")
 }
 
 func TestConstantFoldingScopes(t *testing.T) {