From 2d66eee8722e683a51c6b8f3af6be04b38f25558 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 9 Jan 2023 17:15:17 +0100 Subject: [PATCH] Structured Plan Renderer: Address comments raised in previous PRs (#32478) * Address comments raised in previous PRs * add doc comments for public types --- internal/command/format/diff.go | 2 ++ internal/command/jsonformat/change/block.go | 10 +++----- internal/command/jsonformat/change/change.go | 4 +-- internal/command/jsonformat/change/list.go | 12 +++------ internal/command/jsonformat/change/map.go | 4 +-- internal/command/jsonformat/change/object.go | 4 +-- internal/command/jsonformat/change/set.go | 4 +-- .../command/jsonformat/differ/attribute.go | 10 ++++---- internal/command/jsonformat/differ/block.go | 16 ++++++------ internal/command/jsonformat/differ/list.go | 6 ++--- internal/command/jsonformat/differ/map.go | 6 ++--- internal/command/jsonformat/differ/object.go | 4 +-- internal/command/jsonformat/differ/output.go | 13 ++-------- internal/command/jsonformat/differ/set.go | 6 ++--- internal/command/jsonformat/differ/types.go | 25 +++++++++++++++++++ 15 files changed, 68 insertions(+), 58 deletions(-) create mode 100644 internal/command/jsonformat/differ/types.go diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index 8b4e65aed8..77f8f0ace7 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -2014,6 +2014,8 @@ func DiffActionSymbol(action plans.Action) string { return " [cyan]<=[reset]" case plans.Update: return " [yellow]~[reset]" + case plans.NoOp: + return " " default: return " ?" } diff --git a/internal/command/jsonformat/change/block.go b/internal/command/jsonformat/change/block.go index 5eab3aad61..0e54183bbe 100644 --- a/internal/command/jsonformat/change/block.go +++ b/internal/command/jsonformat/change/block.go @@ -55,10 +55,6 @@ func (renderer blockRenderer) Render(change Change, indent int, opts RenderOpts) buf.WriteString(fmt.Sprintf("{%s\n", change.forcesReplacement())) for _, importantKey := range importantAttributes { if attribute, ok := renderer.attributes[importantKey]; ok { - if attribute.action == plans.NoOp { - buf.WriteString(fmt.Sprintf("%s%s %-*s = %s\n", change.indent(indent+1), attribute.emptySymbol(), renderer.maximumKeyLen, importantKey, attribute.Render(indent+1, opts))) - continue - } buf.WriteString(fmt.Sprintf("%s%s %-*s = %s\n", change.indent(indent+1), format.DiffActionSymbol(attribute.action), renderer.maximumKeyLen, importantKey, attribute.Render(indent+1, opts))) } } @@ -86,7 +82,7 @@ func (renderer blockRenderer) Render(change Change, indent int, opts RenderOpts) } if unchangedAttributes > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("attribute", unchangedAttributes))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("attribute", unchangedAttributes))) } var blockKeys []string @@ -118,9 +114,9 @@ func (renderer blockRenderer) Render(change Change, indent int, opts RenderOpts) } if unchangedBlocks > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("block", unchangedBlocks))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("block", unchangedBlocks))) } - buf.WriteString(fmt.Sprintf("%s%s }", change.indent(indent), change.emptySymbol())) + buf.WriteString(fmt.Sprintf("%s%s }", change.indent(indent), format.DiffActionSymbol(plans.NoOp))) return buf.String() } diff --git a/internal/command/jsonformat/change/change.go b/internal/command/jsonformat/change/change.go index eab85b0321..197d1a9c6a 100644 --- a/internal/command/jsonformat/change/change.go +++ b/internal/command/jsonformat/change/change.go @@ -61,8 +61,8 @@ func (change Change) Warnings(indent int) []string { return change.renderer.Warnings(change, indent) } -// GetAction returns the plans.Action that this change describes. -func (change Change) GetAction() plans.Action { +// Action returns the plans.Action that this change describes. +func (change Change) Action() plans.Action { return change.action } diff --git a/internal/command/jsonformat/change/list.go b/internal/command/jsonformat/change/list.go index c900c5b48d..9045891bc8 100644 --- a/internal/command/jsonformat/change/list.go +++ b/internal/command/jsonformat/change/list.go @@ -67,7 +67,7 @@ func (renderer listRenderer) Render(change Change, indent int, opts RenderOpts) // minus 1 as the most recent unchanged element will be printed out // in full. if len(unchangedElements) > 1 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("element", len(unchangedElements)-1))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("element", len(unchangedElements)-1))) } // If our list of unchanged elements contains at least one entry, // we're going to print out the most recent change in full. That's @@ -95,11 +95,7 @@ func (renderer listRenderer) Render(change Change, indent int, opts RenderOpts) for _, warning := range element.Warnings(indent + 1) { buf.WriteString(fmt.Sprintf("%s%s\n", change.indent(indent+1), warning)) } - if element.action == plans.NoOp { - buf.WriteString(fmt.Sprintf("%s%s %s,\n", change.indent(indent+1), element.emptySymbol(), element.Render(indent+1, unchangedElementOpts))) - } else { - buf.WriteString(fmt.Sprintf("%s%s %s,\n", change.indent(indent+1), format.DiffActionSymbol(element.action), element.Render(indent+1, elementOpts))) - } + buf.WriteString(fmt.Sprintf("%s%s %s,\n", change.indent(indent+1), format.DiffActionSymbol(element.action), element.Render(indent+1, elementOpts))) } // If we were not displaying any context alongside our changes then the @@ -109,9 +105,9 @@ func (renderer listRenderer) Render(change Change, indent int, opts RenderOpts) // If we were displaying context, then this will contain any unchanged // elements since our last change, so we should also print it out. if len(unchangedElements) > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("element", len(unchangedElements)))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("element", len(unchangedElements)))) } - buf.WriteString(fmt.Sprintf("%s%s ]%s", change.indent(indent), change.emptySymbol(), change.nullSuffix(opts.overrideNullSuffix))) + buf.WriteString(fmt.Sprintf("%s%s ]%s", change.indent(indent), format.DiffActionSymbol(plans.NoOp), change.nullSuffix(opts.overrideNullSuffix))) return buf.String() } diff --git a/internal/command/jsonformat/change/map.go b/internal/command/jsonformat/change/map.go index 968f83b20e..28d27870cb 100644 --- a/internal/command/jsonformat/change/map.go +++ b/internal/command/jsonformat/change/map.go @@ -75,9 +75,9 @@ func (renderer mapRenderer) Render(change Change, indent int, opts RenderOpts) s } if unchangedElements > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("element", unchangedElements))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("element", unchangedElements))) } - buf.WriteString(fmt.Sprintf("%s%s }%s", change.indent(indent), change.emptySymbol(), change.nullSuffix(opts.overrideNullSuffix))) + buf.WriteString(fmt.Sprintf("%s%s }%s", change.indent(indent), format.DiffActionSymbol(plans.NoOp), change.nullSuffix(opts.overrideNullSuffix))) return buf.String() } diff --git a/internal/command/jsonformat/change/object.go b/internal/command/jsonformat/change/object.go index 294ca5ed74..f4814a692d 100644 --- a/internal/command/jsonformat/change/object.go +++ b/internal/command/jsonformat/change/object.go @@ -80,9 +80,9 @@ func (renderer objectRenderer) Render(change Change, indent int, opts RenderOpts } if unchangedAttributes > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("attribute", unchangedAttributes))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("attribute", unchangedAttributes))) } - buf.WriteString(fmt.Sprintf("%s%s }%s", change.indent(indent), change.emptySymbol(), change.nullSuffix(opts.overrideNullSuffix))) + buf.WriteString(fmt.Sprintf("%s%s }%s", change.indent(indent), format.DiffActionSymbol(plans.NoOp), change.nullSuffix(opts.overrideNullSuffix))) return buf.String() } diff --git a/internal/command/jsonformat/change/set.go b/internal/command/jsonformat/change/set.go index 997d39ce06..f5686f0653 100644 --- a/internal/command/jsonformat/change/set.go +++ b/internal/command/jsonformat/change/set.go @@ -45,9 +45,9 @@ func (renderer setRenderer) Render(change Change, indent int, opts RenderOpts) s } if unchangedElements > 0 { - buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), change.emptySymbol(), change.unchanged("element", unchangedElements))) + buf.WriteString(fmt.Sprintf("%s%s %s\n", change.indent(indent+1), format.DiffActionSymbol(plans.NoOp), change.unchanged("element", unchangedElements))) } - buf.WriteString(fmt.Sprintf("%s%s ]%s", change.indent(indent), change.emptySymbol(), change.nullSuffix(opts.overrideNullSuffix))) + buf.WriteString(fmt.Sprintf("%s%s ]%s", change.indent(indent), format.DiffActionSymbol(plans.NoOp), change.nullSuffix(opts.overrideNullSuffix))) return buf.String() } diff --git a/internal/command/jsonformat/differ/attribute.go b/internal/command/jsonformat/differ/attribute.go index f544894d76..d8d719a6e7 100644 --- a/internal/command/jsonformat/differ/attribute.go +++ b/internal/command/jsonformat/differ/attribute.go @@ -24,14 +24,14 @@ func (v Value) computeChangeForNestedAttribute(nested *jsonprovider.NestedType) return computed } - switch nested.NestingMode { - case "single", "group": + switch NestingMode(nested.NestingMode) { + case nestingModeSingle, nestingModeGroup: return v.computeAttributeChangeAsNestedObject(nested.Attributes) - case "map": + case nestingModeMap: return v.computeAttributeChangeAsNestedMap(nested.Attributes) - case "list": + case nestingModeList: return v.computeAttributeChangeAsNestedList(nested.Attributes) - case "set": + case nestingModeSet: return v.computeAttributeChangeAsNestedSet(nested.Attributes) default: panic("unrecognized nesting mode: " + nested.NestingMode) diff --git a/internal/command/jsonformat/differ/block.go b/internal/command/jsonformat/differ/block.go index b114910f1f..eacf68d170 100644 --- a/internal/command/jsonformat/differ/block.go +++ b/internal/command/jsonformat/differ/block.go @@ -23,13 +23,13 @@ func (v Value) ComputeChangeForBlock(block *jsonprovider.Block) change.Change { for key, attr := range block.Attributes { childValue := blockValue.getChild(key) childChange := childValue.ComputeChangeForAttribute(attr) - if childChange.GetAction() == plans.NoOp && childValue.Before == nil && childValue.After == nil { + if childChange.Action() == plans.NoOp && childValue.Before == nil && childValue.After == nil { // Don't record nil values at all in blocks. continue } attributes[key] = childChange - current = compareActions(current, childChange.GetAction()) + current = compareActions(current, childChange.Action()) } blocks := make(map[string][]change.Change) @@ -48,16 +48,16 @@ func (v Value) ComputeChangeForBlock(block *jsonprovider.Block) change.Change { } func (v Value) computeChangesForBlockType(blockType *jsonprovider.BlockType) ([]change.Change, plans.Action) { - switch blockType.NestingMode { - case "set": + switch NestingMode(blockType.NestingMode) { + case nestingModeSet: return v.computeBlockChangesAsSet(blockType.Block) - case "list": + case nestingModeList: return v.computeBlockChangesAsList(blockType.Block) - case "map": + case nestingModeMap: return v.computeBlockChangesAsMap(blockType.Block) - case "single", "group": + case nestingModeSingle, nestingModeGroup: ch := v.ComputeChangeForBlock(blockType.Block) - return []change.Change{ch}, ch.GetAction() + return []change.Change{ch}, ch.Action() default: panic("unrecognized nesting mode: " + blockType.NestingMode) } diff --git a/internal/command/jsonformat/differ/list.go b/internal/command/jsonformat/differ/list.go index 32040d9bb6..9dfb175449 100644 --- a/internal/command/jsonformat/differ/list.go +++ b/internal/command/jsonformat/differ/list.go @@ -16,7 +16,7 @@ func (v Value) computeAttributeChangeAsList(elementType cty.Type) change.Change v.processList(elementType.IsObjectType(), func(value Value) { element := value.computeChangeForType(elementType) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.List(elements), current, v.replacePath()) } @@ -30,7 +30,7 @@ func (v Value) computeAttributeChangeAsNestedList(attributes map[string]*jsonpro NestingMode: "single", }) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.NestedList(elements), current, v.replacePath()) } @@ -41,7 +41,7 @@ func (v Value) computeBlockChangesAsList(block *jsonprovider.Block) ([]change.Ch v.processNestedList(func(value Value) { element := value.ComputeChangeForBlock(block) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return elements, current } diff --git a/internal/command/jsonformat/differ/map.go b/internal/command/jsonformat/differ/map.go index b2563f16aa..3593b3e5c1 100644 --- a/internal/command/jsonformat/differ/map.go +++ b/internal/command/jsonformat/differ/map.go @@ -14,7 +14,7 @@ func (v Value) computeAttributeChangeAsMap(elementType cty.Type) change.Change { v.processMap(func(key string, value Value) { element := value.computeChangeForType(elementType) elements[key] = element - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.Map(elements), current, v.replacePath()) } @@ -28,7 +28,7 @@ func (v Value) computeAttributeChangeAsNestedMap(attributes map[string]*jsonprov NestingMode: "single", }) elements[key] = element - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.Map(elements), current, v.replacePath()) } @@ -39,7 +39,7 @@ func (v Value) computeBlockChangesAsMap(block *jsonprovider.Block) ([]change.Cha v.processMap(func(key string, value Value) { element := value.ComputeChangeForBlock(block) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return elements, current } diff --git a/internal/command/jsonformat/differ/object.go b/internal/command/jsonformat/differ/object.go index b56088ab5f..0b92b16d8c 100644 --- a/internal/command/jsonformat/differ/object.go +++ b/internal/command/jsonformat/differ/object.go @@ -48,13 +48,13 @@ func processObject[T any](v Value, attributes map[string]T, computeChange func(V // We use the generic ComputeChange here, as we don't know whether this // is from a nested object or a `normal` object. attributeChange := computeChange(attributeValue, attribute) - if attributeChange.GetAction() == plans.NoOp && attributeValue.Before == nil && attributeValue.After == nil { + if attributeChange.Action() == plans.NoOp && attributeValue.Before == nil && attributeValue.After == nil { // We skip attributes of objects that are null both before and // after. We don't even count these as unchanged attributes. continue } attributeChanges[key] = attributeChange - currentAction = compareActions(currentAction, attributeChange.GetAction()) + currentAction = compareActions(currentAction, attributeChange.Action()) } return attributeChanges, currentAction diff --git a/internal/command/jsonformat/differ/output.go b/internal/command/jsonformat/differ/output.go index 238feb1904..c6a059e53c 100644 --- a/internal/command/jsonformat/differ/output.go +++ b/internal/command/jsonformat/differ/output.go @@ -9,15 +9,6 @@ import ( "github.com/hashicorp/terraform/internal/plans" ) -const ( - jsonNumber = "number" - jsonObject = "object" - jsonArray = "array" - jsonBool = "bool" - jsonString = "string" - jsonNull = "null" -) - func (v Value) ComputeChangeForOutput() change.Change { if sensitive, ok := v.checkForSensitive(); ok { return sensitive @@ -30,7 +21,7 @@ func (v Value) ComputeChangeForOutput() change.Change { beforeType := getJsonType(v.Before) afterType := getJsonType(v.After) - valueToAttribute := func(v Value, jsonType string) change.Change { + valueToAttribute := func(v Value, jsonType JsonType) change.Change { var res change.Change switch jsonType { @@ -75,7 +66,7 @@ func (v Value) ComputeChangeForOutput() change.Change { return change.New(change.TypeChange(before, after), plans.Update, false) } -func getJsonType(json interface{}) string { +func getJsonType(json interface{}) JsonType { switch json.(type) { case []interface{}: return jsonArray diff --git a/internal/command/jsonformat/differ/set.go b/internal/command/jsonformat/differ/set.go index 28308d2942..95c892e4d5 100644 --- a/internal/command/jsonformat/differ/set.go +++ b/internal/command/jsonformat/differ/set.go @@ -16,7 +16,7 @@ func (v Value) computeAttributeChangeAsSet(elementType cty.Type) change.Change { v.processSet(false, func(value Value) { element := value.computeChangeForType(elementType) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.Set(elements), current, v.replacePath()) } @@ -30,7 +30,7 @@ func (v Value) computeAttributeChangeAsNestedSet(attributes map[string]*jsonprov NestingMode: "single", }) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return change.New(change.Set(elements), current, v.replacePath()) } @@ -41,7 +41,7 @@ func (v Value) computeBlockChangesAsSet(block *jsonprovider.Block) ([]change.Cha v.processSet(true, func(value Value) { element := value.ComputeChangeForBlock(block) elements = append(elements, element) - current = compareActions(current, element.GetAction()) + current = compareActions(current, element.Action()) }) return elements, current } diff --git a/internal/command/jsonformat/differ/types.go b/internal/command/jsonformat/differ/types.go new file mode 100644 index 0000000000..487791f5b6 --- /dev/null +++ b/internal/command/jsonformat/differ/types.go @@ -0,0 +1,25 @@ +package differ + +// JsonType is a wrapper around a string type to describe the various different +// kinds of JSON types. This is used when processing dynamic types and outputs. +type JsonType string + +// NestingMode is a wrapper around a string type to describe the various +// different kinds of nesting modes that can be applied to nested blocks and +// objects. +type NestingMode string + +const ( + jsonNumber JsonType = "number" + jsonObject JsonType = "object" + jsonArray JsonType = "array" + jsonBool JsonType = "bool" + jsonString JsonType = "string" + jsonNull JsonType = "null" + + nestingModeSet NestingMode = "set" + nestingModeList NestingMode = "list" + nestingModeMap NestingMode = "map" + nestingModeSingle NestingMode = "single" + nestingModeGroup NestingMode = "group" +)