Fixed issue in len with string concatenation in argument

grantnelson-wf · grantnelson-wf · commit d01a92e91402 · 2024-10-28T13:42:17.000-06:00
diff --git a/compiler/compiler_test.go b/compiler/compiler_test.go
@@ -360,6 +360,50 @@ func TestDeclSelection_RemoveUnusedTypeConstraint(t *testing.T) {
 	sel.InitCode.IsDead(`ghost = new Bar\[\d+ /\* int \*/\]\.ptr\(7\)`)
 }
 
+func TestLengthParenthesizingIssue841(t *testing.T) {
+	// See issue https://github.com/gopherjs/gopherjs/issues/841
+	//
+	// Summary: Given `len(a+b)` where a and b are strings being concatenated
+	// together, the result was `a + b.length` instead of `(a+b).length`.
+	//
+	// The fix was to check if the expression in `len` contains a binary
+	// expression or not. If it does, then the expression is parenthesized.
+	// This will work for concatenations any combination of variables and literals.
+
+	src := `
+		package main
+
+		func main() {
+			a := "a"
+			b := "b"
+			ab := a + b
+			if len(a+b) != len(ab) {
+				panic("unreachable")
+			}
+		}`
+
+	srcFiles := []srctesting.Source{{Name: `main.go`, Contents: []byte(src)}}
+	root := srctesting.ParseSources(t, srcFiles, nil)
+	archives := compileProject(t, root, false)
+	mainPkg := archives[root.PkgPath]
+
+	badRegex := regexp.MustCompile(`a\s*\+\s*b\.length`)
+	goodRegex := regexp.MustCompile(`\(a\s*\+\s*b\)\.length`)
+	goodFound := false
+	for i, decl := range mainPkg.Declarations {
+		if badRegex.Match(decl.DeclCode) {
+			t.Errorf("found length issue in decl #%d: %s", i, decl.FullName)
+			t.Logf("decl code:\n%s", string(decl.DeclCode))
+		}
+		if goodRegex.Match(decl.DeclCode) {
+			goodFound = true
+		}
+	}
+	if !goodFound {
+		t.Error("parenthesized length not found")
+	}
+}
+
 func compareOrder(t *testing.T, sourceFiles []srctesting.Source, minify bool) {
 	t.Helper()
 	outputNormal := compile(t, sourceFiles, minify)
diff --git a/compiler/expressions.go b/compiler/expressions.go
@@ -982,6 +982,9 @@ func (fc *funcContext) translateBuiltin(name string, sig *types.Signature, args
 	case "len":
 		switch argType := fc.typeOf(args[0]).Underlying().(type) {
 		case *types.Basic:
+			if containsBinaryExpr(args[0]) {
+				return fc.formatExpr("(%e).length", args[0])
+			}
 			return fc.formatExpr("%e.length", args[0])
 		case *types.Slice:
 			return fc.formatExpr("%e.$length", args[0])
diff --git a/compiler/utils.go b/compiler/utils.go
@@ -396,6 +396,20 @@ func isPkgLevel(o types.Object) bool {
 	return (o.Parent() != nil && o.Parent().Parent() == types.Universe) || isType
 }
 
+// containsBinaryExpr returns true if the given expression contains a binary
+// expression somewhere in its subtree.
+func containsBinaryExpr(expr ast.Expr) bool {
+	var found bool
+	ast.Inspect(expr, func(n ast.Node) bool {
+		if _, ok := n.(*ast.BinaryExpr); ok {
+			found = true
+			return false
+		}
+		return true
+	})
+	return found
+}
+
 // assignedObjectName checks if the object has been previously assigned a name
 // in this or one of the parent contexts. If not, found will be false.
 func (fc *funcContext) assignedObjectName(o types.Object) (name string, found bool) {
