diff --git a/command/apply.go b/command/apply.go index f9ccddfc1..21063bbfc 100644 --- a/command/apply.go +++ b/command/apply.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" - "github.com/hashicorp/terraform/configs/hcl2shim" "github.com/hashicorp/terraform/repl" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -345,17 +344,7 @@ func outputsAsString(state *states.State, modPath addrs.ModuleInstance, includeH continue } - // Our formatter still wants an old-style raw interface{} value, so - // for now we'll just shim it. - // FIXME: Port the formatter to work with cty.Value directly. - legacyVal := hcl2shim.ConfigValueFromHCL2(v.Value) - result, err := repl.FormatResult(legacyVal) - if err != nil { - // We can't really return errors from here, so we'll just have - // to stub this out. This shouldn't happen in practice anyway. - result = "" - } - + result := repl.FormatValue(v.Value, 0) outputBuf.WriteString(fmt.Sprintf("%s = %s\n", k, result)) } } diff --git a/command/apply_test.go b/command/apply_test.go index c4d0ba949..74719c32a 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -1107,7 +1107,7 @@ func TestApply_sensitiveOutput(t *testing.T) { } output := ui.OutputWriter.String() - if !strings.Contains(output, "notsensitive = Hello world") { + if !strings.Contains(output, "notsensitive = \"Hello world\"") { t.Fatalf("bad: output should contain 'notsensitive' output\n%s", output) } if !strings.Contains(output, "sensitive = ") { diff --git a/command/console_test.go b/command/console_test.go index b8e73fe33..bbfee6951 100644 --- a/command/console_test.go +++ b/command/console_test.go @@ -27,7 +27,7 @@ func TestConsole_basic(t *testing.T) { defer testFixCwd(t, tmp, cwd) p := testProvider() - ui := new(cli.MockUi) + ui := cli.NewMockUi() c := &ConsoleCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), @@ -73,7 +73,7 @@ func TestConsole_tfvars(t *testing.T) { }, } - ui := new(cli.MockUi) + ui := cli.NewMockUi() c := &ConsoleCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), @@ -95,7 +95,7 @@ func TestConsole_tfvars(t *testing.T) { } actual := output.String() - if actual != "bar\n" { + if actual != "\"bar\"\n" { t.Fatalf("bad: %q", actual) } } @@ -120,7 +120,7 @@ func TestConsole_unsetRequiredVars(t *testing.T) { }, }, } - ui := new(cli.MockUi) + ui := cli.NewMockUi() c := &ConsoleCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(p), @@ -140,21 +140,12 @@ func TestConsole_unsetRequiredVars(t *testing.T) { code := c.Run(args) outCloser() - // Because we're running "terraform console" in piped input mode, we're - // expecting it to return a nonzero exit status here but the message - // must be the one indicating that it did attempt to evaluate var.foo and - // got an unknown value in return, rather than an error about var.foo - // not being set or a failure to prompt for it. - if code == 0 { - t.Fatalf("unexpected success\n%s", ui.OutputWriter.String()) + if code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } - // The error message should be the one console produces when it encounters - // an unknown value. - got := ui.ErrorWriter.String() - want := `Error: Result depends on values that cannot be determined` - if !strings.Contains(got, want) { - t.Fatalf("wrong output\ngot:\n%s\n\nwant string containing %q", got, want) + if got, want := output.String(), "(known after apply)\n"; got != want { + t.Fatalf("unexpected output\n got: %q\nwant: %q", got, want) } } @@ -165,7 +156,7 @@ func TestConsole_modules(t *testing.T) { defer testChdir(t, td)() p := applyFixtureProvider() - ui := new(cli.MockUi) + ui := cli.NewMockUi() c := &ConsoleCommand{ Meta: Meta{ @@ -175,8 +166,8 @@ func TestConsole_modules(t *testing.T) { } commands := map[string]string{ - "module.child.myoutput\n": "bar\n", - "module.count_child[0].myoutput\n": "bar\n", + "module.child.myoutput\n": "\"bar\"\n", + "module.count_child[0].myoutput\n": "\"bar\"\n", "local.foo\n": "3\n", } diff --git a/command/output.go b/command/output.go index a9d4441be..01f988cc6 100644 --- a/command/output.go +++ b/command/output.go @@ -10,7 +10,6 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/configs/hcl2shim" "github.com/hashicorp/terraform/repl" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -195,16 +194,7 @@ func (c *OutputCommand) Run(args []string) int { c.Ui.Output(string(jsonOutput)) } else { - // Our formatter still wants an old-style raw interface{} value, so - // for now we'll just shim it. - // FIXME: Port the formatter to work with cty.Value directly. - legacyVal := hcl2shim.ConfigValueFromHCL2(v) - result, err := repl.FormatResult(legacyVal) - if err != nil { - diags = diags.Append(err) - c.showDiagnostics(diags) - return 1 - } + result := repl.FormatValue(v, 0) c.Ui.Output(result) } diff --git a/command/output_test.go b/command/output_test.go index d8e4b862f..4ca121aa1 100644 --- a/command/output_test.go +++ b/command/output_test.go @@ -41,7 +41,7 @@ func TestOutput(t *testing.T) { } actual := strings.TrimSpace(ui.OutputWriter.String()) - if actual != "bar" { + if actual != `"bar"` { t.Fatalf("bad: %#v", actual) } } @@ -80,9 +80,19 @@ func TestOutput_nestedListAndMap(t *testing.T) { } actual := strings.TrimSpace(ui.OutputWriter.String()) - expected := "foo = [\n {\n \"key\" = \"value\"\n \"key2\" = \"value2\"\n },\n {\n \"key\" = \"value\"\n },\n]" + expected := strings.TrimSpace(` +foo = tolist([ + tomap({ + "key" = "value" + "key2" = "value2" + }), + tomap({ + "key" = "value" + }), +]) +`) if actual != expected { - t.Fatalf("wrong output\ngot: %#v\nwant: %#v", actual, expected) + t.Fatalf("wrong output\ngot: %s\nwant: %s", actual, expected) } } @@ -260,7 +270,7 @@ func TestOutput_blank(t *testing.T) { t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) } - expectedOutput := "foo = bar\nname = john-doe\n" + expectedOutput := "foo = \"bar\"\nname = \"john-doe\"\n" output := ui.OutputWriter.String() if output != expectedOutput { t.Fatalf("wrong output\ngot: %#v\nwant: %#v", output, expectedOutput) @@ -393,7 +403,7 @@ func TestOutput_stateDefault(t *testing.T) { } actual := strings.TrimSpace(ui.OutputWriter.String()) - if actual != "bar" { + if actual != `"bar"` { t.Fatalf("bad: %#v", actual) } } diff --git a/repl/format.go b/repl/format.go index d8f95ea3d..051e2425e 100644 --- a/repl/format.go +++ b/repl/format.go @@ -1,116 +1,118 @@ package repl import ( - "bufio" - "bytes" "fmt" - "sort" "strconv" "strings" + + "github.com/zclconf/go-cty/cty" ) -// FormatResult formats the given result value for human-readable output. -// -// The value must currently be a string, list, map, and any nested values -// with those same types. -func FormatResult(value interface{}) (string, error) { - return formatResult(value, false) -} - -func formatResult(value interface{}, nested bool) (string, error) { - if value == nil { - return "null", nil +// FormatValue formats a value in a way that resembles Terraform language syntax +// and uses the type conversion functions where necessary to indicate exactly +// what type it is given, so that equality test failures can be quickly +// understood. +func FormatValue(v cty.Value, indent int) string { + if !v.IsKnown() { + return "(known after apply)" } - switch output := value.(type) { - case string: - if nested { - return fmt.Sprintf("%q", output), nil - } - return output, nil - case int: - return strconv.Itoa(output), nil - case float64: - return fmt.Sprintf("%g", output), nil - case bool: + if v.IsNull() { + ty := v.Type() switch { - case output == true: - return "true", nil + case ty == cty.DynamicPseudoType: + return "null" + case ty == cty.String: + return "tostring(null)" + case ty == cty.Number: + return "tonumber(null)" + case ty == cty.Bool: + return "tobool(null)" + case ty.IsListType(): + return fmt.Sprintf("tolist(null) /* of %s */", ty.ElementType().FriendlyName()) + case ty.IsSetType(): + return fmt.Sprintf("toset(null) /* of %s */", ty.ElementType().FriendlyName()) + case ty.IsMapType(): + return fmt.Sprintf("tomap(null) /* of %s */", ty.ElementType().FriendlyName()) default: - return "false", nil + return fmt.Sprintf("null /* %s */", ty.FriendlyName()) } - case []interface{}: - return formatListResult(output) - case map[string]interface{}: - return formatMapResult(output) - default: - return "", fmt.Errorf("unknown value type: %T", value) } + + ty := v.Type() + switch { + case ty.IsPrimitiveType(): + switch ty { + case cty.String: + // FIXME: If it's a multi-line string, better to render it using + // HEREDOC-style syntax. + return strconv.Quote(v.AsString()) + case cty.Number: + bf := v.AsBigFloat() + return bf.Text('g', -1) + case cty.Bool: + if v.True() { + return "true" + } else { + return "false" + } + } + case ty.IsObjectType(): + return formatMappingValue(v, indent) + case ty.IsTupleType(): + return formatSequenceValue(v, indent) + case ty.IsListType(): + return fmt.Sprintf("tolist(%s)", formatSequenceValue(v, indent)) + case ty.IsSetType(): + return fmt.Sprintf("toset(%s)", formatSequenceValue(v, indent)) + case ty.IsMapType(): + return fmt.Sprintf("tomap(%s)", formatMappingValue(v, indent)) + } + + // Should never get here because there are no other types + return fmt.Sprintf("%#v", v) } -func formatListResult(value []interface{}) (string, error) { - var outputBuf bytes.Buffer - outputBuf.WriteString("[") - if len(value) > 0 { - outputBuf.WriteString("\n") +func formatMappingValue(v cty.Value, indent int) string { + var buf strings.Builder + count := 0 + buf.WriteByte('{') + indent += 2 + for it := v.ElementIterator(); it.Next(); { + count++ + k, v := it.Element() + buf.WriteByte('\n') + buf.WriteString(strings.Repeat(" ", indent)) + buf.WriteString(FormatValue(k, indent)) + buf.WriteString(" = ") + buf.WriteString(FormatValue(v, indent)) } - - for _, v := range value { - raw, err := formatResult(v, true) - if err != nil { - return "", err - } - - outputBuf.WriteString(indent(raw)) - outputBuf.WriteString(",\n") + indent -= 2 + if count > 0 { + buf.WriteByte('\n') + buf.WriteString(strings.Repeat(" ", indent)) } - - outputBuf.WriteString("]") - return outputBuf.String(), nil + buf.WriteByte('}') + return buf.String() } -func formatMapResult(value map[string]interface{}) (string, error) { - ks := make([]string, 0, len(value)) - for k, _ := range value { - ks = append(ks, k) +func formatSequenceValue(v cty.Value, indent int) string { + var buf strings.Builder + count := 0 + buf.WriteByte('[') + indent += 2 + for it := v.ElementIterator(); it.Next(); { + count++ + _, v := it.Element() + buf.WriteByte('\n') + buf.WriteString(strings.Repeat(" ", indent)) + buf.WriteString(FormatValue(v, indent)) + buf.WriteByte(',') } - sort.Strings(ks) - - var outputBuf bytes.Buffer - outputBuf.WriteString("{") - if len(value) > 0 { - outputBuf.WriteString("\n") + indent -= 2 + if count > 0 { + buf.WriteByte('\n') + buf.WriteString(strings.Repeat(" ", indent)) } - - for _, k := range ks { - v := value[k] - rawK, err := formatResult(k, true) - if err != nil { - return "", err - } - rawV, err := formatResult(v, true) - if err != nil { - return "", err - } - - outputBuf.WriteString(indent(fmt.Sprintf("%s = %s", rawK, rawV))) - outputBuf.WriteString("\n") - } - - outputBuf.WriteString("}") - return outputBuf.String(), nil -} - -func indent(value string) string { - var outputBuf bytes.Buffer - s := bufio.NewScanner(strings.NewReader(value)) - newline := false - for s.Scan() { - if newline { - outputBuf.WriteByte('\n') - } - outputBuf.WriteString(" " + s.Text()) - newline = true - } - - return outputBuf.String() + buf.WriteByte(']') + return buf.String() } diff --git a/repl/format_test.go b/repl/format_test.go new file mode 100644 index 000000000..ca1d06232 --- /dev/null +++ b/repl/format_test.go @@ -0,0 +1,149 @@ +package repl + +import ( + "fmt" + "testing" + + "github.com/zclconf/go-cty/cty" +) + +func TestFormatValue(t *testing.T) { + tests := []struct { + Val cty.Value + Want string + }{ + { + cty.NullVal(cty.DynamicPseudoType), + `null`, + }, + { + cty.NullVal(cty.String), + `tostring(null)`, + }, + { + cty.NullVal(cty.Number), + `tonumber(null)`, + }, + { + cty.NullVal(cty.Bool), + `tobool(null)`, + }, + { + cty.NullVal(cty.List(cty.String)), + `tolist(null) /* of string */`, + }, + { + cty.NullVal(cty.Set(cty.Number)), + `toset(null) /* of number */`, + }, + { + cty.NullVal(cty.Map(cty.Bool)), + `tomap(null) /* of bool */`, + }, + { + cty.NullVal(cty.Object(map[string]cty.Type{"a": cty.Bool})), + `null /* object */`, // Ideally this would display the full object type, including its attributes + }, + { + cty.UnknownVal(cty.DynamicPseudoType), + `(known after apply)`, + }, + { + cty.StringVal(""), + `""`, + }, + { + cty.StringVal("hello"), + `"hello"`, + }, + { + cty.StringVal("hello\nworld"), + `"hello\nworld"`, // Ideally we'd use heredoc syntax here for better readability, but we don't yet + }, + { + cty.Zero, + `0`, + }, + { + cty.NumberIntVal(5), + `5`, + }, + { + cty.NumberFloatVal(5.2), + `5.2`, + }, + { + cty.False, + `false`, + }, + { + cty.True, + `true`, + }, + { + cty.EmptyObjectVal, + `{}`, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("b"), + }), + `{ + "a" = "b" +}`, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("b"), + "c": cty.StringVal("d"), + }), + `{ + "a" = "b" + "c" = "d" +}`, + }, + { + cty.MapValEmpty(cty.String), + `tomap({})`, + }, + { + cty.EmptyTupleVal, + `[]`, + }, + { + cty.TupleVal([]cty.Value{ + cty.StringVal("b"), + }), + `[ + "b", +]`, + }, + { + cty.TupleVal([]cty.Value{ + cty.StringVal("b"), + cty.StringVal("d"), + }), + `[ + "b", + "d", +]`, + }, + { + cty.ListValEmpty(cty.String), + `tolist([])`, + }, + { + cty.SetValEmpty(cty.String), + `toset([])`, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%#v", test.Val), func(t *testing.T) { + got := FormatValue(test.Val, 0) + if got != test.Want { + t.Errorf("wrong result\nvalue: %#v\ngot: %s\nwant: %s", test.Val, got, test.Want) + } + }) + } +} diff --git a/repl/session.go b/repl/session.go index f24f3948d..1cb7028a3 100644 --- a/repl/session.go +++ b/repl/session.go @@ -2,14 +2,12 @@ package repl import ( "errors" - "fmt" "strings" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/hashicorp/terraform/configs/hcl2shim" "github.com/hashicorp/terraform/lang" "github.com/hashicorp/terraform/tfdiags" ) @@ -61,25 +59,7 @@ func (s *Session) handleEval(line string) (string, tfdiags.Diagnostics) { return "", diags } - if !val.IsWhollyKnown() { - // FIXME: In future, once we've updated the result formatter to be - // cty-aware, we should just include unknown values as "(not yet known)" - // in the serialized result, allowing the rest (if any) to be seen. - diags = diags.Append(fmt.Errorf("Result depends on values that cannot be determined until after \"terraform apply\".")) - return "", diags - } - - // Our formatter still wants an old-style raw interface{} value, so - // for now we'll just shim it. - // FIXME: Port the formatter to work with cty.Value directly. - legacyVal := hcl2shim.ConfigValueFromHCL2(val) - result, err := FormatResult(legacyVal) - if err != nil { - diags = diags.Append(err) - return "", diags - } - - return result, diags + return FormatValue(val, 0), diags } func (s *Session) handleHelp() (string, tfdiags.Diagnostics) { diff --git a/repl/session_test.go b/repl/session_test.go index 27eb10567..a49708276 100644 --- a/repl/session_test.go +++ b/repl/session_test.go @@ -73,7 +73,7 @@ func TestSession_basicState(t *testing.T) { Inputs: []testSessionInput{ { Input: "test_instance.foo.id", - Output: "bar", + Output: `"bar"`, }, }, })