Skip to content

Commit

Permalink
Fix staticcheck/gocyclo findings
Browse files Browse the repository at this point in the history
Introduce PathElement functions to determine their respective function,
for example whether they point to a map, or a simple list.

Use new helper functions in grab function to make the code easier to read.

Introduce grabByPath function to be used if a Path object is already available.

Fix error message that start with an upper-case character.

Use typed objects in switch .(type) constructs to avoid casting in cases.
  • Loading branch information
HeavyWombat committed May 20, 2019
1 parent 7a1e32d commit a4910b4
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 66 deletions.
4 changes: 2 additions & 2 deletions pkg/v1/ytbx/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ const (

// GetType returns the type of the input value with a YAML specific view
func GetType(value interface{}) string {
switch value.(type) {
switch tobj := value.(type) {
case yaml.MapSlice:
return typeMap

case []interface{}:
if IsComplexSlice(value.([]interface{})) {
if IsComplexSlice(tobj) {
return typeComplexList
}

Expand Down
36 changes: 16 additions & 20 deletions pkg/v1/ytbx/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,32 +39,29 @@ import (
// `github.com/virtuald/go-ordered-json` and will translate this structure into
// the compatible YAML structure.
func mapSlicify(obj interface{}) interface{} {
switch obj.(type) {
switch tobj := obj.(type) {
case ordered.OrderedObject:
orderedObj := obj.(ordered.OrderedObject)
result := make(yaml.MapSlice, 0, len(orderedObj))
for _, member := range orderedObj {
result := make(yaml.MapSlice, 0, len(tobj))
for _, member := range tobj {
result = append(result, yaml.MapItem{Key: member.Key, Value: mapSlicify(member.Value)})
}

return result

case map[string]interface{}:
return mapToYamlSlice(obj.(map[string]interface{}))
return mapToYamlSlice(tobj)

case []interface{}:
list := obj.([]interface{})
result := make([]interface{}, len(list))
for idx, entry := range list {
result := make([]interface{}, len(tobj))
for idx, entry := range tobj {
result[idx] = mapSlicify(entry)
}

return result

case []map[string]interface{}:
list := obj.([]map[string]interface{})
result := make([]yaml.MapSlice, len(list))
for idx, entry := range list {
result := make([]yaml.MapSlice, len(tobj))
for idx, entry := range tobj {
result[idx] = mapToYamlSlice(entry)
}

Expand Down Expand Up @@ -92,21 +89,20 @@ func mapToYamlSlice(input map[string]interface{}) yaml.MapSlice {
}

func castAsComplexList(obj interface{}) ([]yaml.MapSlice, bool) {
switch obj.(type) {
switch tobj := obj.(type) {
case []yaml.MapSlice:
return obj.([]yaml.MapSlice), true
return tobj, true

case []interface{}:
list := obj.([]interface{})
if IsComplexSlice(list) {
result := make([]yaml.MapSlice, len(list))
for idx, entry := range list {
switch entry.(type) {
if IsComplexSlice(tobj) {
result := make([]yaml.MapSlice, len(tobj))
for idx, entry := range tobj {
switch x := entry.(type) {
case yaml.MapSlice:
result[idx] = entry.(yaml.MapSlice)
result[idx] = x

case map[string]interface{}:
result[idx] = mapToYamlSlice(entry.(map[string]interface{}))
result[idx] = mapToYamlSlice(x)
}
}

Expand Down
14 changes: 9 additions & 5 deletions pkg/v1/ytbx/getting.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@ func Grab(obj interface{}, pathString string) (interface{}, error) {
return nil, err
}

pointer := obj
pointerPath := Path{DocumentIdx: path.DocumentIdx}
return grabByPath(obj, path)
}

func grabByPath(obj interface{}, path Path) (interface{}, error) {
pointer, pointerPath := obj, Path{DocumentIdx: path.DocumentIdx}

for _, element := range path.PathElements {
switch {
// Key/Value Map, where the element name is the key for the map
case len(element.Key) == 0 && len(element.Name) > 0:
case element.isMapElement():
if !isMapSlice(pointer) {
return nil, fmt.Errorf("failed to traverse tree, expected a %s but found type %s at %s", typeMap, GetType(pointer), pointerPath.ToGoPatchStyle())
}
Expand All @@ -51,7 +55,7 @@ func Grab(obj interface{}, pathString string) (interface{}, error) {
pointer = entry

// Complex List, where each list entry is a Key/Value map and the entry is identified by name using an indentifier (e.g. name, key, or id)
case len(element.Key) > 0 && len(element.Name) > 0:
case element.isComplexListElement():
complexList, ok := castAsComplexList(pointer)
if !ok {
return nil, fmt.Errorf("failed to traverse tree, expected a %s but found type %s at %s", typeComplexList, GetType(pointer), pointerPath.ToGoPatchStyle())
Expand All @@ -65,7 +69,7 @@ func Grab(obj interface{}, pathString string) (interface{}, error) {
pointer = entry

// Simple List (identified by index)
case len(element.Key) == 0 && len(element.Name) == 0:
case element.isSimpleListElement():
if !isList(pointer) {
return nil, fmt.Errorf("failed to traverse tree, expected a %s but found type %s at %s", typeSimpleList, GetType(pointer), pointerPath.ToGoPatchStyle())
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/v1/ytbx/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func LoadJSONDocuments(input []byte) ([]interface{}, error) {
values[i] = value

default:
return nil, fmt.Errorf("Unsupported type %s in load document function", types[i])
return nil, fmt.Errorf("unsupported type %s in load document function", types[i])
}
}

Expand Down Expand Up @@ -318,7 +318,7 @@ func LoadYAMLDocuments(input []byte) ([]interface{}, error) {
values[i] = value

default:
return nil, fmt.Errorf("Unsupported type %s in load document function", types[i])
return nil, fmt.Errorf("unsupported type %s in load document function", types[i])
}
}

Expand Down Expand Up @@ -368,7 +368,7 @@ func getBytesFromLocation(location string) ([]byte, error) {
}

// In any other case, bail out ...
return nil, fmt.Errorf("Unable to get any content using location %s: it is not a file or usable URI", location)
return nil, fmt.Errorf("unable to get any content using location %s: it is not a file or usable URI", location)
}

// IsStdin checks whether the provided input location refers to the dash
Expand Down
4 changes: 2 additions & 2 deletions pkg/v1/ytbx/list_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ func GetIdentifierFromNamedList(list []interface{}) string {
counters := map[interface{}]int{}

for _, sliceEntry := range list {
switch sliceEntry.(type) {
switch mapslice := sliceEntry.(type) {
case yaml.MapSlice:
for _, mapSliceEntry := range sliceEntry.(yaml.MapSlice) {
for _, mapSliceEntry := range mapslice {
if _, ok := counters[mapSliceEntry.Key]; !ok {
counters[mapSliceEntry.Key] = 0
}
Expand Down
25 changes: 20 additions & 5 deletions pkg/v1/ytbx/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,22 +227,22 @@ func ListPaths(location string, style PathStyle) ([]Path, error) {
}

func traverseTree(path Path, obj interface{}, leafFunc func(path Path, value interface{})) {
switch obj.(type) {
switch tobj := obj.(type) {
case []interface{}:
if identifier := GetIdentifierFromNamedList(obj.([]interface{})); identifier != "" {
for _, entry := range obj.([]interface{}) {
if identifier := GetIdentifierFromNamedList(tobj); identifier != "" {
for _, entry := range tobj {
name, data := splitEntryIntoNameAndData(entry.(yaml.MapSlice), identifier)
traverseTree(NewPathWithNamedListElement(path, identifier, name), data, leafFunc)
}

} else {
for idx, entry := range obj.([]interface{}) {
for idx, entry := range tobj {
traverseTree(NewPathWithIndexedListElement(path, idx), entry, leafFunc)
}
}

case yaml.MapSlice:
for _, mapitem := range obj.(yaml.MapSlice) {
for _, mapitem := range tobj {
traverseTree(NewPathWithNamedElement(path, mapitem.Key), mapitem.Value, leafFunc)
}

Expand Down Expand Up @@ -379,3 +379,18 @@ func ParsePathString(pathString string, obj interface{}) (Path, error) {

return ParseDotStylePathString(pathString, obj)
}

func (element PathElement) isMapElement() bool {
return len(element.Key) == 0 &&
len(element.Name) > 0
}

func (element PathElement) isComplexListElement() bool {
return len(element.Key) > 0 &&
len(element.Name) > 0
}

func (element PathElement) isSimpleListElement() bool {
return len(element.Key) == 0 &&
len(element.Name) == 0
}
62 changes: 38 additions & 24 deletions pkg/v1/ytbx/path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,29 @@ var _ = Describe("path tests", func() {
list, err := ComparePaths(assetsDirectory+"/testbed/sample_a.yml", assetsDirectory+"/testbed/sample_b.yml", GoPatchStyle, false)
Expect(err).ToNot(HaveOccurred())

listOfPaths := make([]Path, 5)
listOfPaths = []Path{{DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "somekey"},
}}, {DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "dot"},
}}, {DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "list"},
{Idx: -1, Key: "name", Name: "sametwo"},
{Idx: -1, Key: "", Name: "somekey"},
}}}
listOfPaths := []Path{
{
DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "somekey"},
},
},
{
DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "dot"},
},
},
{
DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "list"},
{Idx: -1, Key: "name", Name: "sametwo"},
{Idx: -1, Key: "", Name: "somekey"},
},
},
}

Expect(list).To(BeEquivalentTo(listOfPaths))
})
Expand All @@ -174,16 +183,21 @@ var _ = Describe("path tests", func() {
list, err := ComparePaths(assetsDirectory+"/testbed/sample_a.yml", assetsDirectory+"/testbed/sample_b.yml", GoPatchStyle, true)
Expect(err).ToNot(HaveOccurred())

listOfPathsWithSameValue := make([]Path, 5)
listOfPathsWithSameValue = []Path{{DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "dot"},
}}, {DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "list"},
{Idx: -1, Key: "name", Name: "sametwo"},
{Idx: -1, Key: "", Name: "somekey"},
}}}
listOfPathsWithSameValue := []Path{
{
DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "yaml"},
{Idx: -1, Key: "", Name: "structure"},
{Idx: -1, Key: "", Name: "dot"},
}},
{
DocumentIdx: 0, PathElements: []PathElement{
{Idx: -1, Key: "", Name: "list"},
{Idx: -1, Key: "name", Name: "sametwo"},
{Idx: -1, Key: "", Name: "somekey"},
},
},
}

Expect(list).To(BeEquivalentTo(listOfPathsWithSameValue))
})
Expand Down
10 changes: 5 additions & 5 deletions pkg/v1/ytbx/ytbx_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ func yml(input string) yaml.MapSlice {

// Load YAML by parsing the actual string as YAML if it was not a file location
doc := singleDoc(input)
switch doc.(type) {
switch mapslice := doc.(type) {
case yaml.MapSlice:
return doc.(yaml.MapSlice)
return mapslice
}

Fail(fmt.Sprintf("Failed to use YAML, parsed data is not a YAML MapSlice:\n%s\n", input))
Expand All @@ -126,12 +126,12 @@ func yml(input string) yaml.MapSlice {
func list(input string) []interface{} {
doc := singleDoc(input)

switch doc.(type) {
switch tobj := doc.(type) {
case []interface{}:
return doc.([]interface{})
return tobj

case []yaml.MapSlice:
return ytbx.SimplifyList(doc.([]yaml.MapSlice))
return ytbx.SimplifyList(tobj)
}

Fail(fmt.Sprintf("Failed to use YAML, parsed data is not a slice of any kind:\n%s\nIt was parsed as: %#v", input, doc))
Expand Down

0 comments on commit a4910b4

Please sign in to comment.