syntax: honor escaped brace metacharacters

[AlexandreYang]

cc @mvdan

Summary

syntax.SplitBraces currently scans literal word parts for {, ,, .., and } without checking whether those bytes were quoted with a backslash. Bash performs brace expansion before quote removal, but brace syntax is still made of unquoted metacharacters: an unquoted opening brace, an unquoted comma or sequence expression, and an unquoted closing brace.

As a result, expand.Fields can produce fields that differ from bash for words such as:

• \{a,b}: the { is quoted, so this should stay one field: {a,b}
• {a\,b,c}: the comma is quoted, so this should expand to a,b and c
• {a\}b,c}: the first } is quoted, so this should expand to a}b and c
• \{{x},z}: the first { is quoted, but the later brace expression should still be parsed like bash

This PR updates SplitBraces to ignore backslash-escaped brace metacharacters. It also keeps a top-level single-element brace open long enough for a later unescaped comma or sequence operator to form an expansion, which is needed for bash-compatible cases like {x},z} and \{{x},z}.

The actual expansion enumerator, expand.Braces, is unchanged; this only changes how syntax.SplitBraces identifies BraceExp nodes.

Why this is needed

expand.Fields is commonly used as the high-level shell field expansion API. Callers expect it to be close to bash for argument expansion, especially because brace expansion happens before quote removal. Today, callers that need bash-compatible behavior must add their own pre-processing around expand.Fields to protect escaped brace metacharacters before SplitBraces runs.

Fixing this in SplitBraces is the right layer because:

• SplitBraces is where literal text is classified as brace syntax or ordinary text.
• expand.Braces only enumerates already-parsed BraceExp nodes; by then it is too late to know that a metacharacter was escaped.
• The parser preserves backslashes in *syntax.Lit values, so SplitBraces has enough information to distinguish quoted vs unquoted brace metacharacters.
• Downstream users get bash-compatible behavior without maintaining local workarounds.

Downstream context

This issue was found while updating DataDog/rshell from mvdan.cc/sh/v3 v3.13.0 to v3.13.1: DataDog/rshell#197

rshell currently carries a local compatibility shim before calling expand.Fields to preserve bash behavior for escaped brace metacharacters. If this fix lands upstream and is released, rshell should be able to remove most or all of that workaround.

Reproducing and comparing with bash

From a checkout of mvdan/sh, run this Go reproducer:

cat >/tmp/brace-repro.go <<'EOF'
package main

import (
	"fmt"
	"strings"

	"mvdan.cc/sh/v3/expand"
	"mvdan.cc/sh/v3/syntax"
)

func main() {
	cases := []string{
		`\{a,b}`,
		`\\{a,b}`,
		`\\\{a,b}`,
		`{a\,b,c}`,
		`{a\}b,c}`,
		`{1\..3,foo}`,
		`\{{x},z}`,
		`\{{1..3},}`,
		`\{{1\..3},}`,
		`{x},z}`,
		`{{x},z}`,
	}
	for _, src := range cases {
		word, err := syntax.NewParser().Document(strings.NewReader(src))
		if err != nil {
			panic(err)
		}
		fields, err := expand.Fields(nil, word)
		if err != nil {
			panic(err)
		}
		fmt.Printf("case %s\n", src)
		for _, field := range fields {
			fmt.Printf("<%s>\n", field)
		}
	}
}
EOF

go run /tmp/brace-repro.go >/tmp/sh-expand.out

Then run the equivalent bash argv-expansion script:

cat >/tmp/brace-repro.sh <<'EOF'
show() {
	printf 'case %s\n' "$1"
	shift
	printf '<%s>\n' "$@"
}

show '\{a,b}'      \{a,b}
show '\\{a,b}'     \\{a,b}
show '\\\{a,b}'    \\\{a,b}
show '{a\,b,c}'    {a\,b,c}
show '{a\}b,c}'    {a\}b,c}
show '{1\..3,foo}' {1\..3,foo}
show '\{{x},z}'    \{{x},z}
show '\{{1..3},}'  \{{1..3},}
show '\{{1\..3},}' \{{1\..3},}
show '{x},z}'      {x},z}
show '{{x},z}'     {{x},z}
EOF

bash /tmp/brace-repro.sh >/tmp/bash.out

Compare them:

diff -u /tmp/bash.out /tmp/sh-expand.out

On master before this PR, the diff shows multiple mismatches, for example \{a,b} is incorrectly expanded as if the escaped { were brace syntax. With this PR, the diff is empty for these cases.

Tests

go test ./syntax ./expand

[AlexandreYang]

Downstream context: this bash-compatibility issue was found while updating DataDog/rshell to mvdan.cc/sh/v3 v3.13.1: DataDog/rshell#197

rshell currently carries a local workaround before calling expand.Fields; this PR aims to fix the behavior at the syntax.SplitBraces layer so downstream callers no longer need that workaround.

[vmaerten]

Hello !
The same issue has been reported in Task (go-task/task#2812)

[mvdan]

Thanks, I'll take a look at this this weekend. It's likely that there is a bug here, but I don't think this fix is the right approach.

[mvdan]

The fix is now in master; as expected this was almost a one-line fix.

The apparent regression was 7e3be04; note how the expand package was incorrectly omitting backslashes from its output. That bug was papering over this bug, but they are different bugs, we were just getting lucky in some cases. This fixes the underlying issue.

I'll probably tag a bugfix release in the coming days.

[vmaerten]

Thanks a lot!

[AlexandreYang]

Thanks for fixing so quickly!

Would it be possible to have a new release ?

[mvdan]

As I already mentioned, I'll tag a release in a few days. You can use master in the meantime.