From 7610874264b0c39c119c52d6d1821677ad6bd4f1 Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Tue, 15 Sep 2015 09:34:57 +0200 Subject: [PATCH 01/11] Initial implementation of compact() interpolation function --- config/interpolate_funcs.go | 17 +++++++++++++++++ config/string_list.go | 13 +++++++++++++ 2 files changed, 30 insertions(+) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index abaa9817c..67523ddb2 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -30,6 +30,7 @@ func init() { "length": interpolationFuncLength(), "replace": interpolationFuncReplace(), "split": interpolationFuncSplit(), + "compact": interpolationFuncCompact(), "base64encode": interpolationFuncBase64Encode(), "base64decode": interpolationFuncBase64Decode(), } @@ -279,6 +280,22 @@ func interpolationFuncSplit() ast.Function { } } +// interpolationFuncCompact strips a list of values (e.g. as returned by +// "split") of any empty strings +func interpolationFuncCompact() ast.Function { + return ast.Function{ + ArgTypes: []ast.Type{ast.TypeString}, + ReturnType: ast.TypeString, + Variadic: false, + Callback: func(args []interface{}) (interface{}, error) { + if !IsStringList(args[0].(string)) { + return args[0].(string), nil + } + return CompactStringList(StringList(args[0].(string))).String(), nil + }, + } +} + // interpolationFuncLookup implements the "lookup" function that allows // dynamic lookups of map types within a Terraform configuration. func interpolationFuncLookup(vs map[string]ast.Variable) ast.Function { diff --git a/config/string_list.go b/config/string_list.go index 70d43d1e4..d4d1850d0 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -74,3 +74,16 @@ func (sl StringList) String() string { func IsStringList(s string) bool { return strings.Contains(s, stringListDelim) } + +func CompactStringList(sl StringList) StringList { + parts := sl.Slice() + + var newlist []string + // drop the empty strings + for i := range parts { + if parts[i] != "" { + newlist = append(newlist, parts[i]) + } + } + return NewStringList(newlist) +} From 735803ef0995833bfd042d9c7c5832aa37cdcf18 Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Tue, 15 Sep 2015 15:50:31 +0200 Subject: [PATCH 02/11] Test cases for compact() --- config/interpolate_funcs_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index e4d275e58..e3564aee1 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -11,6 +11,34 @@ import ( "github.com/hashicorp/terraform/config/lang/ast" ) + +func TestInterpolateFuncCompact(t *testing.T) { + testFunction(t, testFunctionConfig{ + Cases: []testFunctionCase{ + // empty string within array + { + `${compact(split(",", "b,,c"))}`, + NewStringList([]string{"b", "c"}).String(), + false, + }, + + // empty string at the end of array + { + `${compact(split(",", "b,c,"))}`, + NewStringList([]string{"b", "c"}).String(), + false, + }, + + // single empty string + { + `${compact(split(",", ""))}`, + NewStringList([]string{}).String(), + false, + }, + }, + }) +} + func TestInterpolateFuncDeprecatedConcat(t *testing.T) { testFunction(t, testFunctionConfig{ Cases: []testFunctionCase{ From ef2b0a0b71f5e1c26acb1efdc3ae9098c5c6b48b Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Wed, 16 Sep 2015 11:08:58 +0200 Subject: [PATCH 03/11] Order functions alphabetically --- config/interpolate_funcs.go | 52 ++++++++++++++++++------------------- config/string_list.go | 27 +++++++++---------- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 67523ddb2..7cd7f7476 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -20,22 +20,38 @@ var Funcs map[string]ast.Function func init() { Funcs = map[string]ast.Function{ - "concat": interpolationFuncConcat(), - "element": interpolationFuncElement(), - "file": interpolationFuncFile(), - "format": interpolationFuncFormat(), - "formatlist": interpolationFuncFormatList(), - "index": interpolationFuncIndex(), - "join": interpolationFuncJoin(), - "length": interpolationFuncLength(), - "replace": interpolationFuncReplace(), - "split": interpolationFuncSplit(), "compact": interpolationFuncCompact(), + "concat": interpolationFuncConcat(), + "element": interpolationFuncElement(), + "file": interpolationFuncFile(), + "format": interpolationFuncFormat(), + "formatlist": interpolationFuncFormatList(), + "index": interpolationFuncIndex(), + "join": interpolationFuncJoin(), + "length": interpolationFuncLength(), + "replace": interpolationFuncReplace(), + "split": interpolationFuncSplit(), "base64encode": interpolationFuncBase64Encode(), "base64decode": interpolationFuncBase64Decode(), } } +// interpolationFuncCompact strips a list of multi-variable values +// (e.g. as returned by "split") of any empty strings. +func interpolationFuncCompact() ast.Function { + return ast.Function{ + ArgTypes: []ast.Type{ast.TypeString}, + ReturnType: ast.TypeString, + Variadic: false, + Callback: func(args []interface{}) (interface{}, error) { + if !IsStringList(args[0].(string)) { + return args[0].(string), nil + } + return CompactStringList(StringList(args[0].(string))).String(), nil + }, + } +} + // interpolationFuncConcat implements the "concat" function that // concatenates multiple strings. This isn't actually necessary anymore // since our language supports string concat natively, but for backwards @@ -280,22 +296,6 @@ func interpolationFuncSplit() ast.Function { } } -// interpolationFuncCompact strips a list of values (e.g. as returned by -// "split") of any empty strings -func interpolationFuncCompact() ast.Function { - return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString}, - ReturnType: ast.TypeString, - Variadic: false, - Callback: func(args []interface{}) (interface{}, error) { - if !IsStringList(args[0].(string)) { - return args[0].(string), nil - } - return CompactStringList(StringList(args[0].(string))).String(), nil - }, - } -} - // interpolationFuncLookup implements the "lookup" function that allows // dynamic lookups of map types within a Terraform configuration. func interpolationFuncLookup(vs map[string]ast.Variable) ast.Function { diff --git a/config/string_list.go b/config/string_list.go index d4d1850d0..d68fadfd0 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -24,6 +24,20 @@ type StringList string // ["", ""] => SLDSLDSLD const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` +// Takes a Stringlist and returns one without empty strings in it +func (sl StringList) CompactStringList() StringList { + parts := sl.Slice() + + var newlist []string + // drop the empty strings + for i := range parts { + if parts[i] != "" { + newlist = append(newlist, parts[i]) + } + } + return NewStringList(newlist) +} + // Build a StringList from a slice func NewStringList(parts []string) StringList { // We have to special case the empty list representation @@ -74,16 +88,3 @@ func (sl StringList) String() string { func IsStringList(s string) bool { return strings.Contains(s, stringListDelim) } - -func CompactStringList(sl StringList) StringList { - parts := sl.Slice() - - var newlist []string - // drop the empty strings - for i := range parts { - if parts[i] != "" { - newlist = append(newlist, parts[i]) - } - } - return NewStringList(newlist) -} From 95b2a60b29dabf13830b727d46df6dd2c09bafc8 Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Wed, 16 Sep 2015 11:09:34 +0200 Subject: [PATCH 04/11] Use {a,b} instead of {b,c} How does the alphabet even? --- config/interpolate_funcs_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index e3564aee1..8a33169a6 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -17,15 +17,15 @@ func TestInterpolateFuncCompact(t *testing.T) { Cases: []testFunctionCase{ // empty string within array { - `${compact(split(",", "b,,c"))}`, - NewStringList([]string{"b", "c"}).String(), + `${compact(split(",", "a,,b"))}`, + NewStringList([]string{"a", "b"}).String(), false, }, // empty string at the end of array { - `${compact(split(",", "b,c,"))}`, - NewStringList([]string{"b", "c"}).String(), + `${compact(split(",", "a,b,"))}`, + NewStringList([]string{"a", "b"}).String(), false, }, From aed3f98703335bbfe2e17a17757a54765ac9ce6d Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Wed, 16 Sep 2015 11:23:43 +0200 Subject: [PATCH 05/11] Rename func which is now a method. --- config/interpolate_funcs.go | 2 +- config/string_list.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 7cd7f7476..992171914 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -47,7 +47,7 @@ func interpolationFuncCompact() ast.Function { if !IsStringList(args[0].(string)) { return args[0].(string), nil } - return CompactStringList(StringList(args[0].(string))).String(), nil + return StringList(args[0].(string)).Compact().String(), nil }, } } diff --git a/config/string_list.go b/config/string_list.go index d68fadfd0..5ed9336ab 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -25,7 +25,7 @@ type StringList string const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` // Takes a Stringlist and returns one without empty strings in it -func (sl StringList) CompactStringList() StringList { +func (sl StringList) Compact() StringList { parts := sl.Slice() var newlist []string From f2f4ded97047360ae07521e0201d7736907af3d8 Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Thu, 8 Oct 2015 16:49:41 +0200 Subject: [PATCH 06/11] Initialize list as an empty slice --- config/string_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/string_list.go b/config/string_list.go index 5ed9336ab..a175b821d 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -28,7 +28,7 @@ const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` func (sl StringList) Compact() StringList { parts := sl.Slice() - var newlist []string + newlist := []string{} // drop the empty strings for i := range parts { if parts[i] != "" { From 53f44878ff118f4b59f2381731d4c07da09aed41 Mon Sep 17 00:00:00 2001 From: Svend Sorensen Date: Wed, 7 Oct 2015 13:11:54 -0700 Subject: [PATCH 07/11] Add tests for empty string lists --- config/string_list_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/config/string_list_test.go b/config/string_list_test.go index 64049eb50..3fe57dfe2 100644 --- a/config/string_list_test.go +++ b/config/string_list_test.go @@ -27,3 +27,26 @@ func TestStringList_element(t *testing.T) { list, expected, actual) } } + +func TestStringList_empty_slice(t *testing.T) { + expected := []string{} + l := NewStringList(expected) + actual := l.Slice() + + if !reflect.DeepEqual(expected, actual) { + t.Fatalf("Expected %q, got %q", expected, actual) + } +} + +func TestStringList_empty_slice_length(t *testing.T) { + list := []string{} + l := NewStringList([]string{}) + actual := l.Length() + + expected := 0 + + if actual != expected { + t.Fatalf("Expected length of %q to be %d, got %d", + list, expected, actual) + } +} From 8e4a313f17a22089b53f38437069718469612997 Mon Sep 17 00:00:00 2001 From: Svend Sorensen Date: Wed, 7 Oct 2015 13:28:46 -0700 Subject: [PATCH 08/11] Return an empty slice for empty string lists --- config/string_list.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/string_list.go b/config/string_list.go index a175b821d..da45df736 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -69,11 +69,11 @@ func (sl StringList) Length() int { func (sl StringList) Slice() []string { parts := strings.Split(string(sl), stringListDelim) + // split on an empty StringList will have a length of 2, since there is + // always at least one deliminator switch len(parts) { - case 0, 1: + case 0, 1, 2: return []string{} - case 2: - return []string{""} } // strip empty elements generated by leading and trailing delimiters From 73b51698ada29342e69086eba86e643f2e31bb1a Mon Sep 17 00:00:00 2001 From: Svend Sorensen Date: Wed, 7 Oct 2015 13:56:43 -0700 Subject: [PATCH 09/11] Replace simple case with if --- config/string_list.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/string_list.go b/config/string_list.go index da45df736..ba41d0256 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -71,8 +71,7 @@ func (sl StringList) Slice() []string { // split on an empty StringList will have a length of 2, since there is // always at least one deliminator - switch len(parts) { - case 0, 1, 2: + if len(parts) <= 2 { return []string{} } From 3040d8419fefac935d94450013f1bb7bfc08e741 Mon Sep 17 00:00:00 2001 From: Anthony Stanton Date: Thu, 8 Oct 2015 18:07:07 +0200 Subject: [PATCH 10/11] Test removing weird zero+zero Route53 test case --- terraform/interpolate_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index bbbb1024a..fbce848ea 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -330,11 +330,6 @@ func TestInterpolator_resourceMultiAttributesWithResourceCount(t *testing.T) { Value: config.NewStringList([]string{}).String(), Type: ast.TypeString, }) - // Zero + zero elements - testInterpolate(t, i, scope, "aws_route53_zone.terra.*.nothing", ast.Variable{ - Value: config.NewStringList([]string{"", ""}).String(), - Type: ast.TypeString, - }) // Zero + 1 element testInterpolate(t, i, scope, "aws_route53_zone.terra.*.special", ast.Variable{ Value: config.NewStringList([]string{"extra"}).String(), From 16b11e443d4984e010b85f4a7356c77fd9af7652 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 10 Oct 2015 15:17:25 -0700 Subject: [PATCH 11/11] go fmt the "compact" function changes. --- config/interpolate_funcs.go | 22 +++++++++++----------- config/interpolate_funcs_test.go | 1 - config/string_list.go | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 992171914..5322e46c4 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -20,17 +20,17 @@ var Funcs map[string]ast.Function func init() { Funcs = map[string]ast.Function{ - "compact": interpolationFuncCompact(), - "concat": interpolationFuncConcat(), - "element": interpolationFuncElement(), - "file": interpolationFuncFile(), - "format": interpolationFuncFormat(), - "formatlist": interpolationFuncFormatList(), - "index": interpolationFuncIndex(), - "join": interpolationFuncJoin(), - "length": interpolationFuncLength(), - "replace": interpolationFuncReplace(), - "split": interpolationFuncSplit(), + "compact": interpolationFuncCompact(), + "concat": interpolationFuncConcat(), + "element": interpolationFuncElement(), + "file": interpolationFuncFile(), + "format": interpolationFuncFormat(), + "formatlist": interpolationFuncFormatList(), + "index": interpolationFuncIndex(), + "join": interpolationFuncJoin(), + "length": interpolationFuncLength(), + "replace": interpolationFuncReplace(), + "split": interpolationFuncSplit(), "base64encode": interpolationFuncBase64Encode(), "base64decode": interpolationFuncBase64Decode(), } diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index 8a33169a6..561e1176b 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/terraform/config/lang/ast" ) - func TestInterpolateFuncCompact(t *testing.T) { testFunction(t, testFunctionConfig{ Cases: []testFunctionCase{ diff --git a/config/string_list.go b/config/string_list.go index ba41d0256..e3caea70b 100644 --- a/config/string_list.go +++ b/config/string_list.go @@ -28,11 +28,11 @@ const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` func (sl StringList) Compact() StringList { parts := sl.Slice() - newlist := []string{} + newlist := []string{} // drop the empty strings for i := range parts { if parts[i] != "" { - newlist = append(newlist, parts[i]) + newlist = append(newlist, parts[i]) } } return NewStringList(newlist)