From f55fd6cc5bcf72b463e04fa5b3110a7918b15370 Mon Sep 17 00:00:00 2001 From: Khosrow Moossavi Date: Mon, 24 Feb 2020 10:18:02 -0500 Subject: [PATCH] fix: Fix type conversion for numbers (#204) * Fix type conversion for numbers * add tests * add more tests --- go.mod | 2 +- go.sum | 4 +- internal/types/bool_test.go | 31 ++++++ internal/types/empty_test.go | 31 ++++++ internal/types/list_test.go | 36 ++++++ internal/types/map_test.go | 50 +++++++++ internal/types/nil_test.go | 71 ++++++++++++ internal/types/number_test.go | 31 ++++++ internal/types/string_test.go | 31 ++++++ internal/types/types.go | 79 +++++++++---- internal/types/types_test.go | 86 ++++++++++++++ .../tfconfig/markdown.go | 105 ++++++++++++++++++ vendor/modules.txt | 2 +- 13 files changed, 536 insertions(+), 23 deletions(-) create mode 100644 internal/types/bool_test.go create mode 100644 internal/types/empty_test.go create mode 100644 internal/types/list_test.go create mode 100644 internal/types/map_test.go create mode 100644 internal/types/nil_test.go create mode 100644 internal/types/number_test.go create mode 100644 internal/types/string_test.go create mode 100644 internal/types/types_test.go create mode 100644 vendor/github.com/hashicorp/terraform-config-inspect/tfconfig/markdown.go diff --git a/go.mod b/go.mod index 3775c2a..800e5cc 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/segmentio/terraform-docs go 1.13 require ( - github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7 + github.com/hashicorp/terraform-config-inspect v0.0.0-20200210163047-f7d8399e1194 github.com/imdario/mergo v0.3.8 github.com/spf13/cobra v0.0.6 github.com/stretchr/testify v1.5.1 diff --git a/go.sum b/go.sum index bfb7788..3d1a3d2 100644 --- a/go.sum +++ b/go.sum @@ -54,8 +54,8 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/hcl/v2 v2.0.0 h1:efQznTz+ydmQXq3BOnRa3AXzvCeTq1P4dKj/z5GLlY8= github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90= -github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7 h1:Pc5TCv9mbxFN6UVX0LH6CpQrdTM5YjbVI2w15237Pjk= -github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7/go.mod h1:p+ivJws3dpqbp1iP84+npOyAmTTOLMgCzrXd3GSdn/A= +github.com/hashicorp/terraform-config-inspect v0.0.0-20200210163047-f7d8399e1194 h1:Zn54YFKlZ0tVcR4HCfmVUP9kTTIK1c7G5mW7JKMsaak= +github.com/hashicorp/terraform-config-inspect v0.0.0-20200210163047-f7d8399e1194/go.mod h1:p+ivJws3dpqbp1iP84+npOyAmTTOLMgCzrXd3GSdn/A= github.com/imdario/mergo v0.3.8 h1:CGgOkSJeqMRmt0D9XLWExdT4m4F1vd3FV3VPt+0VxkQ= github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= diff --git a/internal/types/bool_test.go b/internal/types/bool_test.go new file mode 100644 index 0000000..f43ce27 --- /dev/null +++ b/internal/types/bool_test.go @@ -0,0 +1,31 @@ +package types + +import ( + "testing" +) + +func TestBool(t *testing.T) { + values := List{true, false} + testPrimitive(t, []testprimitive{ + { + name: "value not nil and type bool", + values: values, + types: "bool", + expected: expected{ + typeName: "bool", + valueKind: "types.Bool", + hasDefault: true, + }, + }, + { + name: "value not nil and type empty", + values: values, + types: "", + expected: expected{ + typeName: "bool", + valueKind: "types.Bool", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/empty_test.go b/internal/types/empty_test.go new file mode 100644 index 0000000..c57e2bc --- /dev/null +++ b/internal/types/empty_test.go @@ -0,0 +1,31 @@ +package types + +import ( + "testing" +) + +func TestEmpty(t *testing.T) { + values := List{""} + testPrimitive(t, []testprimitive{ + { + name: "value empty and type string", + values: values, + types: "string", + expected: expected{ + typeName: "string", + valueKind: "types.Empty", + hasDefault: true, + }, + }, + { + name: "value empty and type empty", + values: values, + types: "", + expected: expected{ + typeName: "string", + valueKind: "types.Empty", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/list_test.go b/internal/types/list_test.go new file mode 100644 index 0000000..8ce321d --- /dev/null +++ b/internal/types/list_test.go @@ -0,0 +1,36 @@ +package types + +import ( + "testing" +) + +func TestList(t *testing.T) { + values := []List{ + List{"foo", "bar", "baz"}, + List{"1", "2", "3"}, + List{true, false, true}, + List{10, float64(1000), int8(42)}, + } + testList(t, []testlist{ + { + name: "value not nil and type list", + values: values, + types: "list", + expected: expected{ + typeName: "list", + valueKind: "types.List", + hasDefault: true, + }, + }, + { + name: "value not nil and type empty", + values: values, + types: "", + expected: expected{ + typeName: "list", + valueKind: "types.List", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/map_test.go b/internal/types/map_test.go new file mode 100644 index 0000000..1768267 --- /dev/null +++ b/internal/types/map_test.go @@ -0,0 +1,50 @@ +package types + +import ( + "testing" +) + +func TestMap(t *testing.T) { + values := []Map{ + { + "a": 1, + "b": 2, + "c": 3, + }, + { + "name": "hello", + "foo": map[string]string{ + "foo": "foo", + "bar": "foo", + }, + "bar": map[string]string{ + "foo": "bar", + "bar": "bar", + }, + "buzz": []string{"fizz", "buzz"}, + }, + {}, + } + testMap(t, []testmap{ + { + name: "value not nil and type map", + values: values, + types: "map", + expected: expected{ + typeName: "map", + valueKind: "types.Map", + hasDefault: true, + }, + }, + { + name: "value not nil and type empty", + values: values, + types: "", + expected: expected{ + typeName: "map", + valueKind: "types.Map", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/nil_test.go b/internal/types/nil_test.go new file mode 100644 index 0000000..5cf6f3f --- /dev/null +++ b/internal/types/nil_test.go @@ -0,0 +1,71 @@ +package types + +import ( + "testing" +) + +func TestNil(t *testing.T) { + nils := List{nil} + testPrimitive(t, []testprimitive{ + { + name: "value nil and type bool", + values: nils, + types: "bool", + expected: expected{ + typeName: "bool", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + { + name: "value nil and type number", + values: nils, + types: "number", + expected: expected{ + typeName: "number", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + { + name: "value nil and type list", + values: nils, + types: "list", + expected: expected{ + typeName: "list", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + { + name: "value nil and type map", + values: nil, + types: "map", + expected: expected{ + typeName: "map", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + { + name: "value nil and type string", + values: nils, + types: "string", + expected: expected{ + typeName: "string", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + { + name: "value nil and type empty", + values: nils, + types: "", + expected: expected{ + typeName: "any", + valueKind: "*types.Nil", + hasDefault: false, + }, + }, + }) +} diff --git a/internal/types/number_test.go b/internal/types/number_test.go new file mode 100644 index 0000000..b95e8c4 --- /dev/null +++ b/internal/types/number_test.go @@ -0,0 +1,31 @@ +package types + +import ( + "testing" +) + +func TestNumber(t *testing.T) { + values := List{int(0), int8(42), int16(-1200), int32(1140085647), int64(8922336854775807), float32(13.75), float64(2317483.64)} + testPrimitive(t, []testprimitive{ + { + name: "value not nil and type number", + values: values, + types: "number", + expected: expected{ + typeName: "number", + valueKind: "types.Number", + hasDefault: true, + }, + }, + { + name: "value not nil and type empty", + values: values, + types: "", + expected: expected{ + typeName: "number", + valueKind: "types.Number", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/string_test.go b/internal/types/string_test.go new file mode 100644 index 0000000..b5a89c4 --- /dev/null +++ b/internal/types/string_test.go @@ -0,0 +1,31 @@ +package types + +import ( + "testing" +) + +func TestString(t *testing.T) { + values := List{"foo", "42", "false", "true"} + testPrimitive(t, []testprimitive{ + { + name: "value not nil and type string", + values: values, + types: "string", + expected: expected{ + typeName: "string", + valueKind: "types.String", + hasDefault: true, + }, + }, + { + name: "value not nil and type empty", + values: values, + types: "", + expected: expected{ + typeName: "string", + valueKind: "types.String", + hasDefault: true, + }, + }, + }) +} diff --git a/internal/types/types.go b/internal/types/types.go index 2302788..9c37655 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -2,8 +2,8 @@ package types import ( "bytes" - "fmt" "go/types" + "reflect" "strings" ) @@ -28,20 +28,23 @@ func ValueOf(v interface{}) Default { if v == nil { return new(Nil) } - switch xType := fmt.Sprintf("%T", v); xType { - case "string": - if v.(string) == "" { + value := reflect.ValueOf(v) + switch value.Kind() { + case reflect.String: + if value.IsZero() { return Empty("") } - return String(v.(string)) - case "int", "int8", "int16", "int32", "int64", "float32", "float64": - return Number(v.(float64)) - case "bool": - return Bool(v.(bool)) - case "[]interface {}": - return List(v.([]interface{})) - case "map[string]interface {}": - return Map(v.(map[string]interface{})) + return String(value.String()) + case reflect.Float32, reflect.Float64: + return Number(value.Float()) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return Number(float64(value.Int())) + case reflect.Bool: + return Bool(value.Bool()) + case reflect.Slice: + return List(value.Interface().([]interface{})) + case reflect.Map: + return Map(value.Interface().(map[string]interface{})) } return new(Nil) } @@ -54,16 +57,16 @@ func TypeOf(t string, v interface{}) String { return String(t) } if v != nil { - switch xType := fmt.Sprintf("%T", v); xType { - case "string": + switch reflect.ValueOf(v).Kind() { + case reflect.String: return String("string") - case "int", "int8", "int16", "int32", "int64", "float32", "float64": + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64: return String("number") - case "bool": + case reflect.Bool: return String("bool") - case "[]interface {}": + case reflect.Slice: return String("list") - case "map[string]interface {}": + case reflect.Map: return String("map") } } @@ -102,6 +105,11 @@ func (s String) String() string { return string(s) } +// nolint +func (s String) underlying() string { + return string(s) +} + // HasDefault indicates a Terraform variable has a default value set. func (s String) HasDefault() bool { return true @@ -135,6 +143,11 @@ func (s String) MarshalYAML() (interface{}, error) { // marshaled to `""` in JSON and YAML type Empty string +// nolint +func (e Empty) underlying() string { + return string(e) +} + // HasDefault indicates a Terraform variable has a default value set. func (e Empty) HasDefault() bool { return true @@ -150,6 +163,11 @@ func (e Empty) MarshalJSON() ([]byte, error) { // marshaled to `null` when emty in JSON and YAML type Number float64 +// nolint +func (n Number) underlying() float64 { + return float64(n) +} + // HasDefault indicates a Terraform variable has a default value set. func (n Number) HasDefault() bool { return true @@ -158,6 +176,11 @@ func (n Number) HasDefault() bool { // Bool represents a 'bool' value type Bool bool +// nolint +func (b Bool) underlying() bool { + return bool(b) +} + // HasDefault indicates a Terraform variable has a default value set. func (b Bool) HasDefault() bool { return true @@ -166,6 +189,15 @@ func (b Bool) HasDefault() bool { // List represents a 'list' of values type List []interface{} +// nolint +func (l List) underlying() []interface{} { + r := make([]interface{}, 0) + for _, i := range l { + r = append(r, i) + } + return r +} + // HasDefault indicates a Terraform variable has a default value set. func (l List) HasDefault() bool { return true @@ -174,6 +206,15 @@ func (l List) HasDefault() bool { // Map represents a 'map' of values type Map map[string]interface{} +// nolint +func (m Map) underlying() map[string]interface{} { + r := make(map[string]interface{}, 0) + for k, e := range m { + r[k] = e + } + return r +} + // HasDefault indicates a Terraform variable has a default value set. func (m Map) HasDefault() bool { return true diff --git a/internal/types/types_test.go b/internal/types/types_test.go new file mode 100644 index 0000000..1f13f4d --- /dev/null +++ b/internal/types/types_test.go @@ -0,0 +1,86 @@ +package types + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +type expected struct { + typeName string + valueKind string + hasDefault bool +} + +type testprimitive struct { + name string + values List + types string + expected expected +} + +type testlist struct { + name string + values []List + types string + expected expected +} + +type testmap struct { + name string + values []Map + types string + expected expected +} + +func testPrimitive(t *testing.T, tests []testprimitive) { + for _, tt := range tests { + for _, tv := range tt.values { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + actualValue := ValueOf(tv) + actualType := TypeOf(tt.types, tv) + + assert.Equal(tt.expected.typeName, string(actualType)) + assert.Equal(tt.expected.valueKind, reflect.TypeOf(actualValue).String()) + assert.Equal(tt.expected.hasDefault, actualValue.HasDefault()) + }) + } + } +} + +func testList(t *testing.T, tests []testlist) { + for _, tt := range tests { + for _, tv := range tt.values { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + actualValue := ValueOf(tv.underlying()) + actualType := TypeOf(tt.types, tv.underlying()) + + assert.Equal(tt.expected.typeName, string(actualType)) + assert.Equal(tt.expected.valueKind, reflect.TypeOf(actualValue).String()) + assert.Equal(tt.expected.hasDefault, actualValue.HasDefault()) + }) + } + } +} + +func testMap(t *testing.T, tests []testmap) { + for _, tt := range tests { + for _, tv := range tt.values { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + actualValue := ValueOf(tv.underlying()) + actualType := TypeOf(tt.types, tv.underlying()) + + assert.Equal(tt.expected.typeName, string(actualType)) + assert.Equal(tt.expected.valueKind, reflect.TypeOf(actualValue).String()) + assert.Equal(tt.expected.hasDefault, actualValue.HasDefault()) + }) + } + } +} diff --git a/vendor/github.com/hashicorp/terraform-config-inspect/tfconfig/markdown.go b/vendor/github.com/hashicorp/terraform-config-inspect/tfconfig/markdown.go new file mode 100644 index 0000000..9ace608 --- /dev/null +++ b/vendor/github.com/hashicorp/terraform-config-inspect/tfconfig/markdown.go @@ -0,0 +1,105 @@ +package tfconfig + +import ( + "encoding/json" + "io" + "strings" + "text/template" +) + +func RenderMarkdown(w io.Writer, module *Module) error { + tmpl := template.New("md") + tmpl.Funcs(template.FuncMap{ + "tt": func(s string) string { + return "`" + s + "`" + }, + "commas": func(s []string) string { + return strings.Join(s, ", ") + }, + "json": func(v interface{}) (string, error) { + j, err := json.Marshal(v) + return string(j), err + }, + "severity": func(s DiagSeverity) string { + switch s { + case DiagError: + return "Error: " + case DiagWarning: + return "Warning: " + default: + return "" + } + }, + }) + template.Must(tmpl.Parse(markdownTemplate)) + return tmpl.Execute(w, module) +} + +const markdownTemplate = ` +# Module {{ tt .Path }} + +{{- if .RequiredCore}} + +Core Version Constraints: +{{- range .RequiredCore }} +* {{ tt . }} +{{- end}}{{end}} + +{{- if .RequiredProviders}} + +Provider Requirements: +{{- range $name, $req := .RequiredProviders }} +* **{{ $name }}{{ if $req.Source }} ({{ $req.Source | tt }}){{ end }}:** {{ if $req.VersionConstraints }}{{ commas $req.VersionConstraints | tt }}{{ else }}(any version){{ end }} +{{- end}}{{end}} + +{{- if .Variables}} + +## Input Variables +{{- range .Variables }} +* {{ tt .Name }}{{ if .Default }} (default {{ json .Default | tt }}){{else}} (required){{end}} +{{- if .Description}}: {{ .Description }}{{ end }} +{{- end}}{{end}} + +{{- if .Outputs}} + +## Output Values +{{- range .Outputs }} +* {{ tt .Name }}{{ if .Description}}: {{ .Description }}{{ end }} +{{- end}}{{end}} + +{{- if .ManagedResources}} + +## Managed Resources +{{- range .ManagedResources }} +* {{ printf "%s.%s" .Type .Name | tt }} from {{ tt .Provider.Name }} +{{- end}}{{end}} + +{{- if .DataResources}} + +## Data Resources +{{- range .DataResources }} +* {{ printf "data.%s.%s" .Type .Name | tt }} from {{ tt .Provider.Name }} +{{- end}}{{end}} + +{{- if .ModuleCalls}} + +## Child Modules +{{- range .ModuleCalls }} +* {{ tt .Name }} from {{ tt .Source }}{{ if .Version }} ({{ tt .Version }}){{ end }} +{{- end}}{{end}} + +{{- if .Diagnostics}} + +## Problems +{{- range .Diagnostics }} + +## {{ severity .Severity }}{{ .Summary }}{{ if .Pos }} + +(at {{ tt .Pos.Filename }} line {{ .Pos.Line }}{{ end }}) +{{ if .Detail }} +{{ .Detail }} +{{- end }} + +{{- end}}{{end}} + +` diff --git a/vendor/modules.txt b/vendor/modules.txt index 3ad0c44..7354718 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -27,7 +27,7 @@ github.com/hashicorp/hcl/v2/hclparse github.com/hashicorp/hcl/v2/hclsyntax github.com/hashicorp/hcl/v2/hclwrite github.com/hashicorp/hcl/v2/json -# github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7 +# github.com/hashicorp/terraform-config-inspect v0.0.0-20200210163047-f7d8399e1194 github.com/hashicorp/terraform-config-inspect/tfconfig # github.com/imdario/mergo v0.3.8 github.com/imdario/mergo