Warning and Error consolidation CLI options (#1894)

Signed-off-by: Hefeweizen <jmales@gmail.com>
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
Signed-off-by: Syasusu <syasusu@163.com>
Co-authored-by: Hefeweizen <jmales@gmail.com>
Co-authored-by: Syasusu <syasusu@163.com>
This commit is contained in:
Christian Mesh 2024-09-23 07:31:06 -04:00 committed by GitHub
parent bc166ce498
commit 12ed264597
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 308 additions and 108 deletions

View File

@ -356,6 +356,14 @@ Options:
accompanied by errors, show them in a more compact accompanied by errors, show them in a more compact
form that includes only the summary messages. 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. -destroy Destroy OpenTofu-managed infrastructure.
The command "tofu destroy" is a convenience alias The command "tofu destroy" is a convenience alias
for this option. for this option.

View File

@ -14,7 +14,9 @@ type View struct {
// CompactWarnings is used to coalesce duplicate warnings, to reduce the // CompactWarnings is used to coalesce duplicate warnings, to reduce the
// level of noise when multiple instances of the same warning are raised // level of noise when multiple instances of the same warning are raised
// for a configuration. // 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 // Concise is used to reduce the level of noise in the output and display
// only the important details. // only the important details.
@ -28,7 +30,9 @@ type View struct {
// possibly-modified slice of arguments. If any of the supported flags are // possibly-modified slice of arguments. If any of the supported flags are
// found, they will be removed from the slice. // found, they will be removed from the slice.
func ParseView(args []string) (*View, []string) { 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 // Keep track of the length of the returned slice. When we find an
// argument we support, i will not be incremented. // argument we support, i will not be incremented.
@ -39,6 +43,18 @@ func ParseView(args []string) (*View, []string) {
common.NoColor = true common.NoColor = true
case "-compact-warnings": case "-compact-warnings":
common.CompactWarnings = true 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": case "-concise":
common.Concise = true common.Concise = true
default: default:

View File

@ -19,57 +19,62 @@ func TestParseView(t *testing.T) {
}{ }{
"nil": { "nil": {
nil, nil,
&View{NoColor: false, CompactWarnings: false, Concise: false}, &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false},
nil, nil,
}, },
"empty": { "empty": {
[]string{}, []string{},
&View{NoColor: false, CompactWarnings: false, Concise: false}, &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false},
[]string{}, []string{},
}, },
"none matching": { "none matching": {
[]string{"-foo", "bar", "-baz"}, []string{"-foo", "bar", "-baz"},
&View{NoColor: false, CompactWarnings: false, Concise: false}, &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: false},
[]string{"-foo", "bar", "-baz"}, []string{"-foo", "bar", "-baz"},
}, },
"no-color": { "no-color": {
[]string{"-foo", "-no-color", "-baz"}, []string{"-foo", "-no-color", "-baz"},
&View{NoColor: true, CompactWarnings: false, Concise: false}, &View{NoColor: true, CompactWarnings: false, ConsolidateWarnings: true, Concise: false},
[]string{"-foo", "-baz"}, []string{"-foo", "-baz"},
}, },
"compact-warnings": { "compact-warnings": {
[]string{"-foo", "-compact-warnings", "-baz"}, []string{"-foo", "-compact-warnings", "-baz"},
&View{NoColor: false, CompactWarnings: true, Concise: false}, &View{NoColor: false, CompactWarnings: true, ConsolidateWarnings: true, Concise: false},
[]string{"-foo", "-baz"}, []string{"-foo", "-baz"},
}, },
"concise": { "concise": {
[]string{"-foo", "-concise", "-baz"}, []string{"-foo", "-concise", "-baz"},
&View{NoColor: false, CompactWarnings: false, Concise: true}, &View{NoColor: false, CompactWarnings: false, ConsolidateWarnings: true, Concise: true},
[]string{"-foo", "-baz"}, []string{"-foo", "-baz"},
}, },
"no-color and compact-warnings": { "no-color and compact-warnings": {
[]string{"-foo", "-no-color", "-compact-warnings", "-baz"}, []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"}, []string{"-foo", "-baz"},
}, },
"no-color and concise": { "no-color and concise": {
[]string{"-foo", "-no-color", "-concise", "-baz"}, []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"}, []string{"-foo", "-baz"},
}, },
"concise and compact-warnings": { "concise and compact-warnings": {
[]string{"-foo", "-concise", "-compact-warnings", "-baz"}, []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"}, []string{"-foo", "-baz"},
}, },
"all three": { "all three": {
[]string{"-foo", "-no-color", "-compact-warnings", "-concise", "-baz"}, []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"}, []string{"-foo", "-baz"},
}, },
"all three, resulting in empty args": { "all three, resulting in empty args": {
[]string{"-no-color", "-compact-warnings", "-concise"}, []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{}, []string{},
}, },
} }

View File

@ -223,15 +223,27 @@ Usage: tofu [global options] console [options]
Options: Options:
-state=path Legacy option for the local backend only. See the local -compact-warnings If OpenTofu produces any warnings that are not
backend's documentation for more information. 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 -consolidate-warnings If OpenTofu produces any warnings, no consolodation
flag can be set multiple times. will be performed. All locations, for all warnings
will be listed. Enabled by default.
-var-file=foo Set variables in the OpenTofu configuration from -consolidate-errors If OpenTofu produces any errors, no consolodation
a file. If "terraform.tfvars" or any ".auto.tfvars" will be performed. All locations, for all errors
files are present, they will be automatically loaded. 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) return strings.TrimSpace(helpText)
} }

