views: Fix missing source in diagnostic output

The previous implementation of views was copying and embedding the base
View struct in each individual view. While this allowed for easy access
to the interface of that struct (both in the view and externally), it more
importantly completely broke the ability of the diagnostic printer to
output source code snippets.

This is because the `configSources` field on the base view is lazily set
after the config loader is initialized. In the commands ported to use
views, this happens after the base View struct is copied, so we are
updating the wrong copy of the struct.

This commit fixes this with a simple mechanical refactor: keep a pointer
to the base View struct instead, and update all of the individual views
to explicitly refer to that struct to access its fields and methods.

This is not a particularly satisfying solution, but I can't find
anything clearly better. It might be worth exploring the alternative
approach in the view for the new test command, which explicitly pulls
its dependencies out of the base view, rather than retaining a full
reference. Maybe there's a third way which is better still.
This commit is contained in:
Alisdair McDiarmid 2021-02-26 16:31:12 -05:00
parent b5adc33075
commit 86495f93cb
9 changed files with 97 additions and 58 deletions

View File

@ -30,7 +30,7 @@ func (c *PlanCommand) Run(rawArgs []string) int {
if diags.HasErrors() { if diags.HasErrors() {
view.Diagnostics(diags) view.Diagnostics(diags)
view.HelpPrompt("plan") view.HelpPrompt()
return 1 return 1
} }

View File

@ -543,6 +543,9 @@ func TestPlan_validate(t *testing.T) {
if want := "Error: Invalid count argument"; !strings.Contains(actual, want) { if want := "Error: Invalid count argument"; !strings.Contains(actual, want) {
t.Fatalf("unexpected error output\ngot:\n%s\n\nshould contain: %s", actual, want) t.Fatalf("unexpected error output\ngot:\n%s\n\nshould contain: %s", actual, want)
} }
if want := "9: count = timestamp()"; !strings.Contains(actual, want) {
t.Fatalf("unexpected error output\ngot:\n%s\n\nshould contain: %s", actual, want)
}
} }
func TestPlan_vars(t *testing.T) { func TestPlan_vars(t *testing.T) {

View File

@ -31,7 +31,7 @@ func (c *RefreshCommand) Run(rawArgs []string) int {
if diags.HasErrors() { if diags.HasErrors() {
view.Diagnostics(diags) view.Diagnostics(diags)
view.HelpPrompt("refresh") view.HelpPrompt()
return 1 return 1
} }

View File

@ -27,7 +27,7 @@ func NewApply(vt arguments.ViewType, destroy bool, runningInAutomation bool, vie
switch vt { switch vt {
case arguments.ViewHuman: case arguments.ViewHuman:
return &ApplyHuman{ return &ApplyHuman{
View: *view, view: view,
destroy: destroy, destroy: destroy,
inAutomation: runningInAutomation, inAutomation: runningInAutomation,
countHook: &countHook{}, countHook: &countHook{},
@ -40,7 +40,7 @@ func NewApply(vt arguments.ViewType, destroy bool, runningInAutomation bool, vie
// The ApplyHuman implementation renders human-readable text logs, suitable for // The ApplyHuman implementation renders human-readable text logs, suitable for
// a scrolling terminal. // a scrolling terminal.
type ApplyHuman struct { type ApplyHuman struct {
View view *View
destroy bool destroy bool
inAutomation bool inAutomation bool
@ -52,48 +52,52 @@ var _ Apply = (*ApplyHuman)(nil)
func (v *ApplyHuman) ResourceCount(stateOutPath string) { func (v *ApplyHuman) ResourceCount(stateOutPath string) {
if v.destroy { if v.destroy {
v.streams.Printf( v.view.streams.Printf(
v.colorize.Color("[reset][bold][green]\nDestroy complete! Resources: %d destroyed.\n"), v.view.colorize.Color("[reset][bold][green]\nDestroy complete! Resources: %d destroyed.\n"),
v.countHook.Removed, v.countHook.Removed,
) )
} else { } else {
v.streams.Printf( v.view.streams.Printf(
v.colorize.Color("[reset][bold][green]\nApply complete! Resources: %d added, %d changed, %d destroyed.\n"), v.view.colorize.Color("[reset][bold][green]\nApply complete! Resources: %d added, %d changed, %d destroyed.\n"),
v.countHook.Added, v.countHook.Added,
v.countHook.Changed, v.countHook.Changed,
v.countHook.Removed, v.countHook.Removed,
) )
} }
if (v.countHook.Added > 0 || v.countHook.Changed > 0) && stateOutPath != "" { if (v.countHook.Added > 0 || v.countHook.Changed > 0) && stateOutPath != "" {
v.streams.Printf("\n%s\n\n", format.WordWrap(stateOutPathPostApply, v.View.outputColumns())) v.view.streams.Printf("\n%s\n\n", format.WordWrap(stateOutPathPostApply, v.view.outputColumns()))
v.streams.Printf("State path: %s\n", stateOutPath) v.view.streams.Printf("State path: %s\n", stateOutPath)
} }
} }
func (v *ApplyHuman) Outputs(outputValues map[string]*states.OutputValue) { func (v *ApplyHuman) Outputs(outputValues map[string]*states.OutputValue) {
if len(outputValues) > 0 { if len(outputValues) > 0 {
v.streams.Print(v.colorize.Color("[reset][bold][green]\nOutputs:\n\n")) v.view.streams.Print(v.view.colorize.Color("[reset][bold][green]\nOutputs:\n\n"))
NewOutput(arguments.ViewHuman, &v.View).Output("", outputValues) NewOutput(arguments.ViewHuman, v.view).Output("", outputValues)
} }
} }
func (v *ApplyHuman) Operation() Operation { func (v *ApplyHuman) Operation() Operation {
return NewOperation(arguments.ViewHuman, v.inAutomation, &v.View) return NewOperation(arguments.ViewHuman, v.inAutomation, v.view)
} }
func (v *ApplyHuman) Hooks() []terraform.Hook { func (v *ApplyHuman) Hooks() []terraform.Hook {
return []terraform.Hook{ return []terraform.Hook{
v.countHook, v.countHook,
NewUiHook(&v.View), NewUiHook(v.view),
} }
} }
func (v *ApplyHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
func (v *ApplyHuman) HelpPrompt() { func (v *ApplyHuman) HelpPrompt() {
command := "apply" command := "apply"
if v.destroy { if v.destroy {
command = "destroy" command = "destroy"
} }
v.View.HelpPrompt(command) v.view.HelpPrompt(command)
} }
const stateOutPathPostApply = "The state of your infrastructure has been saved to the path below. This state is required to modify and destroy your infrastructure, so keep it safe. To inspect the complete state use the `terraform show` command." const stateOutPathPostApply = "The state of your infrastructure has been saved to the path below. This state is required to modify and destroy your infrastructure, so keep it safe. To inspect the complete state use the `terraform show` command."

View File

@ -32,14 +32,14 @@ type Operation interface {
func NewOperation(vt arguments.ViewType, inAutomation bool, view *View) Operation { func NewOperation(vt arguments.ViewType, inAutomation bool, view *View) Operation {
switch vt { switch vt {
case arguments.ViewHuman: case arguments.ViewHuman:
return &OperationHuman{View: *view, inAutomation: inAutomation} return &OperationHuman{view: view, inAutomation: inAutomation}
default: default:
panic(fmt.Sprintf("unknown view type %v", vt)) panic(fmt.Sprintf("unknown view type %v", vt))
} }
} }
type OperationHuman struct { type OperationHuman struct {
View view *View
// inAutomation indicates that commands are being run by an // inAutomation indicates that commands are being run by an
// automated system rather than directly at a command prompt. // automated system rather than directly at a command prompt.
@ -54,11 +54,11 @@ type OperationHuman struct {
var _ Operation = (*OperationHuman)(nil) var _ Operation = (*OperationHuman)(nil)
func (v *OperationHuman) Interrupted() { func (v *OperationHuman) Interrupted() {
v.streams.Println(format.WordWrap(interrupted, v.outputColumns())) v.view.streams.Println(format.WordWrap(interrupted, v.view.outputColumns()))
} }
func (v *OperationHuman) FatalInterrupt() { func (v *OperationHuman) FatalInterrupt() {
v.streams.Eprintln(format.WordWrap(fatalInterrupt, v.errorColumns())) v.view.streams.Eprintln(format.WordWrap(fatalInterrupt, v.view.errorColumns()))
} }
const fatalInterrupt = ` const fatalInterrupt = `
@ -72,14 +72,14 @@ Gracefully shutting down...
` `
func (v *OperationHuman) Stopping() { func (v *OperationHuman) Stopping() {
v.streams.Println("Stopping operation...") v.view.streams.Println("Stopping operation...")
} }
func (v *OperationHuman) Cancelled(destroy bool) { func (v *OperationHuman) Cancelled(destroy bool) {
if destroy { if destroy {
v.streams.Println("Destroy cancelled.") v.view.streams.Println("Destroy cancelled.")
} else { } else {
v.streams.Println("Apply cancelled.") v.view.streams.Println("Apply cancelled.")
} }
} }
@ -89,17 +89,17 @@ func (v *OperationHuman) EmergencyDumpState(stateFile *statefile.File) error {
if jsonErr != nil { if jsonErr != nil {
return jsonErr return jsonErr
} }
v.streams.Eprintln(stateBuf) v.view.streams.Eprintln(stateBuf)
return nil return nil
} }
func (v *OperationHuman) PlanNoChanges() { func (v *OperationHuman) PlanNoChanges() {
v.streams.Println("\n" + v.colorize.Color(strings.TrimSpace(planNoChanges))) v.view.streams.Println("\n" + v.view.colorize.Color(strings.TrimSpace(planNoChanges)))
v.streams.Println("\n" + strings.TrimSpace(format.WordWrap(planNoChangesDetail, v.outputColumns()))) v.view.streams.Println("\n" + strings.TrimSpace(format.WordWrap(planNoChangesDetail, v.view.outputColumns())))
} }
func (v *OperationHuman) Plan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas) { func (v *OperationHuman) Plan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas) {
renderPlan(plan, baseState, schemas, &v.View) renderPlan(plan, baseState, schemas, v.view)
} }
// PlanNextStep gives the user some next-steps, unless we're running in an // PlanNextStep gives the user some next-steps, unless we're running in an
@ -108,20 +108,24 @@ func (v *OperationHuman) PlanNextStep(planPath string) {
if v.inAutomation { if v.inAutomation {
return return
} }
v.outputHorizRule() v.view.outputHorizRule()
if planPath == "" { if planPath == "" {
v.streams.Print( v.view.streams.Print(
"\n" + strings.TrimSpace(format.WordWrap(planHeaderNoOutput, v.outputColumns())) + "\n", "\n" + strings.TrimSpace(format.WordWrap(planHeaderNoOutput, v.view.outputColumns())) + "\n",
) )
} else { } else {
v.streams.Printf( v.view.streams.Printf(
"\n"+strings.TrimSpace(format.WordWrap(planHeaderYesOutput, v.outputColumns()))+"\n", "\n"+strings.TrimSpace(format.WordWrap(planHeaderYesOutput, v.view.outputColumns()))+"\n",
planPath, planPath, planPath, planPath,
) )
} }
} }
func (v *OperationHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
const planNoChanges = ` const planNoChanges = `
[reset][bold][green]No changes. Infrastructure is up-to-date.[reset][green] [reset][bold][green]No changes. Infrastructure is up-to-date.[reset][green]
` `

View File

@ -28,11 +28,11 @@ type Output interface {
func NewOutput(vt arguments.ViewType, view *View) Output { func NewOutput(vt arguments.ViewType, view *View) Output {
switch vt { switch vt {
case arguments.ViewJSON: case arguments.ViewJSON:
return &OutputJSON{View: *view} return &OutputJSON{view: view}
case arguments.ViewRaw: case arguments.ViewRaw:
return &OutputRaw{View: *view} return &OutputRaw{view: view}
case arguments.ViewHuman: case arguments.ViewHuman:
return &OutputHuman{View: *view} return &OutputHuman{view: view}
default: default:
panic(fmt.Sprintf("unknown view type %v", vt)) panic(fmt.Sprintf("unknown view type %v", vt))
} }
@ -41,7 +41,7 @@ func NewOutput(vt arguments.ViewType, view *View) Output {
// The OutputHuman implementation renders outputs in a format equivalent to HCL // The OutputHuman implementation renders outputs in a format equivalent to HCL
// source. This uses the same formatting logic as in the console REPL. // source. This uses the same formatting logic as in the console REPL.
type OutputHuman struct { type OutputHuman struct {
View view *View
} }
var _ Output = (*OutputHuman)(nil) var _ Output = (*OutputHuman)(nil)
@ -61,7 +61,7 @@ func (v *OutputHuman) Output(name string, outputs map[string]*states.OutputValue
return diags return diags
} }
result := repl.FormatValue(output.Value, 0) result := repl.FormatValue(output.Value, 0)
v.streams.Println(result) v.view.streams.Println(result)
return nil return nil
} }
@ -90,17 +90,21 @@ func (v *OutputHuman) Output(name string, outputs map[string]*states.OutputValue
} }
} }
v.streams.Println(strings.TrimSpace(outputBuf.String())) v.view.streams.Println(strings.TrimSpace(outputBuf.String()))
return nil return nil
} }
func (v *OutputHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
// The OutputRaw implementation renders single string, number, or boolean // The OutputRaw implementation renders single string, number, or boolean
// output values directly and without quotes or other formatting. This is // output values directly and without quotes or other formatting. This is
// intended for use in shell scripting or other environments where the exact // intended for use in shell scripting or other environments where the exact
// type of an output value is not important. // type of an output value is not important.
type OutputRaw struct { type OutputRaw struct {
View view *View
// Unit tests may set rawPrint to capture the output from the Output // Unit tests may set rawPrint to capture the output from the Output
// method, which would normally go to stdout directly. // method, which would normally go to stdout directly.
@ -168,16 +172,20 @@ func (v *OutputRaw) Output(name string, outputs map[string]*states.OutputValue)
// If we get out here then we should have a valid string to print. // If we get out here then we should have a valid string to print.
// We're writing it using Print here so that a shell caller will get // We're writing it using Print here so that a shell caller will get
// exactly the value and no extra whitespace (including trailing newline). // exactly the value and no extra whitespace (including trailing newline).
v.streams.Print(strV.AsString()) v.view.streams.Print(strV.AsString())
return nil return nil
} }
func (v *OutputRaw) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
// The OutputJSON implementation renders outputs as JSON values. When rendering // The OutputJSON implementation renders outputs as JSON values. When rendering
// a single output, only the value is displayed. When rendering all outputs, // a single output, only the value is displayed. When rendering all outputs,
// the result is a JSON object with keys matching the output names and object // the result is a JSON object with keys matching the output names and object
// values including type and sensitivity metadata. // values including type and sensitivity metadata.
type OutputJSON struct { type OutputJSON struct {
View view *View
} }
var _ Output = (*OutputJSON)(nil) var _ Output = (*OutputJSON)(nil)
@ -199,7 +207,7 @@ func (v *OutputJSON) Output(name string, outputs map[string]*states.OutputValue)
return diags return diags
} }
v.streams.Println(string(jsonOutput)) v.view.streams.Println(string(jsonOutput))
return nil return nil
} }
@ -241,11 +249,15 @@ func (v *OutputJSON) Output(name string, outputs map[string]*states.OutputValue)
return diags return diags
} }
v.streams.Println(string(jsonOutputs)) v.view.streams.Println(string(jsonOutputs))
return nil return nil
} }
func (v *OutputJSON) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
// For text and raw output modes, an empty map of outputs is considered a // For text and raw output modes, an empty map of outputs is considered a
// separate and higher priority failure mode than an output not being present // separate and higher priority failure mode than an output not being present
// in a non-empty map. This warning diagnostic explains how this might have // in a non-empty map. This warning diagnostic explains how this might have

View File

@ -21,7 +21,7 @@ type Plan interface {
Hooks() []terraform.Hook Hooks() []terraform.Hook
Diagnostics(diags tfdiags.Diagnostics) Diagnostics(diags tfdiags.Diagnostics)
HelpPrompt(string) HelpPrompt()
} }
// NewPlan returns an initialized Plan implementation for the given ViewType. // NewPlan returns an initialized Plan implementation for the given ViewType.
@ -29,7 +29,7 @@ func NewPlan(vt arguments.ViewType, runningInAutomation bool, view *View) Plan {
switch vt { switch vt {
case arguments.ViewHuman: case arguments.ViewHuman:
return &PlanHuman{ return &PlanHuman{
View: *view, view: view,
inAutomation: runningInAutomation, inAutomation: runningInAutomation,
} }
default: default:
@ -40,7 +40,7 @@ func NewPlan(vt arguments.ViewType, runningInAutomation bool, view *View) Plan {
// The PlanHuman implementation renders human-readable text logs, suitable for // The PlanHuman implementation renders human-readable text logs, suitable for
// a scrolling terminal. // a scrolling terminal.
type PlanHuman struct { type PlanHuman struct {
View view *View
inAutomation bool inAutomation bool
} }
@ -48,15 +48,23 @@ type PlanHuman struct {
var _ Plan = (*PlanHuman)(nil) var _ Plan = (*PlanHuman)(nil)
func (v *PlanHuman) Operation() Operation { func (v *PlanHuman) Operation() Operation {
return NewOperation(arguments.ViewHuman, v.inAutomation, &v.View) return NewOperation(arguments.ViewHuman, v.inAutomation, v.view)
} }
func (v *PlanHuman) Hooks() []terraform.Hook { func (v *PlanHuman) Hooks() []terraform.Hook {
return []terraform.Hook{ return []terraform.Hook{
NewUiHook(&v.View), NewUiHook(v.view),
} }
} }
func (v *PlanHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
func (v *PlanHuman) HelpPrompt() {
v.view.HelpPrompt("plan")
}
// The plan renderer is used by the Operation view (for plan and apply // The plan renderer is used by the Operation view (for plan and apply
// commands) and the Show view (for the show command). // commands) and the Show view (for the show command).
func renderPlan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas, view *View) { func renderPlan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas, view *View) {

View File

@ -17,7 +17,7 @@ type Refresh interface {
Hooks() []terraform.Hook Hooks() []terraform.Hook
Diagnostics(diags tfdiags.Diagnostics) Diagnostics(diags tfdiags.Diagnostics)
HelpPrompt(command string) HelpPrompt()
} }
// NewRefresh returns an initialized Refresh implementation for the given ViewType. // NewRefresh returns an initialized Refresh implementation for the given ViewType.
@ -25,7 +25,7 @@ func NewRefresh(vt arguments.ViewType, runningInAutomation bool, view *View) Ref
switch vt { switch vt {
case arguments.ViewHuman: case arguments.ViewHuman:
return &RefreshHuman{ return &RefreshHuman{
View: *view, view: view,
inAutomation: runningInAutomation, inAutomation: runningInAutomation,
countHook: &countHook{}, countHook: &countHook{},
} }
@ -37,7 +37,7 @@ func NewRefresh(vt arguments.ViewType, runningInAutomation bool, view *View) Ref
// The RefreshHuman implementation renders human-readable text logs, suitable for // The RefreshHuman implementation renders human-readable text logs, suitable for
// a scrolling terminal. // a scrolling terminal.
type RefreshHuman struct { type RefreshHuman struct {
View view *View
inAutomation bool inAutomation bool
@ -48,18 +48,26 @@ var _ Refresh = (*RefreshHuman)(nil)
func (v *RefreshHuman) Outputs(outputValues map[string]*states.OutputValue) { func (v *RefreshHuman) Outputs(outputValues map[string]*states.OutputValue) {
if len(outputValues) > 0 { if len(outputValues) > 0 {
v.streams.Print(v.colorize.Color("[reset][bold][green]\nOutputs:\n\n")) v.view.streams.Print(v.view.colorize.Color("[reset][bold][green]\nOutputs:\n\n"))
NewOutput(arguments.ViewHuman, &v.View).Output("", outputValues) NewOutput(arguments.ViewHuman, v.view).Output("", outputValues)
} }
} }
func (v *RefreshHuman) Operation() Operation { func (v *RefreshHuman) Operation() Operation {
return NewOperation(arguments.ViewHuman, v.inAutomation, &v.View) return NewOperation(arguments.ViewHuman, v.inAutomation, v.view)
} }
func (v *RefreshHuman) Hooks() []terraform.Hook { func (v *RefreshHuman) Hooks() []terraform.Hook {
return []terraform.Hook{ return []terraform.Hook{
v.countHook, v.countHook,
NewUiHook(&v.View), NewUiHook(v.view),
} }
} }
func (v *RefreshHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
func (v *RefreshHuman) HelpPrompt() {
v.view.HelpPrompt("refresh")
}

View File

@ -17,7 +17,7 @@ type StateLocker interface {
func NewStateLocker(vt arguments.ViewType, view *View) StateLocker { func NewStateLocker(vt arguments.ViewType, view *View) StateLocker {
switch vt { switch vt {
case arguments.ViewHuman: case arguments.ViewHuman:
return &StateLockerHuman{View: *view} return &StateLockerHuman{view: view}
default: default:
panic(fmt.Sprintf("unknown view type %v", vt)) panic(fmt.Sprintf("unknown view type %v", vt))
} }
@ -26,15 +26,15 @@ func NewStateLocker(vt arguments.ViewType, view *View) StateLocker {
// StateLockerHuman is an implementation of StateLocker which prints status to // StateLockerHuman is an implementation of StateLocker which prints status to
// a terminal. // a terminal.
type StateLockerHuman struct { type StateLockerHuman struct {
View view *View
} }
var _ StateLocker = (*StateLockerHuman)(nil) var _ StateLocker = (*StateLockerHuman)(nil)
func (v *StateLockerHuman) Locking() { func (v *StateLockerHuman) Locking() {
v.streams.Println("Acquiring state lock. This may take a few moments...") v.view.streams.Println("Acquiring state lock. This may take a few moments...")
} }
func (v *StateLockerHuman) Unlocking() { func (v *StateLockerHuman) Unlocking() {
v.streams.Println("Releasing state lock. This may take a few moments...") v.view.streams.Println("Releasing state lock. This may take a few moments...")
} }