From 12ed26459711d258235507c37eba131010b0c930 Mon Sep 17 00:00:00 2001 From: Christian Mesh Date: Mon, 23 Sep 2024 07:31:06 -0400 Subject: [PATCH] Warning and Error consolidation CLI options (#1894) Signed-off-by: Hefeweizen Signed-off-by: Christian Mesh Signed-off-by: Syasusu Co-authored-by: Hefeweizen Co-authored-by: Syasusu --- internal/command/apply.go | 8 ++ internal/command/arguments/view.go | 20 ++- internal/command/arguments/view_test.go | 27 +++-- internal/command/console.go | 26 ++-- internal/command/format/diagnostic.go | 2 +- internal/command/format/diagnostic_test.go | 2 +- internal/command/import.go | 12 ++ internal/command/init.go | 12 ++ internal/command/meta.go | 44 +++++-- internal/command/plan.go | 8 ++ internal/command/refresh.go | 44 ++++--- internal/command/test.go | 12 ++ internal/command/validate.go | 12 ++ internal/command/views/view.go | 13 +- internal/tfdiags/consolidate_warnings.go | 114 ++++++++++-------- internal/tfdiags/consolidate_warnings_test.go | 60 ++++++++- 16 files changed, 308 insertions(+), 108 deletions(-) diff --git a/internal/command/apply.go b/internal/command/apply.go index f827b02d72..7789b3e18f 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -356,6 +356,14 @@ Options: accompanied by errors, show them in a more compact form that includes only the summary messages. + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -destroy Destroy OpenTofu-managed infrastructure. The command "tofu destroy" is a convenience alias for this option. diff --git a/internal/command/arguments/view.go b/internal/command/arguments/view.go index 3b6e11be26..f473f0bb53 100644 --- a/internal/command/arguments/view.go +++ b/internal/command/arguments/view.go @@ -14,7 +14,9 @@ type View struct { // CompactWarnings is used to coalesce duplicate warnings, to reduce the // level of noise when multiple instances of the same warning are raised // for a configuration. - CompactWarnings bool + CompactWarnings bool + ConsolidateWarnings bool + ConsolidateErrors bool // Concise is used to reduce the level of noise in the output and display // only the important details. @@ -28,7 +30,9 @@ type View struct { // possibly-modified slice of arguments. If any of the supported flags are // found, they will be removed from the slice. func ParseView(args []string) (*View, []string) { - common := &View{} + common := &View{ + ConsolidateWarnings: true, + } // Keep track of the length of the returned slice. When we find an // argument we support, i will not be incremented. @@ -39,6 +43,18 @@ func ParseView(args []string) (*View, []string) { common.NoColor = true case "-compact-warnings": common.CompactWarnings = true + case "-consolidate-warnings": + common.ConsolidateWarnings = true + case "-consolidate-warnings=true": + common.ConsolidateWarnings = true + case "-consolidate-warnings=false": + common.ConsolidateWarnings = false + case "-consolidate-errors": + common.ConsolidateErrors = true + case "-consolidate-errors=true": + common.ConsolidateErrors = true + case "-consolidate-errors=false": + common.ConsolidateErrors = false case "-concise": common.Concise = true default: diff --git a/internal/command/arguments/view_test.go b/internal/command/arguments/view_test.go index de55c556a6..ba239dc438 100644 --- a/internal/command/arguments/view_test.go +++ b/internal/command/arguments/view_test.go @@ -19,57 +19,62 @@ func TestParseView(t *testing.T) { }{ "nil": { nil, - &View{NoColor: false, CompactWarnings: false, Concise: false}, + &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false}, nil, }, "empty": { []string{}, - &View{NoColor: false, CompactWarnings: false, Concise: false}, + &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false}, []string{}, }, "none matching": { []string{"-foo", "bar", "-baz"}, - &View{NoColor: false, CompactWarnings: false, Concise: false}, + &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false}, []string{"-foo", "bar", "-baz"}, }, "no-color": { []string{"-foo", "-no-color", "-baz"}, - &View{NoColor: true, CompactWarnings: false, Concise: false}, + &View{NoColor: true, CompactWarnings: false, ConsolidateWarnings: true, Concise: false}, []string{"-foo", "-baz"}, }, "compact-warnings": { []string{"-foo", "-compact-warnings", "-baz"}, - &View{NoColor: false, CompactWarnings: true, Concise: false}, + &View{NoColor: false, CompactWarnings: true, ConsolidateWarnings: true, Concise: false}, []string{"-foo", "-baz"}, }, "concise": { []string{"-foo", "-concise", "-baz"}, - &View{NoColor: false, CompactWarnings: false, Concise: true}, + &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: true}, []string{"-foo", "-baz"}, }, "no-color and compact-warnings": { []string{"-foo", "-no-color", "-compact-warnings", "-baz"}, - &View{NoColor: true, CompactWarnings: true, Concise: false}, + &View{NoColor: true, CompactWarnings: true, ConsolidateWarnings: true, Concise: false}, []string{"-foo", "-baz"}, }, "no-color and concise": { []string{"-foo", "-no-color", "-concise", "-baz"}, - &View{NoColor: true, CompactWarnings: false, Concise: true}, + &View{NoColor: true, CompactWarnings: false, ConsolidateWarnings: true, Concise: true}, []string{"-foo", "-baz"}, }, "concise and compact-warnings": { []string{"-foo", "-concise", "-compact-warnings", "-baz"}, - &View{NoColor: false, CompactWarnings: true, Concise: true}, + &View{NoColor: false, CompactWarnings: true, ConsolidateWarnings: true, Concise: true}, []string{"-foo", "-baz"}, }, "all three": { []string{"-foo", "-no-color", "-compact-warnings", "-concise", "-baz"}, - &View{NoColor: true, CompactWarnings: true, Concise: true}, + &View{NoColor: true, CompactWarnings: true, ConsolidateWarnings: true, Concise: true}, []string{"-foo", "-baz"}, }, "all three, resulting in empty args": { []string{"-no-color", "-compact-warnings", "-concise"}, - &View{NoColor: true, CompactWarnings: true, Concise: true}, + &View{NoColor: true, CompactWarnings: true, ConsolidateWarnings: true, Concise: true}, + []string{}, + }, + "turn off warning consolidation": { + []string{"-consolidate-warnings=false"}, + &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: false, Concise: false}, []string{}, }, } diff --git a/internal/command/console.go b/internal/command/console.go index 1449ed38be..b8ee429af9 100644 --- a/internal/command/console.go +++ b/internal/command/console.go @@ -223,15 +223,27 @@ Usage: tofu [global options] console [options] Options: - -state=path Legacy option for the local backend only. See the local - backend's documentation for more information. + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. - -var 'foo=bar' Set a variable in the OpenTofu configuration. This - flag can be set multiple times. + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. - -var-file=foo Set variables in the OpenTofu configuration from - a file. If "terraform.tfvars" or any ".auto.tfvars" - files are present, they will be automatically loaded. + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + + -state=path Legacy option for the local backend only. See the local + backend's documentation for more information. + + -var 'foo=bar' Set a variable in the OpenTofu configuration. This + flag can be set multiple times. + + -var-file=foo Set variables in the OpenTofu configuration from + a file. If "terraform.tfvars" or any ".auto.tfvars" + files are present, they will be automatically loaded. ` return strings.TrimSpace(helpText) } diff --git a/internal/command/format/diagnostic.go b/internal/command/format/diagnostic.go index 1f53950b69..392b3189c3 100644 --- a/internal/command/format/diagnostic.go +++ b/internal/command/format/diagnostic.go @@ -187,7 +187,7 @@ func DiagnosticWarningsCompact(diags tfdiags.Diagnostics, color *colorstring.Col var b strings.Builder b.WriteString(color.Color("[bold][yellow]Warnings:[reset]\n\n")) for _, diag := range diags { - sources := tfdiags.WarningGroupSourceRanges(diag) + sources := tfdiags.ConsolidatedGroupSourceRanges(diag) b.WriteString(fmt.Sprintf("- %s\n", diag.Description().Summary)) if len(sources) > 0 { mainSource := sources[0] diff --git a/internal/command/format/diagnostic_test.go b/internal/command/format/diagnostic_test.go index 5b81ef7651..20e6aabeb1 100644 --- a/internal/command/format/diagnostic_test.go +++ b/internal/command/format/diagnostic_test.go @@ -634,7 +634,7 @@ func TestDiagnosticWarningsCompact(t *testing.T) { // ConsolidateWarnings groups together the ones // that have source location information and that // have the same summary text. - diags = diags.ConsolidateWarnings(1) + diags = diags.Consolidate(1, tfdiags.Warning) // A zero-value Colorize just passes all the formatting // codes back to us, so we can test them literally. diff --git a/internal/command/import.go b/internal/command/import.go index 529a4e521f..5b5384604e 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -316,6 +316,18 @@ Usage: tofu [global options] import [options] ADDR ID Options: + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. + + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -config=path Path to a directory of OpenTofu configuration files to use to configure the provider. Defaults to pwd. If no config files are present, they must be provided diff --git a/internal/command/init.go b/internal/command/init.go index c3ea51c233..e638ad9dea 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -1193,6 +1193,18 @@ Options: times. The backend type must be in the configuration itself. + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. + + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -force-copy Suppress prompts about copying state data when initializing a new state backend. This is equivalent to providing a "yes" to all confirmation diff --git a/internal/command/meta.go b/internal/command/meta.go index 18908dd9db..de0b92dcd6 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -252,16 +252,24 @@ type Meta struct { // // compactWarnings (-compact-warnings) selects a more compact presentation // of warnings in the output when they are not accompanied by errors. - statePath string - stateOutPath string - backupPath string - parallelism int - stateLock bool - stateLockTimeout time.Duration - forceInitCopy bool - reconfigure bool - migrateState bool - compactWarnings bool + // + // consolidateWarnings (-consolidate-warnings=false) disables consolodation + // of warnings in the output, printing all instances of a particular warning. + // + // consolidateErrors (-consolidate-errors=true) enables consolodation + // of errors in the output, printing a single instances of a particular warning. + statePath string + stateOutPath string + backupPath string + parallelism int + stateLock bool + stateLockTimeout time.Duration + forceInitCopy bool + reconfigure bool + migrateState bool + compactWarnings bool + consolidateWarnings bool + consolidateErrors bool // Used with commands which write state to allow users to write remote // state even if the remote and local OpenTofu versions don't match. @@ -611,6 +619,8 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { f.BoolVar(&m.input, "input", true, "input") f.Var((*FlagStringSlice)(&m.targetFlags), "target", "resource to target") f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings") + f.BoolVar(&m.consolidateWarnings, "consolidate-warnings", true, "consolidate warnings") + f.BoolVar(&m.consolidateErrors, "consolidate-errors", false, "consolidate errors") m.varFlagSet(f) @@ -661,8 +671,10 @@ func (m *Meta) process(args []string) []string { // views.View and cli.Ui during the migration phase. if m.View != nil { m.View.Configure(&arguments.View{ - CompactWarnings: m.compactWarnings, - NoColor: !m.Color, + CompactWarnings: m.compactWarnings, + ConsolidateWarnings: m.consolidateWarnings, + ConsolidateErrors: m.consolidateErrors, + NoColor: !m.Color, }) } @@ -707,6 +719,7 @@ func (m *Meta) confirm(opts *tofu.InputOpts) (bool, error) { // Internally this function uses Diagnostics.Append, and so it will panic // if given unsupported value types, just as Append does. func (m *Meta) showDiagnostics(vals ...interface{}) { + var diags tfdiags.Diagnostics diags = diags.Append(vals...) diags.Sort() @@ -723,7 +736,12 @@ func (m *Meta) showDiagnostics(vals ...interface{}) { outputWidth := m.ErrorColumns() - diags = diags.ConsolidateWarnings(1) + if m.consolidateWarnings { + diags = diags.Consolidate(1, tfdiags.Warning) + } + if m.consolidateErrors { + diags = diags.Consolidate(1, tfdiags.Error) + } // Since warning messages are generally competing if m.compactWarnings { diff --git a/internal/command/plan.go b/internal/command/plan.go index 036cd9c349..cf1089f19f 100644 --- a/internal/command/plan.go +++ b/internal/command/plan.go @@ -255,6 +255,14 @@ Other Options: accompanied by errors, shows them in a more compact form that includes only the summary messages. + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -detailed-exitcode Return detailed exit codes when the command exits. This will change the meaning of exit codes to: 0 - Succeeded, diff is empty (no changes) diff --git a/internal/command/refresh.go b/internal/command/refresh.go index a435cb5e61..4b15616b07 100644 --- a/internal/command/refresh.go +++ b/internal/command/refresh.go @@ -193,32 +193,40 @@ Usage: tofu [global options] refresh [options] Options: - -compact-warnings If OpenTofu produces any warnings that are not - accompanied by errors, show them in a more compact form - that includes only the summary messages. + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact form + that includes only the summary messages. - -input=true Ask for input for variables if not directly set. + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. - -lock=false Don't hold a state lock during the operation. This is - dangerous if others might concurrently run commands - against the same workspace. + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default - -lock-timeout=0s Duration to retry a state lock. + -input=true Ask for input for variables if not directly set. - -no-color If specified, output won't contain any color. + -lock=false Don't hold a state lock during the operation. This is + dangerous if others might concurrently run commands + against the same workspace. - -parallelism=n Limit the number of concurrent operations. Defaults to 10. + -lock-timeout=0s Duration to retry a state lock. - -target=resource Resource to target. Operation will be limited to this - resource and its dependencies. This flag can be used - multiple times. + -no-color If specified, output won't contain any color. - -var 'foo=bar' Set a variable in the OpenTofu configuration. This - flag can be set multiple times. + -parallelism=n Limit the number of concurrent operations. Defaults to 10. - -var-file=foo Set variables in the OpenTofu configuration from - a file. If "terraform.tfvars" or any ".auto.tfvars" - files are present, they will be automatically loaded. + -target=resource Resource to target. Operation will be limited to this + resource and its dependencies. This flag can be used + multiple times. + + -var 'foo=bar' Set a variable in the OpenTofu configuration. This + flag can be set multiple times. + + -var-file=foo Set variables in the OpenTofu configuration from + a file. If "terraform.tfvars" or any ".auto.tfvars" + files are present, they will be automatically loaded. -state, state-out, and -backup are legacy options supported for the local backend only. For more information, see the local backend's documentation. diff --git a/internal/command/test.go b/internal/command/test.go index c37caaded2..97f1bb1d5c 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -58,6 +58,18 @@ Usage: tofu [global options] test [options] Options: + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. + + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -filter=testfile If specified, OpenTofu will only execute the test files specified by this flag. You can use this option multiple times to execute more than one test file. diff --git a/internal/command/validate.go b/internal/command/validate.go index ffdbddf513..f977039ffd 100644 --- a/internal/command/validate.go +++ b/internal/command/validate.go @@ -196,6 +196,18 @@ Usage: tofu [global options] validate [options] Options: + -compact-warnings If OpenTofu produces any warnings that are not + accompanied by errors, show them in a more compact + form that includes only the summary messages. + + -consolidate-warnings If OpenTofu produces any warnings, no consolodation + will be performed. All locations, for all warnings + will be listed. Enabled by default. + + -consolidate-errors If OpenTofu produces any errors, no consolodation + will be performed. All locations, for all errors + will be listed. Disabled by default + -json Produce output in a machine-readable JSON format, suitable for use in text editor integrations and other automated systems. Always disables color. diff --git a/internal/command/views/view.go b/internal/command/views/view.go index 084c1533e1..cbcd795eb1 100644 --- a/internal/command/views/view.go +++ b/internal/command/views/view.go @@ -21,7 +21,9 @@ type View struct { streams *terminal.Streams colorize *colorstring.Colorize - compactWarnings bool + compactWarnings bool + consolidateWarnings bool + consolidateErrors bool // When this is true it's a hint that OpenTofu is being run indirectly // via a wrapper script or other automation and so we may wish to replace @@ -78,6 +80,8 @@ func (v *View) RunningInAutomation() bool { func (v *View) Configure(view *arguments.View) { v.colorize.Disable = view.NoColor v.compactWarnings = view.CompactWarnings + v.consolidateWarnings = view.ConsolidateWarnings + v.consolidateErrors = view.ConsolidateErrors v.concise = view.Concise } @@ -96,7 +100,12 @@ func (v *View) Diagnostics(diags tfdiags.Diagnostics) { return } - diags = diags.ConsolidateWarnings(1) + if v.consolidateWarnings { + diags = diags.Consolidate(1, tfdiags.Warning) + } + if v.consolidateErrors { + diags = diags.Consolidate(1, tfdiags.Error) + } // Since warning messages are generally competing if v.compactWarnings { diff --git a/internal/tfdiags/consolidate_warnings.go b/internal/tfdiags/consolidate_warnings.go index 9dd303513d..7e301588ec 100644 --- a/internal/tfdiags/consolidate_warnings.go +++ b/internal/tfdiags/consolidate_warnings.go @@ -7,13 +7,13 @@ package tfdiags import "fmt" -// ConsolidateWarnings checks if there is an unreasonable amount of warnings +// Consolidate checks if there is an unreasonable amount of diagnostics // with the same summary in the receiver and, if so, returns a new diagnostics -// with some of those warnings consolidated into a single warning in order +// with some of those diagnostics consolidated into a single diagnostic in order // to reduce the verbosity of the output. // // This mechanism is here primarily for diagnostics printed out at the CLI. In -// other contexts it is likely better to just return the warnings directly, +// other contexts it is likely better to just return the diagnostic directly, // particularly if they are going to be interpreted by software rather than // by a human reader. // @@ -21,28 +21,28 @@ import "fmt" // but some diagnostic values themselves might be shared. // // The definition of "unreasonable" is given as the threshold argument. At most -// that many warnings with the same summary will be shown. -func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics { +// that many diagnostics with the same summary will be shown. +func (diags Diagnostics) Consolidate(threshold int, level Severity) Diagnostics { if len(diags) == 0 { return nil } newDiags := make(Diagnostics, 0, len(diags)) - // We'll track how many times we've seen each warning summary so we can + // We'll track how many times we've seen each diagnostic summary so we can // decide when to start consolidating. Once we _have_ started consolidating, - // we'll also track the object representing the consolidated warning + // we'll also track the object representing the consolidated diagnostic // so we can continue appending to it. - warningStats := make(map[string]int) - warningGroups := make(map[string]*warningGroup) + diagnosticStats := make(map[string]int) + diagnosticGroups := make(map[string]*consolidatedGroup) for _, diag := range diags { severity := diag.Severity() - if severity != Warning || diag.Source().Subject == nil { - // Only warnings can get special treatment, and we only - // consolidate warnings that have source locations because + if severity != level || diag.Source().Subject == nil { + // Only the given level can get special treatment, and we only + // consolidate diagnostics that have source locations because // our primary goal here is to deal with the situation where - // some configuration language feature is producing a warning + // some configuration language feature is producing a diagnostic // each time it's used across a potentially-large config. newDiags = newDiags.Append(diag) continue @@ -56,26 +56,26 @@ func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics { desc := diag.Description() summary := desc.Summary - if g, ok := warningGroups[summary]; ok { + if g, ok := diagnosticGroups[summary]; ok { // We're already grouping this one, so we'll just continue it. g.Append(diag) continue } - warningStats[summary]++ - if warningStats[summary] == threshold { + diagnosticStats[summary]++ + if diagnosticStats[summary] == threshold { // Initially creating the group doesn't really change anything - // visibly in the result, since a group with only one warning + // visibly in the result, since a group with only one diagnostic // is just a passthrough anyway, but once we do this any additional - // warnings with the same summary will get appended to this group. - g := &warningGroup{} + // diagnostics with the same summary will get appended to this group. + g := &consolidatedGroup{} newDiags = newDiags.Append(g) - warningGroups[summary] = g + diagnosticGroups[summary] = g g.Append(diag) continue } - // If this warning is not consolidating yet then we'll just append + // If this diagnostic is not consolidating yet then we'll just append // it directly. newDiags = newDiags.Append(diag) } @@ -83,36 +83,46 @@ func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics { return newDiags } -// A warningGroup is one or more warning diagnostics grouped together for +// A consolidatedGroup is one or more diagnostics grouped together for // UI consolidation purposes. // -// A warningGroup with only one diagnostic in it is just a passthrough for +// A consolidatedGroup with only one diagnostic in it is just a passthrough for // that one diagnostic. If it has more than one then it will behave mostly // like the first one but its detail message will include an additional -// sentence mentioning the consolidation. A warningGroup with no diagnostics +// sentence mentioning the consolidation. A consolidatedGroup with no diagnostics // at all is invalid and will panic when used. -type warningGroup struct { - Warnings Diagnostics +type consolidatedGroup struct { + Consolidated Diagnostics } -var _ Diagnostic = (*warningGroup)(nil) +var _ Diagnostic = (*consolidatedGroup)(nil) -func (wg *warningGroup) Severity() Severity { - return wg.Warnings[0].Severity() +func (wg *consolidatedGroup) Severity() Severity { + return wg.Consolidated[0].Severity() } -func (wg *warningGroup) Description() Description { - desc := wg.Warnings[0].Description() - if len(wg.Warnings) < 2 { +func (wg *consolidatedGroup) Description() Description { + desc := wg.Consolidated[0].Description() + if len(wg.Consolidated) == 1 { return desc } - extraCount := len(wg.Warnings) - 1 + extraCount := len(wg.Consolidated) - 1 var msg string + var diagType string + switch wg.Severity() { + case Error: + diagType = "error" + case Warning: + diagType = "warning" + default: + panic(fmt.Sprintf("Invalid diagnostic severity: %#v", wg.Severity())) + } + switch extraCount { case 1: - msg = "(and one more similar warning elsewhere)" + msg = fmt.Sprintf("(and one more similar %s elsewhere)", diagType) default: - msg = fmt.Sprintf("(and %d more similar warnings elsewhere)", extraCount) + msg = fmt.Sprintf("(and %d more similar %ss elsewhere)", extraCount, diagType) } if desc.Detail != "" { desc.Detail = desc.Detail + "\n\n" + msg @@ -122,39 +132,39 @@ func (wg *warningGroup) Description() Description { return desc } -func (wg *warningGroup) Source() Source { - return wg.Warnings[0].Source() +func (wg *consolidatedGroup) Source() Source { + return wg.Consolidated[0].Source() } -func (wg *warningGroup) FromExpr() *FromExpr { - return wg.Warnings[0].FromExpr() +func (wg *consolidatedGroup) FromExpr() *FromExpr { + return wg.Consolidated[0].FromExpr() } -func (wg *warningGroup) ExtraInfo() interface{} { - return wg.Warnings[0].ExtraInfo() +func (wg *consolidatedGroup) ExtraInfo() interface{} { + return wg.Consolidated[0].ExtraInfo() } -func (wg *warningGroup) Append(diag Diagnostic) { - if diag.Severity() != Warning { +func (wg *consolidatedGroup) Append(diag Diagnostic) { + if len(wg.Consolidated) != 0 && diag.Severity() != wg.Severity() { panic("can't append a non-warning diagnostic to a warningGroup") } - wg.Warnings = append(wg.Warnings, diag) + wg.Consolidated = append(wg.Consolidated, diag) } -// WarningGroupSourceRanges can be used in conjunction with -// Diagnostics.ConsolidateWarnings to recover the full set of original source -// locations from a consolidated warning. +// ConsolidatedGroupSourceRanges can be used in conjunction with +// Diagnostics.Consolidate to recover the full set of original source +// locations from a consolidated diagnostic. // // For convenience, this function accepts any diagnostic and will just return -// the single Source value from any diagnostic that isn't a warning group. -func WarningGroupSourceRanges(diag Diagnostic) []Source { - wg, ok := diag.(*warningGroup) +// the single Source value from any diagnostic that isn't a consolidated group. +func ConsolidatedGroupSourceRanges(diag Diagnostic) []Source { + wg, ok := diag.(*consolidatedGroup) if !ok { return []Source{diag.Source()} } - ret := make([]Source, len(wg.Warnings)) - for i, wrappedDiag := range wg.Warnings { + ret := make([]Source, len(wg.Consolidated)) + for i, wrappedDiag := range wg.Consolidated { ret[i] = wrappedDiag.Source() } return ret diff --git a/internal/tfdiags/consolidate_warnings_test.go b/internal/tfdiags/consolidate_warnings_test.go index a6e8e26728..9fe5e733eb 100644 --- a/internal/tfdiags/consolidate_warnings_test.go +++ b/internal/tfdiags/consolidate_warnings_test.go @@ -96,7 +96,7 @@ func TestConsolidateWarnings(t *testing.T) { // We're using ForRPC here to force the diagnostics to be of a consistent // type that we can easily assert against below. - got := diags.ConsolidateWarnings(2).ForRPC() + got := diags.Consolidate(2, Warning).ForRPC() want := Diagnostics{ // First set &rpcFriendlyDiag{ @@ -261,3 +261,61 @@ var _ DiagnosticExtraDoNotConsolidate = doNotConsolidate(true) func (d doNotConsolidate) DoNotConsolidateDiagnostic() bool { return bool(d) } + +func TestConsolidateError(t *testing.T) { + var diags Diagnostics + + // create a multiplicity of errors + for i := 0; i < 5; i++ { + diags = diags.Append( + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Error 1", + Detail: "Error diag have duplicated subjects", + Subject: &hcl.Range{ + Filename: "foo.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Error 2", + Detail: "Error diag have different subjects", + Subject: &hcl.Range{ + Filename: "foo.tf", + Start: hcl.Pos{Line: i + 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: i + 1, Column: 1, Byte: 0}, + }, + }, + ) + } + + got := diags.Consolidate(1, Error).ForRPC() + want := Diagnostics{ + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 1", + Detail_: "Error diag have duplicated subjects\n\n(and 4 more similar errors elsewhere)", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 2", + Detail_: "Error diag have different subjects\n\n(and 4 more similar errors elsewhere)", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } +}