From ce08221e22660d8111847502a4b1f306abac816e Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Tue, 17 Sep 2024 13:18:41 +1000 Subject: [PATCH 1/2] Add test cases from #14 --- interpolate_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interpolate_test.go b/interpolate_test.go index d33deda..bc192c1 100644 --- a/interpolate_test.go +++ b/interpolate_test.go @@ -290,6 +290,8 @@ func TestEscapingVariables(t *testing.T) { {`Do this \${SUCH_ESCAPE}`, `Do this ${SUCH_ESCAPE}`}, {`Do this $${SUCH_ESCAPE:-$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, {`Do this \${SUCH_ESCAPE:-$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, + {`Do this $${SUCH_ESCAPE:-$$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, + {`Do this \${SUCH_ESCAPE:-\$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, {`echo "my favourite mountain is cotopaxi" | grep 'xi$$'`, `echo "my favourite mountain is cotopaxi" | grep 'xi$'`}, } { result, err := interpolate.Interpolate(nil, tc.Str) From d7d6b88adb2c3f9f8e9294f6fd3d70fcb0c5912d Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Tue, 17 Sep 2024 16:37:58 +1000 Subject: [PATCH 2/2] Fix nested escaped dollar signs --- go.mod | 2 + go.sum | 2 + interpolate.go | 22 ++++-- interpolate_test.go | 57 ++++++++++---- parser.go | 47 +++++++---- parser_test.go | 184 ++++++++++++++++++++++++-------------------- 6 files changed, 193 insertions(+), 121 deletions(-) create mode 100644 go.sum diff --git a/go.mod b/go.mod index 903fcfc..27c4d1c 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/buildkite/interpolate go 1.22 + +require github.com/google/go-cmp v0.6.0 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..5a8d551 --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= diff --git a/interpolate.go b/interpolate.go index 7a24fd8..3079564 100644 --- a/interpolate.go +++ b/interpolate.go @@ -1,8 +1,8 @@ package interpolate import ( - "bytes" "fmt" + "strings" ) // Interpolate takes a set of environment and interpolates it into the provided string using shell script expansions @@ -28,7 +28,13 @@ func Identifiers(str string) ([]string, error) { // An expansion is something that takes in ENV and returns a string or an error type Expansion interface { + // Expand expands the expansion using variables from env. Expand(env Env) (string, error) + + // Identifiers returns any variable names referenced within the expansion. + // Escaped expansions do something special and return identifiers + // (starting with $) that *would* become referenced after a round of + // unescaping. Identifiers() []string } @@ -84,15 +90,17 @@ func (e UnsetValueExpansion) Expand(env Env) (string, error) { // EscapedExpansion is an expansion that is delayed until later on (usually by a later process) type EscapedExpansion struct { - Identifier string + // PotentialIdentifier is an identifier for the purpose of Identifiers, + // but not for the purpose of Expand. + PotentialIdentifier string } func (e EscapedExpansion) Identifiers() []string { - return []string{"$" + e.Identifier} + return []string{"$" + e.PotentialIdentifier} } func (e EscapedExpansion) Expand(Env) (string, error) { - return "$" + e.Identifier, nil + return "$", nil } // SubstringExpansion returns a substring (or slice) of the env @@ -193,7 +201,7 @@ func (e Expression) Identifiers() []string { } func (e Expression) Expand(env Env) (string, error) { - buf := &bytes.Buffer{} + var buf strings.Builder for _, item := range e { if item.Expansion != nil { @@ -201,9 +209,9 @@ func (e Expression) Expand(env Env) (string, error) { if err != nil { return "", err } - _, _ = buf.WriteString(result) + buf.WriteString(result) } else { - _, _ = buf.WriteString(item.Text) + buf.WriteString(item.Text) } } diff --git a/interpolate_test.go b/interpolate_test.go index bc192c1..bab0e56 100644 --- a/interpolate_test.go +++ b/interpolate_test.go @@ -281,25 +281,52 @@ func TestEscapingVariables(t *testing.T) { t.Parallel() for _, tc := range []struct { - Str string - Expected string + input string + want string }{ - {`Do this $$ESCAPE_PARTY`, `Do this $ESCAPE_PARTY`}, - {`Do this \$ESCAPE_PARTY`, `Do this $ESCAPE_PARTY`}, - {`Do this $${SUCH_ESCAPE}`, `Do this ${SUCH_ESCAPE}`}, - {`Do this \${SUCH_ESCAPE}`, `Do this ${SUCH_ESCAPE}`}, - {`Do this $${SUCH_ESCAPE:-$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, - {`Do this \${SUCH_ESCAPE:-$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, - {`Do this $${SUCH_ESCAPE:-$$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, - {`Do this \${SUCH_ESCAPE:-\$OTHERWISE}`, `Do this ${SUCH_ESCAPE:-$OTHERWISE}`}, - {`echo "my favourite mountain is cotopaxi" | grep 'xi$$'`, `echo "my favourite mountain is cotopaxi" | grep 'xi$'`}, + { + input: "Do this $$ESCAPE_PARTY", + want: "Do this $ESCAPE_PARTY", + }, + { + input: `Do this \$ESCAPE_PARTY`, + want: "Do this $ESCAPE_PARTY", + }, + { + input: "Do this $${SUCH_ESCAPE}", + want: "Do this ${SUCH_ESCAPE}", + }, + { + input: `Do this \${SUCH_ESCAPE}`, + want: "Do this ${SUCH_ESCAPE}", + }, + { + input: "Do this $${SUCH_ESCAPE:-$OTHERWISE}", + want: "Do this ${SUCH_ESCAPE:-}", + }, + { + input: `Do this \${SUCH_ESCAPE:-$OTHERWISE}`, + want: "Do this ${SUCH_ESCAPE:-}", + }, + { + input: "Do this $${SUCH_ESCAPE:-$$OTHERWISE}", + want: "Do this ${SUCH_ESCAPE:-$OTHERWISE}", + }, + { + input: `Do this \${SUCH_ESCAPE:-\$OTHERWISE}`, + want: "Do this ${SUCH_ESCAPE:-$OTHERWISE}", + }, + { + input: `echo "my favourite mountain is cotopaxi" | grep 'xi$$'`, + want: `echo "my favourite mountain is cotopaxi" | grep 'xi$'`, + }, } { - result, err := interpolate.Interpolate(nil, tc.Str) + result, err := interpolate.Interpolate(nil, tc.input) if err != nil { - t.Fatal(err) + t.Errorf("interpolate.Interpolate(nil, %q) error = %v", tc.input, err) } - if result != tc.Expected { - t.Fatalf("Test %q failed: Expected substring %q, got %q", tc.Str, tc.Expected, result) + if result != tc.want { + t.Errorf("interpolate.Interpolate(nil, %q) = %q, want %q", tc.input, tc.want, result) } } } diff --git a/parser.go b/parser.go index e02cc7d..b68c09e 100644 --- a/parser.go +++ b/parser.go @@ -87,7 +87,7 @@ func (p *Parser) parseExpression(stop ...rune) (Expression, error) { return nil, err } - expr = append(expr, ee) + expr = append(expr, ExpressionItem{Expansion: ee}) continue } @@ -122,28 +122,39 @@ func (p *Parser) parseExpression(stop ...rune) (Expression, error) { return expr, nil } -func (p *Parser) parseEscapedExpansion() (ExpressionItem, error) { +// parseEscapedExpansion attempts to extract a *potential* identifier or brace +// expression from the text following the escaped dollarsign. +func (p *Parser) parseEscapedExpansion() (EscapedExpansion, error) { + // Since it's not an expansion, we should treat the following text as text. + start := p.pos + defer func() { p.pos = start }() + next := p.peekRune() switch { case next == '{': - // if it's an escaped brace expansion, (eg $${MY_COOL_VAR:-5}) consume text until the close brace - id := p.scanUntil(func(r rune) bool { return r == '}' }) - id = id + string(p.nextRune()) // we know that the next rune is a close brace, chuck it on the end - return ExpressionItem{Expansion: EscapedExpansion{Identifier: id}}, nil + // it *could be* an escaped brace expansion + if _, err := p.parseBraceExpansion(); err != nil { + return EscapedExpansion{}, nil + } + // it was! instead of storing the expansion itself, store the string + // that produced it. + return EscapedExpansion{PotentialIdentifier: p.input[start:p.pos]}, nil case unicode.IsLetter(next): - // it's an escaped identifier (eg $$MY_COOL_VAR) + // it *could be* an escaped identifier (eg $$MY_COOL_VAR) id, err := p.scanIdentifier() if err != nil { - return ExpressionItem{}, err + // this should never happen, since scanIdentifier only errors if the + // first rune is not a letter, and we just checked that. + return EscapedExpansion{}, nil } - return ExpressionItem{Expansion: EscapedExpansion{Identifier: id}}, nil + return EscapedExpansion{PotentialIdentifier: id}, nil default: - // there's no identifier or brace afterward, so it's probably a literal escaped dollar sign - // just return a text item with the dollar sign - return ExpressionItem{Text: "$"}, nil + // there's no identifier or brace afterward, so it's probably a literal + // escaped dollar sign + return EscapedExpansion{}, nil } } @@ -162,7 +173,9 @@ func (p *Parser) parseExpansion() (Expansion, error) { return nil, err } - return VariableExpansion{Identifier: identifier}, nil + return VariableExpansion{ + Identifier: identifier, + }, nil } func (p *Parser) parseBraceExpansion() (Expansion, error) { @@ -177,7 +190,9 @@ func (p *Parser) parseBraceExpansion() (Expansion, error) { if c := p.peekRune(); c == '}' { _ = p.nextRune() - return VariableExpansion{Identifier: identifier}, nil + return VariableExpansion{ + Identifier: identifier, + }, nil } var operator string @@ -298,8 +313,8 @@ func (p *Parser) scanIdentifier() (string, error) { if c := p.peekRune(); !unicode.IsLetter(c) { return "", fmt.Errorf("Expected identifier to start with a letter, got %c", c) } - var notIdentifierChar = func(r rune) bool { - return (!unicode.IsLetter(r) && !unicode.IsNumber(r) && r != '_') + notIdentifierChar := func(r rune) bool { + return !(unicode.IsLetter(r) || unicode.IsNumber(r) || r == '_') } return p.scanUntil(notIdentifierChar), nil } diff --git a/parser_test.go b/parser_test.go index 466a37a..1a89c0b 100644 --- a/parser_test.go +++ b/parser_test.go @@ -1,124 +1,129 @@ -package interpolate_test +package interpolate import ( - "reflect" "testing" - "github.com/buildkite/interpolate" + "github.com/google/go-cmp/cmp" ) func TestParser(t *testing.T) { t.Parallel() - var testCases = []struct { - String string - Expected []interpolate.ExpressionItem + testCases := []struct { + input string + want Expression }{ { - String: `Buildkite... ${HELLO_WORLD} ${ANOTHER_VAR:-🏖}`, - Expected: []interpolate.ExpressionItem{ + input: "Buildkite... ${HELLO_WORLD} ${ANOTHER_VAR:-🏖}", + want: Expression{ {Text: "Buildkite... "}, - {Expansion: interpolate.VariableExpansion{ + {Expansion: VariableExpansion{ Identifier: "HELLO_WORLD", }}, {Text: " "}, - {Expansion: interpolate.EmptyValueExpansion{ + {Expansion: EmptyValueExpansion{ Identifier: "ANOTHER_VAR", - Content: interpolate.Expression([]interpolate.ExpressionItem{{ + Content: Expression{{ Text: "🏖", - }}), + }}, }}, }, }, { - String: `${TEST1:- ${TEST2:-$TEST3}}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.EmptyValueExpansion{ + input: "${TEST1:- ${TEST2:-$TEST3}}", + want: Expression{ + {Expansion: EmptyValueExpansion{ Identifier: "TEST1", - Content: interpolate.Expression([]interpolate.ExpressionItem{ + Content: Expression{ {Text: " "}, - {Expansion: interpolate.EmptyValueExpansion{ + {Expansion: EmptyValueExpansion{ Identifier: "TEST2", - Content: interpolate.Expression([]interpolate.ExpressionItem{ - {Expansion: interpolate.VariableExpansion{ + Content: Expression{ + {Expansion: VariableExpansion{ Identifier: "TEST3", }}, - }), + }, }}, - }), + }, }}, }, }, { - String: `${HELLO_WORLD-blah}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.UnsetValueExpansion{ + input: "${HELLO_WORLD-blah}", + want: Expression{ + {Expansion: UnsetValueExpansion{ Identifier: "HELLO_WORLD", - Content: interpolate.Expression([]interpolate.ExpressionItem{{ + Content: Expression{{ Text: "blah", - }}), + }}, }}, }, }, { - String: `\\${HELLO_WORLD-blah}`, - Expected: []interpolate.ExpressionItem{ + input: `\\${HELLO_WORLD-blah}`, + want: Expression{ {Text: `\\`}, - {Expansion: interpolate.UnsetValueExpansion{ + {Expansion: UnsetValueExpansion{ Identifier: "HELLO_WORLD", - Content: interpolate.Expression([]interpolate.ExpressionItem{{ + Content: Expression{{ Text: "blah", - }}), + }}, }}, }, }, { - String: `\${HELLO_WORLD-blah}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.EscapedExpansion{Identifier: "{HELLO_WORLD-blah}"}}, + input: `\${HELLO_WORLD-blah}`, + want: Expression{ + {Expansion: EscapedExpansion{ + PotentialIdentifier: "{HELLO_WORLD-blah}", + }}, + {Text: "{HELLO_WORLD-blah}"}, }, }, { - String: `Test \\\${HELLO_WORLD-blah}`, - Expected: []interpolate.ExpressionItem{ - {Text: `Test `}, + input: `Test \\\${HELLO_WORLD-blah}`, + want: Expression{ + {Text: "Test "}, {Text: `\\`}, - {Expansion: interpolate.EscapedExpansion{Identifier: "{HELLO_WORLD-blah}"}}, + {Expansion: EscapedExpansion{ + PotentialIdentifier: "{HELLO_WORLD-blah}", + }}, + {Text: "{HELLO_WORLD-blah}"}, }, }, { - String: `${HELLO_WORLD:1}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.SubstringExpansion{ + input: `${HELLO_WORLD:1}`, + want: Expression{ + {Expansion: SubstringExpansion{ Identifier: "HELLO_WORLD", Offset: 1, }}, }, }, { - String: `${HELLO_WORLD: -1}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.SubstringExpansion{ + input: "${HELLO_WORLD: -1}", + want: Expression{ + {Expansion: SubstringExpansion{ Identifier: "HELLO_WORLD", Offset: -1, }}, }, }, { - String: `${HELLO_WORLD:-1}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.EmptyValueExpansion{ + input: `${HELLO_WORLD:-1}`, + want: Expression{ + {Expansion: EmptyValueExpansion{ Identifier: "HELLO_WORLD", - Content: interpolate.Expression([]interpolate.ExpressionItem{{ + Content: Expression{{ Text: "1", - }}), + }}, }}, }, }, { - String: `${HELLO_WORLD:1:7}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.SubstringExpansion{ + input: `${HELLO_WORLD:1:7}`, + want: Expression{ + {Expansion: SubstringExpansion{ Identifier: "HELLO_WORLD", Offset: 1, Length: 7, @@ -127,9 +132,9 @@ func TestParser(t *testing.T) { }, }, { - String: `${HELLO_WORLD:1:-7}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.SubstringExpansion{ + input: `${HELLO_WORLD:1:-7}`, + want: Expression{ + {Expansion: SubstringExpansion{ Identifier: "HELLO_WORLD", Offset: 1, Length: -7, @@ -138,65 +143,78 @@ func TestParser(t *testing.T) { }, }, { - String: `${HELLO_WORLD?Required}`, - Expected: []interpolate.ExpressionItem{ - {Expansion: interpolate.RequiredExpansion{ + input: `${HELLO_WORLD?Required}`, + want: Expression{ + {Expansion: RequiredExpansion{ Identifier: "HELLO_WORLD", - Message: interpolate.Expression([]interpolate.ExpressionItem{ + Message: Expression{ {Text: "Required"}, - }), + }, }}, }, }, { - String: `$`, - Expected: []interpolate.ExpressionItem{ - {Text: `$`}, + input: `$${not actually a brace expression`, + want: Expression{ + {Expansion: EscapedExpansion{}}, + {Text: "{not actually a brace expression"}, + }, + }, + { + input: "$", + want: Expression{ + {Text: "$"}, }, }, { - String: `\`, - Expected: []interpolate.ExpressionItem{ + input: `\`, + want: Expression{ {Text: `\`}, }, }, { - String: `$(echo hello world)`, - Expected: []interpolate.ExpressionItem{ - {Text: `$(`}, - {Text: `echo hello world)`}, + input: "$(echo hello world)", + want: Expression{ + {Text: "$("}, + {Text: "echo hello world)"}, }, }, { - String: "$$MOUNTAIN", - Expected: []interpolate.ExpressionItem{{Expansion: interpolate.EscapedExpansion{Identifier: "MOUNTAIN"}}}, + input: "$$MOUNTAIN", + want: Expression{ + {Expansion: EscapedExpansion{PotentialIdentifier: "MOUNTAIN"}}, + {Text: "MOUNTAIN"}, + }, }, { - String: "this is a regex! /^start.*end$$/", // the dollar sign at the end of the regex has to be escaped to be treated as a literal dollar sign by this library - Expected: []interpolate.ExpressionItem{ - {Text: "this is a regex! /^start.*end"}, {Text: "$"}, {Text: "/"}, + input: "this is a regex! /^start.*end$$/", // the dollar sign at the end of the regex has to be escaped to be treated as a literal dollar sign by this library + want: Expression{ + {Text: "this is a regex! /^start.*end"}, + {Expansion: EscapedExpansion{}}, + {Text: "/"}, }, }, { - String: `this is a more different regex! /^start.*end\$/`, // the dollar sign at the end of the regex has to be escaped to be treated as a literal dollar sign by this library - Expected: []interpolate.ExpressionItem{ - {Text: "this is a more different regex! /^start.*end"}, {Text: "$"}, {Text: "/"}, + input: `this is a more different regex! /^start.*end\$/`, // the dollar sign at the end of the regex has to be escaped to be treated as a literal dollar sign by this library + want: Expression{ + {Text: "this is a more different regex! /^start.*end"}, + {Expansion: EscapedExpansion{}}, + {Text: "/"}, }, }, } for _, tc := range testCases { - t.Run(tc.String, func(t *testing.T) { + t.Run(tc.input, func(t *testing.T) { t.Parallel() - actual, err := interpolate.NewParser(tc.String).Parse() + got, err := NewParser(tc.input).Parse() if err != nil { - t.Fatal(err) + t.Fatalf("NewParser(%q).Parse() error = %v", tc.input, err) } - expected := interpolate.Expression(tc.Expected) - if !reflect.DeepEqual(expected, actual) { - t.Fatalf("Expected vs Actual: \n%s\n\n%s", expected, actual) + if diff := cmp.Diff(got, tc.want); diff != "" { + t.Errorf("parsed expression diff (-got +want):\n%s", diff) } }) }