Refactor and add further unit tests

Author: stilvoid

formatters/bash.go

@@ -2,7 +2,6 @@ package formatters
 
 import (
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/stilvoid/please/util"
@@ -25,25 +24,14 @@ func formatBashInternal(in interface{}) string {
 		return ""
 	}
 
-	val := reflect.ValueOf(in)
+	switch v := in.(type) {
+	case map[string]interface{}:
+		keys := util.SortedKeys(v)
 
-	switch val.Kind() {
-	case reflect.Map:
-		parts := make([]string, val.Len())
+		parts := make([]string, len(v))
 
-		for i, key := range val.MapKeys() {
-			value := val.MapIndex(key).Interface()
-			parts[i] = fmt.Sprintf("[%s]=%s", key.String(), wrapObj(value))
-		}
-
-		return fmt.Sprintf("(%s)", strings.Join(parts, " "))
-	case reflect.Array, reflect.Slice:
-		parts := make([]string, val.Len())
-
-		for i := 0; i < val.Len(); i++ {
-			value := val.Index(i).Interface()
-
-			parts[i] = fmt.Sprintf("[%d]=%s", i, wrapObj(value))
+		for i, key := range keys {
+			parts[i] = fmt.Sprintf("[%s]=%s", key, wrapObj(v[key.(string)]))
 		}
 
 		return fmt.Sprintf("(%s)", strings.Join(parts, " "))
@@ -53,6 +41,7 @@ func formatBashInternal(in interface{}) string {
 }
 
 func formatBash(in interface{}) (string, error) {
+	in = util.ArraysToMaps(in)
 	in = util.ForceStringKeys(in)
 
 	return formatBashInternal(in), nil

formatters/bash_test.go

@@ -0,0 +1,37 @@
+package formatters
+
+import "testing"
+
+func TestBash(t *testing.T) {
+	expecteds := []string{
+		"123",
+		"456.789",
+		"abc",
+		"true",
+		"false",
+		"",
+		"([0]=\"123\" [1]=\"abc\")",
+		"([foo]=\"bar\")",
+		"([123]=\"([0]=\\\"baz\\\" [1]=\\\"quux\\\")\")",
+		"([true]=\"([null]=\\\"\\\")\")",
+		"([0]=\"456\" [1]=\"def\" [2]=\"([3]=\\\"4\\\")\")",
+	}
+
+	if len(expecteds) != len(testCases) {
+		t.Fatalf("insufficient test cases implemented")
+	}
+
+	for i, expected := range expecteds {
+		testCase := testCases[i]
+
+		actual, err := formatBash(testCase)
+
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+
+		if actual != expected {
+			t.Errorf("unexpected '%v', want '%v'", actual, expected)
+		}
+	}
+}

formatters/query.go

@@ -3,44 +3,42 @@ package formatters
 import (
 	"fmt"
 	"net/url"
-	"reflect"
 
 	"github.com/stilvoid/please/util"
 )
 
-// TODO: Tidy this up; it's horrid.
-func formatQuery(in interface{}) (string, error) {
-	in = util.ForceStringKeys(in)
+func formatQueryInternal(in interface{}) string {
+	if in == nil {
+		return ""
+	}
 
 	inMap, ok := in.(map[string]interface{})
 
 	if !ok {
-		return "", fmt.Errorf("query formatter expects a map as input")
+		return fmt.Sprint(in)
 	}
 
 	var output url.Values = make(map[string][]string)
 
 	for key, value := range inMap {
-		val := reflect.ValueOf(value)
-
-		switch val.Kind() {
-		case reflect.Map:
-			return "", fmt.Errorf("query formatter cannot deal with nested values")
-		case reflect.Array, reflect.Slice:
-			for i := 0; i < val.Len(); i++ {
-				iVal := val.Index(i)
-
-				switch iVal.Kind() {
-				case reflect.Map, reflect.Array, reflect.Slice:
-					return "", fmt.Errorf("query formatter cannot deal with nested values")
-				default:
-					output.Add(key, fmt.Sprint(iVal.Interface()))
-				}
-			}
+		switch value.(type) {
+		case map[string]interface{}:
+			result := formatQueryInternal(value)
+
+			output.Add(key, result)
+		case nil:
+			output.Add(key, "")
 		default:
 			output.Add(key, fmt.Sprint(value))
 		}
 	}
 
-	return output.Encode(), nil
+	return output.Encode()
+}
+
+func formatQuery(in interface{}) (string, error) {
+	in = util.ArraysToMaps(in)
+	in = util.ForceStringKeys(in)
+
+	return formatQueryInternal(in), nil
 }

formatters/query_test.go

@@ -1,48 +1,36 @@
 package formatters
 
-import (
-	"reflect"
-	"testing"
-)
+import "testing"
 
 func TestQuery(t *testing.T) {
-	inputs := []interface{}{
-		map[string]string{
-			"foo": "bar",
-		},
-		map[string][]string{
-			"foo": {"bar", "baz"},
-		},
-		map[string]int{
-			"foo": 123,
-		},
-		map[int]int{
-			123: 456,
-		},
-		map[string]string{
-			"foo": "bar",
-			"baz": "quux",
-		},
-	}
-
 	expecteds := []string{
+		"123",
+		"456.789",
+		"abc",
+		"true",
+		"false",
+		"",
+		"0=123&1=abc",
 		"foo=bar",
-		"foo=bar&foo=baz",
-		"foo=123",
-		"123=456",
-		"baz=quux&foo=bar",
+		"123=0%3Dbaz%261%3Dquux",
+		"true=null%3D",
+		"0=456&1=def&2=3%3D4",
+	}
+
+	if len(expecteds) != len(testCases) {
+		t.Fatalf("insufficient test cases implemented")
 	}
 
-	for i, input := range inputs {
-		actual, err := formatQuery(input)
+	for i, expected := range expecteds {
+		testCase := testCases[i]
+
+		actual, err := formatQuery(testCase)
 
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
 
-		expected := expecteds[i]
-
-		if !reflect.DeepEqual(actual, expected) {
+		if actual != expected {
 			t.Errorf("unexpected '%v', want '%v'", actual, expected)
 		}
 	}

formatters/xml_test.go

@@ -18,7 +18,7 @@ func TestXML(t *testing.T) {
 	}
 
 	if len(expecteds) != len(testCases) {
-		//t.Fatalf("insufficient test cases implemented")
+		t.Fatalf("insufficient test cases implemented")
 	}
 
 	for i, expected := range expecteds {

util/map.go

@@ -3,27 +3,32 @@ package util
 import (
 	"fmt"
 	"reflect"
+	"sort"
 )
 
+// toString wraps fmt.Sprint except that it converts nil to "null"
+func toString(in interface{}) string {
+	if in == nil {
+		return "null"
+	}
+
+	return fmt.Sprint(in)
+}
+
 // ForceStringKeys creates a copy of the provided interface{}, with all maps changed to have string keys for use by serialisers that expect string keys
-// This is particularly useful for formatters where the target serialisation format only allows string keys
+// This is useful for formatters where the target serialisation format only allows string keys
 func ForceStringKeys(in interface{}) interface{} {
 	val := reflect.ValueOf(in)
 
 	switch val.Kind() {
 	case reflect.Map:
 		newMap := make(map[string]interface{}, val.Len())
 
-		var stringKey string
-
-		for _, key := range val.MapKeys() {
-			if reflect.TypeOf(key.Interface()) == nil {
-				stringKey = "null"
-			} else {
-				stringKey = fmt.Sprint(key.Interface())
-			}
+		for _, keyVal := range val.MapKeys() {
+			key := toString(keyVal.Interface())
+			value := val.MapIndex(keyVal).Interface()
 
-			newMap[stringKey] = ForceStringKeys(val.MapIndex(key).Interface())
+			newMap[key] = ForceStringKeys(value)
 		}
 
 		return newMap
@@ -40,3 +45,63 @@ func ForceStringKeys(in interface{}) interface{} {
 		return in
 	}
 }
+
+// ArraysToMaps creates a copy of the provided interface{}, with all arrays converted into maps where the keys are the array indices, starting at 0.
+// This is useful for formatters where the target serialisation format does not have a means of representing arrays
+func ArraysToMaps(in interface{}) interface{} {
+	val := reflect.ValueOf(in)
+
+	switch val.Kind() {
+	case reflect.Map:
+		newMap := make(map[interface{}]interface{}, val.Len())
+
+		for _, key := range val.MapKeys() {
+			value := val.MapIndex(key).Interface()
+
+			newMap[key.Interface()] = ArraysToMaps(value)
+		}
+
+		return newMap
+	case reflect.Array, reflect.Slice:
+		newMap := make(map[interface{}]interface{}, val.Len())
+
+		for i := 0; i < val.Len(); i++ {
+			value := val.Index(i).Interface()
+
+			newMap[interface{}(i)] = ArraysToMaps(value)
+		}
+
+		return newMap
+	default:
+		return in
+	}
+}
+
+func SortedKeys(in interface{}) []interface{} {
+	val := reflect.ValueOf(in)
+
+	if val.Kind() != reflect.Map {
+		panic("SortedKeys only works on maps")
+	}
+
+	stringKeys := make([]string, val.Len())
+	stringKeysMap := make(map[string]interface{}, val.Len())
+
+	for i, key := range val.MapKeys() {
+		stringKey := toString(key.Interface())
+
+		stringKeys[i] = stringKey
+
+		stringKeysMap[stringKey] = key.Interface()
+	}
+
+	sort.Strings(stringKeys)
+
+	outKeys := make([]interface{}, val.Len())
+
+	for i, key := range stringKeys {
+		outKeys[i] = stringKeysMap[key]
+	}
+
+	return outKeys
+}