View File

@ -187,7 +187,7 @@ func DiagnosticWarningsCompact(diags tfdiags.Diagnostics, color *colorstring.Col
var b strings.Builder var b strings.Builder
b.WriteString(color.Color("[bold][yellow]Warnings:[reset]\n\n")) b.WriteString(color.Color("[bold][yellow]Warnings:[reset]\n\n"))
for _, diag := range diags { for _, diag := range diags {
sources := tfdiags.WarningGroupSourceRanges(diag) sources := tfdiags.ConsolidatedGroupSourceRanges(diag)
b.WriteString(fmt.Sprintf("- %s\n", diag.Description().Summary)) b.WriteString(fmt.Sprintf("- %s\n", diag.Description().Summary))
if len(sources) > 0 { if len(sources) > 0 {
mainSource := sources[0] mainSource := sources[0]

View File

@ -634,7 +634,7 @@ func TestDiagnosticWarningsCompact(t *testing.T) {
// ConsolidateWarnings groups together the ones // ConsolidateWarnings groups together the ones
// that have source location information and that // that have source location information and that
// have the same summary text. // have the same summary text.
diags = diags.ConsolidateWarnings(1) diags = diags.Consolidate(1, tfdiags.Warning)
// A zero-value Colorize just passes all the formatting // A zero-value Colorize just passes all the formatting
// codes back to us, so we can test them literally. // codes back to us, so we can test them literally.

View File

@ -316,6 +316,18 @@ Usage: tofu [global options] import [options] ADDR ID
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
-config=path Path to a directory of OpenTofu configuration files -config=path Path to a directory of OpenTofu configuration files
to use to configure the provider. Defaults to pwd. to use to configure the provider. Defaults to pwd.
If no config files are present, they must be provided If no config files are present, they must be provided

View File

@ -1193,6 +1193,18 @@ Options:
times. The backend type must be in the configuration times. The backend type must be in the configuration
itself. 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 -force-copy Suppress prompts about copying state data when
initializing a new state backend. This is initializing a new state backend. This is
equivalent to providing a "yes" to all confirmation equivalent to providing a "yes" to all confirmation

View File

@ -252,16 +252,24 @@ type Meta struct {
// //
// compactWarnings (-compact-warnings) selects a more compact presentation // compactWarnings (-compact-warnings) selects a more compact presentation
// of warnings in the output when they are not accompanied by errors. // of warnings in the output when they are not accompanied by errors.
statePath string //
stateOutPath string // consolidateWarnings (-consolidate-warnings=false) disables consolodation
backupPath string // of warnings in the output, printing all instances of a particular warning.
parallelism int //
stateLock bool // consolidateErrors (-consolidate-errors=true) enables consolodation
stateLockTimeout time.Duration // of errors in the output, printing a single instances of a particular warning.
forceInitCopy bool statePath string
reconfigure bool stateOutPath string
migrateState bool backupPath string
compactWarnings bool 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 // Used with commands which write state to allow users to write remote
// state even if the remote and local OpenTofu versions don't match. // 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.BoolVar(&m.input, "input", true, "input")
f.Var((*FlagStringSlice)(&m.targetFlags), "target", "resource to target") f.Var((*FlagStringSlice)(&m.targetFlags), "target", "resource to target")
f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings") 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) m.varFlagSet(f)
@ -661,8 +671,10 @@ func (m *Meta) process(args []string) []string {
// views.View and cli.Ui during the migration phase. // views.View and cli.Ui during the migration phase.
if m.View != nil { if m.View != nil {
m.View.Configure(&arguments.View{ m.View.Configure(&arguments.View{
CompactWarnings: m.compactWarnings, CompactWarnings: m.compactWarnings,
NoColor: !m.Color, 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 // Internally this function uses Diagnostics.Append, and so it will panic
// if given unsupported value types, just as Append does. // if given unsupported value types, just as Append does.
func (m *Meta) showDiagnostics(vals ...interface{}) { func (m *Meta) showDiagnostics(vals ...interface{}) {
var diags tfdiags.Diagnostics var diags tfdiags.Diagnostics
diags = diags.Append(vals...) diags = diags.Append(vals...)
diags.Sort() diags.Sort()
@ -723,7 +736,12 @@ func (m *Meta) showDiagnostics(vals ...interface{}) {
outputWidth := m.ErrorColumns() 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 // Since warning messages are generally competing
if m.compactWarnings { if m.compactWarnings {

View File

@ -255,6 +255,14 @@ Other Options:
accompanied by errors, shows them in a more compact accompanied by errors, shows them in a more compact
form that includes only the summary messages. 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. -detailed-exitcode Return detailed exit codes when the command exits.
This will change the meaning of exit codes to: This will change the meaning of exit codes to:
0 - Succeeded, diff is empty (no changes) 0 - Succeeded, diff is empty (no changes)

View File

@ -193,32 +193,40 @@ Usage: tofu [global options] refresh [options]
Options: Options:
-compact-warnings If OpenTofu produces any warnings that are not -compact-warnings If OpenTofu produces any warnings that are not
accompanied by errors, show them in a more compact form accompanied by errors, show them in a more compact form
that includes only the summary messages. 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 -consolidate-errors If OpenTofu produces any errors, no consolodation
dangerous if others might concurrently run commands will be performed. All locations, for all errors
against the same workspace. 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 -no-color If specified, output won't contain any color.
resource and its dependencies. This flag can be used
multiple times.
-var 'foo=bar' Set a variable in the OpenTofu configuration. This -parallelism=n Limit the number of concurrent operations. Defaults to 10.
flag can be set multiple times.
-var-file=foo Set variables in the OpenTofu configuration from -target=resource Resource to target. Operation will be limited to this
a file. If "terraform.tfvars" or any ".auto.tfvars" resource and its dependencies. This flag can be used
files are present, they will be automatically loaded. 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 -state, state-out, and -backup are legacy options supported for the local
backend only. For more information, see the local backend's documentation. backend only. For more information, see the local backend's documentation.

View File

@ -58,6 +58,18 @@ Usage: tofu [global options] test [options]
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 -filter=testfile If specified, OpenTofu will only execute the test files
specified by this flag. You can use this option multiple specified by this flag. You can use this option multiple
times to execute more than one test file. times to execute more than one test file.

View File

@ -196,6 +196,18 @@ Usage: tofu [global options] validate [options]
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, -json Produce output in a machine-readable JSON format,
suitable for use in text editor integrations and other suitable for use in text editor integrations and other
automated systems. Always disables color. automated systems. Always disables color.

View File

@ -21,7 +21,9 @@ type View struct {
streams *terminal.Streams streams *terminal.Streams
colorize *colorstring.Colorize 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 // 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 // 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) { func (v *View) Configure(view *arguments.View) {
v.colorize.Disable = view.NoColor v.colorize.Disable = view.NoColor
v.compactWarnings = view.CompactWarnings v.compactWarnings = view.CompactWarnings
v.consolidateWarnings = view.ConsolidateWarnings
v.consolidateErrors = view.ConsolidateErrors
v.concise = view.Concise v.concise = view.Concise
} }
@ -96,7 +100,12 @@ func (v *View) Diagnostics(diags tfdiags.Diagnostics) {
return 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 // Since warning messages are generally competing
if v.compactWarnings { if v.compactWarnings {

View File

@ -7,13 +7,13 @@ package tfdiags
import "fmt" 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 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. // to reduce the verbosity of the output.
// //
// This mechanism is here primarily for diagnostics printed out at the CLI. In // 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 // particularly if they are going to be interpreted by software rather than
// by a human reader. // by a human reader.
// //
@ -21,28 +21,28 @@ import "fmt"
// but some diagnostic values themselves might be shared. // but some diagnostic values themselves might be shared.
// //
// The definition of "unreasonable" is given as the threshold argument. At most // The definition of "unreasonable" is given as the threshold argument. At most
// that many warnings with the same summary will be shown. // that many diagnostics with the same summary will be shown.
func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics { func (diags Diagnostics) Consolidate(threshold int, level Severity) Diagnostics {
if len(diags) == 0 { if len(diags) == 0 {
return nil return nil
} }
newDiags := make(Diagnostics, 0, len(diags)) 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, // 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. // so we can continue appending to it.
warningStats := make(map[string]int) diagnosticStats := make(map[string]int)
warningGroups := make(map[string]*warningGroup) diagnosticGroups := make(map[string]*consolidatedGroup)
for _, diag := range diags { for _, diag := range diags {
severity := diag.Severity() severity := diag.Severity()
if severity != Warning || diag.Source().Subject == nil { if severity != level || diag.Source().Subject == nil {
// Only warnings can get special treatment, and we only // Only the given level can get special treatment, and we only
// consolidate warnings that have source locations because // consolidate diagnostics that have source locations because
// our primary goal here is to deal with the situation where // 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. // each time it's used across a potentially-large config.
newDiags = newDiags.Append(diag) newDiags = newDiags.Append(diag)
continue continue
@ -56,26 +56,26 @@ func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics {
desc := diag.Description() desc := diag.Description()
summary := desc.Summary 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. // We're already grouping this one, so we'll just continue it.
g.Append(diag) g.Append(diag)
continue continue
} }
warningStats[summary]++ diagnosticStats[summary]++
if warningStats[summary] == threshold { if diagnosticStats[summary] == threshold {
// Initially creating the group doesn't really change anything // 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 // is just a passthrough anyway, but once we do this any additional
// warnings with the same summary will get appended to this group. // diagnostics with the same summary will get appended to this group.
g := &warningGroup{} g := &consolidatedGroup{}
newDiags = newDiags.Append(g) newDiags = newDiags.Append(g)
warningGroups[summary] = g diagnosticGroups[summary] = g
g.Append(diag) g.Append(diag)
continue 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. // it directly.
newDiags = newDiags.Append(diag) newDiags = newDiags.Append(diag)
} }
@ -83,36 +83,46 @@ func (diags Diagnostics) ConsolidateWarnings(threshold int) Diagnostics {
return newDiags 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. // 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 // 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 // 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. // at all is invalid and will panic when used.
type warningGroup struct { type consolidatedGroup struct {
Warnings Diagnostics Consolidated Diagnostics
} }
var _ Diagnostic = (*warningGroup)(nil) var _ Diagnostic = (*consolidatedGroup)(nil)
func (wg *warningGroup) Severity() Severity { func (wg *consolidatedGroup) Severity() Severity {
return wg.Warnings[0].Severity() return wg.Consolidated[0].Severity()
} }
func (wg *warningGroup) Description() Description { func (wg *consolidatedGroup) Description() Description {
desc := wg.Warnings[0].Description() desc := wg.Consolidated[0].Description()
if len(wg.Warnings) < 2 { if len(wg.Consolidated) == 1 {
return desc return desc
} }
extraCount := len(wg.Warnings) - 1 extraCount := len(wg.Consolidated) - 1
var msg string 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 { switch extraCount {
case 1: case 1:
msg = "(and one more similar warning elsewhere)" msg = fmt.Sprintf("(and one more similar %s elsewhere)", diagType)
default: 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 != "" { if desc.Detail != "" {
desc.Detail = desc.Detail + "\n\n" + msg desc.Detail = desc.Detail + "\n\n" + msg
@ -122,39 +132,39 @@ func (wg *warningGroup) Description() Description {
return desc return desc
} }
func (wg *warningGroup) Source() Source { func (wg *consolidatedGroup) Source() Source {
return wg.Warnings[0].Source() return wg.Consolidated[0].Source()
} }
func (wg *warningGroup) FromExpr() *FromExpr { func (wg *consolidatedGroup) FromExpr() *FromExpr {
return wg.Warnings[0].FromExpr() return wg.Consolidated[0].FromExpr()
} }
func (wg *warningGroup) ExtraInfo() interface{} { func (wg *consolidatedGroup) ExtraInfo() interface{} {
return wg.Warnings[0].ExtraInfo() return wg.Consolidated[0].ExtraInfo()
} }
func (wg *warningGroup) Append(diag Diagnostic) { func (wg *consolidatedGroup) Append(diag Diagnostic) {
if diag.Severity() != Warning { if len(wg.Consolidated) != 0 && diag.Severity() != wg.Severity() {
panic("can't append a non-warning diagnostic to a warningGroup") 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 // ConsolidatedGroupSourceRanges can be used in conjunction with
// Diagnostics.ConsolidateWarnings to recover the full set of original source // Diagnostics.Consolidate to recover the full set of original source
// locations from a consolidated warning. // locations from a consolidated diagnostic.
// //
// For convenience, this function accepts any diagnostic and will just return // For convenience, this function accepts any diagnostic and will just return
// the single Source value from any diagnostic that isn't a warning group. // the single Source value from any diagnostic that isn't a consolidated group.
func WarningGroupSourceRanges(diag Diagnostic) []Source { func ConsolidatedGroupSourceRanges(diag Diagnostic) []Source {
wg, ok := diag.(*warningGroup) wg, ok := diag.(*consolidatedGroup)
if !ok { if !ok {
return []Source{diag.Source()} return []Source{diag.Source()}
} }
ret := make([]Source, len(wg.Warnings)) ret := make([]Source, len(wg.Consolidated))
for i, wrappedDiag := range wg.Warnings { for i, wrappedDiag := range wg.Consolidated {
ret[i] = wrappedDiag.Source() ret[i] = wrappedDiag.Source()
} }
return ret return ret

View File

@ -96,7 +96,7 @@ func TestConsolidateWarnings(t *testing.T) {
// We're using ForRPC here to force the diagnostics to be of a consistent // We're using ForRPC here to force the diagnostics to be of a consistent
// type that we can easily assert against below. // type that we can easily assert against below.
got := diags.ConsolidateWarnings(2).ForRPC() got := diags.Consolidate(2, Warning).ForRPC()
want := Diagnostics{ want := Diagnostics{
// First set // First set
&rpcFriendlyDiag{ &rpcFriendlyDiag{
@ -261,3 +261,61 @@ var _ DiagnosticExtraDoNotConsolidate = doNotConsolidate(true)
func (d doNotConsolidate) DoNotConsolidateDiagnostic() bool { func (d doNotConsolidate) DoNotConsolidateDiagnostic() bool {
return bool(d) 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)
}
}