Skip to content

Commit

Permalink
core: allow distinguishing between empty lists and strings
Browse files Browse the repository at this point in the history
Had to handle a lot of implicit leaning on a few properties of the old
representation:

 * Old representation allowed plain strings to be treated as lists
   without problem (i.e. shoved into strings.Split), now strings need to
   be checked whether they are a list before they are treated as one
   (i.e. shoved into StringList(s).Slice()).
 * Tested behavior of 0 and 1 length lists in formatlist() was a side
   effect of the representation. Needs to be special cased now to
   maintain the behavior.
 * Found a pretty old context test failure that was wrong in several
   different ways. It's covered by TestContext2Apply_multiVar so I
   removed it.
  • Loading branch information
phinze committed Jun 25, 2015
1 parent 7238b3b commit e88aeed
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 70 deletions.
22 changes: 19 additions & 3 deletions config/interpolate_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ func interpolationFuncConcat() ast.Function {

if IsStringList(argument) {
isDeprecated = false
finalList = append(finalList, StringList(argument).Slice()...)
} else {
finalList = append(finalList, argument)
}

finalList = append(finalList, argument)

// Deprecated concat behaviour
b.WriteString(argument)
}
Expand Down Expand Up @@ -131,11 +132,26 @@ func interpolationFuncFormatList() ast.Function {
if !ok {
continue
}
if !IsStringList(s) {
continue
}

parts := StringList(s).Slice()
if len(parts) <= 1 {

// 0 or 1 length lists are treated as scalars and repeated
switch len(parts) {
case 0:
varargs[i-1] = ""
continue
case 1:
varargs[i-1] = parts[0]
continue
}

// otherwise the list is sent down to be indexed
varargs[i-1] = parts

// Check length
if n == 0 {
// first list we've seen
n = len(parts)
Expand Down
14 changes: 11 additions & 3 deletions config/interpolate_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ func TestInterpolateFuncJoin(t *testing.T) {
},

{
`${join(",", "foo")}`,
fmt.Sprintf(`${join(",", "%s")}`,
NewStringList([]string{"foo"}).String()),
"foo",
false,
},
Expand Down Expand Up @@ -354,9 +355,15 @@ func TestInterpolateFuncSplit(t *testing.T) {
true,
},

{
`${split(",", "")}`,
NewStringList([]string{""}).String(),
false,
},

{
`${split(",", "foo")}`,
"foo",
NewStringList([]string{"foo"}).String(),
false,
},

Expand Down Expand Up @@ -524,7 +531,8 @@ func TestInterpolateFuncElement(t *testing.T) {
},

{
`${element("foo", "0")}`,
fmt.Sprintf(`${element("%s", "0")}`,
NewStringList([]string{"foo"}).String()),
"foo",
false,
},
Expand Down
13 changes: 9 additions & 4 deletions config/interpolate_walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error {
// set if it is computed. This behavior is different if we're
// splitting (in a SliceElem) or not.
remove := false
if w.loc == reflectwalk.SliceElem {
if w.loc == reflectwalk.SliceElem && IsStringList(replaceVal) {
parts := StringList(replaceVal).Slice()
for _, p := range parts {
if p == UnknownVariableValue {
Expand Down Expand Up @@ -265,10 +265,15 @@ func (w *interpolationWalker) splitSlice() {
continue
}

// Split on the delimiter
for _, p := range StringList(sv).Slice() {
result = append(result, p)
if IsStringList(sv) {
for _, p := range StringList(sv).Slice() {
result = append(result, p)
}
continue
}

// Not a string list, so just set it
result = append(result, sv)
}

// Our slice is now done, we have to replace the slice now
Expand Down
2 changes: 1 addition & 1 deletion config/interpolate_walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestInterpolationWalker_replace(t *testing.T) {
}

if !reflect.DeepEqual(tc.Input, tc.Output) {
t.Fatalf("%d: bad:\n\n%#v", i, tc.Input)
t.Fatalf("%d: bad:\n\nexpected:%#v\ngot:%#v", i, tc.Output, tc.Input)
}
}
}
65 changes: 38 additions & 27 deletions config/string_list.go
Original file line number Diff line number Diff line change
@@ -1,37 +1,48 @@
package config

import "strings"
import (
"fmt"
"strings"
)

// StringList represents the "poor man's list" that terraform uses
// internally
type StringList string

// This is the delimiter used to recognize and split StringLists
const StringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6`
//
// It plays two semantic roles:
// * It introduces a list
// * It terminates each element
//
// Example representations:
// [] => SLD
// [""] => SLDSLD
// [" "] => SLD SLD
// ["foo"] => SLDfooSLD
// ["foo", "bar"] => SLDfooSLDbarSLD
// ["", ""] => SLDSLDSLD
const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6`

// Build a StringList from a slice
func NewStringList(parts []string) StringList {
// FOR NOW:
return StringList(strings.Join(parts, StringListDelim))
// EVENTUALLY:
// var sl StringList
// for _, p := range parts {
// sl = sl.Append(p)
// }
// return sl
}

// Returns a new StringList with the item appended
func (sl StringList) Append(s string) StringList {
// FOR NOW:
return StringList(strings.Join(append(sl.Slice(), s), StringListDelim))
// EVENTUALLY:
// return StringList(fmt.Sprintf("%s%s%s", sl, s, StringListDelim))
// We have to special case the empty list representation
if len(parts) == 0 {
return StringList(stringListDelim)
}
return StringList(fmt.Sprintf("%s%s%s",
stringListDelim,
strings.Join(parts, stringListDelim),
stringListDelim,
))
}

// Returns an element at the index, wrapping around the length of the string
// when index > list length
func (sl StringList) Element(index int) string {
if sl.Length() == 0 {
return ""
}
return sl.Slice()[index%sl.Length()]
}

Expand All @@ -42,17 +53,17 @@ func (sl StringList) Length() int {

// Returns a slice of strings as represented by this StringList
func (sl StringList) Slice() []string {
parts := strings.Split(string(sl), StringListDelim)
parts := strings.Split(string(sl), stringListDelim)

// FOR NOW:
if sl.String() == "" {
switch len(parts) {
case 0, 1:
return []string{}
} else {
return parts
case 2:
return []string{""}
}
// EVENTUALLY:
// StringLists always have a trailing StringListDelim
// return parts[:len(parts)-1]

// strip empty elements generated by leading and trailing delimiters
return parts[1 : len(parts)-1]
}

func (sl StringList) String() string {
Expand All @@ -61,5 +72,5 @@ func (sl StringList) String() string {

// Determines if a given string represents a StringList
func IsStringList(s string) bool {
return strings.Contains(s, StringListDelim)
return strings.Contains(s, stringListDelim)
}
23 changes: 0 additions & 23 deletions terraform/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1672,29 +1672,6 @@ func TestContext2Plan_provider(t *testing.T) {
}
}

func TestContext2Plan_varMultiCountOne(t *testing.T) {
m := testModule(t, "plan-var-multi-count-one")
p := testProvider("aws")
p.DiffFn = testDiffFn
ctx := testContext2(t, &ContextOpts{
Module: m,
Providers: map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p),
},
})

plan, err := ctx.Plan()
if err != nil {
t.Fatalf("err: %s", err)
}

actual := strings.TrimSpace(plan.String())
expected := strings.TrimSpace(testTerraformPlanVarMultiCountOneStr)
if actual != expected {
t.Fatalf("bad:\n%s", actual)
}
}

func TestContext2Plan_varListErr(t *testing.T) {
m := testModule(t, "plan-var-list-err")
p := testProvider("aws")
Expand Down
9 changes: 0 additions & 9 deletions terraform/test-fixtures/plan-var-multi-count-one/main.tf

This file was deleted.

0 comments on commit e88aeed

Please sign in to comment.